Upgrade to setono/quickpay-php-sdk ^1.1 and act on its release notes

CLAUDE.md

@@ -58,13 +58,23 @@ All HTTP and (de)serialization is delegated to [`setono/quickpay-php-sdk`](https
 typed `payments()` endpoints, request/response DTOs, non-exhaustive `PaymentState`/`OperationType` enums,
 a `QuickpayException` hierarchy, and a timing-safe `CallbackValidator`. Basic auth, the mandatory
 `Accept-Version: v10` header and host pinning to `api.quickpay.net` all live in the SDK. The SDK went
-stable on 2026-08-10, so the constraint is a plain `^1.0` and **consumers need no `minimum-stability`
-setting at all** — that requirement, and the docs describing it, are gone.
+stable on 2026-08-10, so the constraint is a plain caret and **consumers need no `minimum-stability`
+setting at all** — that requirement, and the docs describing it, are gone. The constraint is **`^1.1`**
+(2026-08-17): 1.1 is additive but the gateway uses what it added — `Operation::isApproved()` (not
+pending AND `20000`; every `Operations` helper builds on it) and `isOfType()`, `Shopsystem` on the
+create request (`ConvertPaymentAction` identifies the integration as `setono/payum-quickpay` +
+installed version via Composer's runtime API), and `PaymentsEndpoint::findByOrderId()` for the
+find-or-create in `Convert`. Not used on purpose: the SDK's `Payment::latestOperation()` /
+`hasPendingOperation()` are type-agnostic and its amount helpers sum — the actions need the per-type,
+last-approved views `Operations` provides. `cache:` on the SDK `Client` is reachable through the
+`quickpay.client` option (build the client with it) rather than a gateway option of its own, so the
+package does not have to depend on Valinor's cache interface. Since 1.1 an empty body (cancel) is sent
+as `{}`, not as nothing.
 
 The pre-release history is worth remembering only because it explains why the constraint used to be so
 specific: the gateway cannot run on alpha.1 (no client-wide `synchronized`) and breaks on alpha.1–2
 (request fields only became required constructor params in alpha.3), so the constraint had to keep
-excluding them. `^1.0` excludes every pre-release outright, so that is no longer a concern.
+excluding them. A caret on a stable release excludes every pre-release outright, so that is no longer a concern.
 
 ### Wiring
 
@@ -109,8 +119,15 @@ the `quickpayPaymentId` (int) it carries is the single source of truth — actio
 `Api::payments()->getById()` rather than passing the model through as API params.
 
 Request → Action flow (amounts are integer minor units everywhere — no conversion):
-- **Convert** → `ConvertPaymentAction` — turns a Payum `PaymentInterface` into the details array; creates
-  the Quickpay payment (via `CreatePaymentRequest`) if absent and stores **only scalars** —
+- **Convert** → `ConvertPaymentAction` — turns a Payum `PaymentInterface` into the details array;
+  **finds or creates** the Quickpay payment if the details carry no id: `findByOrderId()` first — Quickpay
+  enforces `order_id` uniqueness per account (a second create is a 400 "already exists on another
+  payment", verified live 2026-08-17) and under Sylius the Payum payment number is the *order* number, so
+  a retry after a decline collides — and an existing payment is adopted only if nothing was ever
+  approved on it (`findReusablePayment()`: initial/declined/authorize-in-flight; the currency must
+  match). One with an approved operation is a `LogicException` naming it — never adopt money silently:
+  it is either this order's earlier paid payment or another environment's under a shared prefix. Otherwise
+  it creates the payment (via `CreatePaymentRequest`, with `Shopsystem` = this package + version) and stores **only scalars** —
   `quickpayPaymentId`, `amount`, `currency`, `order_id` — plus `continue_url` (the token's **target**
   url, so the customer's return re-executes the `Capture`/`Authorize` that sent them out — Payum's
   return-trip convention) and `cancel_url` (the token's after url). It never persists DTO/model objects. On the create path only it asserts its inputs:

README.md

@@ -103,7 +103,8 @@ The gateway accepts either shape, so a list is fine where that reads better:
 Quickpay is an authorize/capture PSP behind a hosted payment window, and the gateway maps that onto
 Payum's requests the way Payum's own controllers — and Sylius — expect:
 
-1. **`Convert`** turns your Payum payment into the details array and creates the payment at Quickpay.
+1. **`Convert`** turns your Payum payment into the details array and creates the payment at Quickpay —
+   or picks up the one that already exists under the same order id, if nobody has paid it (see below).
 2. **`Capture`** or **`Authorize`** against the fresh payment creates the payment link and
    **redirects the customer to the Quickpay payment window** (Payum's `HttpRedirect` reply). Which
    one you execute is how you say what the window should do once the card is authorized:
@@ -152,6 +153,17 @@ The `auto_capture` gateway option is **deprecated** in favour of this: it made a
 capture on authorization too, which is exactly what executing `Capture` means. It still works for
 existing configurations and goes in 3.0.
 
+**Retrying a checkout.** Quickpay's `order_id` — `order_prefix` + the Payum payment number — must be
+unique per account, and under Sylius the payment number is the *order* number, so a customer who was
+declined, came back to the shop and pays again arrives with the same order id. `Convert` therefore
+looks the order id up first: a payment that already exists under it and **was never successfully paid**
+(the window was never completed, the attempt was declined) is picked up where it was left, and the
+customer is sent back to the window on it. A payment that *has* an approved operation — authorized,
+captured, refunded, cancelled — is never adopted silently: `Convert` throws a `LogicException` naming it,
+because it is either this order's earlier payment that really was paid (carry its `quickpayPaymentId`
+over) or another shop or environment sharing the account under a prefix that should not be shared
+(give each its own `order_prefix`). Either way that is your call, not a claim of money.
+
 ### Callbacks
 
 Quickpay confirms operations with a signed server-to-server callback. `NotifyAction` verifies the

composer.json

@@ -15,9 +15,10 @@
         "ext-filter": "*",
         "ext-hash": "*",
         "ext-json": "*",
+        "composer-runtime-api": "^2.0",
         "payum/core": "^1.7.5",
         "php-http/message-factory": "^1.0",
-        "setono/quickpay-php-sdk": "^1.0"
+        "setono/quickpay-php-sdk": "^1.1"
     },
     "require-dev": {
         "ergebnis/composer-normalize": "^2.52",

docs/UPGRADE-2.0.md

@@ -13,7 +13,8 @@ handling, and modernizes the test suite. This is a major release with breaking c
 - The SDK is built on PSR-18 / PSR-17 and discovers them via `php-http/discovery`. Make sure your
   project provides a PSR-18 client and PSR-17 factories (e.g. `composer require kriswallsmith/buzz
   nyholm/psr7`, or any other implementation).
-- `setono/quickpay-php-sdk` is stable as of `1.0.0`, so **no `minimum-stability` change is needed**. If
+- `setono/quickpay-php-sdk` **`^1.1`** (stable since `1.0.0`, and 2.0 uses what 1.1 added), so **no
+  `minimum-stability` change is needed**. If
   you tracked the 2.0 alphas and added `"minimum-stability": "beta"` for the SDK's pre-releases, you can
   drop it again (assuming nothing else in your project needs it).
 
@@ -257,6 +258,31 @@ pre-operation balance. `GetStatus` used to answer `pending` for that; it now ans
 already been approved (`authorized` with a capture queued, `captured` with a refund queued) and says
 `pending` only when nothing has been approved yet.
 
+## `Convert` finds or creates — a retried checkout no longer fails on a duplicate order id
+
+> Changed after `2.0.0-beta.1`.
+
+Quickpay enforces `order_id` uniqueness per account: creating a second payment under an existing order
+id is a `400` "order_id already exists on another payment" (verified live). The gateway builds the order
+id from the Payum payment **number**, which under Sylius is the **order** number — so a customer who was
+declined, came back to the shop and pays again arrived at `Convert` with an order id Quickpay already
+had, and the retry died with a `ValidationException`. (The same on plain Payum whenever a number is
+reused for a new payment.)
+
+`ConvertPaymentAction` now looks the order id up first (`PaymentsEndpoint::findByOrderId()`, SDK ≥ 1.1):
+
+| Under that order id Quickpay has… | `Convert` |
+|---|---|
+| nothing | creates the payment, as before |
+| a payment nobody has paid — no approved operation: created but never completed, declined, an authorize still in flight — in the same currency | **adopts it**: `quickpayPaymentId`, `order_id`, `currency` come from it, and the checkout continues on it (a new link is created; the entry-point actions treat a declined attempt like a fresh payment) |
+| a payment with an approved operation (authorized / captured / refunded / cancelled) | throws a `LogicException` naming it — never adopts money silently: it is either this order's earlier payment that really was paid (carry its `quickpayPaymentId` over yourself) or another shop or environment sharing the account under a prefix that should not be shared (give each its own `order_prefix`) |
+| a payment in another currency | throws a `LogicException` — the payment is what Quickpay charges in |
+
+The lookup is one `GET /payments?order_id=…` per *creating* Convert; a model that already carries a
+`quickpayPaymentId` makes no request, as before. The create request now also carries Quickpay's
+`shopsystem` (`setono/payum-quickpay` + the installed version), so the manager shows what created a
+payment; a Convert action of your own may say something more specific.
+
 ## Order ids are now validated before they are sent
 
 `ConvertPaymentAction` builds `order_id` as `order_prefix` + the Payum payment number, and now enforces
@@ -286,8 +312,9 @@ These were internal implementation details; they are gone in 2.0:
 ## Injecting a custom SDK client
 
 You can pass a preconfigured SDK client through the new `quickpay.client` option (a
-`Setono\Quickpay\Client\ClientInterface`) — useful for wiring a cached Valinor builder or a specific
-PSR-18 client. When omitted, the gateway builds one from `api_key` via discovery.
+`Setono\Quickpay\Client\ClientInterface`) — useful for a specific PSR-18 client, or for the SDK's mapper
+cache in production (`new Client($apiKey, synchronized: …, cache: new FileSystemCache($dir))`, one
+argument since SDK 1.1). When omitted, the gateway builds one from `api_key` via discovery, uncached.
 
 The `synchronized` option is carried by the SDK client itself (`Client::__construct(..., synchronized:
 true)`), and it can only be set there. An injected client must therefore be constructed with the same

src/Action/ConvertPaymentAction.php

@@ -4,6 +4,7 @@
 
 namespace Setono\Payum\Quickpay\Action;
 
+use Composer\InstalledVersions;
 use Payum\Core\Action\ActionInterface;
 use Payum\Core\ApiAwareInterface;
 use Payum\Core\Bridge\Spl\ArrayObject;
@@ -14,13 +15,18 @@
 use Payum\Core\Model\PaymentInterface;
 use Payum\Core\Request\Convert;
 use Setono\Payum\Quickpay\Action\Api\ApiAwareTrait;
+use Setono\Payum\Quickpay\Operations;
 use Setono\Quickpay\Request\Payment\CreatePaymentRequest;
+use Setono\Quickpay\Request\Payment\Shopsystem;
+use Setono\Quickpay\Response\Payment\Payment;
 
 class ConvertPaymentAction implements ActionInterface, ApiAwareInterface, GatewayAwareInterface
 {
     use GatewayAwareTrait;
     use ApiAwareTrait;
 
+    private const PACKAGE = 'setono/payum-quickpay';
+
     /**
      * Quickpay's accepted `order_id` length, verified against the live API.
      */
@@ -33,7 +39,7 @@
      */
     public function execute($request): void
     {
         RequestNotSupportedException::assertSupports($this, $request);
 
         /** @var PaymentInterface $paymentModel */
         $paymentModel = $request->getSource();
@@ -50,10 +56,16 @@
             $currency = self::assertNotEmptyString($paymentModel->getCurrencyCode(), 'currency code', $number);
             $orderId = self::assertOrderId($this->api->getOrderPrefix() . $number);
 
-            $payment = $this->api->payments()->create(new CreatePaymentRequest(
-                orderId: $orderId,
-                currency: $currency,
-            ));
+            $payment = $this->findReusablePayment($orderId, $currency)
+                ?? $this->api->payments()->create(new CreatePaymentRequest(
+                    orderId: $orderId,
+                    currency: $currency,
+                    // Quickpay's "shopsystem" is what the payment was created with — it shows in the
+                    // manager and tells Quickpay support which integration they are looking at. A
+                    // shop's own Convert action (the Sylius plugin has one) can say something more
+                    // specific.
+                    shopsystem: new Shopsystem(name: self::PACKAGE, version: self::version()),
+                ));
 
             $details['quickpayPaymentId'] = $payment->id;
             $details['order_id'] = $payment->orderId;
@@ -73,7 +85,7 @@
             $details['cancel_url'] = $token->getAfterUrl();
         }
 
         $request->setResult((array) $details);
     }
 
     public function supports($request): bool
@@ -132,7 +144,7 @@
 
         if (is_string($stored) && '' !== $stored && $stored !== $currency) {
             throw new LogicException(sprintf(
                 'The Quickpay payment %s was created in %s, but the Payum payment now says %s. A Quickpay '
                 . 'payment cannot change currency — cancel it and convert a new payment instead.',
                 is_scalar($details['quickpayPaymentId']) ? (string) $details['quickpayPaymentId'] : '(unknown)',
                 $stored,
@@ -143,6 +155,75 @@
         $details['currency'] = $currency;
     }
 
+    /**
+     * Find-or-create, the safe half: the Quickpay payment that already carries this order id, if it
+     * can be picked up where it was left — or null, meaning "create one".
+     *
+     * Quickpay enforces `order_id` uniqueness per account (a second create is a 400 "order_id already
+     * exists on another payment" — verified live), and the order id is built from the Payum payment
+     * NUMBER, which under Sylius is the ORDER number: every retry payment for an order — the customer
+     * was declined, came back to the shop and pays again — carries the same number. Without this,
+     * that retry died at Convert with a validation error, for the most ordinary of reasons.
+     *
+     * A payment is picked up only if nothing was ever approved on it — the window was never completed,
+     * or the attempt was declined. That is exactly the retry case, and it can never make anything look
+     * paid that is not: the entry-point actions treat such a payment like a fresh one and send the
+     * customer back to the window. A payment that HAS an approved operation — authorized, captured,
+     * refunded, cancelled — is never adopted silently: it may be this order's earlier payment that
+     * really was paid, or another environment's payment under a prefix that should not be shared, and
+     * either way it is for the shop to decide, so it is a clear exception rather than a claim of money.
+     * The currency has to match too — the payment is what Quickpay charges in.
+     *
+     * @throws LogicException if a payment with this order id exists but cannot be adopted
+     */
+    private function findReusablePayment(string $orderId, string $currency): ?Payment
+    {
+        $existing = $this->api->payments()->findByOrderId($orderId);
+
+        if (null === $existing) {
+            return null;
+        }
+
+        $approved = Operations::latestApproved($existing->operations);
+        if (null !== $approved) {
+            throw new LogicException(sprintf(
+                'A Quickpay payment with order id "%s" already exists (id %d, state %s) and has an approved %s. '
+                . 'The gateway will not adopt it: if it is this order\'s earlier payment, carry its '
+                . 'quickpayPaymentId over; if another shop or environment shares this Quickpay account, '
+                . 'give each its own "order_prefix".',
+                $orderId,
+                $existing->id,
+                $existing->state,
+                $approved->type,
+            ));
+        }
+
+        if ($existing->currency !== $currency) {
+            throw new LogicException(sprintf(
+                'A Quickpay payment with order id "%s" already exists (id %d) in %s, but this payment is in %s. '
+                . 'A Quickpay payment cannot change currency; give the retry a different order id.',
+                $orderId,
+                $existing->id,
+                $existing->currency,
+                $currency,
+            ));
+        }
+
+        return $existing;
+    }
+
+    /**
+     * The installed version of this package, for the `shopsystem` Quickpay records on the payment.
+     * Composer's runtime API knows it wherever the package was installed by Composer; "unknown" covers
+     * a vendored copy.
+     */
+    private static function version(): string
+    {
+        return InstalledVersions::isInstalled(self::PACKAGE)
+            ? (InstalledVersions::getPrettyVersion(self::PACKAGE) ?? 'unknown')
+            : 'unknown';
+    }
+
     /**
      * Quickpay requires `order_id` to be 4–20 characters, and the gateway builds it by concatenating
      * the `order_prefix` option with the Payum payment number. Checking the result here turns two

src/Exception/OperationRejectedException.php

@@ -50,7 +50,8 @@ public static function assertNotRejected(int $paymentId, Payment $payment, Opera
     {
         $operation = Operations::latestOfType($payment->operations, $type);
 
-        if (null === $operation || $operation->pending || Operations::isApproved($operation)) {
+        // isApproved() is false for a pending operation too; the explicit check says why it passes.
+        if (null === $operation || $operation->pending || $operation->isApproved()) {
             return;
         }