fix(PT-13142): Fix order and transaction id mixup

Components/Services/OrderDataService.php

@@ -15,6 +15,7 @@
 use SwagPaymentPayPalUnified\Components\Services\OrderDataServiceResults\OrderAndPaymentStatusResult;
 use SwagPaymentPayPalUnified\PayPalBundle\Components\LoggerServiceInterface;
 use SwagPaymentPayPalUnified\PayPalBundle\PaymentType;
+use SwagPaymentPayPalUnified\WebhookHandlers\OrderAndTransactionIdResult;
 use UnexpectedValueException;
 
 class OrderDataService
@@ -180,6 +181,8 @@ public function setOrderStatus($orderId, $newOrderStatusId)
      * @param string $transactionId
      *
      * @return OrderAndPaymentStatusResult|null
+     *
+     * @deprecated in 6.1.2, and will be removed with 7.0.0 without replacement
      */
     public function getOrderAndPaymentStatusResultByTransactionId($transactionId)
     {
@@ -197,4 +200,26 @@ public function getOrderAndPaymentStatusResultByTransactionId($transactionId)
 
         return new OrderAndPaymentStatusResult((int) $order['id'], (int) $order['status'], (int) $order['cleared']);
     }
+
+    /**
+     * @return OrderAndPaymentStatusResult|null
+     */
+    public function getOrderAndPaymentStatusResultByOrderAndTransactionId(OrderAndTransactionIdResult $orderAndTransactionIdResult)
+    {
+        $order = $this->dbalConnection->createQueryBuilder()
+            ->select(['id', 'status', 'cleared'])
+            ->from('s_order')
+            ->where('transactionID = :orderId')
+            ->orWhere('transactionID = :transactionId')
+            ->setParameter('orderId', $orderAndTransactionIdResult->getOrderId())
+            ->setParameter('transactionId', $orderAndTransactionIdResult->getTransactionId())
+            ->execute()
+            ->fetch(PDO::FETCH_ASSOC);
+
+        if (!\is_array($order)) {
+            return null;
+        }
+
+        return new OrderAndPaymentStatusResult((int) $order['id'], (int) $order['status'], (int) $order['cleared']);
+    }
 }

Tests/Functional/Components/Services/OrderDataServiceTest.php

@@ -19,6 +19,7 @@
 use SwagPaymentPayPalUnified\Tests\Functional\DatabaseTestCaseTrait;
 use SwagPaymentPayPalUnified\Tests\Functional\SettingsHelperTrait;
 use SwagPaymentPayPalUnified\Tests\Functional\ShopRegistrationTrait;
+use SwagPaymentPayPalUnified\WebhookHandlers\OrderAndTransactionIdResult;
 
 class OrderDataServiceTest extends TestCase
 {
@@ -223,6 +224,52 @@ public function testGetShopwareOrderServiceResultByTransactionIdShouldReturnOrde
         static::assertSame(1, $result->getPaymentStatusId());
     }
 
+    /**
+     * @return void
+     */
+    public function testGetOrderAndPaymentStatusResultByOrderAndTransactionId()
+    {
+        $sql = file_get_contents(__DIR__ . '/_fixtures/order_status.sql');
+        static::assertTrue(\is_string($sql));
+
+        $this->getContainer()->get('dbal_connection')->exec($sql);
+
+        $resultOne = $this->getOrderDataService()->getOrderAndPaymentStatusResultByOrderAndTransactionId(
+            new OrderAndTransactionIdResult('unitTestTransactionId', 'any')
+        );
+
+        static::assertInstanceOf(OrderAndPaymentStatusResult::class, $resultOne);
+        static::assertTrue(\is_int($resultOne->getOrderId()));
+        static::assertSame(-1, $resultOne->getOrderStatusId());
+        static::assertSame(1, $resultOne->getPaymentStatusId());
+
+        $resultTwo = $this->getOrderDataService()->getOrderAndPaymentStatusResultByOrderAndTransactionId(
+            new OrderAndTransactionIdResult('any', 'unitTestTransactionId')
+        );
+
+        static::assertInstanceOf(OrderAndPaymentStatusResult::class, $resultTwo);
+        static::assertTrue(\is_int($resultTwo->getOrderId()));
+        static::assertSame(-1, $resultTwo->getOrderStatusId());
+        static::assertSame(1, $resultTwo->getPaymentStatusId());
+    }
+
+    /**
+     * @return void
+     */
+    public function testGetOrderAndPaymentStatusResultByOrderAndTransactionIdShouldReturnNull()
+    {
+        $sql = file_get_contents(__DIR__ . '/_fixtures/order_status.sql');
+        static::assertTrue(\is_string($sql));
+
+        $this->getContainer()->get('dbal_connection')->exec($sql);
+
+        $result = $this->getOrderDataService()->getOrderAndPaymentStatusResultByOrderAndTransactionId(
+            new OrderAndTransactionIdResult('any', 'any')
+        );
+
+        static::assertNull($result);
+    }
+
     /**
      * @return array<string,int>|null
      */

Tests/Functional/WebhookHandler/AbstractWebhookTest.php

