Write the refund and money-handling journeys as readable stories - #1931
Conversation
Ten money tests become two stories: refunding a booking (8 scenarios) and free bookings with repeated actions (5 scenarios). The shared money drivers move out of the e2e folder into test-utils/money, since the stories and the remaining direct tests both use them. Coverage and mutation runs still to come. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 54 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 (1)
📝 WalkthroughWalkthroughThis PR adds payment feature specifications and Cucumber steps for refunds, fees, service charges, free bookings, payment allocation, corrections, and idempotency. It centralizes money test helpers and expands ledger contract coverage. ChangesPayment accounting coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
The money stories are the only place a paid order carries a charge, and a Cucumber run does not feed the coverage gate, so the driver's charge-carrying path gets a direct test: a signed order with a 10 percent charge books and the charge earns its own money. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 208ca932a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Mutation runs over the ledger found seven changes no test caught:
- the account key separator could be deleted, letting ("ab","c") and
("a","bc") share one key; the test now covers that exact collision
- four identity field names could be blanked without any test noticing, so
each is now asserted to be reported by name
- the amount and reverses-id rules could reject the smallest real value and
stay green, so both boundaries are now checked from the valid side
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
The refund steps become three story-focused files — the refund itself, the extra charges, and the money shares — so no step module runs long and a mutation run over one story stays narrow. Reading a booking after checkout now goes through soleBookingOn, which throws with the count when a listing does not hold exactly one, instead of quietly taking the first row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
Mutation runs over the money paths found eight changes no test caught: - a one-unit correction could flow the wrong way and stay green, so the smallest credit now asserts its direction - the three correction kinds could share one event key, which would take the second and third for a replay of the first; each now records its own event - the duplicate-reference and shared-event guards could be deleted, because a later guard's message also matched; both messages are now exact - a one-leg event could be posted twice, so a replay of a single leg now asserts it is skipped Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
The new smallest-credit test repeated the existing credit assertions, so all three direction checks now run through one parameterised check that spells out every expected value rather than re-deriving the rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
The reference a correction is filed under is built from its kind, its id, the size of the change and the moment — the account is not part of it, so the kind is the only thing keeping an income correction apart from a modifier-revenue one of the same size on the same id. With the clock frozen so both share one moment, the test now shows both post. Without the kind in the key the second would be dropped as a replay, and a modifier's correction would silently vanish. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
Each of the three correction kinds now checks its stored key against one rebuilt from the kind, the id, the size of the change and the moment, so dropping the kind from the key can no longer pass unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/test-utils/money/drivers.ts`:
- Around line 271-279: Remove the stale attendee-edit JSDoc describing
hidden/select field extraction and form reposting, leaving only the
ledger-correction documentation for the helper that posts attendee balance
adjustments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 610f29ca-e30c-4246-9ae9-a83cbff21761
📒 Files selected for processing (24)
specs/payments/free-and-repeated-money.featurespecs/payments/refunding-a-booking.featuretest/e2e/accounting/_shared.tstest/e2e/accounting/bulk-merges-and-pwyw.test.tstest/e2e/accounting/free-and-idempotency.test.tstest/e2e/accounting/income-and-corrections.test.tstest/e2e/accounting/refunds-and-conservation.test.tstest/scripts/specs/catalog.test.tstest/shared/accounting/adjustments.test.tstest/shared/accounting/ledger-tx.test.tstest/shared/accounting/store.test.tstest/shared/ledger/account.test.tstest/shared/ledger/reconcile.test.tstest/shared/ledger/validate.test.tstest/specs/steps/extra-charges.tstest/specs/steps/money-actions.tstest/specs/steps/money-shares.tstest/specs/steps/refunds.tstest/specs/support/money.tstest/specs/support/world.tstest/test-utils/money/drivers.test.tstest/test-utils/money/drivers.tstest/test-utils/money/reads.test.tstest/test-utils/money/reads.ts
💤 Files with no reviewable changes (3)
- test/e2e/accounting/refunds-and-conservation.test.ts
- test/e2e/accounting/free-and-idempotency.test.ts
- test/e2e/accounting/_shared.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/test-utils/money/drivers.ts`:
- Around line 271-279: Remove the stale attendee-edit JSDoc describing
hidden/select field extraction and form reposting, leaving only the
ledger-correction documentation for the helper that posts attendee balance
adjustments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 610f29ca-e30c-4246-9ae9-a83cbff21761
📒 Files selected for processing (24)
specs/payments/free-and-repeated-money.featurespecs/payments/refunding-a-booking.featuretest/e2e/accounting/_shared.tstest/e2e/accounting/bulk-merges-and-pwyw.test.tstest/e2e/accounting/free-and-idempotency.test.tstest/e2e/accounting/income-and-corrections.test.tstest/e2e/accounting/refunds-and-conservation.test.tstest/scripts/specs/catalog.test.tstest/shared/accounting/adjustments.test.tstest/shared/accounting/ledger-tx.test.tstest/shared/accounting/store.test.tstest/shared/ledger/account.test.tstest/shared/ledger/reconcile.test.tstest/shared/ledger/validate.test.tstest/specs/steps/extra-charges.tstest/specs/steps/money-actions.tstest/specs/steps/money-shares.tstest/specs/steps/refunds.tstest/specs/support/money.tstest/specs/support/world.tstest/test-utils/money/drivers.test.tstest/test-utils/money/drivers.tstest/test-utils/money/reads.test.tstest/test-utils/money/reads.ts
💤 Files with no reviewable changes (3)
- test/e2e/accounting/refunds-and-conservation.test.ts
- test/e2e/accounting/free-and-idempotency.test.ts
- test/e2e/accounting/_shared.ts
🛑 Comments failed to post (1)
test/test-utils/money/drivers.ts (1)
271-279: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale attendee-edit JSDoc.
Lines 271-275 describe scraping and reposting an edit form, but this helper posts a ledger entry. Retain only the ledger-correction documentation. As per coding guidelines, “Keep comments focused on current behavior; remove historical comments describing replaced implementations.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/test-utils/money/drivers.ts` around lines 271 - 279, Remove the stale attendee-edit JSDoc describing hidden/select field extraction and form reposting, leaving only the ledger-correction documentation for the helper that posts attendee balance adjustments.Source: Coding guidelines
The attendee-balance helper carried a leftover comment describing a form- scraping helper that no longer exists. Comments describe the code as it is, so it goes. Addresses the CodeRabbit review finding on test/test-utils/money/drivers.ts (its inline comment failed to post, so there is no thread to reply on). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
|
Replying here because CodeRabbit's inline comment failed to post, so there is no thread to answer on.
For completeness, the other review findings on this PR are also resolved and answered on their own threads:
Current state: 31 stories / 191 steps pass, 19,496 tests with 100% line and branch coverage, and a 100% mutation kill rate across every money source after killing 18 survivors. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07fd7a5126
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Delete the old refund-and-second-refund test that the story replaced. It was still running the same journey, leaving two paths for one behaviour — my miss when migrating it. - Split the four unrelated rules into their own stories: free bookings, repeated money actions, and how a listing's income adds up. The turned-down refund is a refund rule, so it joins the refund story. - Drive the refund from its own page: the story now opens the refund page, checks it offers the confirm-by-typing box, and submits that form with the page's own token, then reads the message the organiser is shown. - Use the production money formatter instead of a copy of it. The copy signed a zero, which the real page never does, so the test had locked in a format the page does not use. - Check the success message after each income correction, since a failed save redirects too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ce2c67ff5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The income correction now goes through the form on the listing's edit page, the way the refund already does, so a form that stopped rendering the box or pointed somewhere else fails the story. The money breakdown is now read row by row, each figure found by its own label. Before, a page-wide search let the correction row's figure satisfy the net row's check — the two happened to be the same amount. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478f6cd067
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The free booking now goes through the listing's own public page in a visitor's browser, and the Festival correction uses the same shared correction helper as the other stories, so no money story posts a hand-built form any more. Money listings now keep the site's own thank-you page, so a story can read what the customer is shown instead of following a link off the site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b093737a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The one-payment-two-listings rule never refunds anything, so it moves out of the refund story into its own. That story now also proves both listings really hold the booking, not just that the money adds up: a place could be missing while the books still balanced. The service-charge refund now checks the customer actually received the whole 55.00 back. Undoing the charge alone left the books balanced while the customer waited for their money, and the story could not tell. The refund-cash check is now one shared helper both refund stories use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/shared/accounting/ledger-tx.test.ts`:
- Around line 82-86: Update the allTransfers assertion to verify the returned
collection has exactly one element before destructuring or accessing leg.
Replace the indirect rest-array check in the transfer test with a direct count
assertion, then preserve the existing reference and eventGroup assertions for
the single transfer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3e3edd00-b56d-4209-ba31-d3e1c043ea62
📒 Files selected for processing (17)
specs/payments/free-bookings.featurespecs/payments/income-figures-explained.featurespecs/payments/one-payment-many-listings.featurespecs/payments/refunding-a-booking.featurespecs/payments/repeated-money-actions.featuretest/e2e/accounting/bulk-merges-and-pwyw.test.tstest/scripts/specs/catalog.test.tstest/shared/accounting/ledger-tx.test.tstest/specs/steps/extra-charges.tstest/specs/steps/money-actions.tstest/specs/steps/money-shares.tstest/specs/steps/refunds.tstest/specs/support/money.tstest/specs/support/world.tstest/test-utils/money/drivers.tstest/test-utils/money/reads.test.tstest/test-utils/money/reads.ts
💤 Files with no reviewable changes (3)
- test/specs/support/world.ts
- test/test-utils/money/reads.ts
- test/e2e/accounting/bulk-merges-and-pwyw.test.ts
A rest-array check passed on an empty result and then failed while reading the leg, hiding which contract broke. The test now says there is exactly one leg before it reads it.
2 participants
What changed
Ten money tests are now five stories anyone can read.
An organiser refunds a booking
nothing, and the money goes back where it came from — once.
money moves, and the payment provider is not even asked.
and the whole amount comes back on a refund.
turn still add up.
One payment can pay for places on two listings — each listing is paid its
own share, and both places belong to the same order.
A free booking costs nothing — no money is recorded at all, even with a
booking fee set up.
Doing the same money action twice counts it once — a repeated payment
message makes no second booking, and re-saving the same income figure makes no
second correction.
An organiser can see how a listing's income adds up — the page explains its
two income figures line by line, so they can never quietly disagree.
Each story is one capability with its own owner and tags. The ten old tests are
deleted, so there is one way to prove each behaviour.
Real problems this turned up
Running mutation tests over the money code — deliberately breaking it to see
whether anything notices — found eighteen changes that no test caught. Every
one is now covered:
than nothing"; both could be changed to "must be more than one penny" with the
whole suite still passing.
where it should have put it in.
made from what kind it is. Drop the kind, and a charge correction and a
listing correction of the same size on the same day become the same key — the
second is taken for a repeat and silently does nothing.
sharing a reference; every line belonging to one event) because the tests
matched on part of the message and another check's message also matched.
Review caught six more things worth naming, all fixed:
page's own form, so a page that stopped offering the confirm-by-typing box
would still have passed. It now drives the real page.
which the page never does — and a new test had locked that in. The copy is
gone; the story uses the production formatter.
which made it read as if a split were part of refunding. It is now its own
story.
listing had a booking at all. It now names the one booking on each listing and
insists they are the same one.
pocket: undoing the charge keeps the books balanced whether or not the cash
ever left. The story now checks the customer has the whole amount back.
Tidying
The shared money helpers moved out of the
e2efolder intotest/test-utils/money/—drivers.tsfor paying and refunding,reads.tsforreading the books and the money pages — because the stories and the remaining
direct tests both use them. Duplication the checker flagged was merged rather
than worked around, and the refund steps are four story-focused files instead
of one long one.
Cover stays at 100%
Deleting the old tests left no gap in the site's own code — the direct tests
already covered every line. A few helper paths were only reached by a story, and
stories do not count towards cover, so each gained a direct test of its own.
Checks
deno task specs— 31 scenarios, 192 steps, all passdeno task specs:check— 13 stories, 24 rulesdeno task test:coverage— 100% line and branch coverageledger/reverse,account,reconcile,validate,accounting/adjustments,store,listing-money-totals,ledger-txlint:ci,typecheck,cpd(0% duplication) all pass🤖 Generated with Claude Code
https://claude.ai/code/session_01Ga3qdj1PzXyZTcjmXZRqz7