Evening session: porting 17,000 lines of query builder that had never been tested
The Starting Point
Second session of the day. Billing merged at lunch, so the next step is reporting — a generic report builder: data sources, filter trees, computed fields, report pages made of widgets, scheduling, the lot.
The instruction for this step was fidelity. The source's design is the product here; people have configs stored in a database that were typed years ago and must keep working. So the parity table is exhaustive and the engine gets ported piece for piece.
I should say up front: everything below got rebuilt the next morning, because I was porting from the wrong repository. But the decisions held, so it's worth writing down anyway.
What Actually Happened
17,000 lines of query building with no tests at all
That's the headline number. The source's report engine is enormous and has zero tests, and once I started porting I understood why: it could only ever run on one MySQL instance.
Two portability problems made a test impossible:
LIKEescaping never named its escape character. Works on MySQL's default, breaks elsewhere.- The date/min/max functions the compilers emit were hard-coded. So a computed field literally could not run anywhere but production.
Both went behind a four-function dialect seam. That's what let the engine be tested at all — and I tested it by running the compilers rather than comparing SQL strings, because asserting on generated SQL is how you write a test that passes while the query is wrong.
Every field slug is checked against a whitelist
This is the security model and it's worth stating plainly: a report config is user-supplied structure that becomes a query. Every field slug in a config is checked against the source's whitelist before anything is built, and every path goes through that check. Not "most paths". Every one.
Two computed-field languages, both with a decision worth keeping
The weighted-score language has five component shapes, and one rule I carried with its original reasoning intact: a missing component keeps its weight in the denominator. That's not obviously right — you could argue it should be dropped — but it's what the source does and it's what people's stored scores mean.
The formula compiler has twelve functions, inline expansion of sibling fields and cycle detection. Strict when authored, lenient when run — you get told off while writing it, not while reading a page.
Charts as SVG, not a JavaScript library
Decision taken while building, and it's the one I'd defend hardest:
The charts are drawn as inline SVG in PHP rather than assembled by a chart library in the browser.
Two reasons:
- A report page must render as a PDF, and a scheduled report is an email. Neither has a browser.
- A chart a browser assembles cannot be asserted in a test — and this is a module the source shipped with none.
The payload a library would have been handed is ported exactly, so swapping one back in later is one view.
Every record type is a data source
The other decision. Core has carried a datasource contract since the very first gate — "core says what it means to be reportable, the builder is a module" — and nothing had ever consumed it. This is the caller.
Reporting reads the field declarations modules already write for their own screens. So the reference app has 23 data sources and no module mentions reporting at all.
A bug only the dev database could show
grouped query carried a column that is not in the GROUP BY
MySQL refuses that outright under only_full_group_by. SQLite allows it silently. So the test suite was green and the real database wasn't.
The cause: core's record query orders by created_at desc so a list reads newest first, and a report inherited that ordering ahead of its own. Grouping now clears the source's ordering, and a config with its own sort replaces it. A config with no sort still gets the source's order, which is the sensible default it was there for.
Found by running a grouped report against the dev database rather than the test one. I keep re-learning this.
And a whole package that wasn't in the check
At the end of the session: the reporting package was built, green, and not in bin/check. The root config lists every package's suite and its analysis by hand, and nothing fails when one is missing from either list.
Adding it took the check from 993 tests to 1,046. Fifty-three tests that had only ever run when I ran them directly.
Same failure family as the CI matrix that was missing three packages a fortnight ago: green by omission looks exactly like green. The hand-maintained lists are now recorded as an open item, because that's twice.
What Worked
Page transfer: a page, its items, its reports and its computed fields as one portable file. No id of any kind crosses, sharing never crosses, and the import says what will be missing before it writes anything.
Cross-filtering is a link, not a click handler. So it works without script, and a narrowed page can be sent to somebody — which is most of what people actually want from a dashboard.
Drill regroups at a grain that comes from the item's stored config and never from the request. That's the source's rule and it's a good one: a drilldown cannot ask for a field nobody offered.
Scheduling goes through core's mailer, keeps what it sent, and one schedule's failure doesn't stop the rest.
And a finding recorded rather than carried quietly: the source grew a map, a filter card and a kanban board after the analysis was written — three of its four drift commits. No gate answer covers them, so they're on the parity table as open rather than silently absorbed.
What I Learned
- Test a query compiler by running it, not by asserting on the SQL it produces. String comparison passes while the query is wrong.
- SQLite lets you select a column that isn't in the GROUP BY. MySQL doesn't. If your suites are SQLite and your deployments are MySQL, you will ship this.
- A chart library is a dependency on having a browser. If the output can be a PDF or an email, draw it server-side.
- Hard-coded SQL functions are why 17,000 lines arrived untested. A tiny dialect seam is the difference between testable and not.
- Whitelist every field slug that reaches a query, on every path. "Most paths" is not a security model.
- Hand-maintained lists of things to check will drift. Mine had a whole package missing and nothing failed.
- Carry a rule's reasoning across, not just its behaviour. "A missing component keeps its weight" looks like a bug until you know why.
Tech Notes
- Stack: PHP 8.4, Laravel, MySQL (dev) / SQLite (suites), headless-browser PDF renderer
- Engine: 50,000-row ceiling, 100 in a preview; recursive filter tree, twelve operators; eleven relative-date tokens
- Dialect seam for four functions (date, min, max, and
LIKEescape) so computed fields run on both engines - Charts rendered as inline SVG in PHP — no client library, works in PDF and email
- Every record type is a data source automatically, read from declarations modules already make
bin/check # now includes the reporting package: 993 → 1,046 tests
Zero Focus Dev
Breaking things, learning fast, sharing it all.