Reports: name the eight plugin reports, and let them export - #30
Merged
Conversation
Every bundled plugin report showed two names for one screen. The Reports
menu labels an entry with ucwords() of the FILE name, so ou_report.report
.php appeared as "Ou Report" while the page it opened was headed "Export
OUs" -- and the same for LDAP Servers, Locations, Windows Keys, WOL
Broadcasts, Subnet Groups, Task States and Task Types. The file name is
the half nobody chose.
fogproject #1470 added REPORT_TITLE_DATA for exactly this. Each plugin's
existing menu hook now names its own report, keyed the way the menu and
the base64 `f` parameter already are -- the file name with underscores as
spaces, lower case -- and each report reads the same map back through
reportTitle() for its heading, so the two cannot drift apart again.
THE ROWS MOVED TO reportRows(). fogproject #1467 gave report toolbars a
"CSV (All)" button, because the DataTables export buttons beside it can
only see rows the browser is holding -- on a serverSide table, one page.
That button posts to sub=exportAll, which serves reportRows(); a report
still overriding getList() cannot be reached that way, since getList()
exits and nothing can take back control from it. The download would be an
empty file: no error, nothing logged, a CSV that looks like it worked.
So the eight reports drop getList() in favour of reportRows(), and their
tables ask for the button with {fullExport: true}. The option is opt-in in
core for this reason -- a third-party report that has not been converted
must not be handed a button that produces nothing.
Gate: tests/report-titles-are-registered.test.php, 105 checks. Source
analysis rather than execution, because this repository is fetched on its
own and CI has no fogproject checkout -- so the assertions are on the
AGREEMENT between the report file, its class name, its hook and its JS,
which a stray mention cannot satisfy. Six mutations run against it, all
red: misspelling the hook key, dropping the event registration, putting a
literal title back, keeping getList() alongside reportRows(), asking for
fullExport without the seam, and renaming the class so it derives a key
the file does not.
Verified against the lab database with the updated plugins in a shadow
web root: the sidebar reads "Export LDAP Servers", "Export Locations",
"Export OUs" and "Export Windows Keys" (the four installed there), each
page heading matches its entry, and the OU report's CSV (All) returns
export-ous-2026-08-29.csv with real rows.
NEEDS fogproject #1470. Ship this behind a FOG_PLUGINS_VERSION bump made
after that merges; core older than #1467 has no ReportManagement::
getList() for reportRows() to feed.
Co-Authored-By: Claude <noreply@anthropic.com>
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.
Consumes the
REPORT_TITLE_DATAseam added in fogproject#1470.Two names for one screen
The Reports menu labels an entry with
ucwords()of the file name. So every bundled plugin report has a sidebar entry that disagrees with the page it opens:ou_reportldap_reportlocation_reportwindowskey_reportwolbroadcast_reportsubnetgroup_reporttaskstateedit_reporttasktypeedit_reportEach plugin's existing menu hook now names its own report, keyed the way the menu and the base64
fparameter already are. The report reads the same map back throughreportTitle()for its heading, so the two cannot drift apart again.The rows moved to
reportRows()fogproject#1467 gave report toolbars a "CSV (All)" button — the DataTables export buttons beside it can only see rows the browser is holding, which on a serverSide table is one page. That button posts to
sub=exportAll, which servesreportRows().A report still overriding
getList()cannot be reached that way —getList()exits, so nothing can take back control from it — and the download would be an empty file: no error, nothing logged, a CSV that looks like it worked. That is why the option is opt-in in core, and why these eight now dropgetList()forreportRows()and ask for the button with{fullExport: true}.Verification
tests/report-titles-are-registered.test.php— 105 checks. Source analysis rather than execution, because this repo is fetched on its own and CI has no fogproject checkout; so the assertions are on the agreement between four things (report file name, class name, hook key, JS case), which a stray mention cannot satisfy.Six mutations, all red: misspelling the hook key; dropping the event registration; putting a literal title back; keeping
getList()alongsidereportRows(); asking forfullExportwithout the seam; renaming the class so it derives a key the file does not.11/11 in
tests/run-all.sh.Verified against the lab database with these plugins in a shadow web root — the sidebar reads "Export LDAP Servers", "Export Locations", "Export OUs" and "Export Windows Keys" (the four installed there), each page heading matches its entry, and the OU report's CSV (All) returns
export-ous-2026-08-29.csvwith real rows.Order
FOG_PLUGINS_VERSIONbump in core. Core older than fogproject#1467 has noReportManagement::getList()forreportRows()to feed, so shipping this to an unbumped pin would leave those grids empty.