From 0f9dd3fa74858b12c076787424a86d0b561dc76c Mon Sep 17 00:00:00 2001 From: "Beau Beauchamp, WebTigers" Date: Sat, 24 Oct 2026 13:16:59 -0400 Subject: [PATCH 1/2] fix(system): fetch the Updates history grid from /api instead of server-rendering it (TIGER-383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The updates history was the one admin grid that did not follow the client/server contract: the controller handed the view `recent(15)` and the view looped it into rows. WEBSERVICES.md §5 is explicit that list/table data is fetched from /api, never server-rendered. The consistency argument is the point, but there was a real gap behind it: dev had 24 history rows and the screen could only ever show 15. Nine runs existed that nobody could reach, with no paging, no search and no way to ask "what happened last month?". Converted to the house DataTables pattern, following Cms_Service_Page::datatable and the Access user grid rather than inventing a variant: Tiger_Model_UpdateHistory::datatable() owns the query — search, sort, paging, and both counts. A service never builds a predicate (AGENTS.md), so the finder is named and lives on the model. System_Service_Updates::datatable() _dtParams() -> finder -> _dtResponse(). Rows are STRUCTURED data only, never HTML; the step log rides along so the expandable detail costs no second round trip. The controller renders the shell and sets useDataTables. The view is an empty ; the grid init lives in the module's JS ASSET (this screen already loads one — no new inline diff --git a/tests/Integration/System/UpdatesControllerTest.php b/tests/Integration/System/UpdatesControllerTest.php new file mode 100644 index 00000000..f5a3bdb0 --- /dev/null +++ b/tests/Integration/System/UpdatesControllerTest.php @@ -0,0 +1,69 @@ +priorUnitTestMode = Zend_Session::$_unitTestEnabled; + Zend_Session::$_unitTestEnabled = true; + $_SESSION = []; + } + + protected function tearDown(): void + { + $_SESSION = []; + Zend_Session::$_unitTestEnabled = $this->priorUnitTestMode; + parent::tearDown(); + } + + #[Test] + public function index_renders_the_updates_shell_and_opts_into_the_dataTables_assets(): void + { + $this->loginAs('superadmin'); + $res = $this->dispatchAction(System_UpdatesController::class, 'index', [], 'GET'); + $this->assertSame(200, $res->getHttpResponseCode()); + + $view = $this->controller()->view; + $this->assertSame('Updates — Tiger Admin', $view->title); + $this->assertTrue((bool) $view->useDataTables, 'the history grid needs the DataTables assets, or it renders empty'); + } + + #[Test] + public function the_controller_no_longer_server_renders_the_history_rows(): void + { + $this->loginAs('superadmin'); + $this->dispatchAction(System_UpdatesController::class, 'index', [], 'GET'); + + // The guard for the regression this screen just came back from: if a `history` view-var + // reappears, someone has gone back to rendering rows on the server (WEBSERVICES.md §5). + $this->assertNull($this->controller()->view->history ?? null, + 'history rows come from /api, not a controller view-var'); + } +} diff --git a/tests/Integration/System/UpdatesServiceTest.php b/tests/Integration/System/UpdatesServiceTest.php index f379523f..b02ff908 100644 --- a/tests/Integration/System/UpdatesServiceTest.php +++ b/tests/Integration/System/UpdatesServiceTest.php @@ -78,6 +78,62 @@ public function apply_with_no_selection_is_a_clean_error_not_a_crash(): void $this->assertStringNotContainsString('not_allowed', $this->messages($res), 'superadmin cleared the ACL gate first'); } + #[Test] + public function datatable_returns_the_dataTables_envelope_and_structured_rows_only(): void + { + $this->loginAs('superadmin'); + (new \Tiger_Model_UpdateHistory())->record([ + 'item_type' => 'module', 'item_slug' => 'dt-probe', 'item_name' => 'DT Probe', + 'from_version' => '1.0.0', 'to_version' => '1.1.0', 'outcome' => 'success', + 'log' => [['step' => 'download', 'detail' => 'ok', 'ok' => true]], + ]); + + $res = $this->dispatch(['action' => 'datatable', 'draw' => 3, 'start' => 0, 'length' => 10]); + + $this->assertSame(1, (int) $res->result, 'the grid read succeeds'); + $this->assertSame(3, (int) $res->data['draw'], 'draw is echoed back, as DataTables requires'); + $this->assertArrayHasKey('recordsTotal', $res->data); + $this->assertArrayHasKey('recordsFiltered', $res->data); + $this->assertGreaterThan(0, (int) $res->data['recordsTotal'], 'the row we just recorded is counted'); + + $row = $res->data['data'][0]; + // WEBSERVICES.md §5: rows are STRUCTURED data only — never HTML. + foreach (['item_name', 'item_type', 'versions', 'outcome', 'created', 'log'] as $k) { + $this->assertArrayHasKey($k, $row, "the row carries $k"); + } + $this->assertIsArray($row['log'], 'the step log rides along for the child row'); + $this->assertStringNotContainsString('<', json_encode($row), 'no markup is built on the server'); + } + + #[Test] + public function datatable_search_narrows_recordsFiltered_without_changing_the_total(): void + { + $this->loginAs('superadmin'); + $model = new \Tiger_Model_UpdateHistory(); + $tag = 'zz-' . bin2hex(random_bytes(4)); + $model->record(['item_type' => 'module', 'item_slug' => $tag, 'item_name' => $tag, + 'from_version' => '1.0.0', 'to_version' => '1.0.1', 'outcome' => 'success', 'log' => []]); + $model->record(['item_type' => 'module', 'item_slug' => 'other-one', 'item_name' => 'Other One', + 'from_version' => '1.0.0', 'to_version' => '1.0.1', 'outcome' => 'failed', 'log' => []]); + + $all = $this->dispatch(['action' => 'datatable', 'draw' => 1, 'start' => 0, 'length' => 10]); + $hit = $this->dispatch(['action' => 'datatable', 'draw' => 2, 'start' => 0, 'length' => 10, + 'search' => ['value' => $tag]]); + + $this->assertSame((int) $all->data['recordsTotal'], (int) $hit->data['recordsTotal'], + 'the unfiltered total does not move when the search box is used'); + $this->assertSame(1, (int) $hit->data['recordsFiltered'], 'the search narrows to the one match'); + $this->assertSame($tag, $hit->data['data'][0]['item_name']); + } + + #[Test] + public function datatable_is_denied_to_a_plain_admin(): void + { + $this->loginAs('admin'); + $res = $this->dispatch(['action' => 'datatable', 'draw' => 1]); + $this->assertSame(0, (int) $res->result, 'the history grid is superadmin+, like the rest of the updater'); + } + #[Test] public function history_returns_a_list_for_a_superadmin(): void { From 695f966d6f37240f4e6752c098437eab3381306b Mon Sep 17 00:00:00 2001 From: "Beau Beauchamp, WebTigers" Date: Sat, 24 Oct 2026 13:22:54 -0400 Subject: [PATCH 2/2] test(system): update the dispatch test to the new history contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this and my local run did not: SystemControllerDispatchTest asserts the Updates screen's view-vars, including `history` as an array — the very var this change removes. I had filtered my local run to UpdatesService|UpdatesController, and this file's name mentions neither, so it never ran. The assertion is updated to the contract that now holds rather than the behaviour being replaced: useDataTables is set, and `history` is absent because the rows come from /api. Full integration suite run unfiltered this time: 1552 tests green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013vdyTeoDrGE4hKxt53TsE8 --- tests/Integration/System/SystemControllerDispatchTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Integration/System/SystemControllerDispatchTest.php b/tests/Integration/System/SystemControllerDispatchTest.php index fa1c358a..902d2769 100644 --- a/tests/Integration/System/SystemControllerDispatchTest.php +++ b/tests/Integration/System/SystemControllerDispatchTest.php @@ -163,7 +163,11 @@ public function the_updates_screen_reports_detection_without_reaching_the_networ $this->assertIsArray($this->controller()->view->updates, 'the checker result is passed to the view'); // Seeded low "latest" → nothing pending → the pending list is empty (and no notes were fetched). $this->assertSame([], $this->controller()->view->pending); - $this->assertIsArray($this->controller()->view->history); + // History rows are no longer a view-var: the grid fetches them from /api + // (System_Service_Updates::datatable, TIGER-383). The controller's job here is to + // opt into the DataTables assets — without that flag the grid renders empty. + $this->assertTrue((bool) $this->controller()->view->useDataTables, 'the history grid needs the DataTables assets'); + $this->assertNull($this->controller()->view->history ?? null, 'history is fetched, not server-rendered'); } // ---- cache seeding helpers -------------------------------------------------------------------