Stage paid bookings until payment succeeds - #1764
stefan-burke wants to merge 29 commits into
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:
📝 WalkthroughWalkthroughPayment processing now preserves payment ticket tokens before booking commits, carries payment references through ledger replay, and conditionally recovers unexpected booking failures. Database behavior and webhook, replay, locking, PII, and token tests were updated accordingly. ChangesPayment processing safety
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Webhook
participant processReservedSession
participant replaySessionFromLedgerOrNull
participant recoverOrRefundUnexpectedCreate
participant processed_payments
Webhook->>processReservedSession: process payment session
processReservedSession->>replaySessionFromLedgerOrNull: replay payment reference
replaySessionFromLedgerOrNull->>processed_payments: inspect recorded outcome
processed_payments-->>processReservedSession: result or null
processReservedSession->>recoverOrRefundUnexpectedCreate: recover creation failure
recoverOrRefundUnexpectedCreate->>processed_payments: inspect finalized or unresolved reservation
processed_payments-->>Webhook: recovered success, refund, or original error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: ce199cc597
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?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: 6848c5ce09
ℹ️ 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".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/api/payment-processing/index.ts (1)
137-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSentinel choice diverges from
*OrNullconvention.
replaySessionFromLedgeruses a literalfalseto signal "unrecorded" instead ofnull, and the function/type aren't named to signal this (e.g. noOrNull/documented absence marker). Per the repo guideline on optional absence,null+ explicit naming is preferred so callers can't confuse the sentinel with a legitimate falsyPaymentResultfield.♻️ Suggested naming/sentinel alignment
-const replaySessionFromLedger = async ( +/** Returns null when the session is not yet recorded in the ledger. */ +const replaySessionFromLedgerOrNull = async ( sessionId: string, listingId: number, paymentReference: string, -): Promise<PaymentResult | false> => { +): Promise<PaymentResult | null> => { const disposition = await bookingLedgerDisposition(sessionId); switch (disposition.status) { case "unrecorded": - return false; + return null;As per coding guidelines, "For expected optional absence, use explicit nullable naming such as
*OrNull, document the expected absence, and reserve defaults, optional chaining, and nullable returns for genuinely meaningful optional values."🤖 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 `@src/features/api/payment-processing/index.ts` around lines 137 - 156, Update replaySessionFromLedger to return null instead of false for the "unrecorded" disposition, change its return type accordingly, and rename the function with an OrNull suffix to make the expected absence explicit. Update all callers to use the renamed function and handle the nullable result without treating false as the sentinel.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/features/api/payment-processing/index.ts`:
- Around line 137-156: Update replaySessionFromLedger to return null instead of
false for the "unrecorded" disposition, change its return type accordingly, and
rename the function with an OrNull suffix to make the expected absence explicit.
Update all callers to use the renamed function and handle the nullable result
without treating false as the sentinel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 668c9032-84ad-4868-9617-fd6b1c1c0999
📒 Files selected for processing (2)
src/features/api/payment-processing/index.tstest/lib/webhook-price-signature/trusted-and-mismatch.test.ts
|
@coderabbitai Resolved the nullable-sentinel review note in ee33cd8. |
stefan-burke
changed the title
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
git log --oneline -5 ee33cd8e 2>/dev/null || echo "commit not found locally"
git show ee33cd8e --stat 2>/dev/null | head -50Length of output: 1663 🏁 Script executed: #!/bin/bash
git show ee33cd8e -- src/features/api/payment-processing/index.ts | head -150Length of output: 6097
(=^･ω･^=) ﾉ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/api/payment-processing/index.ts (1)
76-82: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the now-ignored
storeTokensoption.The API still accepts
{ storeTokens: false }, but_optionsis discarded and Line 267 always persists a token. Remove the option so existing callers cannot silently rely on behavior that no longer exists.Proposed cleanup
-type SessionProcessorOptions = { storeTokens?: boolean }; - type SessionProcessor = ( sessionId: string, data: ValidatedSession, - options?: SessionProcessorOptions, ) => Promise<PaymentResult>; export const processPaymentSession: SessionProcessor = async ( sessionId, data, - _options, ) => {Also applies to: 321-325
🤖 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 `@src/features/api/payment-processing/index.ts` around lines 76 - 82, Remove the storeTokens property from SessionProcessorOptions and eliminate the now-unused options parameter from SessionProcessor and its implementations, including the call sites around the payment session processing flow. Update callers to stop passing storeTokens while preserving the existing unconditional token persistence behavior.
🤖 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.
Outside diff comments:
In `@src/features/api/payment-processing/index.ts`:
- Around line 76-82: Remove the storeTokens property from
SessionProcessorOptions and eliminate the now-unused options parameter from
SessionProcessor and its implementations, including the call sites around the
payment session processing flow. Update callers to stop passing storeTokens
while preserving the existing unconditional token persistence behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a73416c-3ac1-4a80-89f8-5524603345c5
📒 Files selected for processing (13)
src/features/api/payment-processing/index.tssrc/features/api/payment-processing/recovery.tssrc/shared/db/attendees/pii.tssrc/shared/db/payment-finalize.tssrc/shared/db/processed-payments.tssrc/shared/payment-ticket-token.tstest/lib/code-quality.test.tstest/lib/processed-payments/locking.test.tstest/lib/server-payments/confirm.test.tstest/lib/webhook-price-signature/post-commit.test.tstest/shared/db/attendees/pii.test.tstest/shared/db/processed-payments.test.tstest/shared/payment-ticket-token.test.ts
💤 Files with no reviewable changes (2)
- test/lib/processed-payments/locking.test.ts
- test/lib/code-quality.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee33cd8ea3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4640c37c7
ℹ️ 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".
|
Addressed the final CodeRabbit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ccf91c68a
ℹ️ 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".
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/shared/db/processed-payments.ts`:
- Around line 304-308: Update the pre-store flow around encryptTicketTokens and
execute so it validates that the UPDATE matched a row for the supplied
sessionId. Use execute’s returned affected-row count or equivalent result shape,
and throw a descriptive error when no row is updated; preserve the existing
successful update behavior and let the failure propagate.
In `@test/lib/webhook-price-signature/post-commit.test.ts`:
- Around line 237-238: Update the assertion around isSessionProcessed in the
post-commit test to establish that processed is non-null before checking its
contents, using the TypeScript non-null assertion if the record is guaranteed to
exist. Then assert failure_data directly against the expected non-empty value
without optional chaining.
🪄 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: f86e890e-a184-45c4-a159-be029d185f5a
📒 Files selected for processing (4)
src/features/api/payment-processing/recovery.tssrc/shared/db/processed-payments.tstest/lib/webhook-price-signature/post-commit.test.tstest/shared/db/processed-payments/staleness.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af47a1400f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eaebc6c2e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aac4e0116
ℹ️ 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: b02e9f1920
ℹ️ 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
A paid order must not claim a place before payment succeeds. It must also never refund a payment while leaving a live ticket behind. The previous flow could depend on cleanup after part of an order had already been written. The new flow stores a non-booking checkout record first, then claims every place together only after Stripe confirms payment. Cancelled and abandoned checkouts no longer keep customer data forever.
Verification
deno task precommit