feat(payouts): make creating an operation one screen, and editing it … - #124
Conversation
…one page Creating a payout operation used to hand the operator an empty shell they filled in across six collapsed disclosures, at roughly 23 full page reloads. Three critique rounds fixed local defects inside that shape and it still read as clunky, because the shape was the defect. /payouts/new is now a composer: name, date, loot paste, roster paste, one submit, landing on a fully-populated operation. Appraisal is a network call and runs before the transaction opens, so a paste that fails to price creates nothing rather than leaving an orphaned shell behind. /payouts/[id] is now a ledger. Loot and roster are always-visible sections rather than disclosures, the pool table is editable at the top level instead of one disclosure deeper, and InlineEdit replaces roughly 18 form + server action + full reload triples with edits that save without navigating. The facts grid no longer leads. "State before action" was misapplied here: a draft ten seconds old has no state, so the grid rendered the operator's own two inputs back at them as findings. It now appears once there is loot or a roster to state. "Add one participant" is demoted rather than removed. It is the only non-destructive amendment path once shares are edited, and its summary now says so. Also fixed, found on the way: - React 19 resets uncontrolled fields after a <form action> submit settles, so AppraiseForm's docblock claim that a rejected paste survived "because nothing ever replaced the DOM" was false. Staying mounted is necessary and not sufficient. Both it and the composer now control their values. - One gold primary per view, per DESIGN.md. There were three at once. The primary now moves with the operator's position: Appraise, then Set roster, then Finalize. - Set roster replaces the roster wholesale and had no confirmation while styled as the encouraged action. It now confirms and names what it drops. - globals.css had no bare details/summary rule, so every page-level disclosure rendered as an unstyled UA widget while the one inside .log rendered correctly. - Duplicate "exclude" accessible names, "mark paid" losing its name after the first payment, four ConfirmArmScopes collapsed to one, 36px hit targets. - A member's own roster row is now marked; accountId was loaded and used only for canUnlock. No migration. Nothing here needed one, and coupling a data change to a UI restructure would make the restructure unrevertable.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe PR replaces redirect-based payout creation errors with controlled ChangesPayout creation and persistence
Payout detail editing
Validation coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Main's #124 rewrote the payout detail page and added its own in-place editor, `InlineEdit`, whose docblock stated the invariant "every action passed to InlineEdit rejects by redirecting, never by returning state" and added: "If that day comes, control the value here rather than adding a special case." This branch's `StringFieldEditState` conversion is that day, so rather than ship two overlapping editors the resolution folds this branch's value preservation into main's component and deletes the one added here (`inline-edit-field.tsx`). All eight in-place editors on the page now behave identically: `setNotesAction` and `setCorpShareAction` were converted to the same state-returning shape. `page.tsx` was resolved to main's structure (it changed there by 1758 lines across three PRs; one commit here touched it by 252), then this branch's one behavioural change — the extracted `AddParticipantForm` and `FlatPoolForm` — re-applied on top. One defect fell out of the combination rather than out of either side. Main made the appraise form conditional on `pools.length === 0`, with a second call site under `pools.length > 0` inside "Add another paste". This branch had just moved the dropped-lines payload out of a `redirect()` and into `useActionState`, pushed to `?dropped=` from an effect — and two call sites under opposite conditions unmount the component on the very commit the first paste succeeds, so that effect never ran and the "N items ignored" notice was lost for the first paste of every operation. The form is now one slot with a `collapsed` prop, which keeps it mounted across the switch and renders identically. Verified: typecheck, lint, format:check clean; npm test 77 files / 1132 passed; test:e2e 210 passed.
…one page
Creating a payout operation used to hand the operator an empty shell they filled in across six collapsed disclosures, at roughly 23 full page reloads. Three critique rounds fixed local defects inside that shape and it still read as clunky, because the shape was the defect.
/payouts/new is now a composer: name, date, loot paste, roster paste, one submit, landing on a fully-populated operation. Appraisal is a network call and runs before the transaction opens, so a paste that fails to price creates nothing rather than leaving an orphaned shell behind.
/payouts/[id] is now a ledger. Loot and roster are always-visible sections rather than disclosures, the pool table is editable at the top level instead of one disclosure deeper, and InlineEdit replaces roughly 18 form + server action + full reload triples with edits that save without navigating.
The facts grid no longer leads. "State before action" was misapplied here: a draft ten seconds old has no state, so the grid rendered the operator's own two inputs back at them as findings. It now appears once there is loot or a roster to state.
"Add one participant" is demoted rather than removed. It is the only non-destructive amendment path once shares are edited, and its summary now says so.
Also fixed, found on the way:
No migration. Nothing here needed one, and coupling a data change to a UI restructure would make the restructure unrevertable.
Pull request
What changed and why
What CI cannot check
Deploy notes
Flags
Summary by CodeRabbit
New Features
Bug Fixes