Skip to content

Commit f94f8f1

Browse files
authored
Merge pull request #4541 from woocommerce/dev/PCP-6702-invoice-id-missing-for-existing-customers-paying-with-saved-pay-pal-payment-tokens
Fix missing `invoice_id` for saved PayPal payment token orders (6702)
2 parents 830246c + 6986425 commit f94f8f1

5 files changed

Lines changed: 202 additions & 25 deletions

File tree

modules/ppcp-wc-gateway/services.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@
122122
$container->get( 'wcgateway.settings.admin-settings-enabled' ),
123123
$container->get( 'wcgateway.endpoint.capture-paypal-payment' ),
124124
$container->get( 'api.endpoint.order' ),
125-
$container->get( 'api.prefix' ),
126125
$container->get( 'button.helper.context' )
127126
);
128127
},

modules/ppcp-wc-gateway/src/Endpoint/CapturePayPalPayment.php

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,25 +52,20 @@ public function __construct(
5252
/**
5353
* Creates PayPal order from the given PayPal/Venmo vault ID.
5454
*
55+
* The custom_id/invoice_id fields are not accepted as parameters here:
56+
* the PayPal Orders v2 API only reads them from inside a purchase
57+
* unit, never from the request root, so from_wc_order() populates
58+
* them directly on the purchase unit built above.
59+
*
5560
* @throws RuntimeException When request fails.
5661
*/
5762
public function create_order(
5863
string $vault_id,
59-
string $custom_id,
60-
string $invoice_id,
6164
WC_Order $wc_order,
6265
string $payment_source_name = 'paypal'
6366
): Order {
6467
$intent = strtoupper( $this->settings_provider->payment_intent() ) === 'AUTHORIZE' ? 'AUTHORIZE' : 'CAPTURE';
65-
$items = array( $this->purchase_unit_factory->from_wc_cart( null, false, $wc_order->get_payment_method() ) );
66-
67-
// phpcs:disable WordPress.Security.NonceVerification
68-
$pay_for_order = wc_clean( wp_unslash( $_GET['pay_for_order'] ?? '' ) );
69-
$order_key = wc_clean( wp_unslash( $_GET['key'] ?? '' ) );
70-
// phpcs:enable
71-
if ( $pay_for_order && $order_key === $wc_order->get_order_key() ) {
72-
$items = array( $this->purchase_unit_factory->from_wc_order( $wc_order ) );
73-
}
68+
$items = array( $this->purchase_unit_factory->from_wc_order( $wc_order ) );
7469

7570
$data = array(
7671
'intent' => $intent,
@@ -88,8 +83,6 @@ static function ( PurchaseUnit $item ): array {
8883
),
8984
),
9085
),
91-
'custom_id' => $custom_id,
92-
'invoice_id' => $invoice_id,
9386
);
9487

9588
$bearer = $this->bearer->bearer();

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,6 @@ class PayPalGateway extends \WC_Payment_Gateway {
120120

121121
private OrderEndpoint $order_endpoint;
122122

123-
private string $prefix;
124-
125123
private Context $context;
126124

127125
/**
@@ -208,7 +206,6 @@ class PayPalGateway extends \WC_Payment_Gateway {
208206
* @param bool $admin_settings_enabled Whether settings module is enabled.
209207
* @param CapturePayPalPayment $capture_paypal_payment The PayPal vault payment capture endpoint.
210208
* @param OrderEndpoint $order_endpoint The order endpoint.
211-
* @param string $prefix The invoice prefix.
212209
* @param Context $context The context helper.
213210
*/
214211
public function __construct(
@@ -231,7 +228,6 @@ public function __construct(
231228
bool $admin_settings_enabled,
232229
CapturePayPalPayment $capture_paypal_payment,
233230
OrderEndpoint $order_endpoint,
234-
string $prefix,
235231
Context $context
236232
) {
237233
$this->id = self::ID;
@@ -254,7 +250,6 @@ public function __construct(
254250
$this->admin_settings_enabled = $admin_settings_enabled;
255251
$this->capture_paypal_payment = $capture_paypal_payment;
256252
$this->order_endpoint = $order_endpoint;
257-
$this->prefix = $prefix;
258253
$this->context = $context;
259254

260255
$default_support = array(
@@ -477,11 +472,9 @@ public function process_payment( $order_id ): array {
477472
}
478473

479474
$payment_source_name = $token instanceof PaymentTokenVenmo ? 'venmo' : 'paypal';
480-
$custom_id = (string) $wc_order->get_id();
481-
$invoice_id = $this->prefix . $wc_order->get_order_number();
482475

483476
try {
484-
$created_order = $this->capture_paypal_payment->create_order( $token->get_token(), $custom_id, $invoice_id, $wc_order, $payment_source_name );
477+
$created_order = $this->capture_paypal_payment->create_order( $token->get_token(), $wc_order, $payment_source_name );
485478
} catch ( RuntimeException $exception ) {
486479
$this->logger->error( $exception->getMessage() );
487480
return $this->handle_payment_failure( $wc_order, $exception );
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
<?php
2+
declare( strict_types=1 );
3+
4+
namespace WooCommerce\PayPalCommerce\WcGateway\Endpoint;
5+
6+
use Mockery;
7+
use Psr\Log\LoggerInterface;
8+
use WC_Order;
9+
use WooCommerce\PayPalCommerce\ApiClient\Authentication\Bearer;
10+
use WooCommerce\PayPalCommerce\ApiClient\Entity\Order;
11+
use WooCommerce\PayPalCommerce\ApiClient\Entity\PurchaseUnit;
12+
use WooCommerce\PayPalCommerce\ApiClient\Factory\OrderFactory;
13+
use WooCommerce\PayPalCommerce\ApiClient\Factory\PurchaseUnitFactory;
14+
use WooCommerce\PayPalCommerce\Settings\Data\SettingsProvider;
15+
use WooCommerce\PayPalCommerce\TestCase;
16+
use function Brain\Monkey\Functions\expect;
17+
use function Brain\Monkey\Functions\when;
18+
19+
class CapturePayPalPaymentTest extends TestCase {
20+
21+
/**
22+
* Build a fully-wired CapturePayPalPayment instance.
23+
*
24+
* @param array &$captured_body Passed by reference; populated with the decoded request body inside the wp_remote_get stub.
25+
* @param PurchaseUnit|null $purchase_unit The purchase unit returned by from_wc_order(); defaults to one whose to_array() is empty.
26+
*/
27+
private function make_testee( array &$captured_body, ?PurchaseUnit $purchase_unit = null ): array {
28+
$token = Mockery::mock( \WooCommerce\PayPalCommerce\ApiClient\Entity\Token::class );
29+
$token->shouldReceive( 'token' )->andReturn( 'test-bearer-token' );
30+
31+
$bearer = Mockery::mock( Bearer::class );
32+
$bearer->shouldReceive( 'bearer' )->andReturn( $token );
33+
34+
if ( ! $purchase_unit ) {
35+
$purchase_unit = Mockery::mock( PurchaseUnit::class );
36+
$purchase_unit->shouldReceive( 'to_array' )->andReturn( array() );
37+
}
38+
39+
$purchase_unit_factory = Mockery::mock( PurchaseUnitFactory::class );
40+
$purchase_unit_factory->shouldReceive( 'from_wc_order' )->andReturn( $purchase_unit );
41+
$purchase_unit_factory->shouldNotReceive( 'from_wc_cart' );
42+
43+
$returned_order = Mockery::mock( Order::class );
44+
45+
$order_factory = Mockery::mock( OrderFactory::class );
46+
$order_factory->shouldReceive( 'from_paypal_response' )->andReturn( $returned_order );
47+
48+
$settings_provider = Mockery::mock( SettingsProvider::class );
49+
$settings_provider->shouldReceive( 'payment_intent' )->andReturn( 'capture' );
50+
51+
$logger = Mockery::mock( LoggerInterface::class );
52+
$logger->allows( 'debug' );
53+
54+
when( 'trailingslashit' )->alias( function ( string $value ): string {
55+
return rtrim( $value, '/' ) . '/';
56+
} );
57+
expect( 'wp_json_encode' )->andReturnUsing( 'json_encode' );
58+
59+
// RequestTrait::request_response_string() calls $response['headers']->getAll(), so headers must be an object.
60+
$headers_stub = Mockery::mock( \Requests_Utility_CaseInsensitiveDictionary::class );
61+
$headers_stub->shouldReceive( 'getAll' )->andReturn( array() );
62+
63+
expect( 'wp_remote_get' )->andReturnUsing(
64+
function ( string $url, array $args ) use ( &$captured_body, $headers_stub ): array {
65+
$captured_body = (array) json_decode( $args['body'], true );
66+
return array(
67+
'body' => '{"id":"MOCK-ORDER-ID"}',
68+
'response' => array( 'code' => 200 ),
69+
'headers' => $headers_stub,
70+
);
71+
}
72+
);
73+
when( 'wp_remote_retrieve_response_code' )->alias(
74+
function ( $response ): int {
75+
return (int) ( $response['response']['code'] ?? 0 );
76+
}
77+
);
78+
79+
$testee = new CapturePayPalPayment(
80+
'https://api.paypal.com',
81+
$bearer,
82+
$order_factory,
83+
$purchase_unit_factory,
84+
$settings_provider,
85+
$logger
86+
);
87+
88+
return array( $testee, $returned_order );
89+
}
90+
91+
/**
92+
* GIVEN a saved-token payment is captured for a WC order
93+
* WHEN create_order builds the purchase units
94+
* THEN they come from PurchaseUnitFactory::from_wc_order(), never from_wc_cart()
95+
*
96+
* This is the PCP-6702 regression: from_wc_cart() always produces an
97+
* empty invoice_id and a session-based custom_id, so the order must be
98+
* the single source of truth here regardless of the current request
99+
* (there is no more `?pay_for_order=1` special case).
100+
*/
101+
public function test_create_order_builds_purchase_unit_from_wc_order(): void {
102+
$captured_body = array();
103+
[ $testee ] = $this->make_testee( $captured_body );
104+
105+
$wc_order = Mockery::mock( WC_Order::class );
106+
107+
// No assertion needed beyond make_testee()'s `shouldNotReceive('from_wc_cart')`
108+
// and `shouldReceive('from_wc_order')` expectations; Mockery fails the test
109+
// if either is violated.
110+
$testee->create_order( 'vault-abc-123', $wc_order );
111+
112+
$this->assertArrayHasKey( 'purchase_units', $captured_body );
113+
}
114+
115+
/**
116+
* GIVEN a saved-token payment is captured for a WC order
117+
* WHEN the HTTP request body is built
118+
* THEN it has no top-level custom_id or invoice_id keys
119+
*
120+
* The PayPal Orders v2 API only reads these fields from inside a
121+
* purchase unit; leaving them at the request root (the PCP-6702 bug)
122+
* means PayPal silently ignores them.
123+
*/
124+
public function test_create_order_does_not_send_top_level_custom_id_or_invoice_id(): void {
125+
$captured_body = array();
126+
[ $testee ] = $this->make_testee( $captured_body );
127+
128+
$wc_order = Mockery::mock( WC_Order::class );
129+
130+
$testee->create_order( 'vault-abc-123', $wc_order );
131+
132+
$this->assertArrayNotHasKey( 'custom_id', $captured_body );
133+
$this->assertArrayNotHasKey( 'invoice_id', $captured_body );
134+
}
135+
136+
/**
137+
* GIVEN PurchaseUnitFactory::from_wc_order() populates custom_id/invoice_id
138+
* WHEN create_order serializes the purchase unit into the request body
139+
* THEN purchase_units[0] carries those values through untouched
140+
*/
141+
public function test_create_order_forwards_purchase_unit_custom_id_and_invoice_id(): void {
142+
$purchase_unit = Mockery::mock( PurchaseUnit::class );
143+
$purchase_unit->shouldReceive( 'to_array' )->andReturn(
144+
array(
145+
'custom_id' => '14121',
146+
'invoice_id' => 'PP-FLYERALARMDigital-422136',
147+
)
148+
);
149+
150+
$captured_body = array();
151+
[ $testee ] = $this->make_testee( $captured_body, $purchase_unit );
152+
153+
$wc_order = Mockery::mock( WC_Order::class );
154+
155+
$testee->create_order( 'vault-abc-123', $wc_order );
156+
157+
$this->assertSame( '14121', $captured_body['purchase_units'][0]['custom_id'] ?? null );
158+
$this->assertSame( 'PP-FLYERALARMDigital-422136', $captured_body['purchase_units'][0]['invoice_id'] ?? null );
159+
}
160+
161+
/**
162+
* GIVEN a vault id and an explicit payment source name are passed to create_order
163+
* WHEN the request body is built
164+
* THEN payment_source is keyed by the given name and carries the vault_id
165+
*/
166+
public function test_create_order_forwards_vault_id_and_payment_source_name(): void {
167+
$captured_body = array();
168+
[ $testee ] = $this->make_testee( $captured_body );
169+
170+
$wc_order = Mockery::mock( WC_Order::class );
171+
172+
$testee->create_order( 'vault-venmo-1', $wc_order, 'venmo' );
173+
174+
$this->assertSame(
175+
'vault-venmo-1',
176+
$captured_body['payment_source']['venmo']['vault_id'] ?? null
177+
);
178+
}
179+
180+
/**
181+
* GIVEN create_order is called without an explicit payment source name
182+
* WHEN the request body is built
183+
* THEN payment_source defaults to 'paypal'
184+
*/
185+
public function test_create_order_defaults_payment_source_name_to_paypal(): void {
186+
$captured_body = array();
187+
[ $testee ] = $this->make_testee( $captured_body );
188+
189+
$wc_order = Mockery::mock( WC_Order::class );
190+
191+
$testee->create_order( 'vault-abc-123', $wc_order );
192+
193+
$this->assertArrayHasKey( 'paypal', $captured_body['payment_source'] ?? array() );
194+
}
195+
}

tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ private function createGateway()
123123
false,
124124
$this->capturePayPalPayment,
125125
Mockery::mock(OrderEndpoint::class),
126-
'WC-',
127126
$this->context
128127
);
129128
}
@@ -150,7 +149,6 @@ private function createSpyGateway(): SpyablePayPalGateway
150149
false,
151150
$this->capturePayPalPayment,
152151
Mockery::mock(OrderEndpoint::class),
153-
'WC-',
154152
$this->context
155153
);
156154
}
@@ -177,7 +175,6 @@ private function createReturnUrlStubGateway(): PayPalGatewayReturnUrlStub
177175
false,
178176
$this->capturePayPalPayment,
179177
Mockery::mock(OrderEndpoint::class),
180-
'WC-',
181178
$this->context
182179
);
183180
}

0 commit comments

Comments
 (0)