refactor(actions): parse server-action input with zod - #217
Conversation
.coderabbit.yaml states the rule twice — validate input with zod before touching a service, and parse rather than cast — and no server action followed it. All five action modules now do. Behaviour does not move. Every parse failure maps back to the code the page already renders (`readValidationCode` throws if a schema emits one the destination's error map lacks), and the two error channels stay as they were: payouts returns typed codes as state, the other four throw, matching the posture that page-unreachable input earns no notice copy. `parseYmd` and `battleReportUrlProblem` move to a new payouts/validation module because a "use server" file may only export async functions. The schemas keep every ordering the `if` chains they replace had — declaration order inside `z.object`, refine order inside a chain — and `date_invalid` / `date_future` stay separately reachable rather than collapsing into one refinement. URL validation still precedes the appraisal call, so a bad scheme costs no network round trip. Bound arguments are caller-controlled over the wire, so they are parsed too — after the auth guard, never before, so a caller who fails the guard learns nothing about the shape of what they sent.
…ed it
Unifies parseId/parseCharacterId onto the convention assertValid already
uses: throw the code taken off the rejected issue rather than restating it
at the call site, so each code has one spelling. Spells the code on every
step a schema can reject through, since `.positive({ error })` attaches it
to `positive` alone and a non-integer would otherwise surface zod's own
wording. Comment corrections throughout.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
Comment |
1 participant
Brings all five
src/app/**/actions.tsmodules in line with the zod convention.coderabbit.yamlalready states (lines 94 and 115). Every one of them returned 0 forgrep -c zodbefore this.Convention conformance, not behaviour change: every existing error code is preserved, no code was merged or split, and no zod message reaches a user.
What changed per file
account/actions.tscharacterIdSchema+parseCharacterId; throws (unreachable input)admin/access-lists/actions.tsidSchema(z.preprocess+ refine) +parseId; throwsadmin/accounts/actions.tsassertValid; throwsadmin/sync/actions.tsjobTypeSchema=z.custom<JobType>(isJobType); throwspayouts/actions.tsbuildCreateOperationSchema, returning typed codes as stateparseYmdandbattleReportUrlProblemmoved to a newsrc/app/payouts/validation.tsso the schemas and their tests can import them — a"use server"file may only export async functions, so nothing testable can live beside the actions.createOperationActionFolded into one schema parsed before
appraiseLoot, with the date parse and the future-date check as separate schema steps.date_invalidanddate_futurestay separately reachable — a single collapsing refinement would have been a regression. URL validation still precedes the appraisal call, so a bad scheme never triggers a network call that gets thrown away.todayUtcis threaded in viabuildCreateOperationSchema(todayUtc)rather than read inside the schema, keeping the schema a pure function of its inputs.Error-code preservation
Codes are read back off the rejected issue (
issues[0].message, which is what each schema'serror:option puts there) rather than restated at the call site, so each code has exactly one spelling. That makes theerror:options load-bearing, which is why they are spelled on every rejecting path:z.number().int().positive({ error })attaches the code topositivealone, so a non-integer would otherwise throw zod's own"Invalid input: expected int, received number".Per AGENTS.md item 22, both newly load-bearing options were demonstrated with the fix surgically removed and the assertions untouched:
The payouts coverage tests derive each code from a real parse rather than a hand-copied list, so a typo'd
error:fails them instead of passing a stale map.Ordering
Every site parses after its auth guard, not before, so an unauthenticated caller gets the guard's own redirect rather than a validation error telling them whether their argument had the right shape.
Notes for the reviewer
Number("0x10")→16, so a hex spelling of an id is accepted. This is byte-identical to the chain that was there before; changing it would be a behaviour change this PR is not.admin/accounts' bound arguments had no prior validation and therefore no prior error string, soinvalid_account_id/invalid_tier/invalid_status/invalid_list_search/invalid_identity/invalid_character_idare new. They are thrown, never rendered.satisfies OperationErrorCode. The invariant it would enforce is already covered by tests that derive each code from a real parse, and adding a type layer across 14 sites is the adjacent cleanup this task ruled out./payouts/[id](pre-existing gap, deliberately left alone in design sweep 2: work the ranked backlog #214).Gates
npm run typechecknpm run lintnpm run format:checknpm testnpm run builde2e/{payouts,account,admin,access-lists}.spec.ts