Add one shared builder for signed paid booking rows - #1829
Conversation
Extract bookingDateFields into src/shared/booking-date-fields.ts and route the paid webhook, store-and-refund, and public/free paths through it. Add src/shared/booking-lines.ts with orderBookings: one pure builder for the signed paid listing_attendees rows (date/duration, package path, allocation expansion, sole-parent package stamping, shared order token). A genuine pricePaid 0 stays distinct from an omitted price. Replace create.ts's paid-row construction with orderBookings; payment finalization and error behaviour are unchanged (the DB layer still coerces an undefined pricePaid to 0, so no observable difference). Make expandChildAllocations and expandBooking generic over T extends ListingBooking so required row fields survive expansion. Update store-refund.ts's date-helper import only. Move the bookingDateFields tests to test/shared/booking-date-fields.test.ts and add the missing-dayCount case; add focused orderBookings tests for standalone and tagged package rows, pricePaid 0 vs omitted, empty and multiple allocations, parentless remainder, exact price conservation, shared order token, and sole-parent vs mixed-parent stamping. No checkout-stage schema, activation, refund, cleanup, backup, or admin-lock changes.
|
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 (1)
📝 WalkthroughWalkthroughBooking date derivation and allocation-aware booking construction are centralized in shared modules. Attendee creation uses the shared builder, while payment and refund flows import the shared date helper. Tests cover date rules, price omission, allocation expansion, price conservation, and package stamping. ChangesBooking row centralization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant createAttendeeForSession
participant orderBookings
participant expandChildAllocations
createAttendeeForSession->>orderBookings: signed lines, date, dayCount, allocations
orderBookings->>expandChildAllocations: allocation-aware booking rows
expandChildAllocations-->>orderBookings: expanded rows with quantities and prices
orderBookings-->>createAttendeeForSession: ListingBooking rows
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@src/features/api/payment-processing/create.ts`:
- Around line 264-274: Update the booking-line mapping inside orderBookings to
conditionally spread pricePaid only when
paidByIntentItem.get(pricingIntent.items[index]!) is defined, preserving 0 as a
valid value and omitting the property otherwise; keep the remaining fields
unchanged.
In `@test/shared/booking-lines.test.ts`:
- Around line 88-89: Replace the compound .every(...).toBe(true) assertions in
the booking-lines test with explicit per-row assertions for orderToken and
parentListingId, including the checks around the additional assertions at lines
108-113. Keep the result length and row-presence validation intact so dropped or
incorrect rows are detected with row-specific failure output.
- Around line 162-170: Remove the unnecessary nullish fallback from the
childPackageFor helper’s find result. Keep the non-null assertion on the
matching listing and return its numeric packageGroupId directly.
🪄 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: 771c84d3-f268-47fe-83d7-58e79cc723ac
📒 Files selected for processing (9)
src/features/api/payment-processing/create.tssrc/features/api/payment-processing/store-refund.tssrc/features/public/ticket-payment.tssrc/shared/booking-date-fields.tssrc/shared/booking-lines.tssrc/shared/db/attendees/order-parents.tstest/features/public/ticket-payment.test.tstest/shared/booking-date-fields.test.tstest/shared/booking-lines.test.ts
💤 Files with no reviewable changes (1)
- test/features/public/ticket-payment.test.ts
Address CodeRabbit review feedback on PR #1829: - create.ts: pricePaid is now conditionally spread (not assigned as number|undefined) so a genuine 0 stays while an absent price is omitted, satisfying exactOptionalPropertyTypes. OrderBookingsInput.dayCount and allocations are typed `| undefined` to match BookingIntent's optionals so an intent's values pass through directly (the builder handles undefined via bookingDateFields's default and the allocations-length check). - booking-lines.test.ts: replace compound .every().toBe(true) assertions with per-row loops for row-specific failure output and stronger mutation resistance. Remove the dead ?? 0 fallback in childPackageFor so an unexpected undefined surfaces as a real test failure. Confirmed via deno check on create.ts (the CI-failing file) and Biome/CPD on all changed files.
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/booking-lines.test.ts`:
- Around line 173-181: Update the childPackageFor helper to return number rather
than number | undefined, and assert the guaranteed packageGroupId invariant when
retrieving the listingId 20 result. Keep the existing orderBookings lookup and
return behavior unchanged.
🪄 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: a26de6e7-6261-4421-8518-d147010e8c85
📒 Files selected for processing (3)
src/features/api/payment-processing/create.tssrc/shared/booking-lines.tstest/shared/booking-lines.test.ts
Address CodeRabbit follow-up review on PR #1829: childPackageFor now returns number (not number | undefined) and asserts the invariant with a non-null assertion, per the repo's Trust application invariants rule. The builder always sets packageGroupId on every output row (set on the raw line, preserved by expandChildAllocations' spread, kept or stamped by stampChildRowPackages), so undefined is a state the application says is impossible — model it with ! rather than propagating the optionality.
This pull request is the next staged-checkout foundation step. It creates one pure, canonical representation of the signed paid booking rows that both ordinary payment completion and the later staged runtime can use, and lifts the shared booking date/duration rules into their own helper alongside it — the row builder depends directly on those rules, so splitting them would only add ordering without isolating meaningful risk.
What changes
src/shared/booking-date-fields.ts(new): the single definition of how a booking row'sdateanddurationDaysare derived from a listing's type and the buyer's chosen span. Acustomisable_dayslisting uses the chosendayCount; a non-customisabledailylisting uses its fixedduration_days; astandardlisting spans one dateless day. A missingdayCount(a legacy signed session withoutday_count) still means one day.src/shared/booking-lines.ts(new):orderBookings— an explicitly typed builder for the signed paidlisting_attendeesrows. It builds one row per signed line carrying its listing, quantity, date, duration, package path, and paid price; expands child lines into one row per(child, parent)when allocations are present; and stamps a folded child with its parent's package only when that parent books through exactly one path.src/features/api/payment-processing/create.ts: the paid webhook's row construction/allocation/package-stamping block is replaced by oneorderBookingscall. Payment finalization and error behaviour are unchanged. The DB layer still coerces an undefinedpricePaidto0, so removing the old?? 0makes no observable difference; it just lets a genuine0(a free line that was still signed) stay distinct from an omitted price on the in-memory rows.src/features/api/payment-processing/store-refund.ts: only thebookingDateFieldsimport moves to the shared helper.src/features/public/ticket-payment.ts: uses the sharedbookingDateFieldsfor its existing availability and free-booking callers. CurrentResponseHandlerand modular capacity behaviour are preserved.src/shared/db/attendees/order-parents.ts:expandChildAllocationsandexpandBookingare made generic overT extends ListingBookingso any required row fields a caller carries on each booking survive the expansion.Behaviour contract
pricePaidof0is kept distinct from an omitted price.1; daily/customisable combinations keep the current date/day rules; a missing legacyday_countmeans one day.packageGroupId 0where existing writers expect it.Tests
test/shared/booking-lines.test.ts(new): focused pure tests for standalone and tagged package rows,pricePaid0 versus omitted, empty and multiple allocations, parentless remainder, exact price conservation, shared order token, sole-parent stamping, and mixed-parent non-stamping.test/shared/booking-date-fields.test.ts(new): the standard/daily/customisable cases moved out oftest/features/public/ticket-payment.test.ts, plus a missing-dayCountcase and a non-finite-clamp case.Scope excluded
No
checkout_stages, migrations, activation, rollback fences, refund lifecycle, cleanup, expiry, backup split, admin locks, staged callbacks, webhook reconciliation, or wholesale copies of the old reference branch. Newer work on main from PRs #1821–#1826 and #1828 is preserved. Build on the current main commit31417124; #1827 (Stripe webhook setup) was still open at push time, so no main re-merge was needed.Verification
Biome (
lint:ci) and CPD (src+testconfigs) both run clean. Full typecheck and the test suite are left to CI and the commit hook.Summary by CodeRabbit
pricePaid: 0while omittingpricePaidwhen not present to allow correct downstream defaults.