Skip to content

PRE-3722: bump UPC for the new Unified API contract - #335

Merged
adumont-payplug merged 3 commits into
developfrom
feature/PRE-3722_bump-upc-unified-api-contract
Oct 9, 2026
Merged

adumont-payplug merged 3 commits into
developfrom
feature/PRE-3722_bump-upc-unified-api-contract

Conversation

@adumont-payplug

@adumont-payplug adumont-payplug commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Description

The Unified API changed its payment creation contract: the Hosted Fields token is now read from paymentMethod.hfToken, the saved-card alias from paymentMethod.storedId, and the created alias comes back in paymentMethod.storedId instead of paymentMethod.id. With payplug/unified-plugin-core (UPC) before 1.2.1 every Hosted Fields payment (new card or saved card) is refused with 400 INVALID_REQUEST, and no card is ever saved. This PR moves to UPC 1.2.1 (PRE-3717), which sends both the old and the new fields, and adapts the plugin around it, including develop's new deferred capture and cancel (PRE-3673), which UPC 1.2.1 broke.

1. UPC bump. The constraint goes from ^1.1.3 to ^1.2.1. develop's lock pinned UPC under the label 1.3.0, which is not a published version (neither a git tag nor on Packagist): it is commit 050f14e, older than 1.1.3 and without the new contract. The lock now pins the published 1.2.1 (05423ee). See Impact for the other packages the lock moves. The plugin builds no hfToken, id or storedId itself, so the bump alone fixes payment creation. A contract test (UnifiedApiContractTest) fails if the constraint ever allows a UPC that predates the new contract, and characterization tests lock in that both capture handlers never put those keys in paymentMethod.

2. Alias read from storedId first. CardDataFromPaymentMethodExtractor reads paymentMethod.storedId and falls back to paymentMethod.id, as UPC does. The real webhook carries the alias only as paymentMethod.storedId, while getOperation returns it as paymentMethod.id; reading both lets the card the shopper asked to keep be saved on either path.

3. Creation response without a top-level id. On staging the creation response sometimes lacks its top-level id, which is the id refunds are keyed on (the refund route refuses the operation id).

  • At creation: the payment is processed normally, and a warning is logged with ids only (PaymentCaptureOutcomeApplier::warnWhenPaymentIdMissing(), called by both capture handlers).
  • At refund: full and partial refunds of such a payment are refused cleanly in RefundPaymentProcessor, before the lock, the API call and any RefundHistory or details['refunds'] write. The refusal logs an error, adds a translated back-office flash (payment_refund_missing_payment_id, en/fr/it) in addition to Sylius's own generic error, and throws UpdateHandlingException, instead of the raw InvalidArgumentException raised so far.

4. Configurable Hosted Fields SDK URL. The checkout no longer hardcodes the staging SDK host: templates/shop/hosted_fields/index.html.twig uses the new payplug_hosted_fields_sdk_url() Twig function, backed by the payplug.hosted_fields_sdk_url parameter and overridable with PAYPLUG_HOSTED_FIELDS_SDK_URL. The default is the production SDK https://js.dalenys.com/hosted-fields/v2.2.0/hosted-fields.min.js.

5. Deferred capture and cancel on UPC 1.2.1. UPC 1.2.1 also contains PRE-3724 (BREAKING): AmountHelper::toCents() and fromCents() now require a currency and throw InvalidCurrencyException for an empty, malformed or 3-decimal one. develop's PRE-3673 admin capture/cancel called them without one in three places, which failed with ArgumentCountError once on 1.2.1. They now pass the payment's currency: AuthorizedPaymentController::parseAmount() (the typed amount), AuthorizedPaymentOperationProcessor (the remaining amount in the "amount exceeds what remains" message) and AuthorizationExtension (the amount fields' placeholder). See Impact for the currency restriction this implies.

Motivation: every Hosted Fields payment is refused on staging with UPC before 1.2.1 and saved cards are never persisted; the plugin must follow the Unified API contract change confirmed by the API team on 2026-10-06, must not fail with a raw error when a payment created without an id is refunded, and must keep develop's deferred capture and cancel working on the UPC that carries the new contract.

Related issue(s): PRE-3722 (builds on the UPC change PRE-3717)


Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue) [x]
  • ✨ New feature (non-breaking change that adds functionality) [x]
  • 💥 Breaking change (fix or feature that causes existing functionality to change and that could impact other libs) [x]
  • 🔧 Refactor (no functional changes, code improvement only) [ ]
  • 📦 Dependency update [x]
  • 🔒 Security fix [ ]
  • 📝 Documentation update [ ]

