Skip to content

Commit 21a0801

Browse files
DinamikoDinamiko
authored
Merge pull request #4536 from woocommerce/dev/PCP-6681-saved-pay-pal-token-in-subscription-change-payment-flow-moves-active-subscription-to-pending
fix(wc-gateway): skip $0 PayPal capture when changing subscription payment to saved token (6681)
2 parents 025cb77 + 1691122 commit 21a0801

2 files changed

Lines changed: 185 additions & 2 deletions

File tree

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,20 @@ public function process_payment( $order_id ): array {
447447
// phpcs:ignore WordPress.Security.NonceVerification.Missing
448448
$vault_approved_order_id = wc_clean( wp_unslash( $_POST['paypal_order_id'] ?? '' ) );
449449

450+
/**
451+
* WC Subscriptions zeroes the order total during a change-payment request, so
452+
* attempting a real capture here would send a $0 create-order request to PayPal
453+
* (rejected with CANNOT_BE_ZERO_OR_NEGATIVE). Just attach the saved token instead,
454+
* mirroring CreditCardGateway's saved-token change-payment handling.
455+
*/
456+
if (
457+
$paypal_payment_token_id
458+
&& 'new' !== $paypal_payment_token_id
459+
&& $this->is_customer_changing_subscription_payment( $this->subscription_helper, $wc_order )
460+
) {
461+
return $this->add_payment_token_to_order( $wc_order, (int) $paypal_payment_token_id, $this->get_return_url( $wc_order ), $this->session_handler );
462+
}
463+
450464
// Skip saved token handling when an approved order exists.
451465
if ( $paypal_payment_token_id && 'new' !== $paypal_payment_token_id && ! $vault_approved_order_id ) {
452466
$tokens = WC_Payment_Tokens::get_customer_tokens( get_current_user_id() );
@@ -698,4 +712,53 @@ public function admin_options(): void {
698712

699713
do_action( 'woocommerce_paypal_payments_gateway_admin_options_wrapper', $this );
700714
}
715+
716+
/**
717+
* Check whether customer is changing subscription payment.
718+
*
719+
* @param SubscriptionHelper $subscription_helper
720+
* @param WC_Order $wc_order
721+
* @return bool
722+
*/
723+
private function is_customer_changing_subscription_payment( SubscriptionHelper $subscription_helper, WC_Order $wc_order ): bool {
724+
// phpcs:ignore WordPress.Security.NonceVerification.Missing
725+
return isset( $_POST['woocommerce_change_payment'] ) && $subscription_helper->has_subscription( $wc_order->get_id() ) && $subscription_helper->is_subscription_change_payment();
726+
}
727+
728+
/**
729+
* Adds the given WC payment token into the given WC Order.
730+
*
731+
* @param WC_Order $wc_order
732+
* @param int $wc_payment_token_id
733+
* @param string $return_url
734+
* @param SessionHandler $session_handler
735+
* @return array{result: string, redirect: string, errorMessage?: string}
736+
*/
737+
private function add_payment_token_to_order(
738+
WC_Order $wc_order,
739+
int $wc_payment_token_id,
740+
string $return_url,
741+
SessionHandler $session_handler
742+
): array {
743+
$payment_token = WC_Payment_Tokens::get( $wc_payment_token_id );
744+
if ( $payment_token && (int) $payment_token->get_user_id() === get_current_user_id() ) {
745+
$wc_order->add_payment_token( $payment_token );
746+
$wc_order->save();
747+
748+
$session_handler->destroy_session_data();
749+
750+
return array(
751+
'result' => 'success',
752+
'redirect' => $return_url,
753+
);
754+
}
755+
756+
wc_add_notice( __( 'Could not change payment.', 'woocommerce-paypal-payments' ), 'error' );
757+
758+
return array(
759+
'result' => 'failure',
760+
'redirect' => wc_get_checkout_url(),
761+
'errorMessage' => __( 'Could not change payment.', 'woocommerce-paypal-payments' ),
762+
);
763+
}
701764
}

‎tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php‎

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class WcGatewayTest extends TestCase
4949
private $wcPaymentTokens;
5050
private $assetGetter;
5151
private $context;
52+
private $capturePayPalPayment;
5253

5354
public function setUp(): void {
5455
parent::setUp();
@@ -97,6 +98,7 @@ public function setUp(): void {
9798
$this->paymentTokensEndpoint = Mockery::mock(PaymentTokensEndpoint::class);
9899
$this->wcPaymentTokens = Mockery::mock(WooCommercePaymentTokens::class);
99100
$this->context = Mockery::mock(Context::class);
101+
$this->capturePayPalPayment = Mockery::mock(CapturePayPalPayment::class);
100102
}
101103

102104
private function createGateway()
@@ -119,7 +121,7 @@ private function createGateway()
119121
$this->wcPaymentTokens,
120122
$this->assetGetter,
121123
false,
122-
Mockery::mock(CapturePayPalPayment::class),
124+
$this->capturePayPalPayment,
123125
Mockery::mock(OrderEndpoint::class),
124126
'WC-',
125127
$this->context
@@ -146,7 +148,34 @@ private function createSpyGateway(): SpyablePayPalGateway
146148
$this->wcPaymentTokens,
147149
$this->assetGetter,
148150
false,
149-
Mockery::mock(CapturePayPalPayment::class),
151+
$this->capturePayPalPayment,
152+
Mockery::mock(OrderEndpoint::class),
153+
'WC-',
154+
$this->context
155+
);
156+
}
157+
158+
private function createReturnUrlStubGateway(): PayPalGatewayReturnUrlStub
159+
{
160+
return new PayPalGatewayReturnUrlStub(
161+
$this->funding_source_renderer,
162+
$this->orderProcessor,
163+
$this->settingsProvider,
164+
$this->sessionHandler,
165+
$this->refundProcessor,
166+
$this->isConnected,
167+
$this->transactionUrlProvider,
168+
$this->subscriptionHelper,
169+
$this->environment,
170+
$this->logger,
171+
$this->apiShopCountry,
172+
static fn ($id) => 'checkoutnow=' . $id,
173+
'Pay via PayPal',
174+
$this->paymentTokensEndpoint,
175+
$this->wcPaymentTokens,
176+
$this->assetGetter,
177+
false,
178+
$this->capturePayPalPayment,
150179
Mockery::mock(OrderEndpoint::class),
151180
'WC-',
152181
$this->context
@@ -497,6 +526,97 @@ public function test_payment_fields_with_vaulting_disabled_suppresses_saved_meth
497526
$this->assertSame( 0, $gateway->tokenization_script_call_count, 'tokenization_script() must not be called when vaulting is disabled' );
498527
$this->assertSame( 0, $gateway->saved_payment_methods_call_count, 'saved_payment_methods() must not be called when vaulting is disabled' );
499528
}
529+
530+
/**
531+
* @runInSeparateProcess
532+
* @preserveGlobalState disabled
533+
* @scenario Current user changes their subscription payment to a saved PayPal
534+
* token they own. The token must be attached to the order without attempting a
535+
* $0 capture (WC Subscriptions zeroes the order total during change-payment
536+
* requests, and PayPal rejects $0 create-order calls with
537+
* CANNOT_BE_ZERO_OR_NEGATIVE).
538+
*/
539+
public function test_process_payment_attaches_token_when_ownership_check_passes_for_change_payment(): void {
540+
$orderId = 1;
541+
$_POST['woocommerce_change_payment'] = '1';
542+
$_POST['wc-ppcp-gateway-payment-token'] = '99';
543+
544+
$this->subscriptionHelper->shouldReceive('has_subscription')->with($orderId)->andReturn(true);
545+
$this->subscriptionHelper->shouldReceive('is_subscription_change_payment')->andReturn(true);
546+
547+
$wcOrder = Mockery::mock(\WC_Order::class);
548+
$wcOrder->shouldReceive('get_id')->andReturn($orderId);
549+
$wcOrder->shouldReceive('get_meta')->andReturn('');
550+
when('wc_get_order')->justReturn($wcOrder);
551+
552+
$tokens = Mockery::mock('alias:WC_Payment_Tokens');
553+
$payment_token = Mockery::mock(\WC_Payment_Token::class);
554+
$payment_token->shouldReceive('get_user_id')->andReturn(1);
555+
$tokens->shouldReceive('get')->with(99)->andReturn($payment_token);
556+
557+
when('get_current_user_id')->justReturn(1);
558+
559+
$wcOrder->shouldReceive('add_payment_token')->once()->with($payment_token);
560+
$wcOrder->shouldReceive('save')->once();
561+
$this->sessionHandler->shouldReceive('destroy_session_data')->once();
562+
563+
$this->capturePayPalPayment->shouldNotReceive('create_order');
564+
565+
$result = $this->createReturnUrlStubGateway()->process_payment($orderId);
566+
567+
$this->assertEquals('success', $result['result']);
568+
}
569+
570+
/**
571+
* @runInSeparateProcess
572+
* @preserveGlobalState disabled
573+
* @scenario Current user supplies a token id belonging to a different user while
574+
* changing their subscription payment. The token must not be attached, the
575+
* change must fail, and no $0 capture must be attempted either.
576+
*/
577+
public function test_process_payment_rejects_token_owned_by_different_user_during_change_payment(): void {
578+
$orderId = 1;
579+
$_POST['woocommerce_change_payment'] = '1';
580+
$_POST['wc-ppcp-gateway-payment-token'] = '99';
581+
582+
$this->subscriptionHelper->shouldReceive('has_subscription')->with($orderId)->andReturn(true);
583+
$this->subscriptionHelper->shouldReceive('is_subscription_change_payment')->andReturn(true);
584+
585+
$wcOrder = Mockery::mock(\WC_Order::class);
586+
$wcOrder->shouldReceive('get_id')->andReturn($orderId);
587+
$wcOrder->shouldReceive('get_meta')->andReturn('');
588+
when('wc_get_order')->justReturn($wcOrder);
589+
590+
$tokens = Mockery::mock('alias:WC_Payment_Tokens');
591+
$payment_token = Mockery::mock(\WC_Payment_Token::class);
592+
$payment_token->shouldReceive('get_user_id')->andReturn(2);
593+
$tokens->shouldReceive('get')->with(99)->andReturn($payment_token);
594+
595+
when('get_current_user_id')->justReturn(1);
596+
when('wc_add_notice')->justReturn(null);
597+
when('wc_get_checkout_url')->justReturn('http://example.test/checkout');
598+
599+
$wcOrder->shouldNotReceive('add_payment_token');
600+
$wcOrder->shouldNotReceive('save');
601+
$this->sessionHandler->shouldNotReceive('destroy_session_data');
602+
$this->capturePayPalPayment->shouldNotReceive('create_order');
603+
604+
$result = $this->createReturnUrlStubGateway()->process_payment($orderId);
605+
606+
$this->assertEquals('failure', $result['result']);
607+
}
608+
}
609+
610+
/**
611+
* Test double that returns a string return URL. The shared WC_Payment_Gateway
612+
* stub returns the order object, which would violate the string type hint of
613+
* PayPalGateway::add_payment_token_to_order().
614+
*/
615+
class PayPalGatewayReturnUrlStub extends PayPalGateway
616+
{
617+
protected function get_return_url( $order = null ) {
618+
return 'http://example.test/return';
619+
}
500620
}
501621

502622
/**

0 commit comments

Comments
 (0)