Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry counter meta to order to avoid duplicate invoice error on c… #371

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Add retry counter meta to order to avoid duplicate invoice error on c… #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c072e85
Add retry counter meta to order to avoid duplicate invoice error on c…
Dinamiko Nov 18, 2021
626f480
Remove unused stuff on test
Dinamiko Nov 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions modules/ppcp-api-client/src/Factory/PurchaseUnitFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,11 @@ public function from_wc_order( \WC_Order $order ): PurchaseUnit {
$payee = $this->payee_repository->payee();
$wc_order_id = $order->get_order_number();
$custom_id = $this->prefix . $wc_order_id;
$invoice_id = $this->prefix . $wc_order_id;
$retry = $order->get_meta( 'ppcp-retry' ) ? '-' . $order->get_meta( 'ppcp-retry' ) : '';
$invoice_id = $this->prefix . $wc_order_id . $retry;
$soft_descriptor = '';
$purchase_unit = new PurchaseUnit(

$purchase_unit = new PurchaseUnit(
$amount,
$items,
$shipping,
Expand Down
6 changes: 6 additions & 0 deletions modules/ppcp-wc-gateway/src/Gateway/ProcessPaymentTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,19 @@ function ( Authorization $authorization ): bool {
}
}

// Adds retry counter meta to avoid duplicate invoice id error on consequent tries.
$wc_order->update_meta_data( 'ppcp-retry', (int) $wc_order->get_meta( 'ppcp-retry' ) + 1 );
$wc_order->save_meta_data();

$this->session_handler->destroy_session_data();
wc_add_notice( $error_message, 'error' );

return $failure_data;
}

WC()->cart->empty_cart();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we do this? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I´ve moved it from OrderProcessor::process to here because otherwise there were no chance to retry the payment when the payment is not saved on PayPal.

Copy link
Contributor Author

@Dinamiko Dinamiko Nov 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexP11223

In order to prevent subscriptions renewals to fail because no payment stored for customer we are introducing a check after the payment to see if the payment is saved on PayPal or not, if it´s not saved then we set the WC order as failed.

The problem is that if the customer tries again with the same order it gets a DUPLICATE_INVOICE_ID, here is how I currently implemented it:

  1. POST order with Auth intent
  2. PATCH
  3. Authorize
  4. Check if payment is saved, if so capture auth order.

The above works, however if point 4 fails, we void the auth order and start again:

  1. POST order with Auth intent
  2. PATCH
  3. Authorize, here we get DUPLICATE_INVOICE_ID

The error is because as the PayPal authorization was successful, that order ID is “used”, so we have to increment or otherwise alter the invoice ID.

I´ve implemented the later by adding a counter at the end, so if WC-001 is used, we alter it to WC-001-1.

$this->session_handler->destroy_session_data();

return array(
'result' => 'success',
'redirect' => $this->get_return_url( $wc_order ),
Expand Down
2 changes: 0 additions & 2 deletions modules/ppcp-wc-gateway/src/Processor/OrderProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,6 @@ public function process( \WC_Order $wc_order ): bool {
$wc_order->update_meta_data( AuthorizedPaymentsProcessor::CAPTURED_META_KEY, 'true' );
$wc_order->update_status( 'processing' );
}
WC()->cart->empty_cart();
$this->session_handler->destroy_session_data();
$this->last_error = '';
return true;
}
Expand Down
3 changes: 3 additions & 0 deletions tests/PHPUnit/ApiClient/Factory/PurchaseUnitFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public function testWcOrderDefault()
$wcOrder = Mockery::mock(\WC_Order::class);
$wcOrder
->expects('get_order_number')->andReturn($wcOrderId);
$wcOrder->expects('get_meta')->andReturn('');
$amount = Mockery::mock(Amount::class);
$amountFactory = Mockery::mock(AmountFactory::class);
$amountFactory
Expand Down Expand Up @@ -89,6 +90,7 @@ public function testWcOrderShippingGetsDroppedWhenNoPostalCode()
$wcOrder = Mockery::mock(\WC_Order::class);
$wcOrder
->expects('get_order_number')->andReturn(1);
$wcOrder->expects('get_meta')->andReturn('');
$amount = Mockery::mock(Amount::class);
$amountFactory = Mockery::mock(AmountFactory::class);
$amountFactory
Expand Down Expand Up @@ -144,6 +146,7 @@ public function testWcOrderShippingGetsDroppedWhenNoCountryCode()
$wcOrder = Mockery::mock(\WC_Order::class);
$wcOrder
->expects('get_order_number')->andReturn(1);
$wcOrder->expects('get_meta')->andReturn('');
$amount = Mockery::mock(Amount::class);
$amountFactory = Mockery::mock(AmountFactory::class);
$amountFactory
Expand Down
8 changes: 7 additions & 1 deletion tests/PHPUnit/WcGateway/Gateway/WcGatewayTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function testProcessPaymentSuccess() {
$orderId = 1;
$wcOrder = Mockery::mock(\WC_Order::class);
$wcOrder->shouldReceive('get_customer_id')->andReturn(1);

$wcOrder->shouldReceive('get_meta')->andReturn('');
$settingsRenderer = Mockery::mock(SettingsRenderer::class);
$orderProcessor = Mockery::mock(OrderProcessor::class);
$orderProcessor
Expand Down Expand Up @@ -106,6 +106,12 @@ function(\WC_Order $order) use ($wcOrder) : bool {
when('wc_get_checkout_url')
->justReturn('test');

$woocommerce = Mockery::mock(\WooCommerce::class);
$cart = Mockery::mock(\WC_Cart::class);
when('WC')->justReturn($woocommerce);
$woocommerce->cart = $cart;
$cart->shouldReceive('empty_cart');

$result = $testee->process_payment($orderId);

$this->assertIsArray($result);
Expand Down
35 changes: 0 additions & 35 deletions tests/PHPUnit/WcGateway/Processor/OrderProcessorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ public function testAuthorize() {
$sessionHandler
->expects('order')
->andReturn($currentOrder);
$sessionHandler
->expects('destroy_session_data');

$orderEndpoint = Mockery::mock(OrderEndpoint::class);
$orderEndpoint
Expand Down Expand Up @@ -129,15 +127,6 @@ public function testAuthorize() {
$this->environment
);

$cart = Mockery::mock(\WC_Cart::class);
$cart
->expects('empty_cart');
$woocommerce = Mockery::mock(\WooCommerce::class);
when('WC')
->justReturn($woocommerce);

$woocommerce->cart = $cart;

$wcOrder
->expects('update_meta_data')
->with(
Expand Down Expand Up @@ -211,8 +200,6 @@ public function testCapture() {
$sessionHandler
->expects('order')
->andReturn($currentOrder);
$sessionHandler
->expects('destroy_session_data');
$orderEndpoint = Mockery::mock(OrderEndpoint::class);
$orderEndpoint
->expects('patch_order_with')
Expand All @@ -234,21 +221,8 @@ public function testCapture() {
->shouldReceive('has')
->andReturnFalse();

$cart = Mockery::mock(\WC_Cart::class);
$cart
->shouldReceive('empty_cart');

$woocommerce = Mockery::Mock(\Woocommerce::class);
$woocommerce
->shouldReceive('__get')
->with('cart')
->set('cart', $cart);
when('WC')
->justReturn($woocommerce);

$logger = Mockery::mock(LoggerInterface::class);


$testee = new OrderProcessor(
$sessionHandler,
$orderEndpoint,
Expand All @@ -260,15 +234,6 @@ public function testCapture() {
$this->environment
);

$cart = Mockery::mock(\WC_Cart::class);
$cart
->expects('empty_cart');
$woocommerce = Mockery::mock(\WooCommerce::class);
$woocommerce->cart = $cart;

when('WC')
->justReturn($woocommerce);

$wcOrder
->expects('update_meta_data')
->with(
Expand Down