-
-
Notifications
You must be signed in to change notification settings - Fork 3
Keep existing payments working when new sales are off #2020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
dc137e9
Keep existing payments refundable when new sales are off
stefan-burke bc4a6dc
Address review: shared provider type, atomic provider transition
stefan-burke dd7c324
Update equivalent-mutant line numbers after review refactor
stefan-burke 3fbb3d6
Tighten wording and use using-declarations for test stubs
stefan-burke f0d6acc
Keep operator UI correct when sales are off; fix broken flake
stefan-burke 5345a26
Fix pre-existing none recovery and concurrency race
stefan-burke 16e318d
Address review: unified provider resolver, RETURNING, revert templates
stefan-burke 248967e
Add raw-writes mirror test; lint fixes
stefan-burke 17a9483
Replace rawWritesApi with valibot tupleWithRest; restore distinct las…
stefan-burke 9c67112
Kill template mutation survivors; fix stale equivalent entries
stefan-burke b6dbcd7
Kill 65/94 mutation survivors with compact table-driven assertions
stefan-burke 5f757d5
Kill remaining 27 mutation survivors; compact tests under line cap
stefan-burke d98a098
Remove as-unknown-as cast; use typed signedMeta input for thank_you_url
stefan-burke 80e7542
Save WIP
stefan-burke db336b4
Delete dead loaded.add writes; kill client.ts mutants
stefan-burke aa3aaaa
Move invalidation test to client suite; use CREATE TABLE for bare DB
stefan-burke 3ea050a
Use unique test-only table for invalidation assertion
stefan-burke f18d7b0
Use INSERT not CREATE so invalidation parser fires on mutant
stefan-burke 2b8bc24
Behavior-focused webhooks tests without mutation jargon
stefan-burke 81a9425
Add behavior tests for distinguishable webhook mutants
stefan-burke 7ec84c7
Extract failureDetail helper; separate specific log assertions
stefan-burke b906cbf
Fix equivalent-mutant line numbers after failureDetail extraction
stefan-burke 6a835c8
Fix missing setupStripe in helper; add 503/data-payment-result/empty-…
stefan-burke d6c57df
Split webhook edge-case tests by route
stefan-burke 656c954
Cover every payment callback mutation
stefan-burke c988434
Complete existing payment provider recovery
stefan-burke 45e7840
Add direct settings route contract
stefan-burke 704933b
Move provider recovery tests to mirror
stefan-burke cd669d0
Kill remaining payment safety mutants
stefan-burke 92fc06c
Verify settings POST route dispatch
stefan-burke 7daf37c
Harden payment provider recovery
stefan-burke f1eec7a
Add direct domain settings tests
stefan-burke a8f275d
Mirror payment provider form tests
stefan-burke 55d10e2
Add payment provider recovery stories
stefan-burke 6df7bd4
Exclude concurrent mutation test work
stefan-burke ff728d1
Finish payment provider recovery review fixes
stefan-burke 05ca3c6
Cover payment provider form mutations
stefan-burke 862bdd0
Finish payment recovery review fixes
stefan-burke bdefe2d
Cover atomic provider recovery state
stefan-burke 0c4b76e
Preserve concurrent provider settings
stefan-burke ff70865
Merge remote-tracking branch 'origin/main' into payment-aggregate/01-…
stefan-burke 17e3ea5
Use split Stripe configuration in tests
stefan-burke 909b396
Serialize provider settings changes
stefan-burke 2871d65
Merge remote-tracking branch 'origin/main' into payment-aggregate/01-…
stefan-burke a2676cc
Fix settings revision CI failures
stefan-burke 6f4fafc
Resolve final provider settings checks
stefan-burke 550b6ce
Cover provider settings guards
stefan-burke 8fba391
Merge remote-tracking branch 'origin/main' into payment-aggregate/01-…
stefan-burke adab54e
Check what the customer was actually charged, and never lose a paymen…
stefan-burke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # Payment aggregate — accepted safety rules | ||
|
|
||
| These seven rules are the safety contract the payment aggregate must satisfy. | ||
| They describe behaviour that does not all exist on `main` yet: the owner-review | ||
| flow, the queued owner email, and "aggregate activation" all belong to the | ||
| future aggregate work. They are recorded here as acceptance constraints so the | ||
| work that lands first — starting with "keep existing payments refundable when | ||
| new sales are off" — does not paint the aggregate into a corner. | ||
|
|
||
| Each rule names the current behaviour on `main` where one exists, so a reader | ||
| can see what already holds and what is still future work. Nothing here is | ||
| implemented ahead of its time; this document is the contract, not scaffolding. | ||
|
|
||
| ## The separation this PR establishes | ||
|
|
||
| New sales and existing payments are now resolved by different questions: | ||
|
|
||
| - **New sales** — `getActivePaymentProvider()` / `isPaymentsEnabled()`. Returns | ||
| null when the operator has saved the provider as "none", so no buyer can | ||
| start a new checkout. | ||
| - **Existing payments** — `getPaymentProviderForExistingPayments()`. Refunds, | ||
| provider reconciliation, replayed callbacks, and completion of already-started | ||
| payment work use this. When new sales are off, it falls back to the last | ||
| provider the operator activated (whose credentials stay stored), so money | ||
| already captured is never stranded. | ||
|
|
||
| The rules below assume that separation holds. | ||
|
|
||
| ## 1. Failed checkout plus captured money becomes owner review with complete-or-refund choices | ||
|
|
||
| A captured charge whose booking cannot be honoured at the charged amount does | ||
| not disappear and is not silently refunded. It is surfaced for the operator, | ||
| who must be able to either complete it (issue the ticket) or refund it. | ||
|
|
||
| *On `main` today:* a signed captured payment that cannot be honoured is kept as | ||
| a quantity-0 placeholder and refunded automatically (`storeRefundedBooking` / | ||
| `refundAndFail`), or acknowledged as "already handled" when its booking is gone. | ||
| There is no owner-review choice yet — the refund is automatic. The | ||
| "complete-or-refund" choice is future aggregate work; the current path must | ||
| keep refunding safely (covered by this PR's regression tests) so the aggregate | ||
| can later replace the automatic refund with a review. | ||
|
|
||
| ## 2. Completed refunds count immediately while overlapping refunds are blocked until provider totals catch up | ||
|
|
||
| A refund that succeeds is recorded at once. A second refund attempt for the same | ||
| charge must not pay out twice: it is treated as already-done once the provider's | ||
| own refund total confirms it. | ||
|
|
||
| *On `main` today:* `tryRefund` treats a payment the provider reports as already | ||
| refunded (`isPaymentRefunded`) as success, and each charge carries a | ||
| `provider_refunded_at` marker so a later attempt skips the provider call for | ||
| charges already returned. This is the current rule and must stay. | ||
|
|
||
| ## 3. Multiple captured charges require owner review | ||
|
|
||
| When a buyer has more than one captured charge for the same booking (a deposit | ||
| plus a balance, or charges combined by a merge), the system must not pick a | ||
| resolution by default. It surfaces the set for the operator to decide. | ||
|
|
||
| *On `main` today:* a merged attendee can carry several references, and the | ||
| bulk/single refund paths handle them as a batch without a review step. The | ||
| "require owner review" choice is future aggregate work. | ||
|
|
||
| ## 4. Queued owner email uses the current business address but stored body/buyer facts | ||
|
|
||
| When an owner notification is queued and sent later, the recipient address is | ||
| read at send time (so it reaches the current operator), but the message body | ||
| and the buyer facts it describes come from when the case was raised (so it | ||
| describes what actually happened, not a later state). | ||
|
|
||
| *On `main` today:* there is no queued owner-email path; notifications are sent | ||
| inline. This is future aggregate work. | ||
|
|
||
| ## 5. Malformed legacy records migrate without invented facts into owner review | ||
|
|
||
| A legacy or malformed payment record that cannot be interpreted must not have | ||
| facts invented for it. It is carried forward only as far as it can be honestly | ||
| read and lands in owner review for a person to resolve. | ||
|
|
||
| *On `main` today:* legacy migration copies what is provably there (references, | ||
| statuses) and invents nothing; an unreadable record is left for the operator | ||
| rather than guessed. This must stay true as the aggregate's migration lands. | ||
|
|
||
| ## 6. A buyer with a paid booking under review sees "payment received / do not pay again" and gets a stable reload | ||
|
|
||
| A buyer whose payment was captured but whose booking is not yet resolved must be | ||
| told their payment was received and must not be offered a way to pay again. A | ||
| reload of the page must return the same state, not re-charge or re-process. | ||
|
|
||
| *On `main` today:* a paid session the ledger already records replays as | ||
| success ("payment received") and is never re-processed or re-refunded | ||
| (`replaySessionFromLedger`). The explicit "under review" state and its stable | ||
| messaging are future aggregate work; the replay safety it depends on is | ||
| current and must stay. | ||
|
|
||
| ## 7. Aggregate activation waits for complete owner case pages/actions | ||
|
|
||
| The aggregate is only switched on once every owner-case page and action it | ||
| depends on exists and works. No half-enabled state where some payments flow | ||
| through the aggregate and some do not. | ||
|
|
||
| *On `main` today:* there is no aggregate to activate. This is the gate the | ||
| future work must clear before it takes over any payment path. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,17 @@ | ||
| import { | ||
| getActivePaymentProvider, | ||
| getPaymentProviderForExistingPayments, | ||
| type PaymentProvider, | ||
| } from "#shared/payments.ts"; | ||
|
|
||
| /** The active payment provider, or the caller's `onMissing` fallback (a redirect | ||
| * or error Response) when none is configured. The refund and refresh POSTs share | ||
| * this "need a provider before we touch money" guard instead of each re-checking. */ | ||
| /** The provider for refunding or refreshing an existing payment, or the | ||
| * caller's `onMissing` fallback (a redirect or error Response) when none was | ||
| * ever configured. Uses {@link getPaymentProviderForExistingPayments} so an | ||
| * operator can still refund and reconcile after switching new sales off. The | ||
| * refund and refresh POSTs share this "need a provider before we touch money" | ||
| * guard instead of each re-checking. */ | ||
|
stefan-burke marked this conversation as resolved.
Outdated
|
||
| export const requirePaymentProvider = async <T>( | ||
| onMissing: () => T, | ||
| ): Promise<PaymentProvider | T> => { | ||
| const provider = await getActivePaymentProvider(); | ||
| const provider = await getPaymentProviderForExistingPayments(); | ||
| return provider ?? onMissing(); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.