From c09c1efc9cc2ea19d2757a96b9231686ac5f95ff Mon Sep 17 00:00:00 2001 From: Alex P Date: Thu, 30 Sep 2021 09:11:10 +0300 Subject: [PATCH 01/14] Improve logging --- .../src/Endpoint/class-paymentsendpoint.php | 31 +++---------------- modules/ppcp-wc-gateway/services.php | 3 +- .../src/Processor/class-refundprocessor.php | 16 ++++++++-- 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php index 6e3ec6b6d..d9e20350b 100644 --- a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php +++ b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php @@ -195,10 +195,11 @@ class PaymentsEndpoint { * * @param Refund $refund The refund to be processed. * - * @return bool + * @return void * @throws RuntimeException If the request fails. + * @throws PayPalApiException If the request fails. */ - public function refund( Refund $refund ) : bool { + public function refund( Refund $refund ) : void { $bearer = $this->bearer->bearer(); $url = trailingslashit( $this->host ) . 'v2/payments/captures/' . $refund->for_capture()->id() . '/refund'; $args = array( @@ -215,37 +216,15 @@ class PaymentsEndpoint { $json = json_decode( $response['body'] ); if ( is_wp_error( $response ) ) { - $error = new RuntimeException( - __( 'Could not refund payment.', 'woocommerce-paypal-payments' ) - ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; + throw new RuntimeException( 'Could not refund payment.' ); } $status_code = (int) wp_remote_retrieve_response_code( $response ); if ( 201 !== $status_code ) { - $error = new PayPalApiException( + throw new PayPalApiException( $json, $status_code ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; } - - return true; } } diff --git a/modules/ppcp-wc-gateway/services.php b/modules/ppcp-wc-gateway/services.php index 93c04b827..9c3e45a13 100644 --- a/modules/ppcp-wc-gateway/services.php +++ b/modules/ppcp-wc-gateway/services.php @@ -215,7 +215,8 @@ return array( 'wcgateway.processor.refunds' => static function ( $container ): RefundProcessor { $order_endpoint = $container->get( 'api.endpoint.order' ); $payments_endpoint = $container->get( 'api.endpoint.payments' ); - return new RefundProcessor( $order_endpoint, $payments_endpoint ); + $logger = $container->get( 'woocommerce.logger.woocommerce' ); + return new RefundProcessor( $order_endpoint, $payments_endpoint, $logger ); }, 'wcgateway.processor.authorized-payments' => static function ( $container ): AuthorizedPaymentsProcessor { $order_endpoint = $container->get( 'api.endpoint.order' ); diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index 0ace800d0..0c5fc7279 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -9,6 +9,7 @@ declare( strict_types=1 ); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; +use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Amount; @@ -36,16 +37,25 @@ class RefundProcessor { */ private $payments_endpoint; + /** + * The logger. + * + * @var LoggerInterface + */ + private $logger; + /** * RefundProcessor constructor. * * @param OrderEndpoint $order_endpoint The order endpoint. * @param PaymentsEndpoint $payments_endpoint The payments endpoint. + * @param LoggerInterface $logger The logger. */ - public function __construct( OrderEndpoint $order_endpoint, PaymentsEndpoint $payments_endpoint ) { + public function __construct( OrderEndpoint $order_endpoint, PaymentsEndpoint $payments_endpoint, LoggerInterface $logger ) { $this->order_endpoint = $order_endpoint; $this->payments_endpoint = $payments_endpoint; + $this->logger = $logger; } /** @@ -91,8 +101,10 @@ class RefundProcessor { new Money( $amount, $wc_order->get_currency() ) ) ); - return $this->payments_endpoint->refund( $refund ); + $this->payments_endpoint->refund( $refund ); + return true; } catch ( RuntimeException $error ) { + $this->logger->error( 'Refund failed: ' . $error->getMessage() ); return false; } } From a1f3cb4a49d18d680332059664bb4fd3cb2f512c Mon Sep 17 00:00:00 2001 From: Alex P Date: Thu, 30 Sep 2021 09:14:48 +0300 Subject: [PATCH 02/14] Add status code in unknown PayPalApiException message --- .../src/Exception/class-paypalapiexception.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/modules/ppcp-api-client/src/Exception/class-paypalapiexception.php b/modules/ppcp-api-client/src/Exception/class-paypalapiexception.php index d5da7357e..6f227d001 100644 --- a/modules/ppcp-api-client/src/Exception/class-paypalapiexception.php +++ b/modules/ppcp-api-client/src/Exception/class-paypalapiexception.php @@ -39,9 +39,13 @@ class PayPalApiException extends RuntimeException { $response = new \stdClass(); } if ( ! isset( $response->message ) ) { - $response->message = __( - 'Unknown error while connecting to PayPal.', - 'woocommerce-paypal-payments' + $response->message = sprintf( + /* translators: %1$d - HTTP status code number (404, 500, ...) */ + __( + 'Unknown error while connecting to PayPal. Status code: %1$d.', + 'woocommerce-paypal-payments' + ), + $this->status_code ); } if ( ! isset( $response->name ) ) { From 393e3116b8a90a1913278eff134f3d64d8ba99b7 Mon Sep 17 00:00:00 2001 From: Alex P Date: Thu, 30 Sep 2021 11:26:17 +0300 Subject: [PATCH 03/14] Improve logging --- .../src/Processor/class-refundprocessor.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index 0c5fc7279..0c49aece1 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -9,6 +9,7 @@ declare( strict_types=1 ); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; +use Exception; use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; @@ -66,30 +67,30 @@ class RefundProcessor { * @param string $reason The reason for the refund. * * @return bool + * + * @phpcs:ignore Squiz.Commenting.FunctionCommentThrowTag.Missing */ public function process( \WC_Order $wc_order, float $amount = null, string $reason = '' ) : bool { - $order_id = $wc_order->get_meta( PayPalGateway::ORDER_ID_META_KEY ); - if ( ! $order_id ) { - return false; - } try { - $order = $this->order_endpoint->order( $order_id ); - if ( ! $order ) { - return false; + $order_id = $wc_order->get_meta( PayPalGateway::ORDER_ID_META_KEY ); + if ( ! $order_id ) { + throw new RuntimeException( 'PayPal order ID not found in meta.' ); } + $order = $this->order_endpoint->order( $order_id ); + $purchase_units = $order->purchase_units(); if ( ! $purchase_units ) { - return false; + throw new RuntimeException( 'No purchase units.' ); } $payments = $purchase_units[0]->payments(); if ( ! $payments ) { - return false; + throw new RuntimeException( 'No payments.' ); } $captures = $payments->captures(); if ( ! $captures ) { - return false; + throw new RuntimeException( 'No capture.' ); } $capture = $captures[0]; @@ -103,7 +104,7 @@ class RefundProcessor { ); $this->payments_endpoint->refund( $refund ); return true; - } catch ( RuntimeException $error ) { + } catch ( Exception $error ) { $this->logger->error( 'Refund failed: ' . $error->getMessage() ); return false; } From 4b3bba941a1a694796b2e248722efb79dbd6c11b Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 10:37:52 +0300 Subject: [PATCH 04/14] Void non-captured authorizations instead of refunding --- .../src/Endpoint/class-paymentsendpoint.php | 35 +++++++++ .../src/Processor/class-refundprocessor.php | 76 ++++++++++++++++--- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php index d9e20350b..3dbec14f3 100644 --- a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php +++ b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php @@ -227,4 +227,39 @@ class PaymentsEndpoint { ); } } + + /** + * Voids a transaction. + * + * @param Authorization $authorization The PayPal payment authorization to void. + * + * @return void + * @throws RuntimeException If the request fails. + * @throws PayPalApiException If the request fails. + */ + public function void( Authorization $authorization ) : void { + $bearer = $this->bearer->bearer(); + $url = trailingslashit( $this->host ) . 'v2/payments/authorizations/' . $authorization->id() . '/void'; + $args = array( + 'method' => 'POST', + 'headers' => array( + 'Authorization' => 'Bearer ' . $bearer->token(), + 'Content-Type' => 'application/json', + 'Prefer' => 'return=representation', + ), + ); + + $response = $this->request( $url, $args ); + + if ( is_wp_error( $response ) ) { + throw new RuntimeException( 'Could not void transaction.' ); + } + + $status_code = (int) wp_remote_retrieve_response_code( $response ); + // Currently it can return body with 200 status, despite the docs saying that it should be 204 No content. + // We don't care much about body, so just checking that it was successful. + if ( $status_code < 200 || $status_code > 299 ) { + throw new PayPalApiException( null, $status_code ); + } + } } diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index 0c49aece1..e0a91ac13 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -14,7 +14,9 @@ use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Amount; +use WooCommerce\PayPalCommerce\ApiClient\Entity\AuthorizationStatus; use WooCommerce\PayPalCommerce\ApiClient\Entity\Money; +use WooCommerce\PayPalCommerce\ApiClient\Entity\Payments; use WooCommerce\PayPalCommerce\ApiClient\Entity\Refund; use WooCommerce\PayPalCommerce\ApiClient\Exception\RuntimeException; use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway; @@ -24,6 +26,10 @@ use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway; */ class RefundProcessor { + private const REFUND_MODE_REFUND = 'refund'; + private const REFUND_MODE_VOID = 'void'; + private const REFUND_MODE_UNKNOWN = 'unknown'; + /** * The order endpoint. * @@ -88,25 +94,71 @@ class RefundProcessor { if ( ! $payments ) { throw new RuntimeException( 'No payments.' ); } - $captures = $payments->captures(); - if ( ! $captures ) { - throw new RuntimeException( 'No capture.' ); - } - $capture = $captures[0]; - $refund = new Refund( - $capture, - $capture->invoice_id(), - $reason, - new Amount( - new Money( $amount, $wc_order->get_currency() ) + $this->logger->debug( + sprintf( + 'Trying to refund/void order %1$s, payments: %2$s.', + $order->id(), + wp_json_encode( $payments->to_array() ) ) ); - $this->payments_endpoint->refund( $refund ); + + $mode = $this->determine_refund_mode( $payments ); + + switch ( $mode ) { + case self::REFUND_MODE_REFUND: + $captures = $payments->captures(); + if ( ! $captures ) { + throw new RuntimeException( 'No capture.' ); + } + + $capture = $captures[0]; + $refund = new Refund( + $capture, + $capture->invoice_id(), + $reason, + new Amount( + new Money( $amount, $wc_order->get_currency() ) + ) + ); + $this->payments_endpoint->refund( $refund ); + break; + case self::REFUND_MODE_VOID: + $authorizations = $payments->authorizations(); + if ( ! $authorizations ) { + throw new RuntimeException( 'No authorizations.' ); + } + + $this->payments_endpoint->void( $authorizations[0] ); + break; + default: + throw new RuntimeException( 'Nothing to refund/void.' ); + } + return true; } catch ( Exception $error ) { $this->logger->error( 'Refund failed: ' . $error->getMessage() ); return false; } } + + /** + * Determines the refunding mode. + * + * @param Payments $payments The order payments state. + * + * @return string One of the REFUND_MODE_ constants. + */ + private function determine_refund_mode( Payments $payments ): string { + $authorizations = $payments->authorizations(); + if ( $authorizations && $authorizations[0]->status()->is( AuthorizationStatus::CREATED ) ) { + return self::REFUND_MODE_VOID; + } + + if ( $payments->captures() ) { + return self::REFUND_MODE_REFUND; + } + + return self::REFUND_MODE_UNKNOWN; + } } From 85f3b184b2f9146152df4f25327704155662c2ad Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 15:47:44 +0300 Subject: [PATCH 05/14] Void all voidable authorizations --- .../src/Processor/class-refundprocessor.php | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index e0a91ac13..cb972de55 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -14,6 +14,7 @@ use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Amount; +use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; use WooCommerce\PayPalCommerce\ApiClient\Entity\AuthorizationStatus; use WooCommerce\PayPalCommerce\ApiClient\Entity\Money; use WooCommerce\PayPalCommerce\ApiClient\Entity\Payments; @@ -124,12 +125,18 @@ class RefundProcessor { $this->payments_endpoint->refund( $refund ); break; case self::REFUND_MODE_VOID: - $authorizations = $payments->authorizations(); - if ( ! $authorizations ) { - throw new RuntimeException( 'No authorizations.' ); + $voidable_authorizations = array_filter( + $payments->authorizations(), + array( $this, 'is_voidable_authorization' ) + ); + if ( ! $voidable_authorizations ) { + throw new RuntimeException( 'No voidable authorizations.' ); + } + + foreach ( $voidable_authorizations as $authorization ) { + $this->payments_endpoint->void( $authorization ); } - $this->payments_endpoint->void( $authorizations[0] ); break; default: throw new RuntimeException( 'Nothing to refund/void.' ); @@ -151,8 +158,12 @@ class RefundProcessor { */ private function determine_refund_mode( Payments $payments ): string { $authorizations = $payments->authorizations(); - if ( $authorizations && $authorizations[0]->status()->is( AuthorizationStatus::CREATED ) ) { - return self::REFUND_MODE_VOID; + if ( $authorizations ) { + foreach ( $authorizations as $authorization ) { + if ( $this->is_voidable_authorization( $authorization ) ) { + return self::REFUND_MODE_VOID; + } + } } if ( $payments->captures() ) { @@ -161,4 +172,14 @@ class RefundProcessor { return self::REFUND_MODE_UNKNOWN; } + + /** + * Checks whether the authorization can be voided. + * + * @param Authorization $authorization The authorization to check. + * @return bool + */ + private function is_voidable_authorization( Authorization $authorization ): bool { + return $authorization->status()->is( AuthorizationStatus::CREATED ); + } } From c6229c99444f8d62bb58f010023cc3561a5d56e8 Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 18:25:35 +0300 Subject: [PATCH 06/14] Simplify test --- .../AuthorizedPaymentsProcessorTest.php | 275 ++++++++---------- 1 file changed, 122 insertions(+), 153 deletions(-) diff --git a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php index f9583742f..4abba7d4c 100644 --- a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php +++ b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; +use WC_Order; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; @@ -17,186 +18,154 @@ use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway; use Mockery; class AuthorizedPaymentsProcessorTest extends TestCase { + private $wcOrder; + private $paypalOrderId = 'abc'; - public function testDefault() { - $orderId = 'abc'; - $authorizationId = 'def'; - $authorizationStatus = Mockery::mock(AuthorizationStatus::class); - $authorizationStatus - ->shouldReceive('is') - ->with(AuthorizationStatus::CREATED) - ->andReturn(true); - $authorization = Mockery::mock(Authorization::class); - $authorization - ->shouldReceive('id') - ->andReturn($authorizationId); - $authorization - ->shouldReceive('status') - ->andReturn($authorizationStatus); - $payments = Mockery::mock(Payments::class); - $payments - ->expects('authorizations') - ->andReturn([$authorization]); - $purchaseUnit = Mockery::mock(PurchaseUnit::class); - $purchaseUnit - ->expects('payments') - ->andReturn($payments); - $order = Mockery::mock(Order::class); - $order - ->expects('purchase_units') - ->andReturn([$purchaseUnit]); - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint - ->expects('order') - ->with($orderId) - ->andReturn($order); - $paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); - $paymentsEndpoint - ->expects('capture') - ->with($authorizationId) - ->andReturn($authorization); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $paymentsEndpoint); + private $authorizationId = 'qwe'; - $wcOrder = Mockery::mock(\WC_Order::class); - $wcOrder - ->expects('get_meta') - ->with(PayPalGateway::ORDER_ID_META_KEY) - ->andReturn($orderId); - $this->assertTrue($testee->process($wcOrder)); + private $paypalOrder; + + private $orderEndpoint; + + private $paymentsEndpoint; + + public function setUp(): void { + parent::setUp(); + + $this->wcOrder = $this->createWcOrder($this->paypalOrderId); + + $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::CREATED)]); + + $this->orderEndpoint = Mockery::mock(OrderEndpoint::class); + $this->orderEndpoint + ->shouldReceive('order') + ->with($this->paypalOrderId) + ->andReturnUsing(function () { + return $this->paypalOrder; + }); + + $this->paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); + } + + public function testSuccess() { + $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); + + $this->paymentsEndpoint + ->expects('capture') + ->with($this->authorizationId) + ->andReturn($this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)); + + $this->assertTrue($testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->last_status()); + } + + public function testCapturesAllCaptureable() { + $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); + + $authorizations = [ + $this->createAuthorization('id1', AuthorizationStatus::CREATED), + $this->createAuthorization('id2', AuthorizationStatus::VOIDED), + $this->createAuthorization('id3', AuthorizationStatus::PENDING), + $this->createAuthorization('id4', AuthorizationStatus::CAPTURED), + $this->createAuthorization('id5', AuthorizationStatus::DENIED), + $this->createAuthorization('id6', AuthorizationStatus::EXPIRED), + $this->createAuthorization('id7', AuthorizationStatus::COMPLETED), + ]; + $this->paypalOrder = $this->createPaypalOrder($authorizations); + + foreach ([$authorizations[0], $authorizations[2]] as $authorization) { + $this->paymentsEndpoint + ->expects('capture') + ->with($authorization->id()) + ->andReturn($this->createAuthorization($authorization->id(), AuthorizationStatus::CAPTURED)); + } + + $this->assertTrue($testee->process($this->wcOrder)); $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->last_status()); } public function testInaccessible() { - $orderId = 'abc'; $orderEndpoint = Mockery::mock(OrderEndpoint::class); $orderEndpoint ->expects('order') - ->with($orderId) + ->with($this->paypalOrderId) ->andThrow(RuntimeException::class); - $paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $paymentsEndpoint); - $wcOrder = Mockery::mock(\WC_Order::class); - $wcOrder - ->expects('get_meta') - ->with(PayPalGateway::ORDER_ID_META_KEY) - ->andReturn($orderId); - $this->assertFalse($testee->process($wcOrder)); + $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); + + $this->assertFalse($testee->process($this->wcOrder)); $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $testee->last_status()); } public function testNotFound() { - $orderId = 'abc'; $orderEndpoint = Mockery::mock(OrderEndpoint::class); $orderEndpoint ->expects('order') - ->with($orderId) - ->andThrow(new RuntimeException("text", 404)); - $paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $paymentsEndpoint); + ->with($this->paypalOrderId) + ->andThrow(new RuntimeException('text', 404)); - $wcOrder = Mockery::mock(\WC_Order::class); - $wcOrder - ->expects('get_meta') - ->with(PayPalGateway::ORDER_ID_META_KEY) - ->andReturn($orderId); - $this->assertFalse($testee->process($wcOrder)); + $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); + + $this->assertFalse($testee->process($this->wcOrder)); $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $testee->last_status()); } public function testCaptureFails() { - $orderId = 'abc'; - $authorizationId = 'def'; - $authorizationStatus = Mockery::mock(AuthorizationStatus::class); - $authorizationStatus - ->shouldReceive('is') - ->with(AuthorizationStatus::CREATED) - ->andReturn(true); - $authorization = Mockery::mock(Authorization::class); - $authorization - ->shouldReceive('id') - ->andReturn($authorizationId); - $authorization - ->shouldReceive('status') - ->andReturn($authorizationStatus); - $payments = Mockery::mock(Payments::class); - $payments - ->expects('authorizations') - ->andReturn([$authorization]); - $purchaseUnit = Mockery::mock(PurchaseUnit::class); - $purchaseUnit - ->expects('payments') - ->andReturn($payments); - $order = Mockery::mock(Order::class); - $order - ->expects('purchase_units') - ->andReturn([$purchaseUnit]); - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint - ->expects('order') - ->with($orderId) - ->andReturn($order); - $paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); - $paymentsEndpoint - ->expects('capture') - ->with($authorizationId) - ->andThrow(RuntimeException::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $paymentsEndpoint); + $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $wcOrder = Mockery::mock(\WC_Order::class); - $wcOrder - ->expects('get_meta') - ->with(PayPalGateway::ORDER_ID_META_KEY) - ->andReturn($orderId); - $this->assertFalse($testee->process($wcOrder)); + $this->paymentsEndpoint + ->expects('capture') + ->with($this->authorizationId) + ->andThrow(RuntimeException::class); + + $this->assertFalse($testee->process($this->wcOrder)); $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $testee->last_status()); } - public function testAllAreCaptured() { - $orderId = 'abc'; - $authorizationId = 'def'; - $authorizationStatus = Mockery::mock(AuthorizationStatus::class); - $authorizationStatus - ->shouldReceive('is') - ->with(AuthorizationStatus::CREATED) - ->andReturn(false); - $authorizationStatus - ->shouldReceive('is') - ->with(AuthorizationStatus::PENDING) - ->andReturn(false); - $authorization = Mockery::mock(Authorization::class); - $authorization - ->shouldReceive('id') - ->andReturn($authorizationId); - $authorization - ->shouldReceive('status') - ->andReturn($authorizationStatus); - $payments = Mockery::mock(Payments::class); - $payments - ->expects('authorizations') - ->andReturn([$authorization]); - $purchaseUnit = Mockery::mock(PurchaseUnit::class); - $purchaseUnit - ->expects('payments') - ->andReturn($payments); - $order = Mockery::mock(Order::class); - $order - ->expects('purchase_units') - ->andReturn([$purchaseUnit]); - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint - ->expects('order') - ->with($orderId) - ->andReturn($order); - $paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $paymentsEndpoint); + public function testAlreadyCaptured() { + $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $wcOrder = Mockery::mock(\WC_Order::class); - $wcOrder - ->expects('get_meta') - ->with(PayPalGateway::ORDER_ID_META_KEY) - ->andReturn($orderId); - $this->assertFalse($testee->process($wcOrder)); + $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)]); + + $this->assertFalse($testee->process($this->wcOrder)); $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $testee->last_status()); } -} \ No newline at end of file + + private function createWcOrder(string $paypalOrderId): WC_Order { + $wcOrder = Mockery::mock(WC_Order::class); + $wcOrder + ->shouldReceive('get_meta') + ->with(PayPalGateway::ORDER_ID_META_KEY) + ->andReturn($paypalOrderId); + return $wcOrder; + } + + private function createAuthorization(string $id, string $status): Authorization { + $authorization = Mockery::mock(Authorization::class); + $authorization + ->shouldReceive('id') + ->andReturn($id); + $authorization + ->shouldReceive('status') + ->andReturn(new AuthorizationStatus($status)); + return $authorization; + } + + private function createPaypalOrder(array $authorizations): Order { + $payments = Mockery::mock(Payments::class); + $payments + ->shouldReceive('authorizations') + ->andReturn($authorizations); + + $purchaseUnit = Mockery::mock(PurchaseUnit::class); + $purchaseUnit + ->shouldReceive('payments') + ->andReturn($payments); + + $order = Mockery::mock(Order::class); + $order + ->shouldReceive('purchase_units') + ->andReturn([$purchaseUnit]); + return $order; + } +} From 07928c99359fe3fdeb9c2956a53e5b3f0b9d7c83 Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 18:36:30 +0300 Subject: [PATCH 07/14] Return status instead of bool + last_status --- .../src/Gateway/class-paypalgateway.php | 8 ++--- .../class-authorizedpaymentsprocessor.php | 36 ++++--------------- .../src/Processor/class-orderprocessor.php | 2 +- .../WcGateway/Gateway/WcGatewayTest.php | 15 ++------ .../AuthorizedPaymentsProcessorTest.php | 18 ++++------ 5 files changed, 21 insertions(+), 58 deletions(-) diff --git a/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php b/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php index 4ca2f947b..875277810 100644 --- a/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php +++ b/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php @@ -239,10 +239,10 @@ class PayPalGateway extends \WC_Payment_Gateway { * @return bool */ public function capture_authorized_payment( \WC_Order $wc_order ): bool { - $is_processed = $this->authorized_payments->process( $wc_order ); - $this->render_authorization_message_for_status( $this->authorized_payments->last_status() ); + $result_status = $this->authorized_payments->process( $wc_order ); + $this->render_authorization_message_for_status( $result_status ); - if ( $is_processed ) { + if ( AuthorizedPaymentsProcessor::SUCCESSFUL === $result_status ) { $wc_order->add_order_note( __( 'Payment successfully captured.', 'woocommerce-paypal-payments' ) ); @@ -252,7 +252,7 @@ class PayPalGateway extends \WC_Payment_Gateway { return true; } - if ( $this->authorized_payments->last_status() === AuthorizedPaymentsProcessor::ALREADY_CAPTURED ) { + if ( AuthorizedPaymentsProcessor::ALREADY_CAPTURED === $result_status ) { if ( $wc_order->get_status() === 'on-hold' ) { $wc_order->add_order_note( __( 'Payment successfully captured.', 'woocommerce-paypal-payments' ) diff --git a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php index 4b21e7198..86f542cc0 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php @@ -42,13 +42,6 @@ class AuthorizedPaymentsProcessor { */ private $payments_endpoint; - /** - * The last status. - * - * @var string - */ - private $last_status = ''; - /** * AuthorizedPaymentsProcessor constructor. * @@ -69,46 +62,31 @@ class AuthorizedPaymentsProcessor { * * @param \WC_Order $wc_order The WooCommerce order. * - * @return bool + * @return string One of the AuthorizedPaymentsProcessor status constants. */ - public function process( \WC_Order $wc_order ): bool { + public function process( \WC_Order $wc_order ): string { try { $order = $this->paypal_order_from_wc_order( $wc_order ); } catch ( Exception $exception ) { if ( $exception->getCode() === 404 ) { - $this->last_status = self::NOT_FOUND; - return false; + return self::NOT_FOUND; } - $this->last_status = self::INACCESSIBLE; - return false; + return self::INACCESSIBLE; } $authorizations = $this->all_authorizations( $order ); if ( ! $this->are_authorzations_to_capture( ...$authorizations ) ) { - $this->last_status = self::ALREADY_CAPTURED; - return false; + return self::ALREADY_CAPTURED; } try { $this->capture_authorizations( ...$authorizations ); } catch ( Exception $exception ) { - $this->last_status = self::FAILED; - return false; + return self::FAILED; } - $this->last_status = self::SUCCESSFUL; - return true; - } - - /** - * Returns the last status. - * - * @return string - */ - public function last_status(): string { - - return $this->last_status; + return self::SUCCESSFUL; } /** diff --git a/modules/ppcp-wc-gateway/src/Processor/class-orderprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-orderprocessor.php index 6f908c01c..022604f7e 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-orderprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-orderprocessor.php @@ -188,7 +188,7 @@ class OrderProcessor { $wc_order->payment_complete(); } - if ( $this->capture_authorized_downloads( $order ) && $this->authorized_payments_processor->process( $wc_order ) ) { + if ( $this->capture_authorized_downloads( $order ) && AuthorizedPaymentsProcessor::SUCCESSFUL === $this->authorized_payments_processor->process( $wc_order ) ) { $wc_order->add_order_note( __( 'Payment successfully captured.', 'woocommerce-paypal-payments' ) ); diff --git a/tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php b/tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php index 65ed9776c..3070a2966 100644 --- a/tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php +++ b/tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php @@ -227,10 +227,7 @@ class WcGatewayTest extends TestCase $authorizedPaymentsProcessor ->expects('process') ->with($wcOrder) - ->andReturnTrue(); - $authorizedPaymentsProcessor - ->expects('last_status') - ->andReturn(AuthorizedPaymentsProcessor::SUCCESSFUL); + ->andReturn(AuthorizedPaymentsProcessor::SUCCESSFUL); $authorizedOrderActionNotice = Mockery::mock(AuthorizeOrderActionNotice::class); $authorizedOrderActionNotice ->expects('display_message') @@ -286,10 +283,7 @@ class WcGatewayTest extends TestCase $authorizedPaymentsProcessor ->expects('process') ->with($wcOrder) - ->andReturnFalse(); - $authorizedPaymentsProcessor - ->shouldReceive('last_status') - ->andReturn(AuthorizedPaymentsProcessor::ALREADY_CAPTURED); + ->andReturn(AuthorizedPaymentsProcessor::ALREADY_CAPTURED); $authorizedOrderActionNotice = Mockery::mock(AuthorizeOrderActionNotice::class); $authorizedOrderActionNotice ->expects('display_message') @@ -338,10 +332,7 @@ class WcGatewayTest extends TestCase $authorizedPaymentsProcessor ->expects('process') ->with($wcOrder) - ->andReturnFalse(); - $authorizedPaymentsProcessor - ->shouldReceive('last_status') - ->andReturn($lastStatus); + ->andReturn($lastStatus); $authorizedOrderActionNotice = Mockery::mock(AuthorizeOrderActionNotice::class); $authorizedOrderActionNotice ->expects('display_message') diff --git a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php index 4abba7d4c..2c8ad9e75 100644 --- a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php +++ b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php @@ -55,8 +55,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->with($this->authorizationId) ->andReturn($this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)); - $this->assertTrue($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); } public function testCapturesAllCaptureable() { @@ -80,8 +79,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->andReturn($this->createAuthorization($authorization->id(), AuthorizationStatus::CAPTURED)); } - $this->assertTrue($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); } public function testInaccessible() { @@ -93,8 +91,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - $this->assertFalse($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $testee->process($this->wcOrder)); } public function testNotFound() { @@ -106,8 +103,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - $this->assertFalse($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $testee->process($this->wcOrder)); } public function testCaptureFails() { @@ -118,8 +114,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->with($this->authorizationId) ->andThrow(RuntimeException::class); - $this->assertFalse($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $testee->process($this->wcOrder)); } public function testAlreadyCaptured() { @@ -127,8 +122,7 @@ class AuthorizedPaymentsProcessorTest extends TestCase $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)]); - $this->assertFalse($testee->process($this->wcOrder)); - $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $testee->last_status()); + $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $testee->process($this->wcOrder)); } private function createWcOrder(string $paypalOrderId): WC_Order { From 06e437e0c3607510fae728f0be4c95217213242f Mon Sep 17 00:00:00 2001 From: Alex P Date: Mon, 4 Oct 2021 19:16:50 +0300 Subject: [PATCH 08/14] Refactor logging, add missing logging --- .../src/Endpoint/class-paymentsendpoint.php | 28 ++------------ modules/ppcp-wc-gateway/services.php | 3 +- .../class-authorizedpaymentsprocessor.php | 14 ++++++- .../Endpoint/PaymentsEndpointTest.php | 13 ++----- .../AuthorizedPaymentsProcessorTest.php | 38 ++++++++----------- 5 files changed, 37 insertions(+), 59 deletions(-) diff --git a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php index 6e3ec6b6d..66e0a6bcd 100644 --- a/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php +++ b/modules/ppcp-api-client/src/Endpoint/class-paymentsendpoint.php @@ -138,6 +138,7 @@ class PaymentsEndpoint { * * @return Authorization * @throws RuntimeException If the request fails. + * @throws PayPalApiException If the request fails. */ public function capture( string $authorization_id ): Authorization { $bearer = $this->bearer->bearer(); @@ -155,39 +156,18 @@ class PaymentsEndpoint { $json = json_decode( $response['body'] ); if ( is_wp_error( $response ) ) { - $error = new RuntimeException( - __( 'Could not capture authorized payment.', 'woocommerce-paypal-payments' ) - ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; + throw new RuntimeException( 'Could not capture authorized payment.' ); } $status_code = (int) wp_remote_retrieve_response_code( $response ); if ( 201 !== $status_code ) { - $error = new PayPalApiException( + throw new PayPalApiException( $json, $status_code ); - $this->logger->log( - 'warning', - $error->getMessage(), - array( - 'args' => $args, - 'response' => $response, - ) - ); - throw $error; } - $authorization = $this->authorizations_factory->from_paypal_response( $json ); - return $authorization; + return $this->authorizations_factory->from_paypal_response( $json ); } /** diff --git a/modules/ppcp-wc-gateway/services.php b/modules/ppcp-wc-gateway/services.php index 93c04b827..a8b001c9a 100644 --- a/modules/ppcp-wc-gateway/services.php +++ b/modules/ppcp-wc-gateway/services.php @@ -220,7 +220,8 @@ return array( 'wcgateway.processor.authorized-payments' => static function ( $container ): AuthorizedPaymentsProcessor { $order_endpoint = $container->get( 'api.endpoint.order' ); $payments_endpoint = $container->get( 'api.endpoint.payments' ); - return new AuthorizedPaymentsProcessor( $order_endpoint, $payments_endpoint ); + $logger = $container->get( 'woocommerce.logger.woocommerce' ); + return new AuthorizedPaymentsProcessor( $order_endpoint, $payments_endpoint, $logger ); }, 'wcgateway.admin.render-authorize-action' => static function ( $container ): RenderAuthorizeAction { diff --git a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php index 86f542cc0..453f0652d 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php @@ -10,6 +10,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; use Exception; +use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; @@ -42,19 +43,29 @@ class AuthorizedPaymentsProcessor { */ private $payments_endpoint; + /** + * The logger. + * + * @var LoggerInterface + */ + private $logger; + /** * AuthorizedPaymentsProcessor constructor. * * @param OrderEndpoint $order_endpoint The Order endpoint. * @param PaymentsEndpoint $payments_endpoint The Payments endpoint. + * @param LoggerInterface $logger The logger. */ public function __construct( OrderEndpoint $order_endpoint, - PaymentsEndpoint $payments_endpoint + PaymentsEndpoint $payments_endpoint, + LoggerInterface $logger ) { $this->order_endpoint = $order_endpoint; $this->payments_endpoint = $payments_endpoint; + $this->logger = $logger; } /** @@ -83,6 +94,7 @@ class AuthorizedPaymentsProcessor { try { $this->capture_authorizations( ...$authorizations ); } catch ( Exception $exception ) { + $this->logger->error( 'Failed to capture authorization: ' . $exception->getMessage() ); return self::FAILED; } diff --git a/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php b/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php index 87b2ca9e9..65112a61d 100644 --- a/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php +++ b/tests/PHPUnit/ApiClient/Endpoint/PaymentsEndpointTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\ApiClient\Endpoint; +use Psr\Log\NullLogger; use Requests_Utility_CaseInsensitiveDictionary; use WooCommerce\PayPalCommerce\ApiClient\Authentication\Bearer; use WooCommerce\PayPalCommerce\ApiClient\Entity\Authorization; @@ -235,10 +236,6 @@ class PaymentsEndpointTest extends TestCase $authorizationFactory = Mockery::mock(AuthorizationFactory::class); - $logger = Mockery::mock(LoggerInterface::class); - $logger->expects('log'); - $logger->expects('debug'); - $headers = Mockery::mock(Requests_Utility_CaseInsensitiveDictionary::class); $headers->shouldReceive('getAll'); $rawResponse = [ @@ -250,7 +247,7 @@ class PaymentsEndpointTest extends TestCase $host, $bearer, $authorizationFactory, - $logger + new NullLogger() ); expect('wp_remote_get')->andReturn($rawResponse); @@ -281,15 +278,11 @@ class PaymentsEndpointTest extends TestCase 'headers' => $headers, ]; - $logger = Mockery::mock(LoggerInterface::class); - $logger->expects('log'); - $logger->expects('debug'); - $testee = new PaymentsEndpoint( $host, $bearer, $authorizationFactory, - $logger + new NullLogger() ); expect('wp_remote_get')->andReturn($rawResponse); diff --git a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php index 2c8ad9e75..d653f466a 100644 --- a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php +++ b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Processor; +use Psr\Log\NullLogger; use WC_Order; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PaymentsEndpoint; @@ -29,6 +30,8 @@ class AuthorizedPaymentsProcessorTest extends TestCase private $paymentsEndpoint; + private $testee; + public function setUp(): void { parent::setUp(); @@ -42,25 +45,24 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->with($this->paypalOrderId) ->andReturnUsing(function () { return $this->paypalOrder; - }); + }) + ->byDefault(); $this->paymentsEndpoint = Mockery::mock(PaymentsEndpoint::class); + + $this->testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint, new NullLogger()); } public function testSuccess() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paymentsEndpoint ->expects('capture') ->with($this->authorizationId) ->andReturn($this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)); - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $this->testee->process($this->wcOrder)); } public function testCapturesAllCaptureable() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $authorizations = [ $this->createAuthorization('id1', AuthorizationStatus::CREATED), $this->createAuthorization('id2', AuthorizationStatus::VOIDED), @@ -79,50 +81,40 @@ class AuthorizedPaymentsProcessorTest extends TestCase ->andReturn($this->createAuthorization($authorization->id(), AuthorizationStatus::CAPTURED)); } - $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::SUCCESSFUL, $this->testee->process($this->wcOrder)); } public function testInaccessible() { - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint + $this->orderEndpoint ->expects('order') ->with($this->paypalOrderId) ->andThrow(RuntimeException::class); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - - $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::INACCESSIBLE, $this->testee->process($this->wcOrder)); } public function testNotFound() { - $orderEndpoint = Mockery::mock(OrderEndpoint::class); - $orderEndpoint + $this->orderEndpoint ->expects('order') ->with($this->paypalOrderId) ->andThrow(new RuntimeException('text', 404)); - $testee = new AuthorizedPaymentsProcessor($orderEndpoint, $this->paymentsEndpoint); - - $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::NOT_FOUND, $this->testee->process($this->wcOrder)); } public function testCaptureFails() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paymentsEndpoint ->expects('capture') ->with($this->authorizationId) ->andThrow(RuntimeException::class); - $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::FAILED, $this->testee->process($this->wcOrder)); } public function testAlreadyCaptured() { - $testee = new AuthorizedPaymentsProcessor($this->orderEndpoint, $this->paymentsEndpoint); - $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::CAPTURED)]); - $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $testee->process($this->wcOrder)); + $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $this->testee->process($this->wcOrder)); } private function createWcOrder(string $paypalOrderId): WC_Order { From ea7f56c402e55a846741c031c0ec7b95921b065e Mon Sep 17 00:00:00 2001 From: Alex P Date: Tue, 5 Oct 2021 08:57:55 +0300 Subject: [PATCH 09/14] Handle denied authorization when capturing via order action If nothing to capture and do not have captures, then show error. --- .../src/Gateway/class-paypalgateway.php | 9 +-- .../class-authorizeorderactionnotice.php | 30 +++++---- .../class-authorizedpaymentsprocessor.php | 63 ++++++++++++------- .../AuthorizedPaymentsProcessorTest.php | 6 ++ 4 files changed, 72 insertions(+), 36 deletions(-) diff --git a/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php b/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php index 875277810..fabce710a 100644 --- a/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php +++ b/modules/ppcp-wc-gateway/src/Gateway/class-paypalgateway.php @@ -275,10 +275,11 @@ class PayPalGateway extends \WC_Payment_Gateway { private function render_authorization_message_for_status( string $status ) { $message_mapping = array( - AuthorizedPaymentsProcessor::SUCCESSFUL => AuthorizeOrderActionNotice::SUCCESS, - AuthorizedPaymentsProcessor::ALREADY_CAPTURED => AuthorizeOrderActionNotice::ALREADY_CAPTURED, - AuthorizedPaymentsProcessor::INACCESSIBLE => AuthorizeOrderActionNotice::NO_INFO, - AuthorizedPaymentsProcessor::NOT_FOUND => AuthorizeOrderActionNotice::NOT_FOUND, + AuthorizedPaymentsProcessor::SUCCESSFUL => AuthorizeOrderActionNotice::SUCCESS, + AuthorizedPaymentsProcessor::ALREADY_CAPTURED => AuthorizeOrderActionNotice::ALREADY_CAPTURED, + AuthorizedPaymentsProcessor::INACCESSIBLE => AuthorizeOrderActionNotice::NO_INFO, + AuthorizedPaymentsProcessor::NOT_FOUND => AuthorizeOrderActionNotice::NOT_FOUND, + AuthorizedPaymentsProcessor::BAD_AUTHORIZATION => AuthorizeOrderActionNotice::BAD_AUTHORIZATION, ); $display_message = ( isset( $message_mapping[ $status ] ) ) ? $message_mapping[ $status ] diff --git a/modules/ppcp-wc-gateway/src/Notice/class-authorizeorderactionnotice.php b/modules/ppcp-wc-gateway/src/Notice/class-authorizeorderactionnotice.php index 663273217..114257a80 100644 --- a/modules/ppcp-wc-gateway/src/Notice/class-authorizeorderactionnotice.php +++ b/modules/ppcp-wc-gateway/src/Notice/class-authorizeorderactionnotice.php @@ -18,11 +18,12 @@ class AuthorizeOrderActionNotice { const QUERY_PARAM = 'ppcp-authorized-message'; - const NO_INFO = 81; - const ALREADY_CAPTURED = 82; - const FAILED = 83; - const SUCCESS = 84; - const NOT_FOUND = 85; + const NO_INFO = 81; + const ALREADY_CAPTURED = 82; + const FAILED = 83; + const SUCCESS = 84; + const NOT_FOUND = 85; + const BAD_AUTHORIZATION = 86; /** * Returns the current message if there is one. @@ -45,35 +46,42 @@ class AuthorizeOrderActionNotice { * @return array */ private function current_message(): array { - $messages[ self::NO_INFO ] = array( + $messages[ self::NO_INFO ] = array( 'message' => __( 'Could not retrieve information. Try again later.', 'woocommerce-paypal-payments' ), 'type' => 'error', ); - $messages[ self::ALREADY_CAPTURED ] = array( + $messages[ self::ALREADY_CAPTURED ] = array( 'message' => __( 'Payment already captured.', 'woocommerce-paypal-payments' ), 'type' => 'error', ); - $messages[ self::FAILED ] = array( + $messages[ self::FAILED ] = array( 'message' => __( - 'Failed to capture. Try again later.', + 'Failed to capture. Try again later or checks the logs.', 'woocommerce-paypal-payments' ), 'type' => 'error', ); - $messages[ self::NOT_FOUND ] = array( + $messages[ self::BAD_AUTHORIZATION ] = array( + 'message' => __( + 'Cannot capture, no valid payment authorization.', + 'woocommerce-paypal-payments' + ), + 'type' => 'error', + ); + $messages[ self::NOT_FOUND ] = array( 'message' => __( 'Could not find payment to process.', 'woocommerce-paypal-payments' ), 'type' => 'error', ); - $messages[ self::SUCCESS ] = array( + $messages[ self::SUCCESS ] = array( 'message' => __( 'Payment successfully captured.', 'woocommerce-paypal-payments' diff --git a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php index 453f0652d..153e8ebbc 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-authorizedpaymentsprocessor.php @@ -23,11 +23,12 @@ use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway; */ class AuthorizedPaymentsProcessor { - const SUCCESSFUL = 'SUCCESSFUL'; - const ALREADY_CAPTURED = 'ALREADY_CAPTURED'; - const FAILED = 'FAILED'; - const INACCESSIBLE = 'INACCESSIBLE'; - const NOT_FOUND = 'NOT_FOUND'; + const SUCCESSFUL = 'SUCCESSFUL'; + const ALREADY_CAPTURED = 'ALREADY_CAPTURED'; + const FAILED = 'FAILED'; + const INACCESSIBLE = 'INACCESSIBLE'; + const NOT_FOUND = 'NOT_FOUND'; + const BAD_AUTHORIZATION = 'BAD_AUTHORIZATION'; /** * The Order endpoint. @@ -87,8 +88,12 @@ class AuthorizedPaymentsProcessor { $authorizations = $this->all_authorizations( $order ); - if ( ! $this->are_authorzations_to_capture( ...$authorizations ) ) { - return self::ALREADY_CAPTURED; + if ( ! $this->authorizations_to_capture( ...$authorizations ) ) { + if ( $this->captured_authorizations( ...$authorizations ) ) { + return self::ALREADY_CAPTURED; + } + + return self::BAD_AUTHORIZATION; } try { @@ -131,17 +136,6 @@ class AuthorizedPaymentsProcessor { return $authorizations; } - /** - * Whether Authorizations need to be captured. - * - * @param Authorization ...$authorizations All Authorizations. - * - * @return bool - */ - private function are_authorzations_to_capture( Authorization ...$authorizations ): bool { - return (bool) count( $this->authorizations_to_capture( ...$authorizations ) ); - } - /** * Captures the authorizations. * @@ -161,11 +155,38 @@ class AuthorizedPaymentsProcessor { * @return Authorization[] */ private function authorizations_to_capture( Authorization ...$authorizations ): array { + return $this->filter_authorizations( + $authorizations, + array( AuthorizationStatus::CREATED, AuthorizationStatus::PENDING ) + ); + } + + /** + * The authorizations which were captured. + * + * @param Authorization ...$authorizations All Authorizations. + * @return Authorization[] + */ + private function captured_authorizations( Authorization ...$authorizations ): array { + return $this->filter_authorizations( + $authorizations, + array( AuthorizationStatus::CAPTURED ) + ); + } + + /** + * The authorizations which need to be filtered. + * + * @param Authorization[] $authorizations All Authorizations. + * @param string[] $statuses Allowed statuses, the constants from AuthorizationStatus. + * @return Authorization[] + */ + private function filter_authorizations( array $authorizations, array $statuses ): array { return array_filter( $authorizations, - static function ( Authorization $authorization ): bool { - return $authorization->status()->is( AuthorizationStatus::CREATED ) - || $authorization->status()->is( AuthorizationStatus::PENDING ); + static function ( Authorization $authorization ) use ( $statuses ): bool { + $status = $authorization->status(); + return in_array( $status->name(), $statuses, true ); } ); } diff --git a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php index d653f466a..2672d2f13 100644 --- a/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php +++ b/tests/PHPUnit/WcGateway/Processor/AuthorizedPaymentsProcessorTest.php @@ -117,6 +117,12 @@ class AuthorizedPaymentsProcessorTest extends TestCase $this->assertEquals(AuthorizedPaymentsProcessor::ALREADY_CAPTURED, $this->testee->process($this->wcOrder)); } + public function testBadAuthorization() { + $this->paypalOrder = $this->createPaypalOrder([$this->createAuthorization($this->authorizationId, AuthorizationStatus::DENIED)]); + + $this->assertEquals(AuthorizedPaymentsProcessor::BAD_AUTHORIZATION, $this->testee->process($this->wcOrder)); + } + private function createWcOrder(string $paypalOrderId): WC_Order { $wcOrder = Mockery::mock(WC_Order::class); $wcOrder From 48a6a8069f5f423809c2685137afcbcae8b069a2 Mon Sep 17 00:00:00 2001 From: Alex P Date: Tue, 5 Oct 2021 15:19:50 +0300 Subject: [PATCH 10/14] Set refunded status after void --- .../ppcp-wc-gateway/src/Processor/class-refundprocessor.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index cb972de55..bf3d63ad1 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -137,6 +137,9 @@ class RefundProcessor { $this->payments_endpoint->void( $authorization ); } + $wc_order->set_status('refunded'); + $wc_order->save(); + break; default: throw new RuntimeException( 'Nothing to refund/void.' ); From 2405c7e801427483d0652dae597c39168fcfce33 Mon Sep 17 00:00:00 2001 From: Alex P Date: Tue, 5 Oct 2021 15:24:39 +0300 Subject: [PATCH 11/14] Fix indent --- modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php index bf3d63ad1..c5ca98057 100644 --- a/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php +++ b/modules/ppcp-wc-gateway/src/Processor/class-refundprocessor.php @@ -137,7 +137,7 @@ class RefundProcessor { $this->payments_endpoint->void( $authorization ); } - $wc_order->set_status('refunded'); + $wc_order->set_status( 'refunded' ); $wc_order->save(); break; From 25b3bd148fb741145af137b67934975094768aa7 Mon Sep 17 00:00:00 2001 From: Alex P Date: Wed, 6 Oct 2021 12:26:54 +0300 Subject: [PATCH 12/14] Refactor capture status rendering --- modules/ppcp-wc-gateway/services.php | 3 ++- .../class-ordertablepaymentstatuscolumn.php | 11 +++++---- .../Admin/class-paymentstatusorderdetail.php | 24 +++++++++++++------ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/modules/ppcp-wc-gateway/services.php b/modules/ppcp-wc-gateway/services.php index bd84a86f3..fe1d32aa9 100644 --- a/modules/ppcp-wc-gateway/services.php +++ b/modules/ppcp-wc-gateway/services.php @@ -229,7 +229,8 @@ return array( return new RenderAuthorizeAction(); }, 'wcgateway.admin.order-payment-status' => static function ( $container ): PaymentStatusOrderDetail { - return new PaymentStatusOrderDetail(); + $column = $container->get( 'wcgateway.admin.orders-payment-status-column' ); + return new PaymentStatusOrderDetail( $column ); }, 'wcgateway.admin.orders-payment-status-column' => static function ( $container ): OrderTablePaymentStatusColumn { $settings = $container->get( 'wcgateway.settings' ); diff --git a/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php b/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php index 8bce07661..2d98a3ff5 100644 --- a/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php +++ b/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php @@ -81,7 +81,7 @@ class OrderTablePaymentStatusColumn { $wc_order = wc_get_order( $wc_order_id ); - if ( ! is_a( $wc_order, \WC_Order::class ) || ! $this->render_for_order( $wc_order ) ) { + if ( ! is_a( $wc_order, \WC_Order::class ) || ! $this->should_render_for_order( $wc_order ) ) { return; } @@ -100,8 +100,11 @@ class OrderTablePaymentStatusColumn { * * @return bool */ - private function render_for_order( \WC_Order $order ): bool { - return ! empty( $order->get_meta( PayPalGateway::CAPTURED_META_KEY ) ); + public function should_render_for_order( \WC_Order $order ): bool { + $intent = $order->get_meta( PayPalGateway::INTENT_META_KEY ); + $captured = $order->get_meta( PayPalGateway::CAPTURED_META_KEY ); + return ! empty( $intent ) && strtoupper( self::INTENT ) === strtoupper( $intent ) && + ! empty( $captured ); } /** @@ -111,7 +114,7 @@ class OrderTablePaymentStatusColumn { * * @return bool */ - private function is_captured( \WC_Order $wc_order ): bool { + public function is_captured( \WC_Order $wc_order ): bool { $captured = $wc_order->get_meta( PayPalGateway::CAPTURED_META_KEY ); return wc_string_to_bool( $captured ); } diff --git a/modules/ppcp-wc-gateway/src/Admin/class-paymentstatusorderdetail.php b/modules/ppcp-wc-gateway/src/Admin/class-paymentstatusorderdetail.php index 619f53eff..fceb8f523 100644 --- a/modules/ppcp-wc-gateway/src/Admin/class-paymentstatusorderdetail.php +++ b/modules/ppcp-wc-gateway/src/Admin/class-paymentstatusorderdetail.php @@ -16,6 +16,22 @@ use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway; */ class PaymentStatusOrderDetail { + /** + * The capture info column. + * + * @var OrderTablePaymentStatusColumn + */ + private $column; + + /** + * PaymentStatusOrderDetail constructor. + * + * @param OrderTablePaymentStatusColumn $column The capture info column. + */ + public function __construct( OrderTablePaymentStatusColumn $column ) { + $this->column = $column; + } + /** * Renders the not captured information. * @@ -23,14 +39,8 @@ class PaymentStatusOrderDetail { */ public function render( int $wc_order_id ) { $wc_order = new \WC_Order( $wc_order_id ); - $intent = $wc_order->get_meta( PayPalGateway::INTENT_META_KEY ); - $captured = $wc_order->get_meta( PayPalGateway::CAPTURED_META_KEY ); - if ( strcasecmp( $intent, 'AUTHORIZE' ) !== 0 ) { - return; - } - - if ( ! empty( $captured ) && wc_string_to_bool( $captured ) ) { + if ( ! $this->column->should_render_for_order( $wc_order ) || $this->column->is_captured( $wc_order ) ) { return; } From 571b85ec6168b807ed716726a784ad38f65dec6b Mon Sep 17 00:00:00 2001 From: Alex P Date: Wed, 6 Oct 2021 12:30:26 +0300 Subject: [PATCH 13/14] Do not render capture status for refunded --- .../src/Admin/class-ordertablepaymentstatuscolumn.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php b/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php index 2d98a3ff5..392edc458 100644 --- a/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php +++ b/modules/ppcp-wc-gateway/src/Admin/class-ordertablepaymentstatuscolumn.php @@ -101,10 +101,13 @@ class OrderTablePaymentStatusColumn { * @return bool */ public function should_render_for_order( \WC_Order $order ): bool { - $intent = $order->get_meta( PayPalGateway::INTENT_META_KEY ); - $captured = $order->get_meta( PayPalGateway::CAPTURED_META_KEY ); + $intent = $order->get_meta( PayPalGateway::INTENT_META_KEY ); + $captured = $order->get_meta( PayPalGateway::CAPTURED_META_KEY ); + $status = $order->get_status(); + $not_allowed_statuses = array( 'refunded' ); return ! empty( $intent ) && strtoupper( self::INTENT ) === strtoupper( $intent ) && - ! empty( $captured ); + ! empty( $captured ) && + ! in_array( $status, $not_allowed_statuses, true ); } /** From 403b82d5d719e676074d999cc2ce6916f889f311 Mon Sep 17 00:00:00 2001 From: Alex P Date: Wed, 6 Oct 2021 12:45:51 +0300 Subject: [PATCH 14/14] Check that 3ds_contingency option is not null --- modules/ppcp-button/src/Assets/class-smartbutton.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/ppcp-button/src/Assets/class-smartbutton.php b/modules/ppcp-button/src/Assets/class-smartbutton.php index d8b655806..c9ebc52fc 100644 --- a/modules/ppcp-button/src/Assets/class-smartbutton.php +++ b/modules/ppcp-button/src/Assets/class-smartbutton.php @@ -608,7 +608,10 @@ class SmartButton implements SmartButtonInterface { */ private function get_3ds_contingency(): string { if ( $this->settings->has( '3d_secure_contingency' ) ) { - return $this->settings->get( '3d_secure_contingency' ); + $value = $this->settings->get( '3d_secure_contingency' ); + if ( $value ) { + return $value; + } } return 'SCA_WHEN_REQUIRED';