Skip to content

Reports: name the eight plugin reports, and let them export - #30

Merged
mastacontrola merged 1 commit into
mainfrom
report-titles
Aug 29, 2026
Merged

Reports: name the eight plugin reports, and let them export#30
mastacontrola merged 1 commit into
mainfrom
report-titles

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

Consumes the REPORT_TITLE_DATA seam 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:

Report Sidebar said The page said
ou_report Ou Report Export OUs
ldap_report Ldap Report Export LDAP Servers
location_report Location Report Export Locations
windowskey_report Windowskey Report Export Windows Keys
wolbroadcast_report Wolbroadcast Report Export WOL Broadcasts
subnetgroup_report Subnetgroup Report Export Subnet Groups
taskstateedit_report Taskstateedit Report Export Task States
tasktypeedit_report Tasktypeedit Report Export Task Types

Each plugin's existing menu hook now names its own report, keyed the way the menu and the base64 f parameter already are. The 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 — 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 serves reportRows().

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 drop getList() for reportRows() and ask for the button with {fullExport: true}.

Verification

tests/report-titles-are-registered.test.php105 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() alongside reportRows(); asking for fullExport without 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.csv with real rows.

Order

⚠️ Needs fogproject#1470 merged first, then a release here, then a FOG_PLUGINS_VERSION bump in core. Core older than fogproject#1467 has no ReportManagement::getList() for reportRows() to feed, so shipping this to an unbumped pin would leave those grids empty.

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>
@mastacontrola
mastacontrola merged commit fa8fbd5 into main Aug 29, 2026
2 checks passed
@mastacontrola
mastacontrola deleted the report-titles branch August 29, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant