Skip to content

feat(payouts): make creating an operation one screen, and editing it … - #124

Merged
guarzo merged 1 commit into
mainfrom
worktree-payout-flow-restructure
Aug 5, 2026
Merged

feat(payouts): make creating an operation one screen, and editing it …#124
guarzo merged 1 commit into
mainfrom
worktree-payout-flow-restructure

Conversation

@guarzo

@guarzo guarzo commented Aug 5, 2026

Copy link
Copy Markdown
Owner

…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 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.

Pull request

What changed and why

What CI cannot check

Deploy notes

Flags

Summary by CodeRabbit

  • New Features

    • Redesigned payout details with inline editing, clearer workflow controls, visible item tables, participant actions, and status warnings.
    • Create operations with optional loot and roster data in a single submission.
    • Preserve entered information and show clear validation or appraisal errors after rejected submissions.
    • Added inline edits that save without navigating away, with keyboard-friendly cancel and focus behavior.
    • Added confirmation protection for roster replacement and stage-aware primary actions.
  • Bug Fixes

    • Preserved loot-paste content after unsuccessful submissions.
    • Improved narrow-screen form sizing and restored consistent control dimensions.

…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.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a85b25ba-7848-4ccf-8c21-8a83736626a4

📥 Commits

Reviewing files that changed from the base of the PR and between 31b21a7 and 19f9d4c.

📒 Files selected for processing (11)
  • e2e/payouts.spec.ts
  • src/app/globals.css
  • src/app/payouts/[id]/appraise-form.tsx
  • src/app/payouts/[id]/inline-edit.tsx
  • src/app/payouts/[id]/page.tsx
  • src/app/payouts/actions.ts
  • src/app/payouts/errors.ts
  • src/app/payouts/new/new-operation-form.tsx
  • src/app/payouts/new/page.tsx
  • src/services/payouts.ts
  • tests/payouts-service.test.ts

📝 Walkthrough

Walkthrough

The PR replaces redirect-based payout creation errors with controlled useActionState forms, adds transactional creation with optional loot and roster contents, redesigns payout details around inline editing and staged controls, and updates styling and E2E coverage.

Changes

Payout creation and persistence

Layer / File(s) Summary
Composer submission and transactional creation
src/app/payouts/actions.ts, src/app/payouts/errors.ts, src/app/payouts/new/*, src/services/payouts.ts, tests/payouts-service.test.ts
The new composer submits controlled name, date, loot, and roster fields. The action returns typed rejection state, appraises loot, resolves roster entries, and creates contents in one transaction. Service tests cover audit records, recalculation, and rollback.

Payout detail editing

Layer / File(s) Summary
Inline editing and staged payout controls
src/app/payouts/[id]/*, src/app/globals.css
The detail page adds inline editing for operation, loot, roster, and participant fields. It reorganizes loot and roster controls, adds confirmation scopes and workflow-stage primary controls, preserves rejected appraisal input, and updates responsive disclosure styling.

Validation coverage

Layer / File(s) Summary
Workflow interaction coverage
e2e/payouts.spec.ts
E2E tests cover rejected creation, preserved form data, inline validation, independent editors, roster replacement confirmation, flat-pool placement, composer-based creation, and draft-stage primary controls.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

  • guarzo/authGD#65: Directly overlaps the payout creation action, appraisal form, payout service, and detail page.
  • guarzo/authGD#96: Previously modified the payout detail page, roster UI, inline-edit styling, and related E2E coverage.
  • guarzo/authGD#91: Shares payout form primitives and submission flows such as Submit and ConfirmSubmit.

Poem

I’m a rabbit reviewing payouts tonight,
Inline edits hop into sight.
Loot and rosters travel as one,
Failed forms keep what they’ve begun.
Confirm, recalculate, and save—
The payout flow is neat and brave!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-payout-flow-restructure
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch worktree-payout-flow-restructure

Comment @coderabbitai help to get the list of available commands.

@guarzo
guarzo enabled auto-merge (squash) August 5, 2026 21:02
@guarzo
guarzo merged commit 8e71969 into main Aug 5, 2026
6 of 7 checks passed
guarzo added a commit that referenced this pull request Aug 6, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant