Detect a currency change after the Quickpay payment was created

src/Action/ConvertPaymentAction.php

@@ -40,7 +40,6 @@ public function execute($request): void
 
         $details = ArrayObject::ensureArrayObject($paymentModel->getDetails());
         $details['amount'] = $paymentModel->getTotalAmount();
-        $details['currency'] = $paymentModel->getCurrencyCode();
 
         // Only scalars are stored in the details so they survive serialization by the consumer.
         // `quickpayPaymentId` is the single source of truth; the payment is re-fetched when needed.
@@ -58,6 +57,9 @@ public function execute($request): void
 
             $details['quickpayPaymentId'] = $payment->id;
             $details['order_id'] = $payment->orderId;
+            $details['currency'] = $currency;
+        } else {
+            self::assertCurrencyUnchanged($details, $paymentModel->getCurrencyCode());
         }
 
         if (null !== $token = $request->getToken()) {
@@ -98,6 +100,42 @@ private static function assertNotEmptyString(mixed $value, string $what, string
         return $value;
     }
 
+    /**
+     * A Quickpay payment's currency is fixed when it is created — the authorize will happen in that
+     * currency no matter what the details say. Before this guard, a Payum payment whose currency
+     * changed after conversion simply had its stored `currency` overwritten: the shop then believed
+     * one currency while Quickpay kept charging in the other, with the amount silently read in the
+     * wrong unit. A drifted currency is a hard error; the payment must be cancelled and a fresh one
+     * converted instead.
+     *
+     * A model without a currency (both properties are nullable on Payum's model) leaves the stored
+     * value alone, and a model that agrees is a no-op refresh.
+     *
+     * @param ArrayObject<string, mixed> $details
+     *
+     * @throws LogicException if the model's currency no longer matches the created payment's
+     */
+    private static function assertCurrencyUnchanged(ArrayObject $details, mixed $currency): void
+    {
+        if (!is_string($currency) || '' === $currency) {
+            return;
+        }
+
+        $stored = $details['currency'] ?? null;
+
+        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,
+                $currency,
+            ));
+        }
+
+        $details['currency'] = $currency;
+    }
+
     /**
      * 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

tests/Action/ConvertPaymentActionTest.php

@@ -216,6 +216,63 @@ public function shouldAcceptOrderIdsAtBothEndsOfTheRange(): void
         }
     }
 
+    /**
+     * A Quickpay payment's currency is fixed at creation — the authorize happens in that currency no
+     * matter what the details say. Overwriting the stored currency, as the action used to, let the
+     * shop believe one currency while Quickpay kept charging in the other.
+     *
+     * @test
+     */
+    public function shouldThrowWhenTheCurrencyChangedAfterCreation(): void
+    {
+        $payment = $this->createPayment();
+        $payment->setCurrencyCode('EUR');
+        $payment->setDetails(['quickpayPaymentId' => 555, 'currency' => 'DKK']);
+
+        $convert = new Convert($payment, 'array');
+
+        $action = new ConvertPaymentAction();
+        $action->setGateway($this->gateway);
+        $action->setApi($this->api);
+
+        $this->expectException(LogicException::class);
+        $this->expectExceptionMessage('cannot change currency');
+
+        try {
+            $action->execute($convert);
+        } finally {
+            self::assertCount(0, $this->getRequests(), 'No request may be issued for a drifted payment');
+        }
+    }
+
+    /**
+     * Payum's model allows a null currency. That is nothing to compare against, so the stored value
+     * — the one the Quickpay payment was actually created with — must survive instead of being
+     * overwritten with null.
+     *
+     * @test
+     */
+    public function shouldKeepTheStoredCurrencyWhenTheModelCarriesNone(): void
+    {
+        $payment = new Payment();
+        $payment->setNumber('000000000001');
+        $payment->setTotalAmount(100);
+        $payment->setDetails(['quickpayPaymentId' => 555, 'currency' => 'DKK']);
+
+        $convert = new Convert($payment, 'array');
+
+        $action = new ConvertPaymentAction();
+        $action->setGateway($this->gateway);
+        $action->setApi($this->api);
+        $action->execute($convert);
+
+        /** @var array<string, mixed> $result */
+        $result = $convert->getResult();
+
+        self::assertSame('DKK', $result['currency']);
+        self::assertCount(0, $this->getRequests());
+    }
+
     /**
      * @test
      */