feat: refund PB→merchant payments#40
Open
knutties wants to merge 30 commits into
Open
Conversation
Admin-initiated refund of settled PB→merchant payments. Each refund is a new compensating transaction (1 or 2 rows mirroring the payment's pool split) plus matching code-210 TB transfer(s) debiting the merchant sentinel. Multiple partial refunds allowed per payment; refund credits self-pool first then others-pool. The schema change is forward-compatible so transfer reversal can later move to the same multi-partial pattern without further repo work. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
13 bite-sized TDD-driven tasks (with task 6 split into 8 sub-tasks for the service method) covering migration, error variants, domain type_label, repo helpers, ledger code 210, refund_payment service, DTOs, handler, route, Smithy + SDK regen, cucumber + UI cucumber, admin UI, docs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces uq_transactions_reverses with uq_transactions_reverses_transfer restricted to type='transfer', so payment refunds can have multiple rows pointing at the same original payment row while transfer reversal's at-most-one invariant is preserved.
RefundNotRefundable (409), RefundAmountInvalid (400), and PaymentFullyRefunded (409).
Type-agnostic helpers: find_refunds_of returns every row linked back to a given original via reverses_transaction_id (ordered by created_at, id); sum_refunds_of aggregates their amount. Designed to be reused when transfer reversal moves to the multi-partial pattern. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…split Debit MERCHANT_SETTLEMENT_TB_ID, credit one or two PB pool accounts. The split variant uses linked TB transfers for atomicity. The merchant sentinel has no balance constraint, so over-amount is caught upstream in the service layer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Multi-partial refund of settled PB→merchant payments. Self-first pool allocation, idempotency replay, and atomic insert+TB-transfer flow. End-to-end behaviour pinned by Cucumber payment_refund.feature (Task 10); no unit tests added here per plan (pb_payment_service has no DB-touching unit-test scaffold).
…ent replay loop bound Cosmetic: original_amount and remaining_refundable are pure functions of data loaded before the PG transaction; computing them after tx.commit read misleadingly. Move them above the begin() call. Add a one-line comment on the idempotency-replay sum_refunds_of loop noting that the input always has ≤2 rows for payments written by make_payment. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Thin handler wrapping PbPaymentService::refund_payment.
The /accounts/... routes exist as compat aliases for endpoints that previously lived there. Refund is brand new; nothing to preserve. Keep only the /pb-accounts/.../refund route the Smithy spec declares.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add five refund-tracking fields to PbaWorld (last_refund_correlation_id, last_refund_amount_to_self, last_refund_amount_to_others, last_refund_remaining, previous_refund_correlation_id) and wire them into the Default impl. Append refund step bindings to payment_steps.rs (When/Then steps covering happy-path, idempotency replay, wrong account, refund-of-refund, frozen account, and error classification). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add 11 automated Cucumber scenarios covering full/partial refund of others-only and split payments, sequential partial refunds up to PaymentFullyRefunded, RefundAmountInvalid (over-amount and zero), RefundNotRefundable (refund-of-refund), frozen-account rejection with reactivation retry, idempotency replay (same key → same correlation_id), and wrong-account rejection (TransactionNotFound). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ing assertions Scenario 6 now asserts the 'remaining refundable' value embedded in the RefundAmountInvalid error message. Scenario 8 asserts the 'is_itself_a_refund' reason on RefundNotRefundable. Scenario 11 corrected to expect RefundNotRefundable with reason='wrong_account' (the per-row check fires before the empty-result-set TransactionNotFound branch).
Build the refund form, POST handler calls pb_payment_service.refund_payment, redirects to the original payment's detail page on success, re-renders the form with the AppError Display string on failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…transaction detail Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- account_id check uses !all(==) instead of any(!=) for clearer intent. - process_refund_payment redirect uses the prefixed() helper, matching the convention every other redirect in the admin layer uses. - Hoist 'use BTreeMap' to the top of the file. - Log DB errors swallowed in the refund-history display path instead of silently falling back to an empty list. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add last_payment_id / last_refund_id fields to UiWorld (initialized to None). Add refund navigation helpers + when/then step bindings in payment_steps.rs. Add combined 'I transfer N paisa from the normal account to the PB account' convenience step to UI transfer_steps.rs used by payment_refund_admin.feature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five UI Cucumber scenarios exercising the admin refund button, form, and history through chromiumoxide: visible on settled payment, absent on refund row, absent after full refund (history shown), present after partial refund with reduced remaining, and inline error on over-amount refund. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…match Removing the input's max attribute lets Scenario 5 (over-amount) reach the server-rendered error path instead of being blocked by Chrome's native constraint validation. The refund-history assertion now matches the specific <td>₹X.XX</td> cell rather than the bare amount, so it can't spuriously match the payment's amount stat-card. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add /pb-accounts/{id}/payments/{id}/refund row to the README API table.
Add 'Refunding a PB → merchant payment' subsection to WHAT.md covering
multi-partial refunds, self-first pool routing, admin-only access, and
the no-MCC-recheck / no-InsufficientFunds invariants.
CI's just fmt-check (cargo fmt -p pba-service -- --check) flagged formatting in error.rs, transaction_repo.rs, pb_payment_service.rs, admin/handlers.rs, api/handlers/pb.rs, tests/steps/payment_steps.rs, and tests/ui_steps/payment_steps.rs. Generated pba_client is excluded from fmt-check by design.
… PB account
The UI runner picks up tests/features/*.feature (except @api-tagged scenarios)
and runs them via chromium. Its 'Given a normal account exists' step sets
world.account_id to the normal account id, so if it runs after 'Given a
"health" account exists' the chromium-based 'I pay …' step would navigate
to /admin/accounts/{normal_id}/payment and fail. Match transfer_reversal.feature's
convention: normal first, then health, so account_id ends as the PB account.
Mirror make_payment's payment_id pattern. The primary refund leg (self if
both pools touched, else the single leg) now uses refund_correlation_id as
its row id, so /admin/transactions/{correlation_id} resolves to a real row.
The secondary leg keeps a fresh Uuid::now_v7(). Idempotency-key placement
and find_by_correlation_id semantics are unchanged.
After a refund, the refund row is the most-recent transaction on the account, so re-discovery would clobber the payment id with a refund-row id and break subsequent 'Refund button visible on the payment' assertions. Use the cached id when present.
The cache-id change in 0002a9b chained .await on a single line; rustfmt breaks the chain to three lines. CI's just fmt-check caught it.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
create_payment_refund/create_payment_refund_split(linked) ledger helpers.find_refunds_of/sum_refunds_ofrepo helpers (type-agnostic — reusable later for multi-partial transfer reversal).reverses_transaction_idpartial unique index toWHERE type='transfer', preserving transfer-reversal's at-most-one invariant while letting payment refunds stack.Surface area
POST /pb-accounts/{account_id}/payments/{payment_id}/refund(Smithy:RefundPBAccountPayment).RefundNotRefundable(409),RefundAmountInvalid(400),PaymentFullyRefunded(409).Spec + plan
docs/superpowers/specs/2026-05-30-payment-refund-design.mddocs/superpowers/plans/2026-05-30-payment-refund.mdTest plan
cargo check -p pba-service && cargo check --tests -p pba-servicecleanjust fmt-check/just lint(clippy -D warnings) /just buildpassjust e2e-all:tests/ui_features/: 5 features / 29 scenarios / 189 steps — all pass (incl. newpayment_refund_admin.feature)Future symmetry
The schema change and the new repo helpers are designed for reuse: a follow-up PR can relax transfer reversal to multi-partial by dropping the partial unique index and adopting
sum_refunds_ofinreverse_transfer. No further repo or schema work needed beyond that.🤖 Generated with Claude Code