Agent Activity: group by host in the grid, and expand in it - #1715
Merged
Conversation
The expand on this page never worked. Clicking a host's chevron rendered DataTables Responsive's hidden-column list; the nested table it was meant to open was never constructed. Measured on a live install at 1920px with zero columns hidden, so it was not a narrow-viewport artifact. A DataTables row has ONE child slot. registerTable() turns Responsive on for every grid and Responsive claims that slot, so row.child() handed it a table it then overwrote. Nothing threw -- which is why 7b207c2 could fix "the expanded host was stuck at ten rows" by repairing a pager inside a table that did not exist. So there is no child row now. The page is one grid, grouped by host with rowGroup: the header carries the host, its event count and the expand control, and expanding adds that host's events to the same grid as ordinary rows. No table nested in a table, no second scrollbar, no second pager. Three things had to be true for that to hold, and each was found by measuring rather than by reasoning: - A group whose rows are all filtered out renders NO header, so collapsing by filtering would make every collapsed host vanish. Each host's newest event is therefore seeded into the table and never filtered. It anchors the header and doubles as the thing worth seeing when all is collapsed: what each agent last did. - rowGroup starts a new group every time its dataSrc changes down the ORDERED rows. Ordering by time alone let one host's older events fall past the next host's newest and drew its header twice, so a hidden column sorts every row of a host on that host's last-activity time plus its id -- groups stay whole and stay ordered by recency rather than by name. - listem()'s recordsTotal is every row in auditLog, not the host's. The cap notice now reads recordsFiltered; against recordsTotal it told a 134-event host it was truncated at 500. The flat event set stays unbounded (FOG_AUDIT_RETENTION_DAYS defaults to 0, keep forever), which is why the seed is a summary query and each expansion is capped, and why rowGroup over a serverSide grid was never an option. Separately, and not specific to this page: a grid that says `select: false` no longer gets Select All and Deselect All, and a page that says it is not selectable no longer gets "Delete selected". That was decided by a hardcoded list of node names, which this page was never added to -- so it shipped a red Delete selected over a table with no delete route anywhere in FOG (ADR 0021 Decision 8). FOGPage::$selectable replaces the list, and registerTable() drops the two buttons, so both halves are stated where they are enforced. The 33 tables already passing `select: false` stop showing two enabled buttons that did nothing. tests/agent-activity-grouping.test.php covers all of it, and each of its eight gates was proven by reintroducing the defect and watching it go red. It strips comment lines before scanning, because the first run failed on row.child() and on the old node list where both appear only inside the comments saying never to go back to them. tests/agent-activity-page.test.php had pinned the arrangement this replaces -- "the grid does not use rowGroup" and the child table going through registerTable. Those three checks now assert the opposite, and one of them that no child table comes back by any route. FOG_BCACHE_VER 362 -> 363. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The expand never worked
Clicking a host's chevron on this page rendered DataTables Responsive's hidden-column
list. The nested table it was meant to open was never constructed. Measured on a live
install at 1920px with zero columns hidden, so it was not a narrow-viewport artifact:
A DataTables row has one child slot.
registerTable()turns Responsive on for everygrid and Responsive claims that slot, so
row.child()handed it a table it thenoverwrote. Nothing threw — which is why
7b207c2c9could fix "the expanded host wasstuck at ten rows" by repairing a pager inside a table that did not exist.
What it is now
One grid, grouped by host with
rowGroup. The header carries the host, its event countand the expand control; expanding adds that host's events to the same grid as
ordinary rows. No table nested in a table, no second scrollbar, no second pager. The page
opens with every host visible and every group collapsed.
Three things had to be true for that to hold, each found by measuring rather than
reasoning:
rowGroupstarts a new group wheneverdataSrcchanges down the ordered rowslistem()'srecordsTotalis every row inauditLog(1435 on the lab install), not the host'srecordsFiltered; againstrecordsTotalit told a 134-event host it was truncated at 500The flat event set stays unbounded —
FOG_AUDIT_RETENTION_DAYSdefaults to0, keepforever — which is why the seed is a summary query, each expansion is capped, and
rowGroupover aserverSidegrid was never an option (it groups within one page).Select and delete, on every page
Not specific to this page. A grid that passes
select: falseno longer gets Select Alland Deselect All, and a page that declares itself unselectable no longer gets "Delete
selected".
That was decided by a hardcoded list of node names —
['plugin', 'task', 'activity', 'audit']— which this page was never added to, so it shipped a red Delete selectedover a table with no delete route anywhere in FOG (ADR 0021 Decision 8).
FOGPage::$selectablereplaces the list andregisterTable()drops the two buttons, soeach half is stated in the layer that enforces it. The 33 tables already passing
select: falsestop showing two enabled buttons that did nothing.Tests
tests/agent-activity-grouping.test.php— 21 checks. Each of its eight gates wasproven by reintroducing the defect and watching it go red, then restored. It strips
comment lines before scanning: the first run failed on
row.child()and on the old nodelist, both of which appear only inside the comments that say never to go back to them.
tests/agent-activity-page.test.phphad pinned the arrangement this replaces ("the griddoes not use rowGroup", the child table going through
registerTable). Those threechecks now assert the opposite, and one of them that no child table returns by any route.
Both PHPStan passes clean, unscoped. Full suite 335 passed, 1 failed —
certificate-table.test.php, which fails identically on an unmodified checkout of thebase and is green in CI.
FOG_BCACHE_VER362 → 363. No route changed, soOpenAPI::document()is untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2