fix(webhook): settle the right order when several share a quote (#69) - #81
Merged
Merged
Conversation
Core calls afterPlaceOrder() with no arguments, so the renderer never knew which order a popup transaction was for. Capture the entity id the payment-information response resolves with, and send it so the webhook can settle the right order when several share a quote (issue #69). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Payable) register()'s order-state guard moves onto the new predicate (state new or pending_payment, and a positive base amount due) so the upcoming webhook order resolver can apply exactly the same rule. Recreate keeps its own state list on purpose: it gates an anonymous cancel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rded A charge whose order can never take it (canceled/closed/complete, or nothing left due) used to return ORDER_NOT_PAYABLE, which the webhook retries for Paystack's whole ~72h budget and which risks endpoint back-off. It now returns the new ORDER_CLOSED, a permanent reason, but only once the rejection is durably on the order's history; if that write fails it stays ORDER_NOT_PAYABLE so the retry keeps trying to record it. Held or payment-review orders keep retrying as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inline retries cancel the previous order and reuse its quote, so the webhook's quoteId lookup found several orders, required exactly one, and left the paid order pending. Order lookup moves into WebhookOrderResolver: increment id, then an order the reference is already bound to, then the popup's metadata.orderId (matched with its quoteId), then the quote - a lone order as before, otherwise the single payable Paystack order, and never a guess among several. Metadata is read from the verify response and strictly parsed; repository errors propagate (503) instead of falling through to a weaker step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecorded Diff review of #69: the record-before-ack rule lived inside the shared settlement service and only covered closed orders. register() now reports historyRecorded on every result and returns ORDER_CLOSED deterministically; the webhook acknowledges any permanent reason only when the rejection is on the order's history or no real money moved, and retries (503) otherwise. Also: closed-order history now says the payment was received but not applied (refund or reconcile) and logs at critical; the resolver skips a non-Paystack order bound to the reference and logs unresolvable quotes at error with the candidate orders; stale retry-policy docblock corrected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the cases a mutation pass showed missing: an orphaned bound transaction falls through to the quote lookup, chargeIsReal's full table, and historyRecorded=false on the bound-elsewhere and registration-failed paths when the history save fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Security re-review of the ack-once-recorded change: an order paid before reference binding existed has no transaction row, so a replay of its own successful charge reaches ORDER_CLOSED. The history now asks the merchant to refund only if the charge is not already reflected on the order. Critical is logged when a closed-order rejection is first recorded or when any history write fails, not on every replay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One history sprintf instead of two near-identical blocks, the closed-order decision derived once and passed to writeHistory(), guard clauses instead of nested ifs, a findOrders() helper for the resolver's three order lookups, and the retry-policy rationale kept on the reason constants with pointers elsewhere. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CHANGELOG Unreleased entry, Reference Manual order-lookup and retry semantics, a User Guide troubleshooting entry for the closed-order history comment, and CLAUDE.md's webhook flow and key classes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 29, 2026
Merged
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.
Fixes #69.
Problem
In inline (popup) mode, closing the popup runs
/paystack/payment/recreate, which cancels the order and restores the same quote. A retry therefore leaves several orders on onequote_id. The webhook's fallback lookup required exactly one (getTotalCount() == 1), so whenever the webhook was the path that confirmed payment (customer closed the tab, inline verify failed) the paid order stayed pending.Changes
Model/WebhookOrderResolver(new) finds the order, first match wins: increment id → an order the reference is already bound to →metadata.orderId+quoteId(any state) → the quote: its lone order as before, otherwise the single payable Paystack order. Never guesses among several. Metadata is read from the verify response and strictly parsed; repository errors propagate (503) instead of falling through.getPlaceOrderDeferredObject()override) and sends it asmetadata.orderId.TransactionValidator::isPayable()/isClosedForPayment()/chargeIsReal()— one definition of payability shared by settlement and the resolver (Recreatekeeps its own list on purpose).order_closedreason: a charge for a canceled/closed/complete or fully-paid order is acknowledged with 200 once a history comment is saved ("received after this order was closed … refund or reconcile if not already reflected"), logged atcriticalon first record — instead of 503 for Paystack's ~72h retry budget.historyRecordedonregister()'s result).Behaviour changes to be aware of
order_closed(terminal, same customer message asorder_not_payable) for these orders.Known limitations (in CHANGELOG)
metadata.orderId(pre-release, or checkouts that replace Magento's payment renderer such as Hyvä) still resolve by quote; a late bank-transfer/USSD settlement for a cancelled attempt can bind to the live retry order.errorwith the candidates).Verification
orderId→ retry order settles, cancelled order untouched; legacy (noorderId) → settles; redelivery → idempotent 200, one invoice; late charge naming the cancelled order → 200rejectedwith the history comment. Control withWebhook.phpreverted to master reproduces Webhook handler does not advance order when multiple transactions exist for the same quoteId #69 (503 "order not found", order pending).metadata.orderIdequal to the placed order's entity id.🤖 Generated with Claude Code