Split the payment-processing file into smaller, focused files - #1692
Conversation
…xemption The 1,985-line payment-processing module was grandfathered out of the noExcessiveLinesPerFile rule. Split it into a payment-processing/ folder of single-purpose files, each well under the limit, and drop its biome.json entry: - metadata.ts — read signed session metadata into a BookingIntent - classify.ts — validate the paid session and prove it is ours (price proof) - cancel.ts — the payment-cancelled/declined page and its retry link - refunds.ts — refund mechanics plus the typed refund reasons (RefundSpec) - package-pricing.ts — re-validate package structure and per-line price drift - items.ts — validate every signed line against the current database - pricing.ts — build the checkout intent, re-price, and gate on price drift - create.ts — turn a validated session into a real attendee - store-refund.ts — keep-and-refund placeholder + balance settle paths - index.ts — the two-phase locked orchestration Splitting surfaced duplication jscpd could not see inside the monolith; fixed with shared helpers rather than ignores: bookingSlot (listing id + package path), datelessGhostBookings, allocatedUnitsByChild, a SessionProcessor type for the two processors, and merging currentOrderTree into orderEdgeDrifted. Importers (webhooks.ts, the two test files) now import from the specific files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J251sRHStWRgsijyynWunC
|
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)
📝 WalkthroughWalkthroughThe payment-processing implementation was split from one module into focused modules for session metadata, validation, pricing revalidation, cancellation, refunds, booking creation, placeholder storage, and orchestration. Imports and tests now reference the new module paths. ChangesPayment processing flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Webhooks
participant processPaymentSession
participant validatePaidSession
participant createAttendeeForSession
participant storeRefundedBooking
Webhooks->>processPaymentSession: payment session id
processPaymentSession->>validatePaidSession: validate provider session
validatePaidSession-->>processPaymentSession: validated session
processPaymentSession->>createAttendeeForSession: create attendee and bookings
createAttendeeForSession-->>processPaymentSession: success or honour failure
processPaymentSession->>storeRefundedBooking: persist failed payment when required
storeRefundedBooking-->>processPaymentSession: terminal payment result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/index.ts`:
- Around line 298-316: Prevent post-commit failures from triggering
storeRefundedBooking: narrow the try/catch around createAttendeeForSession to
only pre-commit operations, or ensure its post-write cleanup path (including
ensureAllBookings) is non-throwing. Preserve refund handling for genuine
pre-commit failures while allowing persisted bookings to complete without
refunding.
In `@src/features/api/payment-processing/items.ts`:
- Around line 135-154: Batch listing and package-pricing database reads in
validateAllItems and loadPackagePricingByGroup instead of awaiting
getListingWithCount and group pricing queries sequentially per item/group; add
or use getListingsWithCount for all required listing IDs and fetch pricing data
in grouped queries while preserving validation and fail-closed behavior.
🪄 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: 7f8852d9-2252-419c-85a1-2639ee3dbede
📒 Files selected for processing (15)
biome.jsonsrc/features/api/payment-processing.tssrc/features/api/payment-processing/cancel.tssrc/features/api/payment-processing/classify.tssrc/features/api/payment-processing/create.tssrc/features/api/payment-processing/index.tssrc/features/api/payment-processing/items.tssrc/features/api/payment-processing/metadata.tssrc/features/api/payment-processing/package-pricing.tssrc/features/api/payment-processing/pricing.tssrc/features/api/payment-processing/refunds.tssrc/features/api/payment-processing/store-refund.tssrc/features/api/webhooks.tstest/features/api/payment-processing/packages.test.tstest/features/api/payment-processing/pair-entries-by-listing.test.ts
💤 Files with no reviewable changes (2)
- biome.json
- src/features/api/payment-processing.ts
Two pre-existing behaviours CodeRabbit flagged on PR #1692 (post-commit refund window in processReservedSession; unbatched per-item DB reads in validateAllItems/loadPackagePricingByGroup). Both predate the split — the code was moved verbatim — so they're out of scope for the reorganisation PR and are captured here for a future change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J251sRHStWRgsijyynWunC
The merge of main placed PR #1692's follow-up heading directly after this branch's TODO entry, tripping markdownlint MD022. Add the blank line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpihwVNefhvq7XLVwq1wX4
What changed
The payment-processing code was one very large file (nearly 2,000 lines) that was allowed to skip our "keep files small" rule as a special exception. This splits it into a folder of smaller files that each do one job, and removes that exception so the rule now applies normally.
The behaviour is exactly the same — this is purely a reorganisation. All the same functions run in the same order; they just live in tidier, easier-to-find files now.
The new files
Everything moved into a
payment-processing/folder:Duplication cleanup
Splitting a file this large reveals repeated snippets that our duplication checker couldn't see while they were buried in one giant file. These were removed by sharing small helpers rather than silencing the checker — for example one helper that builds a booking's listing-and-bundle identity, one that builds the placeholder rows for a deleted listing, and one that totals up allocated items.
Checks
Type-checking, linting, and the duplication check all pass, and the payment and webhook test suites (230+ tests) pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01J251sRHStWRgsijyynWunC
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation