Add Payzum as a crypto/stablecoin payment provider (async settlement via signed notification) - #4713
Add Payzum as a crypto/stablecoin payment provider (async settlement via signed notification)#4713payzum-hq wants to merge 2 commits into
Conversation
Adds Payzum through Omnipay, with the asynchronous settlement design agreed in LycheeOrg#4712: a signed inbound notification endpoint completes the order, and the buyer's return no longer fails a payment that is still confirming on-chain. - POST /api/v2/Shop/Checkout/Notify/Payzum/{order_id}: the driver verifies the HMAC-SHA-512 signature over the raw request bytes before any field is readable; order, amount and currency are checked and completion is idempotent across redeliveries. - handlePaymentReturn refreshes the invoice once for Payzum and leaves the order in PROCESSING while it confirms; the checkout page renders a new 'processing' state. - Config, enum, .env.example and composer entries for payzum/omnipay-payzum.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (27)
🚧 Files skipped from review as they are similar to previous changes (7)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughPayzum support adds provider configuration, asynchronous checkout handling, signed notification processing, pending-payment views, localized messages, and endpoint tests. ChangesPayzum provider setup
Asynchronous checkout returns
Signed Payzum notifications
Processing views and coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to Payzum adds asynchronous crypto-payment settlement with signed notifications and a processing state. The reviewed coverage confirms completed payments remain reachable by browser returns and are fulfilled only once, with no remaining merge-blocking risk. Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Full details: Docstring CoverageExplanation Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 32 files. (2 skipped: 2 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lang/ar/webshop.php (1)
288-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse snake_case for the new translation keys.
Rename
paymentProcessingandpaymentProcessingMessagetopayment_processingandpayment_processing_messagein all changed locale files. Update both the v7 and v8 Vue consumers to use the renamed keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 3e77f143-0002-49fd-b0b9-f2fcff57aaf8
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.env.exampleapp/Actions/Shop/CheckoutService.phpapp/Enum/OmnipayProviderType.phpapp/Factories/OmnipayFactory.phpapp/Http/Controllers/Shop/CheckoutController.phpapp/Http/Requests/Checkout/FinalizeRequest.phpapp/Http/Requests/Checkout/NotifyRequest.phpcomposer.jsonconfig/omnipay.phplang/ar/webshop.phplang/bg/webshop.phplang/cz/webshop.phplang/de/webshop.phplang/el/webshop.phplang/en/webshop.phplang/es/webshop.phplang/fa/webshop.phplang/fr/webshop.phplang/hu/webshop.phplang/it/webshop.phplang/ja/webshop.phplang/nl/webshop.phplang/no/webshop.phplang/pl/webshop.phplang/pt/webshop.phplang/ru/webshop.phplang/sk/webshop.phplang/sv/webshop.phplang/tr/webshop.phplang/vi/webshop.phplang/zh_CN/webshop.phplang/zh_TW/webshop.phpresources/js/v7/components/webshop/CancelledFailed.vueresources/js/v8/components/webshop/CancelledFailed.vueroutes/api_v2_shop.phptests/Webshop/Checkout/CheckoutNotifyControllerTest.php
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| // notification before the buyer's browser returns; that return is | ||
| // still legitimate and must be able to show the completed order. | ||
| $is_valid_status = $this->order?->status === PaymentStatusType::PROCESSING || | ||
| ($this->order?->status === PaymentStatusType::COMPLETED && $this->provider_type === OmnipayProviderType::PAYZUM); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Verify the identifier used by the Payzum browser return.
rg -n -C 8 \
'shop\.checkout\.return|returnUrl|findByTransactionId|markAsPaid|transaction_id' \
app routes testsRepository: LycheeOrg/Lychee
Length of output: 50373
🤖 get_repo_knowledge executed:
get_repo_knowledge LycheeOrg/Lychee /tmp/coderabbit-repo-knowledge/lycheeorg-lychee-cc271fdf/learnings
Length of output: 9424
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- FinalizeRequest ---'
cat -n app/Http/Requests/Checkout/FinalizeRequest.php | sed -n '1,130p'
printf '%s\n' '--- NotifyRequest ---'
fd -i 'NotifyRequest.php' app | xargs -r -n1 sh -c 'cat -n "$0" | sed -n "1,180p"'
printf '%s\n' '--- Checkout controller and Payzum references ---'
rg -n -C 12 'function (finalize|notify)|shop\.checkout\.return|shop\.checkout\.notify|Payzum|payment_url|return_url|returnUrl|order_id' app routes tests --glob '*.php' | head -n 320
printf '%s\n' '--- Order lookup and payment mutation ---'
rg -n -C 10 'findByTransactionId|markAsPaid|transaction_id' app/Models app/Services app/Http --glob '*.php' | head -n 240Repository: LycheeOrg/Lychee
Length of output: 49358
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Checkout controller definitions ---'
fd -i 'CheckoutController.php' app | xargs -r -n1 sh -c 'rg -n -C 18 "function (createSession|process|finalize|notify)" "$0"'
printf '%s\n' '--- Checkout service payment URL and notification flow ---'
fd -i 'CheckoutService.php' app | xargs -r -n1 sh -c 'rg -n -C 16 "returnUrl|return_url|notify|notification|markAsPaid|transaction_id|shop.checkout" "$0"'
printf '%s\n' '--- Payzum provider implementation ---'
rg -l -i 'payzum' app config composer.json | sort | while read -r file; do
printf '%s\n' "--- $file"
rg -n -C 10 -i 'payzum|returnUrl|return_url|notify|transaction_id|order_id' "$file"
doneRepository: LycheeOrg/Lychee
Length of output: 45036
Preserve a stable lookup key for Payzum returns.
CheckoutController::process() puts the original Order::transaction_id in the Payzum return URL. handlePaymentNotification() later replaces it with the provider reference. FinalizeRequest::processValidatedValues() then calls Order::findByTransactionId() before the COMPLETED Payzum branch, so the browser return can fail with “Order not found.”
Use the order ID in the return route or preserve the original transaction ID. Add a regression test for notification completion before the browser return.
…table Addresses the review on LycheeOrg#4713: - Settlement now happens inside a transaction that locks and re-reads the order, so the browser return and an inbound notification racing each other can no longer both transition it and dispatch OrderCompleted twice. The loser adopts the fresh state and reports that it changed nothing. - Completing an order no longer replaces its transaction id with the gateway reference: that id is the lookup key of both the return and the notification URLs, so replacing it made the buyer's browser return fail to resolve whenever the notification arrived first. The Payzum invoice is still reachable by it, as their API reads an invoice by payment id or by order id. - Regression tests: exactly one OrderCompleted across redeliveries, the browser return still resolving (and not re-fulfilling) after an early notification. - Translation keys renamed to snake_case in every locale and both Vue consumers.
|
Thanks — all three were worth fixing, and two of them were real defects. Pushed in 5a8e46d. Atomic settlement. Settlement now runs inside a transaction that locks and re-reads the order ( Stable lookup key. You were right, and this was the more serious one: completing the order replaced Redelivery test. Added Translation keys renamed to
|
1 participant
Implements the plan agreed in #4712 (option 1, the inbound notification endpoint @ildyria preferred). Adds Payzum, a non-custodial crypto/stablecoin gateway (USDC, USDT and more, multi-chain — funds settle directly to the photographer's own wallet), as an Omnipay provider.
Since crypto confirmation is asynchronous, the interesting part is settlement — following the pattern discussed in the issue:
POST /api/v2/Shop/Checkout/Notify/Payzum/{order_id}(shop.checkout.notify; keyed by order id, because completing an order replaces its transaction id with the provider reference and the gateway retries to the original URL): the Omnipay driver verifies the HMAC-SHA-512 signature over the raw request bytes (with a replay window) before any payload field is readable. After signature, order, amount and currency checks, the order is completed idempotently — a redelivered notification can never fulfil twice.OrderCompletedis dispatched here (guarded bywasChanged('status')) because the buyer may never come back to the browser.handlePaymentReturnno longer fails confirming payments: for Payzum the return handler polls the invoice once (fetchTransaction) and either completes, fails (expired/cancelled), or leaves the order in PROCESSING — never FAILED just because the buyer returned before the chain confirmed.finalizethen redirects to the checkout status page, where a newprocessingstate inCancelledFailed.vue(v7+v8, i18n key added to all locales with the English fallback) tells the buyer the payment is being confirmed and the page can be closed safely.PAYZUMcase inOmnipayProviderType(requiresapiKey+webhookSecret),config/omnipay.phpblock,.env.exampleentries.composer.json/lock gainpayzum/omnipay-payzum(MIT) and its only dependency, the officialpayzum/payzum-phpSDK (zero runtime deps); nothing else in the lock changes.FinalizeRequestadditionally authorizesCOMPLETEDPayzum orders, because the notification can legitimately complete the order before the buyer's browser returns.Tests:
CheckoutNotifyControllerTestcovers the happy path, forged signatures, pending statuses, expiry, amount mismatches, redelivery idempotency, unknown orders, and the finalize-keeps-processing behaviour. The wholetests/Webshopsuite passes (295 tests), as dophpstanandphp-cs-fixer.Disclosure: I work on Payzum and will maintain the integration long-term.
Summary by CodeRabbit
New Features
Bug Fixes