Fix booking balances and mobile controls#1863
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe PR updates attendee balance projections, screenshot scenario setup, reservation child-block rendering, and order widget behavior. It adds related tests, localized guidance, screenshot cleanup documentation, and equivalent-mutant records. ChangesAttendee balance accounting
Screenshot scenario tooling
Reservation child-block rendering
Order widget
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant captureScenario
participant setupAdmin
participant ScreenshotScenario
participant balancePathFor
participant signBalanceToken
captureScenario->>setupAdmin: Pass scenario.setupUsername
setupAdmin-->>captureScenario: Complete setup and login
captureScenario->>ScreenshotScenario: Run scenario with context
ScreenshotScenario->>balancePathFor: Request attendee balance URL
balancePathFor->>signBalanceToken: Sign attendee id
signBalanceToken-->>balancePathFor: Return signed token
balancePathFor-->>ScreenshotScenario: Return /pay/ URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@scripts/screenshots/scenario.ts`:
- Around line 28-29: Update the setupUsername validation in the scenario
candidate validator to accept undefined or only non-blank strings, rejecting
empty and whitespace-only values before setupAdmin uses them. Add regression
coverage for both rejected inputs while preserving valid username and
missing-value behavior.
In `@src/shared/db/attendees/balance.ts`:
- Around line 129-137: Add a clear inline comment beside the non-null assertion
in getAttendeeAmountPaid documenting that the scalar SELECT without a FROM
clause always returns exactly one row. Keep the existing non-null assertion and
amount conversion unchanged.
- Around line 153-157: Update the balance-loading flow around the Promise.all
call to combine getAttendeeBalanceState and getAttendeeAmountPaid into one
batched database query, while keeping getAttendeeOrderRows as the separate
query. Preserve both projections and the existing returned values so the
order-summary path remains within the two-query budget.
🪄 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: 1c3f9fe9-3938-43f4-ac45-e6b6678eed04
📒 Files selected for processing (9)
TODO.mdscripts/mutation/equivalent-mutants.txtscripts/screenshots.tsscripts/screenshots/scenario.tssrc/shared/accounting/projection-sql.tssrc/shared/db/attendees/balance.tstest/lib/server-reservation/edge-cases.test.tstest/shared/accounting/projection-sql.test.tstest/shared/db/attendees/balance.test.ts
![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
tickets/src/shared/db/attendees/balance.ts
Lines 167 to 168 in a8c198a
Avoid flagging discounted paid reservations as unpaid
After this change fullPrice and depositPaid are net cash/owed totals, but buildTemplateData still passes summary.listedFullPrice into attendeeBalanceNotice, whose reservation branch uses Math.max(fullPrice, listedFullPrice) - amountPaid. For a discounted reservation whose balance is cleared while it remains in a reservation status (for example a manual ledger payment, or no paid-default status), this now compares the discounted paid total against the undiscounted list price and shows “still unpaid” for the discount amount instead of the paid-status nudge.
ℹ️ 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".
stefan-burke
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc5c4d050
ℹ️ 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 discounted paid-reservation finding from the Codex review is fixed in a62d831. The attendee form now compares net cash with the immutable saved booking total instead of the listing’s current price. |
stefan-burke
changed the title
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 `@src/shared/db/attendees/balance.ts`:
- Around line 128-132: Update attendeeOrderMoneyStatement to derive the argument
count from the generated SQL rather than hardcoding Array(8). Build the SQL
once, count its “?” placeholders, and use that count to populate args with
attendeeId, preserving the existing query and SqlStatement shape.
🪄 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: 1d022142-d23b-43db-a277-f6bc2ed82eec
📒 Files selected for processing (24)
scripts/mutation/equivalent-mutants.txtscripts/screenshots/scenario.tssrc/features/admin/attendee-page-data.tssrc/shared/accounting/projection-sql.tssrc/shared/db/attendees/balance.tstest/fixtures/screenshots/scenarios/base.tstest/fixtures/screenshots/scenarios/blank-username.tstest/fixtures/screenshots/scenarios/conflicting-options.tstest/fixtures/screenshots/scenarios/empty-username.tstest/fixtures/screenshots/scenarios/invalid-css.tstest/fixtures/screenshots/scenarios/invalid-full-page.tstest/fixtures/screenshots/scenarios/invalid-name-format.tstest/fixtures/screenshots/scenarios/invalid-name-type.tstest/fixtures/screenshots/scenarios/invalid-run.tstest/fixtures/screenshots/scenarios/invalid-selector.tstest/fixtures/screenshots/scenarios/invalid-username-type.tstest/fixtures/screenshots/scenarios/non-object.tstest/fixtures/screenshots/scenarios/null.tstest/fixtures/screenshots/scenarios/with-username.tstest/fixtures/screenshots/scenarios/without-username.tstest/lib/server-attendee-form/status.test.tstest/scripts/screenshots-scenario.test.tstest/shared/accounting/projection-sql.test.tstest/shared/db/attendees/balance.test.ts
💤 Files with no reviewable changes (1)
- src/features/admin/attendee-page-data.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a62d831a3b
ℹ️ 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".
1 participant
What changed
Why
Marketing screenshot work exposed three customer-facing problems. Balance recaps could overstate the amount paid or lose the original order price after a refund. Child guidance could show a stale maximum beside the live quantity. External cart controls could collide on mobile.
The balance recap now separates net cash, the fee-free reservation subtotal, and the full amount billed. A discounted paid booking stays paid when a listing's current price is higher. Refunded orders keep their original total. Manual charges are included without changing the reservation deposit base.
Checks
deno task precommitRisk
Summary by CodeRabbit