Merge pull request #329 from woocommerce/pcp-338-missing-transaction-id

Fix missing transaction id

modules/ppcp-wc-gateway/services.php

@@ -55,6 +55,7 @@ return array(
 		$subscription_helper = $container->get( 'subscription.helper' );
 		$page_id             = $container->get( 'wcgateway.current-ppcp-settings-page-id' );
 		$environment         = $container->get( 'onboarding.environment' );
+		$logger              = $container->get( 'woocommerce.logger.woocommerce' );
 		return new PayPalGateway(
 			$settings_renderer,
 			$order_processor,
@@ -67,7 +68,8 @@ return array(
 			$transaction_url_provider,
 			$subscription_helper,
 			$page_id,
-			$environment
+			$environment,
+			$logger
 		);
 	},
 	'wcgateway.credit-card-gateway'                => static function ( ContainerInterface $container ): CreditCardGateway {

modules/ppcp-wc-gateway/src/Gateway/PayPalGateway.php

@@ -9,6 +9,7 @@ declare(strict_types=1);
 
 namespace WooCommerce\PayPalCommerce\WcGateway\Gateway;
 
+use Psr\Log\LoggerInterface;
 use WooCommerce\PayPalCommerce\ApiClient\Entity\CaptureStatus;
 use WooCommerce\PayPalCommerce\Onboarding\Environment;
 use WooCommerce\PayPalCommerce\Onboarding\State;
@@ -120,6 +121,13 @@ class PayPalGateway extends \WC_Payment_Gateway {
 	 */
 	protected $environment;
 
+	/**
+	 * The logger.
+	 *
+	 * @var LoggerInterface
+	 */
+	private $logger;
+
 	/**
 	 * PayPalGateway constructor.
 	 *
@@ -135,6 +143,7 @@ class PayPalGateway extends \WC_Payment_Gateway {
 	 * @param SubscriptionHelper          $subscription_helper The subscription helper.
 	 * @param string                      $page_id ID of the current PPCP gateway settings page, or empty if it is not such page.
 	 * @param Environment                 $environment The environment.
+	 * @param LoggerInterface             $logger The logger.
 	 */
 	public function __construct(
 		SettingsRenderer $settings_renderer,
@@ -148,7 +157,8 @@ class PayPalGateway extends \WC_Payment_Gateway {
 		TransactionUrlProvider $transaction_url_provider,
 		SubscriptionHelper $subscription_helper,
 		string $page_id,
-		Environment $environment
+		Environment $environment,
+		LoggerInterface $logger
 	) {
 
 		$this->id                       = self::ID;
@@ -162,6 +172,7 @@ class PayPalGateway extends \WC_Payment_Gateway {
 		$this->transaction_url_provider = $transaction_url_provider;
 		$this->page_id                  = $page_id;
 		$this->environment              = $environment;
+		$this->logger                   = $logger;
 		$this->onboarded                = $state->current_state() === State::STATE_ONBOARDED;
 
 		if ( $this->onboarded ) {
@@ -274,6 +285,8 @@ class PayPalGateway extends \WC_Payment_Gateway {
 
 		$capture = end( $captures );
 
+		$this->update_transaction_id( $capture->id(), $wc_order, $this->logger );
+
 		$this->handle_capture_status( $capture, $wc_order );
 
 		if ( AuthorizedPaymentsProcessor::SUCCESSFUL === $result_status ) {

modules/ppcp-wc-gateway/src/Gateway/ProcessPaymentTrait.php

@@ -16,13 +16,14 @@ use WooCommerce\PayPalCommerce\ApiClient\Exception\RuntimeException;
 use WooCommerce\PayPalCommerce\Onboarding\Environment;
 use WooCommerce\PayPalCommerce\WcGateway\Processor\OrderMetaTrait;
 use WooCommerce\PayPalCommerce\WcGateway\Processor\PaymentsStatusHandlingTrait;
+use WooCommerce\PayPalCommerce\WcGateway\Processor\TransactionIdHandlingTrait;
 
 /**
  * Trait ProcessPaymentTrait
  */
 trait ProcessPaymentTrait {
 
-	use OrderMetaTrait, PaymentsStatusHandlingTrait;
+	use OrderMetaTrait, PaymentsStatusHandlingTrait, TransactionIdHandlingTrait;
 
 	/**
 	 * Process a payment for an WooCommerce order.
@@ -102,6 +103,11 @@ trait ProcessPaymentTrait {
 					$wc_order->update_meta_data( PayPalGateway::CAPTURED_META_KEY, 'false' );
 				}
 
+				$transaction_id = $this->get_paypal_order_transaction_id( $order );
+				if ( $transaction_id ) {
+					$this->update_transaction_id( $transaction_id, $wc_order );
+				}
+
 				$this->handle_new_order_status( $order, $wc_order );
 
 				$this->session_handler->destroy_session_data();

modules/ppcp-wc-gateway/src/Processor/OrderProcessor.php

@@ -26,7 +26,7 @@ use WooCommerce\PayPalCommerce\WcGateway\Settings\Settings;
  */
 class OrderProcessor {
 
-	use OrderMetaTrait, PaymentsStatusHandlingTrait;
+	use OrderMetaTrait, PaymentsStatusHandlingTrait, TransactionIdHandlingTrait;
 
 	/**
 	 * The environment.
@@ -176,8 +176,8 @@ class OrderProcessor {
 
 		$transaction_id = $this->get_paypal_order_transaction_id( $order );
 
-		if ( '' !== $transaction_id ) {
-			$this->set_order_transaction_id( $transaction_id, $wc_order );
+		if ( $transaction_id ) {
+			$this->update_transaction_id( $transaction_id, $wc_order );
 		}
 
 		$this->handle_new_order_status( $order, $wc_order );
@@ -195,55 +195,6 @@ class OrderProcessor {
 		return true;
 	}
 
-	/**
-	 * Set transaction id to WC order meta data.
-	 *
-	 * @param string    $transaction_id Transaction id to set.
-	 * @param \WC_Order $wc_order Order to set transaction ID to.
-	 */
-	public function set_order_transaction_id( string $transaction_id, \WC_Order $wc_order ) {
-		try {
-			$wc_order->set_transaction_id( $transaction_id );
-		} catch ( \WC_Data_Exception $exception ) {
-			$this->logger->log(
-				'warning',
-				sprintf(
-					'Failed to set transaction ID. Exception caught when tried: %1$s',
-					$exception->getMessage()
-				)
-			);
-		}
-	}
-
-	/**
-	 * Retrieve transaction id from PayPal order.
-	 *
-	 * @param Order $order Order to get transaction id from.
-	 *
-	 * @return string
-	 */
-	private function get_paypal_order_transaction_id( Order $order ): string {
-		$purchase_units = $order->purchase_units();
-
-		if ( ! isset( $purchase_units[0] ) ) {
-			return '';
-		}
-
-		$payments = $purchase_units[0]->payments();
-
-		if ( null === $payments ) {
-			return '';
-		}
-
-		$captures = $payments->captures();
-
-		if ( isset( $captures[0] ) ) {
-			return $captures[0]->id();
-		}
-
-		return '';
-	}
-
 	/**
 	 * Returns if an order should be captured immediately.
 	 *

modules/ppcp-wc-gateway/src/Processor/TransactionIdHandlingTrait.php

@@ -0,0 +1,85 @@
+<?php
+/**
+ * Functions for retrieving/saving order transaction ID.
+ *
+ * @package WooCommerce\PayPalCommerce\WcGateway\Processor
+ */
+
+declare(strict_types=1);
+
+namespace WooCommerce\PayPalCommerce\WcGateway\Processor;
+
+use Exception;
+use Psr\Log\LoggerInterface;
+use WC_Order;
+use WooCommerce\PayPalCommerce\ApiClient\Entity\Order;
+
+/**
+ * Trait PaymentsStatusHandlingTrait.
+ */
+trait TransactionIdHandlingTrait {
+
+	/**
+	 * Sets transaction ID to the WC order.
+	 *
+	 * @param string               $transaction_id The transaction ID to set.
+	 * @param WC_Order             $wc_order The order to set transaction ID to.
+	 * @param LoggerInterface|null $logger The logger to log errors.
+	 *
+	 * @return bool
+	 */
+	protected function update_transaction_id(
+		string $transaction_id,
+		WC_Order $wc_order,
+		LoggerInterface $logger = null
+	): bool {
+		try {
+			$wc_order->set_transaction_id( $transaction_id );
+			$wc_order->save();
+			return true;
+		} catch ( Exception $exception ) {
+			if ( $logger ) {
+				$logger->warning(
+					sprintf(
+						'Failed to set transaction ID %1$s. %2$s',
+						$transaction_id,
+						$exception->getMessage()
+					)
+				);
+			}
+			return false;
+		}
+	}
+
+	/**
+	 * Retrieves transaction id from PayPal order.
+	 *
+	 * @param Order $order The order to get transaction id from.
+	 *
+	 * @return string|null
+	 */
+	protected function get_paypal_order_transaction_id( Order $order ): ?string {
+		$purchase_unit = $order->purchase_units()[0] ?? null;
+		if ( ! $purchase_unit ) {
+			return null;
+		}
+
+		$payments = $purchase_unit->payments();
+		if ( null === $payments ) {
+			return null;
+		}
+
+		$capture = $payments->captures()[0] ?? null;
+		if ( $capture ) {
+			return $capture->id();
+		}
+
+		$authorization = $payments->authorizations()[0] ?? null;
+		if ( $authorization ) {
+			return $authorization->id();
+		}
+
+		return null;
+	}
+
+}

modules/ppcp-webhooks/services.php

@@ -68,7 +68,7 @@ return array(
 			new CheckoutOrderCompleted( $logger, $prefix ),
 			new PaymentCaptureRefunded( $logger, $prefix ),
 			new PaymentCaptureReversed( $logger, $prefix ),
-			new PaymentCaptureCompleted( $logger, $prefix ),
+			new PaymentCaptureCompleted( $logger, $prefix, $order_endpoint ),
 		);
 	},
 

modules/ppcp-webhooks/src/Handler/PaymentCaptureCompleted.php

@@ -9,15 +9,19 @@ declare(strict_types=1);
 
 namespace WooCommerce\PayPalCommerce\Webhooks\Handler;
 
+use Exception;
+use WooCommerce\PayPalCommerce\ApiClient\Endpoint\OrderEndpoint;
 use WooCommerce\PayPalCommerce\WcGateway\Gateway\PayPalGateway;
 use Psr\Log\LoggerInterface;
+use WooCommerce\PayPalCommerce\WcGateway\Processor\TransactionIdHandlingTrait;
+use WP_REST_Response;
 
 /**
  * Class PaymentCaptureCompleted
  */
 class PaymentCaptureCompleted implements RequestHandler {
 
-	use PrefixTrait;
+	use PrefixTrait, TransactionIdHandlingTrait;
 
 	/**
 	 * The logger.
@@ -26,15 +30,28 @@ class PaymentCaptureCompleted implements RequestHandler {
 	 */
 	private $logger;
 
+	/**
+	 * The order endpoint.
+	 *
+	 * @var OrderEndpoint
+	 */
+	private $order_endpoint;
+
 	/**
 	 * PaymentCaptureCompleted constructor.
 	 *
 	 * @param LoggerInterface $logger The logger.
 	 * @param string          $prefix The prefix.
+	 * @param OrderEndpoint   $order_endpoint The order endpoint.
 	 */
-	public function __construct( LoggerInterface $logger, string $prefix ) {
-		$this->logger = $logger;
-		$this->prefix = $prefix;
+	public function __construct(
+		LoggerInterface $logger,
+		string $prefix,
+		OrderEndpoint $order_endpoint
+	) {
+		$this->logger         = $logger;
+		$this->prefix         = $prefix;
+		$this->order_endpoint = $order_endpoint;
 	}
 
 	/**
@@ -62,56 +79,41 @@ class PaymentCaptureCompleted implements RequestHandler {
 	 *
 	 * @param \WP_REST_Request $request The request.
 	 *
-	 * @return \WP_REST_Response
+	 * @return WP_REST_Response
 	 */
-	public function handle_request( \WP_REST_Request $request ): \WP_REST_Response {
+	public function handle_request( \WP_REST_Request $request ): WP_REST_Response {
 		$response = array( 'success' => false );
-		$order_id = isset( $request['resource']['custom_id'] ) ?
-			$this->sanitize_custom_id( $request['resource']['custom_id'] ) : 0;
-		if ( ! $order_id ) {
-			$message = sprintf(
-				// translators: %s is the PayPal webhook Id.
-				__(
-					'No order for webhook event %s was found.',
-					'woocommerce-paypal-payments'
-				),
-				isset( $request['id'] ) ? $request['id'] : ''
-			);
-			$this->logger->log(
-				'warning',
-				$message,
-				array(
-					'request' => $request,
-				)
-			);
-			$response['message'] = $message;
-			return rest_ensure_response( $response );
-		}
-		$wc_order = wc_get_order( $order_id );
 
-		if ( ! is_a( $wc_order, \WC_Order::class ) ) {
-			$message = sprintf(
-			// translators: %s is the PayPal webhook Id.
-				__(
-					'No order for webhook event %s was found.',
-					'woocommerce-paypal-payments'
-				),
-				isset( $request['id'] ) ? $request['id'] : ''
-			);
-			$this->logger->log(
-				'warning',
-				$message,
-				array(
-					'request' => $request,
-				)
-			);
+		$webhook_id = (string) ( $request['id'] ?? '' );
+
+		$resource = $request['resource'];
+		if ( ! is_array( $resource ) ) {
+			$message = 'Resource data not found in webhook request.';
+			$this->logger->warning( $message, array( 'request' => $request ) );
 			$response['message'] = $message;
-			return rest_ensure_response( $response );
+			return new WP_REST_Response( $response );
+		}
+
+		$wc_order_id = isset( $resource['custom_id'] ) ?
+			$this->sanitize_custom_id( (string) $resource['custom_id'] ) : 0;
+		if ( ! $wc_order_id ) {
+			$message = sprintf( 'No order for webhook event %s was found.', $webhook_id );
+			$this->logger->warning( $message, array( 'request' => $request ) );
+			$response['message'] = $message;
+			return new WP_REST_Response( $response );
+		}
+
+		$wc_order = wc_get_order( $wc_order_id );
+		if ( ! is_a( $wc_order, \WC_Order::class ) ) {
+			$message = sprintf( 'No order for webhook event %s was found.', $webhook_id );
+			$this->logger->warning( $message, array( 'request' => $request ) );
+			$response['message'] = $message;
+			return new WP_REST_Response( $response );
 		}
 
 		if ( $wc_order->get_status() !== 'on-hold' ) {
 			$response['success'] = true;
-			return rest_ensure_response( $response );
+			return new WP_REST_Response( $response );
 		}
 		$wc_order->add_order_note(
 			__( 'Payment successfully captured.', 'woocommerce-paypal-payments' )
@@ -135,7 +137,25 @@ class PaymentCaptureCompleted implements RequestHandler {
 				'order'   => $wc_order,
 			)
 		);
+
+		$order_id = $resource['supplementary_data']['related_ids']['order_id'] ?? null;
+
+		if ( $order_id ) {
+			try {
+				$this->logger->emergency( (string) $order_id );
+
+				$order = $this->order_endpoint->order( $order_id );
+
+				$transaction_id = $this->get_paypal_order_transaction_id( $order );
+				if ( $transaction_id ) {
+					$this->update_transaction_id( $transaction_id, $wc_order, $this->logger );
+				}
+			} catch ( Exception $exception ) {
+				$this->logger->warning( 'Failed to get transaction ID: ' . $exception->getMessage() );
+			}
+		}
+
 		$response['success'] = true;
-		return rest_ensure_response( $response );
+		return new WP_REST_Response( $response );
 	}
 }

tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php

@@ -5,6 +5,7 @@ namespace WooCommerce\PayPalCommerce\WcGateway\Gateway;
 
 
 use Psr\Container\ContainerInterface;
+use Psr\Log\NullLogger;
 use WooCommerce\PayPalCommerce\ApiClient\Entity\Capture;
 use WooCommerce\PayPalCommerce\ApiClient\Entity\CaptureStatus;
 use WooCommerce\PayPalCommerce\Onboarding\Environment;
@@ -80,7 +81,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         expect('wc_get_order')
@@ -130,7 +132,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         expect('wc_get_order')
@@ -197,7 +200,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         expect('wc_get_order')
@@ -224,6 +228,8 @@ class WcGatewayTest extends TestCase
     public function testCaptureAuthorizedPayment() {
 	    expect('is_admin')->andReturn(false);
 
+		$transactionId = 'abcd1234';
+
         $wcOrder = Mockery::mock(\WC_Order::class);
         $wcOrder
             ->expects('add_order_note');
@@ -233,13 +239,20 @@ class WcGatewayTest extends TestCase
         $wcOrder
 	        ->expects('payment_complete');
         $wcOrder
-            ->expects('save');
+            ->expects('set_transaction_id')
+			->with($transactionId);
+        $wcOrder
+            ->shouldReceive('save')
+			->atLeast()->once();
         $settingsRenderer = Mockery::mock(SettingsRenderer::class);
         $orderProcessor = Mockery::mock(OrderProcessor::class);
 		$capture = Mockery::mock(Capture::class);
 		$capture
 			->shouldReceive('status')
 			->andReturn(new CaptureStatus(CaptureStatus::COMPLETED));
+		$capture
+			->shouldReceive('id')
+			->andReturn($transactionId);
         $authorizedPaymentsProcessor = Mockery::mock(AuthorizedPaymentsProcessor::class);
         $authorizedPaymentsProcessor
             ->expects('process')
@@ -276,7 +289,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         $this->assertTrue($testee->capture_authorized_payment($wcOrder));
@@ -332,7 +346,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         $this->assertTrue($testee->capture_authorized_payment($wcOrder));
@@ -385,7 +400,8 @@ class WcGatewayTest extends TestCase
             $transactionUrlProvider,
             $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
         );
 
         $this->assertFalse($testee->capture_authorized_payment($wcOrder));
@@ -426,7 +442,8 @@ class WcGatewayTest extends TestCase
 		    $transactionUrlProvider,
 		    $subscriptionHelper,
 			PayPalGateway::ID,
-			$this->environment
+			$this->environment,
+			new NullLogger()
 	    );
 
     	$this->assertSame($needSetup, $testee->needs_setup());

tests/PHPUnit/WcGateway/Processor/OrderProcessorTest.php

@@ -159,7 +159,8 @@ class OrderProcessorTest extends TestCase
         $wcOrder
             ->expects('update_status')
             ->with('on-hold', 'Awaiting payment.');
-
+		$wcOrder->expects('set_transaction_id')
+			->with($transactionId);
 
         $this->assertTrue($testee->process($wcOrder));
     }