Repository navigation
feat(reports): the four questions a back office asks, behind reports.read - #25
Merged
Merged
Conversation
Item 3 of docs/admin-api.md: the permission has existed since auth with no route behind it. Four read-only aggregates, and half the spec is about what counts as a sale — CREATED is not money yet, and CANCELLED/REFUNDED both restock, so neither is revenue either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…read Four read-only aggregate routes, gated on the permission that has existed since auth with nothing behind it. Aggregation happens in Postgres; the Node side only transports. One definition of a sale, shared by all four: PAID, SHIPPED or DELIVERED, timed by paidAt. CREATED is stock off the shelf with no money behind it, and CANCELLED/REFUNDED both restock — so leaving them out also keeps 'units sold' and 'stock that left the shelf' agreeing. Weeks and months are cut in REPORTS_TIMEZONE (default UTC), and the SQL anchors the naive TIMESTAMP column in UTC before moving it there: the short 'AT TIME ZONE tz' form converts in the opposite direction, which is a silent three-hour error for a Brazilian store. They answer 403, not 404 — the 404 pattern protects the existence of a resource, and no route here takes an id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The unit tests prove what gets bound; only a real database falsifies the query. Three claims in particular: a sale at 01:30 UTC on 1 September buckets into AUGUST for a Sao Paulo store, a month with no sales comes back as zeros rather than as a gap, and 'not moving' is the exact complement of 'sold' against the same rows. Orders are written straight to the database: paidAt is stamped by the state machine and no route backdates it, so a suite driving checkout could only ever report on today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes item 3 of the admin-api survey, out of the order suggested there: the AVESSO panel already existed and already asked. The survey's own warning that 'derived revenue is easy to get wrong' became half the spec, and its three traps were real. Also qualifies the survey's 'every new back-office read must answer 404': that pattern protects the existence of a RESOURCE, so it applies to routes that take an id. These four take none. The test counts in status.md were stale since variant management; they are now the verified figures. Co-Authored-By: Claude Opus 5 <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.
Item 3 of
docs/admin-api.md—reports.readhas been inthe permission catalogue since auth with no route behind it. Four read-only
aggregate routes, and nothing else: the AVESSO panel asked four questions, and
these answer exactly those.
Spec first, as usual:
docs/specs/reports.md.GET /reports/product-salesGET /reports/revenueGET /reports/cartsGET /reports/unsold-productsWhat counts as a sale — half the spec, and the point of it
PAID,SHIPPEDorDELIVERED, timed bypaidAt. OneSOLD_STATUSESconstant, used by all four routes including "not moving", so the two screens
cannot contradict each other.
CREATEDis out, as the survey's own warning predicted: the stock left theshelf but nothing was paid.
CANCELLEDandREFUNDEDare out too — and theargument got stronger on inspection, because both call
StockService.restock, so excluding them is also what keeps "units sold" and"stock that left the shelf" agreeing.
The clock is
paidAt, nevercreatedAt: an order created on the 31st and paidon the 1st is the new month's revenue, which is how the bank statement sees it.
Two things the four questions did not anticipate
paid_atisTIMESTAMP(3)without a zone. Sopaid_at AT TIME ZONE $tzconverts in the opposite direction — it declares the naive reading was
already local. The SQL anchors in UTC first:
A silent three-hour error that any test written around midday would miss, so
there is an e2e that puts a 01:30 UTC September sale into the store's August.
The zone is
REPORTS_TIMEZONE(defaultUTC, validated at boot) — cutting aBrazilian store's weeks in UTC pushes every Sunday evening into the following
Monday.
The 409 from variant removal counts cart lines, not units.
/reports/cartsanswers
SUM(quantity), withlineCountandcartCountalongside — 40 unitsacross 2 carts and 40 across 30 are opposite situations.
cartCountcomes fromthe lines, because checkout leaves the cart row alive and empty.
Two decisions worth arguing with
These answer 403, not 404. The house pattern protects the existence of a
resource from being confirmed to whoever probes an id. No route here takes an
id of any kind, so there is nothing whose existence could leak — a 403 only
reveals that
/reportsexists, which the published OpenAPI already says. ThisPR also qualifies the survey's blanket "every new back-office read must answer
404" to say where it applies. Spec invariant 10.
reportsreads tables it does not own. The one place in the repo thatcrosses a module boundary at the database rather than through a service. The
alternatives were report-shaped methods (
date_trunc, weekGROUP BY) hangingoff
OrdersServiceandProductsService, or summing money in JavaScript — whichthe spec forbids outright. It is safe because of what the module cannot do: only
SELECTs of aggregates, no writes, and it exports nothing, so nothing can dependon it. Spec invariant 11, and
docs/architecture/modules.mdnow records it as the exception rather than leaving it to be discovered.
Verification
lint:check,typecheckandbuildclean. 545 unit tests (43 new) and258 e2e (46 new), the full e2e suite green against the disposable
e2eschema.
openapi.jsonregenerated — 46 operations, four more, in a ninthcontroller.
The split between the suites is deliberate. The unit tests read the bound
parameters back to prove
CREATED,CANCELLEDandREFUNDEDnever reach thedatabase as something to count — an assertion about how the query is built, which
no mock can hide. What only a real Postgres falsifies is the SQL: the timezone
bucketing above, that a month with no sales returns zeros rather than a gap, and
that "not moving" is the exact complement of "sold" against the same rows.
The e2e writes orders straight to the database rather than driving checkout,
because
paidAtis stamped by the state machine and no route backdates it — asuite driving checkout could only ever report on today.
Left out on purpose
Size breakdown, cart value, refunds as their own report, period-over-period
comparison, export, average ticket, conversion. Each is registered in the spec's
deferred decisions with its reason. The panel asked four questions.
🤖 Generated with Claude Code