Agent Activity: stop paging a grid whose unit is hosts - #1716
Merged
Conversation
Three faults, one root. Paging counts ROWS; this page's unit is HOSTS. Expanding one host with 83 events at 25 rows a page filled pages one to three with that host and pushed every other host onto page four. rowGroup redraws a group's header on every page the group spans, so the same host then appeared four times, each apparently expanded -- which is what it looked like to the person reading it, and it is not a rowGroup bug. It is what paging by row does when the thing grouped is larger than a page. No page length fixes that, because the number of rows an expansion adds is a property of the host and not of the setting. So paging is off. Collapsed, the grid is one row per host; expanded, it gets longer and you scroll. The seed is still bounded by MAX_HOSTS and each expansion by ROWS_PER_HOST, so this is not "no limit". Scroller is not the alternative -- registerTable() excludes any rowGroup table from it. With paging gone the "entries per page" control has nothing to put in itself, and rendered as an empty box beside its own label. Reported as unreadable in both themes, which it was: there was nothing in it to read. lengthChange: false removes the control rather than styling an empty one. And the toolbar's Refresh is dt.clear().draw() + ajax.reload() -- it throws the rows away and re-fetches the seed. The per-host maps survived that, so a host still marked `loaded` was never re-fetched, its rows were gone, and clicking its header did nothing at all. It read as the expander breaking permanently after one press of Refresh. They now reset on xhr.dt, which fires when the new seed lands. Verified on the live install: 21 group headers collapsed and 21 expanded with no duplicates, no host displaced, the length control and pager both absent, and expand working again after a Refresh. Three more gates in tests/agent-activity-grouping.test.php, each proven by reintroducing the defect and watching it go red. FOG_BCACHE_VER 363 -> 364. 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.
Follow-up to #1715, from testing it in a real browser. Three faults, one root.
Paging counts rows; this page's unit is hosts.
Expanding
telliottwin11(83 events) at 25 rows a page filled pages 1–3 with that hostand pushed every other host onto page 4 — and rowGroup redraws a group's header on every
page the group spans, so the host appeared four times, each apparently expanded:
That is not a rowGroup bug; it is what paging by row does when the grouped thing is
bigger than a page. And no page length fixes it, because the rows an expansion adds are
a property of the host, not of the setting.
So paging is off. Collapsed, the grid is one row per host; expanded, it gets longer
and you scroll. The seed is still bounded by
MAX_HOSTSand each expansion byROWS_PER_HOST— this is not "no limit". Scroller is not the alternative:registerTable()excludes any rowGroup table from it.The empty "entries per page" box. With paging gone the length control has nothing to
put in itself and rendered as an empty box beside its own label — reported as unreadable
in both themes, which it was, because there was nothing in it to read.
lengthChange: falseremoves the control instead of styling an empty one.Refresh killed the expander. The toolbar's Refresh is
dt.clear().draw()+ajax.reload(), which throws the rows away and re-fetches the seed. The per-host mapssurvived it, so a host still marked
loadedwas never re-fetched, its rows were gone,and clicking its header did nothing. It read as the expander breaking permanently after
one press. They now reset on
xhr.dt.Verified on the live install
Checked in both themes.
Three more gates in
tests/agent-activity-grouping.test.php(24 checks now), eachproven by reintroducing the defect and watching it go red. Both PHPStan passes clean,
unscoped; suite 335 passed, 1 failed —
certificate-table.test.php, which failsidentically on an unmodified base checkout and is green in CI.
FOG_BCACHE_VER363 → 364.🤖 Generated with Claude Code
https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2