fix(payouts): name the operation in the tab, and 404 a malformed id - #80
Conversation
Two ways `/payouts/[id]` told the member something untrue. The tab read "Payout operation" for an operation that isn't there. The segment-scoped `not-found.tsx` shipped in #78 recorded why it couldn't fix that from its own end: a not-found boundary doesn't get to set the title, and `page.tsx`'s static `metadata` is applied even when the page throws. So the title moves to `generateMetadata`, which resolves the same lookup and names the operation — or says it isn't there. That was left as follow-up on the grounds it would cost a second lookup of the same row. It doesn't: both callers go through one `cache()`d loader, and Next resolves metadata and render in the same request. Measured with a counter on the miss path — one lookup per page load, not two. And `/payouts/<not-a-uuid>` returned a 500. The id reached the `uuid` column as a parameter, postgres rejected the cast with 22P02, and the member got "Something broke" — an apology for a server fault, for a mistyped or truncated URL. This is the defect class #74 spent itself fixing across `/payouts/*`. A shape check next to the existing `notFound()` sends it down the 404 path the well-formed-but-missing id already took. The regex is deliberately narrower than postgres's own parser, which was measured accepting braces, hyphenless and oddly-hyphenated forms. None of those can come from this app or a link it renders, and being narrower than the database fails toward the 404 rather than the 500.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Comment |
1 participant
The follow-up from #78 that didn't get actioned. Two ways
/payouts/[id]told the member something untrue.The tab said "Payout operation" for an operation that isn't there
payouts/[id]/not-found.tsxrecorded why it couldn't fix that from its own end: a segment-scoped not-found boundary doesn't get to set the title —page.tsx's ownmetadatais resolved and applied even though the page threw, on hard and soft navigation alike. (The root boundary is the opposite case: no page segment matched, so itsmetadataexport does apply.)So the title moves to the other end.
page.tsxexportsgenerateMetadatainstead of a staticmetadata, and returns the operation's name — or "No such operation", the same words as the heading — for the same lookup that returns null.The second lookup that seemed to cost is free. Both callers go through one
cache()d loader, and Next resolves metadata and render in the same request. Measured rather than assumed: a counter on the miss path logged exactly one lookup per page load, not two.requirePayoutReader()is wrapped the same way — it is several queries, and it now runs once for both.The guard still runs first.
generateMetadataresolves before render, so it calls the guard before the lookup; a visitor who can't read payouts gets the neutral title and no query./payouts/<not-a-uuid>returned a 500error-boundary-audit.mdfinding 7, observed rather than inferred. The id reached theuuidcolumn as a query parameter, postgres rejected the cast with22P02, and the member was shown "Something broke" — an apology for a server fault, for a mistyped or truncated URL. Same defect class #74 spent itself fixing across/payouts/*.A shape check sits next to the existing
notFound(), so a malformed id takes the 404 path the well-formed-but-missing id already took.The regex is deliberately narrower than postgres. Rather than guess what the database would reject, I probed a
::uuidcast on the test database: it also accepts{…}braces, no hyphens at all, and hyphens after any group of four. Those are not accepted here — they can't be produced by this app or any link it renders, so accepting them would only widen the surface, and being narrower than the database fails toward the 404 rather than the 500, which is the safe direction. Uppercase is the one divergence allowed, because postgres normalizes it and the row would genuinely match.Tests
e2e/not-found.spec.ts— new: a malformed id 404s, doesn't hit the error boundary, and titles the tab. The existing soft-nav 404 test now asserts the title too; that's the path where the document is never reloaded, so a stale title would simply persist.e2e/payouts.spec.ts— the title is pinned on a page that exists, opposite the 404 case. Without it, a loader that returned null for everything would still pass.Verification
npm run typechecknpm run lintno-img-elementnpm run format:checknpm testnpm run test:e2enpm run buildƒ /payouts/[id]error.tsxitself is out of scope here.