@@ -26,7 +26,7 @@ class AbstractWebhookTest extends TestCase
      */
     public function testGetOrderServiceResultWithoutResourceShouldReturnNull()
     {
-        $abstractWebhook = $this->createAbstractWebhook(true);
+        $abstractWebhook = $this->createAbstractWebhook(true, 'error');
 
         $result = $abstractWebhook->getResult(new Webhook());
 
@@ -53,7 +53,7 @@ public function testGetOrderServiceResultResourceIdIsNotSetShouldReturnNull()
      */
     public function testGetOrderServiceResultOrderServiceReturnsNullShouldReturnNull()
     {
-        $abstractWebhook = $this->createAbstractWebhook(true);
+        $abstractWebhook = $this->createAbstractWebhook(true, 'error');
 
         $webhook = new Webhook();
         $webhook->setResource(TestWebhookResource::create('anyPayPalOrderID'));
@@ -177,16 +177,17 @@ public function testGetOrderServiceResultLogErrorWithResourceArrayWithOrderIdIsN
     }
 
     /**
-     * @param bool $loggerExpectCallErrorMethod
+     * @param bool   $loggerExpectCallErrorMethod
+     * @param string $method
      *
      * @return AbstractWebhookMock
      */
-    private function createAbstractWebhook($loggerExpectCallErrorMethod = false)
+    private function createAbstractWebhook($loggerExpectCallErrorMethod = false, $method = 'debug')
     {
         $logger = $this->createMock(LoggerServiceInterface::class);
 
         if ($loggerExpectCallErrorMethod) {
-            $logger->expects(static::once())->method('error');
+            $logger->expects(static::once())->method($method);
         }
 
         return new AbstractWebhookMock(

WebhookHandlers/AbstractWebhook.php

@@ -51,17 +51,25 @@ protected function getOrderServiceResult(Webhook $webhook)
         }
 
         try {
-            $transactionId = $this->getOrderIdFromResource($resource);
+            $orderAndTransactionIdResult = $this->getOrderAndTransactionIdFromResource($resource);
         } catch (UnexpectedValueException $exception) {
-            $this->logger->error(sprintf('[Webhook]Event: %s. Resource structure is not valid. Message: %s', $this->getEventType(), $exception->getMessage()));
+            $this->logger->debug(sprintf('[Webhook]Event: %s. Resource structure is not valid. Message: %s', $this->getEventType(), $exception->getMessage()));
 
             return null;
         }
 
-        $shopwareOrderServiceResult = $this->orderDataService->getOrderAndPaymentStatusResultByTransactionId($transactionId);
+        $shopwareOrderServiceResult = $this->orderDataService->getOrderAndPaymentStatusResultByOrderAndTransactionId($orderAndTransactionIdResult);
 
         if (!$shopwareOrderServiceResult instanceof OrderAndPaymentStatusResult) {
-            $this->logger->error(sprintf('[Webhook]Event: %s. Cannot find orderID by transactionID: %s', $this->getEventType(), $transactionId), $webhook->toArray());
+            $this->logger->error(
+                sprintf(
+                    '[Webhook]Event: %s. Cannot find orderID by PayPalOrderId %s and transactionID: %s',
+                    $this->getEventType(),
+                    $orderAndTransactionIdResult->getOrderId(),
+                    $orderAndTransactionIdResult->getTransactionId()
+                ),
+                $webhook->toArray()
+            );
 
             return null;
         }
@@ -72,9 +80,9 @@ protected function getOrderServiceResult(Webhook $webhook)
     /**
      * @param array<string,mixed> $resource
      *
-     * @return string
+     * @return OrderAndTransactionIdResult
      */
-    private function getOrderIdFromResource(array $resource)
+    private function getOrderAndTransactionIdFromResource(array $resource)
     {
         if (!\array_key_exists('supplementary_data', $resource) || !\is_array($resource['supplementary_data'])) {
             throw new UnexpectedValueException('Expect resource has array key "supplementary_data" with array value');
@@ -90,6 +98,6 @@ private function getOrderIdFromResource(array $resource)
             throw new UnexpectedValueException('Expect related_ids has array key "order_id" with string value');
         }
 
-        return $relatedIds['order_id'];
+        return new OrderAndTransactionIdResult($relatedIds['order_id'], $resource['id']);
     }
 }

WebhookHandlers/OrderAndTransactionIdResult.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ * (c) shopware AG <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace SwagPaymentPayPalUnified\WebhookHandlers;
+
+class OrderAndTransactionIdResult
+{
+    /**
+     * @var string
+     */
+    private $orderId;
+
+    /**
+     * @var string
+     */
+    private $transactionId;
+
+    /**
+     * @param string $orderId
+     * @param string $transactionId
+     */
+    public function __construct($orderId, $transactionId)
+    {
+        $this->orderId = $orderId;
+        $this->transactionId = $transactionId;
+    }
+
+    /**
+     * @return string
+     */
+    public function getOrderId()
+    {
+        return $this->orderId;
+    }
+
+    /**
+     * @return string
+     */
+    public function getTransactionId()
+    {
+        return $this->transactionId;
+    }
+}

plugin.xml

@@ -5,13 +5,22 @@
     <label lang="de">PayPal</label>
     <label lang="en">PayPal</label>
 
-    <version>6.1.1</version>
+    <version>6.1.2</version>
     <copyright>(c) by shopware AG</copyright>
     <license>MIT</license>
     <link>http://store.shopware.com</link>
     <author>shopware AG</author>
     <compatibility minVersion="5.2.27" maxVersion="5.99.99"/>
 
+    <changelog version="6.1.2">
+        <changes lang="de">
+            PT-13142 - Verbessert die Bearbeitung von Webhooks;
+        </changes>
+        <changes lang="en">
+            PT-13142 - Improves the processing of webhooks;
+        </changes>
+    </changelog>
+
     <changelog version="6.1.1">
         <changes lang="de">
             PT-13127 - Verbessert die Benutzerfreundlichkeit für den Rechnungskauf im Checkout-Formular;