Breaking for setups that need UPC below 1.2.1, for code constructing PayPlugExtension by hand, and for merchants typing a partial capture or cancel amount in a currency that is not 2-decimal: see Impact below.


Checklist

Code Quality

  • Code is linted and formatted
  • No unnecessary commented-out code or debug logs
  • No hardcoded values (use env variables or config)

Testing

  • Unit tests added / updated
  • New/changed code is covered by tests — SonarCloud Quality Gate (coverage on new code) passes on the sonarcloud CI job

Security & Ops

  • No sensitive data or secrets introduced
  • Logging and error handling are appropriate

Notes for Reviewer

Impact / BC

  • Requires payplug/unified-plugin-core ^1.2.1. UPC's PHP floor is unchanged (>=7.4, plugin ^8.2).
  • Typed partial amounts in deferred capture/cancel are now limited to 2-decimal currencies. A typed amount is accepted only for a currency UPC's AmountHelper converts with a factor of 100 (EUR, USD...), detected by asking the helper itself (AuthorizedPaymentOperationProcessor::supportsTypedAmounts()), not with a currency table. Zero-decimal (JPY, XPF), 3-decimal (KWD), empty and malformed currencies are refused with a flash (new key admin.authorization.error.unsupported_currency, en/fr/it) before any API call or state change; the refusal is logged with ids and the currency only. A blank amount (the whole remaining amount) involves no conversion and works in every currency, as before. Reason: the plugin stores every amount in Sylius's two-decimal scale whatever the currency, while UPC's helper converts to the currency's real minor unit, so a typed JPY amount would have moved a hundredth of what the merchant meant. EUR/USD behaviour is unchanged.
  • PayPlugExtension::__construct() gains string $hostedFieldsSdkUrl (autowired, bound to %payplug.hosted_fields_sdk_url%); this is the only extender-facing constructor change and is listed in the CHANGELOG BC table.
  • Local and staging setups must now set PAYPLUG_HOSTED_FIELDS_SDK_URL to the staging SDK URL, since the default is the production SDK. Merchants have nothing to configure.
  • composer.lock moves more than UPC. Besides UPC 1.3.0 (050f14e) → 1.2.1, the author ran a broad composer update after the rebase to check nothing breaks: 58 packages differ from develop's lock (55 runtime, 3 dev, UPC included). Notably sylius/refund-plugin v2.1.0 → v2.2.1 (a minor version), symfony/framework-bundle v6.4.46 → v6.4.47 and most of Symfony 6.4 to v6.4.47, api-platform/* v4.3.20 → v4.3.21, and phpunit/phpunit 9.6.37 → 9.6.38. sylius/sylius stays v2.2.10 and doctrine/orm 3.6.9; the package count is unchanged. The content-hash differs from develop's only because of the UPC constraint in composer.json.
  • No data migration, no upgrade script. CHANGELOG updated under ## [2.0.0] - Unreleased.

History of this PR

  • The first commit also recovered a missing payment id from the webhook and added a fallback payment lookup in UnifiedApiIpnAction. That rested on the webhook's top-level id being the payment id; it is the operation id, so the recovery could never work. The second commit removes it entirely: UnifiedApiIpnAction, PaymentRepository and PaymentRepositoryInterface are back to their develop state.
  • The branch was then rebased onto develop (PRE-3673 deferred capture/cancel), and the third commit adapts PRE-3673's AmountHelper calls to UPC 1.2.1. The earlier statement that PRE-3724 did not affect the plugin was true on the old base and is no longer.
  • Code review: changes requested, then approved, then the backfill was removed on the corrected premise; final verdict pass (8/10) on the current state.

Testing

  • Run on PHP 8.2 against the committed state, lock update included: phpunit exit 0 (744 tests, 1477 assertions), phpstan exit 0, ecs exit 0, composer validate exit 0. phpmd exits 2 with 45 violations, the same list as develop, except that the existing ExcessiveClassComplexity of AuthorizedPaymentOperationProcessor grows from 59 to 64.
  • GitHub CI on a422ec1: Coverage, PHP Quality, SonarCloud, the Wiz scans and the four PHPUnit matrix jobs pass, including PHP 8.4 (which failed before the rebase: develop's refreshed lock with Sylius v2.2.10 fixed the knp_gaufrette install failure).
  • Verified on staging (local shop against the Unified API staging, UPC 1.2.1, before the rebase and the AmountHelper change): saved-card payment with 3DS paid (the request carried both id and storedId, 200); new-card payment with 3DS and card saving paid (hfToken sent top-level and inside paymentMethod), card persisted via the status poll; partial refund succeeded using the creation payment id (200).
  • Not verified:
    • full refund on staging;
    • a real webhook delivery: the notifications for these payments went to another developer's URL, so no webhook reached the shop, and the webhook storedId read is covered by a unit test with the real payload shape only;
    • a creation response without a top-level id: not reproducible on staging, unit-tested only;
    • new card without 3DS on staging;
    • the admin deferred capture/cancel flow after the AmountHelper change: unit-tested only, not run on staging;
    • Behat.

Risks / follow-ups

  • Zero-decimal currencies: the amount sent to the Unified API at creation, refund and capture is Sylius's stored amount, passed to CommonFieldsDto without AmountHelper. For JPY that may be the 100x issue UPC 1.2.1 now handles on its side. This needs the API team's answer and a separate ticket; meanwhile JPY and KWD merchants cannot type a partial capture or cancel amount (a blank amount still works).
  • A payment created without a top-level id remains unrefundable from the back office: the refusal is the intended behaviour. Open question for the API team: when can the creation response omit id, and how should such a payment be refunded?
  • Pre-existing and unchanged by this PR: the IPN receiver is unauthenticated and resolves payments with a LIKE match on details. A follow-up security ticket is being prepared.
  • Pre-existing: the debug logs of the Unified API raw request and response (SyliusUnifiedApiHttpClient) and of the hosted payment request payload (CaptureHostedPaymentRequestHandler) contain the single-use hfToken, now in both places UPC sends it.
  • Open Low findings, not fixed in this PR:
    • the CHANGELOG line about AmountHelper says deferred capture and cancel keep working, without saying that typed amounts are limited to 2-decimal currencies and that a blank amount works everywhere;
    • doc/authorized_payment.md (Capture and Cancel sections) does not mention that currency restriction;
    • the creation warning ends with "until PayPlug returns a payment id" (PaymentCaptureOutcomeApplier::warnWhenPaymentIdMissing(), text pinned in its test), which implies a recovery that does not exist.

🤖 Generated with Claude Code

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

Require UPC 1.2.1 and read the alias from storedId first.
Backfill the payment id from the webhook, refuse refunds
cleanly when it is missing, and make the SDK URL a
parameter that defaults to production.
The webhook top-level id is the operation id, not the
payment id, so the backfill could never apply. Remove it
with the fallback lookup; keep the refund refusal.
UPC 1.2.1 requires a currency in AmountHelper. Deferred
capture and cancel now pass the payment currency, and a
typed amount in a currency that is not 2-decimal is
refused before any API call.
@adumont-payplug
adumont-payplug force-pushed the feature/PRE-3722_bump-upc-unified-api-contract branch from a8139b6 to a422ec1 Compare October 9, 2026 08:10
@adumont-payplug
adumont-payplug merged commit 6c972d7 into develop Oct 9, 2026
13 checks passed
@adumont-payplug
adumont-payplug deleted the feature/PRE-3722_bump-upc-unified-api-contract branch October 9, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants