From 53ccd212d5d5cfcae43fff562be14ac3b0d16e92 Mon Sep 17 00:00:00 2001 From: fs Date: Tue, 21 Apr 2026 22:50:43 +0200 Subject: [PATCH] docs(agents): encode list-export convention as GR-UI-EXPORT guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the core-primitive-end-to-end rule for Grid.js list exports binding instead of advisory — so future consumers (and reviewers) cannot quietly drift back into inline fputcsv / hand-rolled headers / bespoke click listeners / lurl() for query-carrying URLs. - CLAUDE.md: new UI Patterns bullet + two "Never Do This" entries spelling out the server/view/client contract in one place. - .agents/checks/guard-catalog.json: new GR-UI-EXPORT entry describing the end-to-end requirement (exportRequireGetRequest + exportCapLimit + exportSendCsv + ExportColumn + shared filter-schema + dropdown partial + endpointUrl() + initListExport). - .agents/checks/guard-enforcement-map.json: wire the new guard to tests/Architecture/ListExportContractTest.php as the automated evidence source. - .agents/checks/guard-checklist.md & .agents/prompts/reviewer-code.md: add GR-UI-EXPORT so the Code Reviewer prompt and the checklist flag violations alongside GR-UI-LIST. - tests/Architecture/ListExportContractFiles: registry of every list export (currently helpdesk-domains, admin/users, audit/system-audit). Adding a new list = one entry, contract test covers the rest. - tests/Architecture/ListExportContractTest: six checks per registered entry — endpoint uses the primitive, no hand-rolled output, data and export share the same filter-schema, template includes the dropdown partial and uses endpointUrl(), page module imports and invokes initListExport, and the shared dropdown partial stays zero-config. - pages/admin/users/export().php: align with the new contract by switching from ad-hoc requestInput()->queryAll() reads to gridParseFiltersFromSchemaFile(__DIR__ . '/filter-schema.php'), same as the data endpoint — eliminates the last place Grid and export could disagree on what "the current filter" means. Gates: PHPUnit 1900 OK (+6 contract checks), PHPStan 0 errors. Co-Authored-By: Claude Opus 4.7 (1M context) --- .agents/checks/guard-catalog.json | 7 + .agents/checks/guard-checklist.md | 1 + .agents/checks/guard-enforcement-map.json | 7 + .agents/prompts/reviewer-code.md | 1 + CLAUDE.md | 6 + pages/admin/users/export().php | 43 ++--- .../Architecture/ListExportContractFiles.php | 60 +++++++ tests/Architecture/ListExportContractTest.php | 158 ++++++++++++++++++ 8 files changed, 256 insertions(+), 27 deletions(-) create mode 100644 tests/Architecture/ListExportContractFiles.php create mode 100644 tests/Architecture/ListExportContractTest.php diff --git a/.agents/checks/guard-catalog.json b/.agents/checks/guard-catalog.json index 98a8674..b05ac07 100644 --- a/.agents/checks/guard-catalog.json +++ b/.agents/checks/guard-catalog.json @@ -92,6 +92,13 @@ "title": "Reuse-first (JS + CSS)", "requirement": "MUST check web/js/core/ and web/js/components/ for existing exports before adding new functions. MUST check existing CSS layers and app-* classes before writing new rules. No duplication of logic from core modules." }, + { + "id": "GR-UI-EXPORT", + "area": "ui", + "reviewer": "code", + "title": "List export via core primitive", + "requirement": "Grid.js list pages that offer CSV/Excel downloads MUST use the core export primitive end-to-end. Server endpoints MUST call exportRequireGetRequest(), cap rows via exportCapLimit(), stream with exportSendCsv() from core/Support/helpers/export.php, and declare columns as MintyPHP\\Service\\Export\\ExportColumn. The SAME filter-schema.php as the list's data endpoint MUST be parsed via gridParseFiltersFromSchemaFile() so exported rows match the grid under identical filters. Views MUST include templates/partials/app-list-export-dropdown.phtml in $listTitleActionsHtml and pass 'exportUrl' => endpointUrl('') in the page config (never plain lurl() — query-string survival on module routes where path != target depends on endpointUrl). Page JS MUST call initListExport({ gridConfig, exportUrl }) from /js/core/app-list-export.js. No inline fputcsv, no hand-rolled headers, no bespoke click listener." + }, { "id": "GR-SEC-001", "area": "security", diff --git a/.agents/checks/guard-checklist.md b/.agents/checks/guard-checklist.md index 7e3bd02..3893943 100644 --- a/.agents/checks/guard-checklist.md +++ b/.agents/checks/guard-checklist.md @@ -17,6 +17,7 @@ Guard IDs live in `.agents/checks/guard-catalog.json` (guards where `reviewer` = ## UI (skip if backend-only) - `GR-UI-014` new views/components handle loading, empty, error, success states - `GR-UI-LIST` list/grid standards preserved (row actions, page size, initStandardListPage) +- `GR-UI-EXPORT` list exports go through the core primitive: server uses exportRequireGetRequest + exportCapLimit + exportSendCsv + ExportColumn, shares filter-schema.php with the data endpoint; view uses `app-list-export-dropdown.phtml` partial and `endpointUrl()` (never `lurl()`) for the exportUrl; JS calls `initListExport` from `/js/core/app-list-export.js`. Add the new list to `tests/Architecture/ListExportContractFiles.php`. - `GR-UI-DETAIL` detail page contracts, shared partials, no inline confirm, layout system - `GR-UI-A11Y` no a11y regression; new components use semantic HTML, keyboard-operable - `GR-UI-I18N` all visible text uses t(); keys present in all language files diff --git a/.agents/checks/guard-enforcement-map.json b/.agents/checks/guard-enforcement-map.json index cce1d2e..9e909c9 100644 --- a/.agents/checks/guard-enforcement-map.json +++ b/.agents/checks/guard-enforcement-map.json @@ -127,6 +127,13 @@ ".agents/runs//review-code.json" ] }, + { + "guard_id": "GR-UI-EXPORT", + "enforcement_mode": "automated", + "evidence_source": [ + "tests/Architecture/ListExportContractTest.php" + ] + }, { "guard_id": "GR-SEC-001", "enforcement_mode": "automated", diff --git a/.agents/prompts/reviewer-code.md b/.agents/prompts/reviewer-code.md index e921908..9ccb0d2 100644 --- a/.agents/prompts/reviewer-code.md +++ b/.agents/prompts/reviewer-code.md @@ -25,6 +25,7 @@ Scope — verify these guards: - GR-TEST-002: code is testable - GR-UI-014: UI state completeness (if UI touched) - GR-UI-LIST: list/grid standards (if lists touched) +- GR-UI-EXPORT: list export via core primitive (if list exports touched) - GR-UI-DETAIL: detail page standards (if detail pages touched) - GR-UI-A11Y: accessibility (if UI touched) - GR-UI-I18N: i18n completeness (if visible text touched) diff --git a/CLAUDE.md b/CLAUDE.md index 8a53b6f..b3404c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -171,6 +171,10 @@ docker/ # Dockerfiles, Nginx configs, PHP configs - **Components inside forms**: Reset core `margin-bottom` on buttons/inputs/selects/labels at component scope. Checkboxes in flex need explicit `1.25em` sizing + `flex-shrink: 0`. - **Sticky elements**: Always use `top: calc(var(--app-topbar-height) + ...)` — never `top: 0`. Multi-column grids must have a responsive single-column `@media` fallback. - **File uploads** use `app-file-upload.phtml` partial (deliberate exception to plain-HTML inputs — the markup is too complex for inline use). See `templates/partials/app-file-upload.phtml` for the `$fileUpload` array contract. Never write file-upload markup inline; always use the partial. +- **List exports**: every Grid.js list that offers a download MUST go through the core primitive — no inline `fputcsv`, no hand-rolled headers, no bespoke dropdown. (GR-UI-EXPORT) + - **Server:** endpoint calls `exportRequireGetRequest()` + `exportCapLimit()` + `exportSendCsv()` from `core/Support/helpers/export.php`, builds columns as `ExportColumn[]` (`core/Service/Export/CsvExportService.php`), and reuses the SAME `filter-schema.php` via `gridParseFiltersFromSchemaFile()` as the data endpoint. Extract a shared service or presenter if row-formatting is non-trivial (see `DomainListService` / `SystemAuditRowPresenter`). + - **View:** `require templatePath('partials/app-list-export-dropdown.phtml');` inside `$listTitleActionsHtml`. Pass the URL via `'exportUrl' => endpointUrl('')` in the page config — **always `endpointUrl()`, never plain `lurl()`**. The helper resolves module routes to their target path so query strings survive. + - **Client:** `initListExport({ gridConfig, exportUrl })` from `/js/core/app-list-export.js`. No hand-rolled click listener, no inline URL construction. ### Never Do This @@ -186,6 +190,8 @@ docker/ # Dockerfiles, Nginx configs, PHP configs - Inline SQL or string-concatenated queries (GR-SEC-003) - Raw `echo` in views without `// raw-html-ok` marker — use `e()` (GR-SEC-010) - OpenAPI changes without updating the spec (GR-CORE-007) +- Hand-rolled list exports (inline `fputcsv`, own header setup, bespoke click-to-download JS) — always use `core/Service/Export/*` + `exportSendCsv()` + `app-list-export-dropdown.phtml` + `initListExport()` (GR-UI-EXPORT) +- Building an export/data endpoint URL with `lurl()` when it will carry query params — use `endpointUrl()` so module routes where `path ≠ target` still resolve (GR-UI-EXPORT) ## Database diff --git a/pages/admin/users/export().php b/pages/admin/users/export().php index d8cbc59..3f3f4e6 100644 --- a/pages/admin/users/export().php +++ b/pages/admin/users/export().php @@ -28,35 +28,24 @@ if (!$decision->isAllowed()) { $request = requestInput(); $currentUserId = (int) ($session['user']['id'] ?? 0); -$search = $request->queryString('search'); -$order = (string) $request->query('order', 'id'); -$dir = strtolower((string) $request->query('dir', 'desc')) === 'asc' ? 'asc' : 'desc'; -$active = (string) $request->query('active', 'all'); -$createdFrom = $request->queryString('created_from'); -$createdTo = $request->queryString('created_to'); -$tenant = $request->queryString('tenant'); -$roles = $request->query('roles', ''); -$departments = $request->query('departments', ''); - -$allowedOrder = ['id', 'uuid', 'first_name', 'last_name', 'email', 'locale', 'theme', 'created', 'modified', 'active']; -if (!in_array($order, $allowedOrder, true)) { - $order = 'id'; -} - -$limit = exportCapLimit($request->query('limit'), 5000); +$filters = gridParseFiltersFromSchemaFile(__DIR__ . '/filter-schema.php'); +$filters['limit'] = exportCapLimit($request->query('limit'), 5000); +$filters['offset'] = 0; $result = app(\MintyPHP\Service\User\UserAccountService::class)->listPaged([ - 'limit' => $limit, - 'offset' => 0, - 'search' => $search, - 'order' => $order, - 'dir' => $dir, - 'active' => $active, - 'created_from' => $createdFrom, - 'created_to' => $createdTo, - 'tenant' => $tenant, - 'roles' => $roles, - 'departments' => $departments, + 'limit' => $filters['limit'], + 'offset' => $filters['offset'], + 'search' => $filters['search'], + 'order' => $filters['order'], + 'dir' => $filters['dir'], + 'active' => $filters['active'], + 'created_from' => $filters['created_from'], + 'created_to' => $filters['created_to'], + 'tenant' => $filters['tenant'], + 'roles' => $filters['roles'], + 'departments' => $filters['departments'], + 'email_verified' => $filters['email_verified'], + 'login_status' => $filters['login_status'], 'tenantUserId' => $currentUserId, ]); diff --git a/tests/Architecture/ListExportContractFiles.php b/tests/Architecture/ListExportContractFiles.php new file mode 100644 index 0000000..ebe6488 --- /dev/null +++ b/tests/Architecture/ListExportContractFiles.php @@ -0,0 +1,60 @@ + + */ + private function listExportRegistry(): array + { + return [ + [ + 'endpoint' => 'modules/helpdesk/pages/helpdesk/domains/export().php', + 'data_endpoint' => 'modules/helpdesk/pages/helpdesk/domains-data().php', + 'filter_schema' => 'modules/helpdesk/pages/helpdesk/domains/filter-schema.php', + 'index_template' => 'modules/helpdesk/pages/helpdesk/domains/index(default).phtml', + 'page_module' => 'modules/helpdesk/web/js/pages/helpdesk-domains-index.js', + 'route_path' => 'helpdesk/domains/export', + ], + [ + 'endpoint' => 'pages/admin/users/export().php', + 'data_endpoint' => 'pages/admin/users/data().php', + 'filter_schema' => 'pages/admin/users/filter-schema.php', + 'index_template' => 'pages/admin/users/index(default).phtml', + 'page_module' => 'web/js/pages/app-users-list.js', + 'route_path' => 'admin/users/export', + ], + [ + 'endpoint' => 'modules/audit/pages/audit/system-audit/export().php', + 'data_endpoint' => 'modules/audit/pages/audit/system-audit/data().php', + 'filter_schema' => 'modules/audit/pages/audit/system-audit/filter-schema.php', + 'index_template' => 'modules/audit/pages/audit/system-audit/index(default).phtml', + 'page_module' => 'modules/audit/web/js/pages/admin-system-audit-index.js', + 'route_path' => 'admin/system-audit/export', + ], + ]; + } +} diff --git a/tests/Architecture/ListExportContractTest.php b/tests/Architecture/ListExportContractTest.php new file mode 100644 index 0000000..74e882d --- /dev/null +++ b/tests/Architecture/ListExportContractTest.php @@ -0,0 +1,158 @@ +listExportRegistry() as $entry) { + $content = $this->readProjectFile($entry['endpoint']); + + $this->assertStringContainsString('Guard::requireLogin();', $content, $entry['endpoint']); + $this->assertStringContainsString('exportRequireGetRequest();', $content, $entry['endpoint']); + $this->assertStringContainsString('exportCapLimit(', $content, $entry['endpoint']); + $this->assertStringContainsString('exportSendCsv(', $content, $entry['endpoint']); + $this->assertStringContainsString('exportResolveFlavor(', $content, $entry['endpoint']); + $this->assertStringContainsString('new ExportColumn(', $content, $entry['endpoint']); + $this->assertStringContainsString( + 'MintyPHP\\Service\\Export\\ExportColumn', + $content, + $entry['endpoint'] + ); + } + } + + public function testExportEndpointsBanHandRolledOutputPrimitives(): void + { + foreach ($this->listExportRegistry() as $entry) { + $content = $this->readProjectFile($entry['endpoint']); + + $this->assertStringNotContainsString('fputcsv(', $content, $entry['endpoint']); + $this->assertStringNotContainsString( + "header('Content-Type: text/csv", + $content, + $entry['endpoint'] + ); + $this->assertStringNotContainsString( + "header('Content-Disposition:", + $content, + $entry['endpoint'] + ); + $this->assertStringNotContainsString('php://output', $content, $entry['endpoint']); + } + } + + public function testExportAndDataEndpointsShareTheSameFilterSchema(): void + { + foreach ($this->listExportRegistry() as $entry) { + $export = $this->readProjectFile($entry['endpoint']); + $data = $this->readProjectFile($entry['data_endpoint']); + + $schemaDir = dirname($entry['filter_schema']); + $endpointDir = dirname($entry['endpoint']); + $dataEndpointDir = dirname($entry['data_endpoint']); + $schemaRefFromExport = $this->relativeSchemaRef($endpointDir, $schemaDir); + $schemaRefFromData = $this->relativeSchemaRef($dataEndpointDir, $schemaDir); + + $this->assertStringContainsString( + "gridParseFiltersFromSchemaFile(__DIR__ . '" . $schemaRefFromExport . "/filter-schema.php'", + $export, + $entry['endpoint'] . ' must parse the shared filter-schema.php' + ); + $this->assertStringContainsString( + "gridParseFiltersFromSchemaFile(__DIR__ . '" . $schemaRefFromData . "/filter-schema.php'", + $data, + $entry['data_endpoint'] . ' must parse the same filter-schema.php as the export endpoint' + ); + $this->assertFileExists( + $this->projectRootPath() . '/' . $entry['filter_schema'], + $entry['filter_schema'] + ); + } + } + + public function testIndexTemplateIncludesExportDropdownAndUsesEndpointUrl(): void + { + foreach ($this->listExportRegistry() as $entry) { + $template = $this->readProjectFile($entry['index_template']); + + $this->assertStringContainsString( + "templatePath('partials/app-list-export-dropdown.phtml')", + $template, + $entry['index_template'] . ' must require the shared export dropdown partial' + ); + $this->assertStringContainsString( + "endpointUrl('" . $entry['route_path'] . "')", + $template, + $entry['index_template'] . ' must build exportUrl via endpointUrl(), not lurl()' + ); + $this->assertStringNotContainsString( + "'exportUrl' => lurl(", + $template, + $entry['index_template'] . ' must not use lurl() for exportUrl (use endpointUrl())' + ); + } + } + + public function testPageModuleInvokesInitListExportFromCore(): void + { + foreach ($this->listExportRegistry() as $entry) { + $module = $this->readProjectFile($entry['page_module']); + + $this->assertMatchesRegularExpression( + "#\\bimport\\s*\\{[^}]*\\binitListExport\\b[^}]*\\}\\s*from\\s*['\"](?:\\.\\./core|/js/core)/app-list-export\\.js['\"]#s", + $module, + $entry['page_module'] . ' must import initListExport from the core module' + ); + $this->assertStringContainsString( + 'initListExport(', + $module, + $entry['page_module'] . ' must invoke initListExport({ gridConfig, exportUrl })' + ); + } + } + + public function testExportDropdownPartialRemainsZeroConfigAndReusesCoreClasses(): void + { + $partial = $this->readProjectFile('templates/partials/app-list-export-dropdown.phtml'); + $this->assertStringContainsString('data-list-export="csv"', $partial); + $this->assertStringContainsString('data-list-export="excel"', $partial); + $this->assertStringContainsString("
assertStringNotContainsString('assertStringNotContainsString('