Skip to content

Commit

Permalink
fix(PT-13142): Fix order and transaction id mixup
Browse files Browse the repository at this point in the history
  • Loading branch information
DennisGarding committed Oct 12, 2023
1 parent e781bc2 commit 522b5da
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 13 deletions.
25 changes: 25 additions & 0 deletions Components/Services/OrderDataService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -180,6 +181,8 @@ public function setOrderStatus($orderId, $newOrderStatusId)
* @param string $transactionId
*
* @return OrderAndPaymentStatusResult|null
*
* @deprecated in 6.1.1, and will be removed with 7.0.0 without replacement
*/
public function getOrderAndPaymentStatusResultByTransactionId($transactionId)
{
Expand All @@ -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']);
}
}
47 changes: 47 additions & 0 deletions Tests/Functional/Components/Services/OrderDataServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
*/
Expand Down
11 changes: 6 additions & 5 deletions Tests/Functional/WebhookHandler/AbstractWebhookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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'));
Expand Down Expand Up @@ -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(
Expand Down
22 changes: 15 additions & 7 deletions WebhookHandlers/AbstractWebhook.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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');
Expand All @@ -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']);
}
}
48 changes: 48 additions & 0 deletions WebhookHandlers/OrderAndTransactionIdResult.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
11 changes: 10 additions & 1 deletion plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 522b5da

Please sign in to comment.