From 4b7fe0be04bd29b24c7b4f8ae0242483bd96b92d Mon Sep 17 00:00:00 2001 From: Frederik Rommel Date: Fri, 13 Sep 2024 14:12:09 +0200 Subject: [PATCH] PAYOSWXP-156: webhooks: add sort-order for handlers to prevent wrong order --- src/DependencyInjection/webhooks.xml | 27 ++++---- .../Handler/AutoCaptureStatusHandler.php | 39 +++++++++++ .../TransactionStatusWebhookHandler.php | 15 +---- .../Handler/AutoCaptureStatusHandlerTest.php | 65 +++++++++++++++++++ .../TransactionStatusWebhookHandlerTest.php | 25 ++----- .../Processor/WebhookProcessorTest.php | 9 +-- ...TransactionStatusWebhookHandlerFactory.php | 5 +- 7 files changed, 129 insertions(+), 56 deletions(-) create mode 100644 src/Payone/Webhook/Handler/AutoCaptureStatusHandler.php create mode 100644 tests/Payone/Webhook/Handler/AutoCaptureStatusHandlerTest.php diff --git a/src/DependencyInjection/webhooks.xml b/src/DependencyInjection/webhooks.xml index b575687d0..7aed8b8c8 100644 --- a/src/DependencyInjection/webhooks.xml +++ b/src/DependencyInjection/webhooks.xml @@ -10,21 +10,28 @@ - - + + - - + - + + - - + + + + + + + + + @@ -32,11 +39,7 @@ - - - - - + diff --git a/src/Payone/Webhook/Handler/AutoCaptureStatusHandler.php b/src/Payone/Webhook/Handler/AutoCaptureStatusHandler.php new file mode 100644 index 000000000..e2467a6a6 --- /dev/null +++ b/src/Payone/Webhook/Handler/AutoCaptureStatusHandler.php @@ -0,0 +1,39 @@ +transactionDataHandler->getPaymentTransactionByPayoneTransactionId( + $salesChannelContext->getContext(), + $request->request->getInt('txid') + ); + + if (!$paymentTransaction instanceof PaymentTransaction) { + return; + } + + $this->automaticCaptureService->captureIfPossible($paymentTransaction, $salesChannelContext); + } + + public function supports(SalesChannelContext $salesChannelContext, array $data): bool + { + return isset($data['txid']) && ($data['txaction'] ?? null) === 'appointed'; + } +} diff --git a/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php b/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php index a7790b960..6f8e4d336 100644 --- a/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php +++ b/src/Payone/Webhook/Handler/TransactionStatusWebhookHandler.php @@ -4,7 +4,6 @@ namespace PayonePayment\Payone\Webhook\Handler; -use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface; use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface; use PayonePayment\Components\TransactionStatus\TransactionStatusServiceInterface; use PayonePayment\Struct\PaymentTransaction; @@ -17,8 +16,7 @@ class TransactionStatusWebhookHandler implements WebhookHandlerInterface public function __construct( private readonly TransactionStatusServiceInterface $transactionStatusService, private readonly TransactionDataHandlerInterface $transactionDataHandler, - private readonly LoggerInterface $logger, - private readonly AutomaticCaptureServiceInterface $automaticCaptureService + private readonly LoggerInterface $logger ) { } @@ -54,17 +52,6 @@ public function process(SalesChannelContext $salesChannelContext, Request $reque $this->transactionDataHandler->saveTransactionData($paymentTransaction, $salesChannelContext->getContext(), $payoneTransactionData); $this->transactionStatusService->transitionByConfigMapping($salesChannelContext, $paymentTransaction, $data); - - // Reload the paymentTransaction for automatic capture - /** @var PaymentTransaction|null $paymentTransaction */ - $paymentTransaction = $this->transactionDataHandler->getPaymentTransactionByPayoneTransactionId( - $salesChannelContext->getContext(), - (int) $data['txid'] - ); - - if ($paymentTransaction) { - $this->automaticCaptureService->captureIfPossible($paymentTransaction, $salesChannelContext); - } } private function utf8EncodeRecursive(array $transactionData): array diff --git a/tests/Payone/Webhook/Handler/AutoCaptureStatusHandlerTest.php b/tests/Payone/Webhook/Handler/AutoCaptureStatusHandlerTest.php new file mode 100644 index 000000000..bf1e1754f --- /dev/null +++ b/tests/Payone/Webhook/Handler/AutoCaptureStatusHandlerTest.php @@ -0,0 +1,65 @@ +createMock(TransactionDataHandlerInterface::class), + $this->createMock(AutomaticCaptureServiceInterface::class), + ); + + $salesChannelMock = $this->createMock(SalesChannelContext::class); + + static::assertTrue($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => 'appointed']), 'supports should return true, cause of valid txid nd correct status'); + + static::assertFalse($handler->supports($salesChannelMock, ['txaction' => 'appointed']), 'supports should return false, cause of missing txid'); + static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1]), 'supports should return false, cause of missing status'); + static::assertFalse($handler->supports($salesChannelMock, []), 'supports should return false, cause of missing data'); + static::assertFalse($handler->supports($salesChannelMock, []), 'supports should return false, cause of missing data'); + + static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_CAPTURE]), 'supports should return false, cause of wrong status'); + static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_COMPLETED]), 'supports should return false, cause of wrong status'); + static::assertFalse($handler->supports($salesChannelMock, ['txid' => 1, 'txaction' => TransactionStatusService::ACTION_FAILED]), 'supports should return false, cause of wrong status'); + } + + public function testIsCaptureGotExecuted(): void + { + $dataHandler = $this->createMock(TransactionDataHandlerInterface::class); + $dataHandler->method('getPaymentTransactionByPayoneTransactionId')->willReturn($this->createMock(PaymentTransaction::class)); + $captureService = $this->createMock(AutomaticCaptureServiceInterface::class); + $captureService->expects(static::once())->method('captureIfPossible'); + + $request = new Request(); + $request->request->set('txid', 123); + + (new AutoCaptureStatusHandler($dataHandler, $captureService)) + ->process($this->createMock(SalesChannelContext::class), $request); + } + + public function testIsCaptureGotNotExecutedIfTransactionIsNotFound(): void + { + $dataHandler = $this->createMock(TransactionDataHandlerInterface::class); + $dataHandler->method('getPaymentTransactionByPayoneTransactionId')->willReturn(null); + $captureService = $this->createMock(AutomaticCaptureServiceInterface::class); + $captureService->expects(static::never())->method('captureIfPossible'); + + $request = new Request(); + $request->request->set('txid', 123); + + (new AutoCaptureStatusHandler($dataHandler, $captureService)) + ->process($this->createMock(SalesChannelContext::class), $request); + } +} diff --git a/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php b/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php index c3552706e..65bf92c4e 100644 --- a/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php +++ b/tests/Payone/Webhook/Handler/TransactionStatusWebhookHandlerTest.php @@ -4,7 +4,6 @@ namespace PayonePayment\Payone\Webhook\Handler; -use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface; use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface; use PayonePayment\Constants; use PayonePayment\PaymentHandler\PayoneCreditCardPaymentHandler; @@ -46,16 +45,12 @@ public function testItAppointsCreditCardWithoutMapping(): void ]; $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class); - $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); + $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData); - $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class); - $automaticCaptureService->expects(static::once())->method('captureIfPossible'); - $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler( $transactionStatusService, - $transactionDataHandler, - $automaticCaptureService + $transactionDataHandler ); $transactionStatusHandler->process( @@ -97,16 +92,12 @@ public function testItAppointsCreditCardWithMapping(): void ]; $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class); - $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); + $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData); - $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class); - $automaticCaptureService->expects(static::once())->method('captureIfPossible'); - $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler( $transactionStatusService, - $transactionDataHandler, - $automaticCaptureService + $transactionDataHandler ); $transactionStatusHandler->process( @@ -148,16 +139,12 @@ public function testItAppointsCreditCardWithSpecificMapping(): void ]; $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class); - $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); + $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData); - $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class); - $automaticCaptureService->expects(static::once())->method('captureIfPossible'); - $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler( $transactionStatusService, - $transactionDataHandler, - $automaticCaptureService + $transactionDataHandler ); $transactionStatusHandler->process( diff --git a/tests/Payone/Webhook/Processor/WebhookProcessorTest.php b/tests/Payone/Webhook/Processor/WebhookProcessorTest.php index 7eb1a39d0..cb4092906 100644 --- a/tests/Payone/Webhook/Processor/WebhookProcessorTest.php +++ b/tests/Payone/Webhook/Processor/WebhookProcessorTest.php @@ -4,7 +4,6 @@ namespace PayonePayment\Payone\Webhook\Processor; -use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface; use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface; use PayonePayment\Components\TransactionStatus\TransactionStatusService; use PayonePayment\Constants; @@ -213,16 +212,12 @@ protected function getWebhookProcessor(string $transitionName, array $transactio $paymentTransaction = PaymentTransaction::fromOrderTransaction($orderTransactionEntity, $orderEntity); $transactionDataHandler = $this->createMock(TransactionDataHandlerInterface::class); - $transactionDataHandler->expects(static::exactly(2))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); + $transactionDataHandler->expects(static::exactly(1))->method('getPaymentTransactionByPayoneTransactionId')->willReturn($paymentTransaction); $transactionDataHandler->expects(static::once())->method('getTransactionDataFromWebhook')->willReturn($transactionData); - $automaticCaptureService = $this->createMock(AutomaticCaptureServiceInterface::class); - $automaticCaptureService->expects(static::once())->method('captureIfPossible'); - $transactionStatusHandler = TransactionStatusWebhookHandlerFactory::createHandler( $transactionStatusService, - $transactionDataHandler, - $automaticCaptureService + $transactionDataHandler ); return new WebhookProcessor( diff --git a/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php b/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php index 5e4210002..d5fa11915 100644 --- a/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php +++ b/tests/TestCaseBase/Factory/TransactionStatusWebhookHandlerFactory.php @@ -4,7 +4,6 @@ namespace PayonePayment\TestCaseBase\Factory; -use PayonePayment\Components\AutomaticCaptureService\AutomaticCaptureServiceInterface; use PayonePayment\Components\Currency\CurrencyPrecision; use PayonePayment\Components\DataHandler\Transaction\TransactionDataHandlerInterface; use PayonePayment\Components\TransactionStatus\TransactionStatusService; @@ -29,13 +28,11 @@ class TransactionStatusWebhookHandlerFactory public static function createHandler( TransactionStatusServiceInterface $transactionStatusService, TransactionDataHandlerInterface $transactionDataHandler, - AutomaticCaptureServiceInterface $automaticCaptureService ): TransactionStatusWebhookHandler { return new TransactionStatusWebhookHandler( $transactionStatusService, $transactionDataHandler, - new NullLogger(), - $automaticCaptureService + new NullLogger() ); }