forked from fa/breadcrumb-the-shire
test(architecture): lock down two routing/filter silent-failure modes
Encodes two bugs that recently cost debugging time as architecture tests so they cannot recur. PageTemplateNamingTest MintyPHP's Router splits page filenames on parens. `view(default).phtml` → view="view", template="default". A nested group like `view($id)(none).phtml` produces an empty view name, resolves to the wrong file, and the response silently becomes an empty body. URL parameters belong only in the .php action filename. This test flags any .phtml with more than one paren group. FilterSchemaConsistencyTest Every query key declared in a list's filter-schema.php must also appear in the toolbar section (standard pagination/sort keys exempted). If a filter param is not in the toolbar, Grid.js's URL-sync drops it on every refetch — the filter works on initial page load and breaks after any user interaction. This test caught one dormant violation (system-audit's target_type), which is now declared as a hidden toolbar field for future use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -46,6 +46,14 @@ return gridFilterSchema([
|
||||
'dir' => ['type' => 'dir', 'default' => 'desc'],
|
||||
],
|
||||
'toolbar' => [
|
||||
[
|
||||
// Dormant capability — kept in toolbar as hidden so Grid.js preserves
|
||||
// the param across refetches if/when a UI is added.
|
||||
'key' => 'target_type',
|
||||
'type' => 'hidden',
|
||||
'input_id' => 'system-audit-target-type-filter',
|
||||
'default' => '',
|
||||
],
|
||||
[
|
||||
'key' => 'search',
|
||||
'type' => 'text',
|
||||
|
||||
110
tests/Architecture/FilterSchemaConsistencyTest.php
Normal file
110
tests/Architecture/FilterSchemaConsistencyTest.php
Normal file
@@ -0,0 +1,110 @@
|
||||
<?php
|
||||
|
||||
namespace MintyPHP\Tests\Architecture;
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Every query key declared in a list's `filter-schema.php` must also appear
|
||||
* in the `toolbar` section (either as a visible field or hidden). Reason:
|
||||
*
|
||||
* - Backend reads filter values via `gridParseFiltersFromSchemaFile()`, which
|
||||
* parses against the `query` schema.
|
||||
* - Frontend's Grid.js URL-sync only forwards params that are declared in the
|
||||
* `toolbar` section (derived via `gridSchemaClientFilters()`). If a query
|
||||
* param is not in `toolbar`, Grid.js drops it on every subsequent request
|
||||
* (pagination, sort, filter change) — the filter appears to work on page
|
||||
* load but silently breaks after any user interaction.
|
||||
*
|
||||
* This test ran us into real production pain before it existed: the address
|
||||
* book's tenant-register tabs worked on first load but lost the filter on
|
||||
* every grid refetch. Declaring `tenant` as a hidden toolbar field fixed it.
|
||||
* This test prevents repeat occurrences.
|
||||
*
|
||||
* Exempted keys (framework-provided, always preserved by Grid.js):
|
||||
* limit, offset, order, dir
|
||||
*/
|
||||
class FilterSchemaConsistencyTest extends TestCase
|
||||
{
|
||||
use ProjectFileAssertionSupport;
|
||||
|
||||
private const FRAMEWORK_KEYS = ['limit', 'offset', 'order', 'dir'];
|
||||
|
||||
public function testEveryQueryKeyHasToolbarDeclaration(): void
|
||||
{
|
||||
$root = $this->projectRootPath();
|
||||
$schemaFiles = [];
|
||||
$this->collectFilterSchemas($root . '/pages', $schemaFiles);
|
||||
$this->collectFilterSchemas($root . '/modules', $schemaFiles);
|
||||
|
||||
$this->assertNotEmpty($schemaFiles, 'No filter-schema.php files found — regex stale?');
|
||||
|
||||
$violations = [];
|
||||
|
||||
foreach ($schemaFiles as $schemaFile) {
|
||||
$schema = @include $schemaFile;
|
||||
if (!is_array($schema)) {
|
||||
$violations[] = sprintf('%s — filter-schema.php did not return an array', $this->relative($schemaFile, $root));
|
||||
continue;
|
||||
}
|
||||
|
||||
$queryKeys = array_keys(is_array($schema['query'] ?? null) ? $schema['query'] : []);
|
||||
$toolbarFields = is_array($schema['toolbar'] ?? null) ? $schema['toolbar'] : [];
|
||||
|
||||
// Schemas without a toolbar are for non-Grid endpoints (e.g. global
|
||||
// search data endpoint consumed by a custom JS dialog). Skip those —
|
||||
// they never participate in Grid.js URL sync.
|
||||
if ($toolbarFields === []) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$toolbarKeys = [];
|
||||
foreach ($toolbarFields as $field) {
|
||||
if (!is_array($field)) {
|
||||
continue;
|
||||
}
|
||||
$key = trim((string) ($field['key'] ?? ''));
|
||||
if ($key !== '') {
|
||||
$toolbarKeys[] = $key;
|
||||
}
|
||||
}
|
||||
|
||||
$missing = array_diff($queryKeys, $toolbarKeys, self::FRAMEWORK_KEYS);
|
||||
if ($missing) {
|
||||
$violations[] = sprintf(
|
||||
"%s — query keys missing from toolbar: %s.\n Add them to the 'toolbar' section (type 'hidden' is fine) so Grid.js forwards them on refetch.",
|
||||
$this->relative($schemaFile, $root),
|
||||
implode(', ', $missing)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
$this->assertSame(
|
||||
[],
|
||||
$violations,
|
||||
"Filter-schema consistency violations:\n" . implode("\n\n", $violations)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<string> &$out
|
||||
*/
|
||||
private function collectFilterSchemas(string $base, array &$out): void
|
||||
{
|
||||
if (!is_dir($base)) {
|
||||
return;
|
||||
}
|
||||
$iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($base, \FilesystemIterator::SKIP_DOTS));
|
||||
/** @var \SplFileInfo $file */
|
||||
foreach ($iterator as $file) {
|
||||
if ($file->isFile() && $file->getFilename() === 'filter-schema.php') {
|
||||
$out[] = $file->getPathname();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private function relative(string $path, string $root): string
|
||||
{
|
||||
return str_replace($root . '/', '', $path);
|
||||
}
|
||||
}
|
||||
86
tests/Architecture/PageTemplateNamingTest.php
Normal file
86
tests/Architecture/PageTemplateNamingTest.php
Normal file
@@ -0,0 +1,86 @@
|
||||
<?php
|
||||
|
||||
namespace MintyPHP\Tests\Architecture;
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* MintyPHP's Router parses page filenames by splitting on parentheses. URL
|
||||
* parameters belong ONLY in the `.php` action filename (e.g. `view($id).php`);
|
||||
* the `.phtml` view filename must contain exactly ONE paren group for the
|
||||
* template name (`view(default).phtml` or `view(none).phtml`).
|
||||
*
|
||||
* Nested paren groups like `view($id)(none).phtml` cause the router's regex
|
||||
* split to produce an empty view name — the file silently resolves to the
|
||||
* wrong path and the response is an empty body with no visible error.
|
||||
*
|
||||
* Reference: vendor/mintyphp/core/src/Router.php::extractParts()
|
||||
*/
|
||||
class PageTemplateNamingTest extends TestCase
|
||||
{
|
||||
use ProjectFileAssertionSupport;
|
||||
|
||||
public function testPhtmlFilenamesHaveExactlyOneParenGroup(): void
|
||||
{
|
||||
$root = $this->projectRootPath();
|
||||
$scanRoots = [$root . '/pages'];
|
||||
$modulesDir = $root . '/modules';
|
||||
if (is_dir($modulesDir)) {
|
||||
foreach (scandir($modulesDir) ?: [] as $moduleEntry) {
|
||||
if ($moduleEntry === '.' || $moduleEntry === '..') {
|
||||
continue;
|
||||
}
|
||||
$modulePages = $modulesDir . '/' . $moduleEntry . '/pages';
|
||||
if (is_dir($modulePages)) {
|
||||
$scanRoots[] = $modulePages;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$violations = [];
|
||||
|
||||
foreach ($scanRoots as $scanRoot) {
|
||||
if (!is_dir($scanRoot)) {
|
||||
continue;
|
||||
}
|
||||
$iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($scanRoot, \FilesystemIterator::SKIP_DOTS));
|
||||
/** @var \SplFileInfo $file */
|
||||
foreach ($iterator as $file) {
|
||||
if (!$file->isFile() || $file->getExtension() !== 'phtml') {
|
||||
continue;
|
||||
}
|
||||
$name = $file->getFilename();
|
||||
$openCount = substr_count($name, '(');
|
||||
$closeCount = substr_count($name, ')');
|
||||
$relativePath = str_replace($root . '/', '', $file->getPathname());
|
||||
|
||||
if ($openCount !== $closeCount) {
|
||||
$violations[] = sprintf('%s — mismatched parens', $relativePath);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Partials (`_form.phtml`, shared includes) have zero paren groups — skip them.
|
||||
if ($openCount === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Routed views must have exactly one paren group: `<view>(<template>).phtml`.
|
||||
if ($openCount !== 1) {
|
||||
$violations[] = sprintf(
|
||||
'%s — found %d paren groups, expected exactly 1 (template only). '
|
||||
. 'URL parameters belong in the .php action filename, not the .phtml view. '
|
||||
. "Nested paren groups break MintyPHP's Router silently (empty response).",
|
||||
$relativePath,
|
||||
$openCount
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$this->assertSame(
|
||||
[],
|
||||
$violations,
|
||||
".phtml filename convention violations:\n" . implode("\n", $violations)
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user