adyen-magento2 icon indicating copy to clipboard operation
adyen-magento2 copied to clipboard

[PW-7149] - POS terminal Payment Notification for non-existing Order

Open pmzandbergen opened this issue 2 years ago • 11 comments

For POS terminal payments the current flow is:

  1. Create a Cart
  2. Initialize the POS payment (REST call): This will reserve an Order ID but does not create an Order yet
  3. Finish the payment on the terminal
  4. Create the order (REST call): The reserved Order ID will be used

This can cause problems if something goes wrong between step 3 and 4. If for example the Customer, closes the page (e.g. clicking on a link, goes back, closes the browser) or the Internet connection fails, no Order will be created. If in the Payment on the terminal is successful we end up with a Payment without an Order. The Adyen module receives the Notification from Adyen, does not recognize the Order and ignores the Notification.

Suggested Fix

  1. Do not use a reserved Order ID but create the Order instead. If the Customer cancels the POS payment re-create the Cart using an API call.

OR

  1. On receiving the Notification from Adyen check if the Order exists. If it does not search for a Quote with the Reserved Order ID and place the Order before processing the Notification

Note: Since we needed a solution right away I've implemented the solution nr. 2 in our project.

pmzandbergen avatar Apr 01 '22 09:04 pmzandbergen

Please also note that initializing a POS payment does not lock the cart (e.g. unset the is_active flag). Therefore the Cart contents can be altered during the payment.

pmzandbergen avatar Apr 01 '22 13:04 pmzandbergen

Hi @pmzandbergen,

Thank you for opening this issue. We agree that this could cause some issues and will be looking into it. Also thanks for providing the potential solutions. Your first suggestion would not fit our current flow but we are interested in the second one, could you provide to us? If you could open a PR for it, that would be great!

Cheers, Titus

tnaber avatar Apr 06 '22 14:04 tnaber

Hi @tnaber we are currently testing our fix / plug-in. There are still some situations in which our fix doesn't seems work.

Terminal Unavailable The first problem is with the processing of the (success) notification from Adyen. On processing the Notification the terminal is being contacted (creating a receipt). From time to time this does not work, for example when the terminal is disconnected. In these cases our plugin isn't able to create the Order (yet).

Unlocked Cart Another problem is with the unlocked Cart. We're using terminal payments for in-store sales on iPads. If a employee uses the following ("wrong") workflow:

  1. Create a cart with a product (10 Euro)
  2. Start a POS payment
  3. Adds another product, updating the Cart (20 Euro)
  4. Finish the initial POS payment (10 Euro)

You'll end up with a payment which doesn't match the Cart total. Of course we can't create the Order in this case. To overcome this issue we could lock the cart when a POS payment is initialized and unlock it on failure.

Follow-Up I think the initial suggested fix, creating an Order before starting the POS payment, is a far more robust workflow. I'll try to implement it tomorrow and report afterwards.

Note: We are using version 7.x, but the problem is also present in version 8.x.

pmzandbergen avatar Apr 11 '22 15:04 pmzandbergen

With some delay:

  • Upgraded to version 8.1.1 (can't use 8.2.x yet since it requires Magento 2.4.x, see: https://github.com/Adyen/adyen-magento2/pull/1490)
  • Added the following to our Adyen fixes / features module:
    1. Automatically place order on processing Notification with unknown Order Increment ID
    2. Lock Quote during POS payment
    3. Update status in background during POS payment, preventing timeouts when the browser is being closed / unresponsive during payment

When its tested in production and everything seems to be stable, I'll try to find a moment to merge it in the Adyen Payment module. Might take a while.

pmzandbergen avatar May 09 '22 14:05 pmzandbergen

Hi @pmzandbergen,

Good to hear you chasing this! If you would like to speed up the process, an option would be to open a Draft PR so we can have a look together :)

Cheers, Titus

tnaber avatar May 09 '22 14:05 tnaber

Hi @tnaber , I'm not going to create a PR for this bug. Reason:

https://github.com/Adyen/adyen-magento2/issues/1489#issuecomment-1123632702

Not really a substantiated discussion IMO. Apparently improvements / fixes are better kept private.

pmzandbergen avatar May 11 '22 12:05 pmzandbergen

Hi @pmzandbergen

We are planning to merge #1610 soon. Basically this PR should help decrease the frequency of the problem you were describing by avoiding another js call to the plugin backend between steps 3 and 4 of the original flow you mentioned.

In the meantime we're still looking into the possibility of adding point number 2 of your suggested fixes. However before that we would like to implement some refactoring to our webhook handling functionality.

Regards, Jean Adyen

Morerice avatar Jul 12 '22 08:07 Morerice

Hi @Morerice ,

Since we have a PWA we do not use these JS's, however the flow is similar. Currently we have multiple fixes in place to make sure the order will be created:

  1. The Quote will be locked when a POS payment is pending (you do not want the grand total to be changed during payment)
  2. In the background (using a Consumer) the status is updated until the POS payment has reached a final state (similar to the XHR calls)
  3. When we receive an notification for a POS payment and no such order exists it will be created based on the quote with the reserved order ID

We have implemented these fixes in the 8.1.x version of the module, it fixes 95% of the problems with POS payments. Sometimes the order can't be created because the POS terminal is unreachable. Fix 3 (or 2 in this thread) is easy to implement.

Currently investigating if we are going to update the module or drop it and implement the Adyen API ourselves.

Regards, Pieter

pmzandbergen avatar Jul 12 '22 11:07 pmzandbergen

Hi @pmzandbergen,

Thanks for the update. Since your flow is similar to the one we're proposing, we will go ahead with this change. The next step would then be to implement point 2 (webhook related). As you said this particular point should be quite straightforward, however before implementing this we will need to refactor a bit our webhook handling.

Still, this point should be handled in the near future. We will keep you updated about this next development.

Regards, Jean Adyen

Morerice avatar Jul 13 '22 13:07 Morerice

Hi @pmzandbergen,

While adding the code you provided us to our native module, we were able to easily reproduce this issue by waiting more than 30s to swipe/insert the card in the pos device. If the card was swiped more than 30s afterwards, the end result was that the payment would still go trough, and the order would not be created on Magento.

Hence, instead of adding the code you provided us as a plugin within our module, for now we would like to see if the timeout change done on #1761 will dramatically decrease the occurrence of this issue. This is also because we want to explore other alternatives to fixing this issue, mainly because we feel that adding your code as a plugin would substantially increase the complexity of the code, just to cover one edge case.

Regards, Jean Adyen

Morerice avatar Oct 11 '22 09:10 Morerice

Hi @Morerice ,

I think the best solution would be to actually create the order when initiating the POS transaction. Otherwise you would have to lock the quote somehow, keep polling in the background, etc. That would all not be necessary if the order is simply created beforehand and canceled if needed (just like with, for example, iDeal payments).

Regards,

Pieter

pmzandbergen avatar Oct 11 '22 09:10 pmzandbergen

Hello @pmzandbergen,

Recently, we've released version 9 of our plugin with new features and fixes for the known issues. As you mentioned before, POS flow was fragile and order couldn't be revived if the connection would be broken.

We implemented following PRs with the V9 for POS improvements. With those PRs, we got rid of terminal initiation. Instead, terminal payments request is being sent after internal /payment-information call. Default timeout for this call is equal to the timeout of Adyen's Terminal API on headless implementation. So, if the payment has not been completed in the desired time span, /payment-information call will fail together with terminals /syncPayments call and Cancelled message will be shown on the terminal display.

https://github.com/Adyen/adyen-magento2/pull/2223 https://github.com/Adyen/adyen-magento2/pull/2132

An example payload of /payment-information endpoint can be found below.

{
   "paymentMethod": {
       "method": "adyen_pos_cloud",
       "additional_data": {
           "terminal_id": "YOUR_TERMINAL_ID"   // Terminal ID returning from extension attributes.
           // "number_of_installments": "5"   // Include this line if installment is required.
           // "funding_source": "credit"      // Value can be debit or credit. Default is credit.
       }
   }
}

With this new implementation, we were not able to reproduce the previous issue.

Please let us know if you could test this new flow.

V9 migration guide

Best Regards, Can

candemiralp avatar Dec 05 '23 10:12 candemiralp

Hi @candemiralp ,

I've tested the new V9 module using a vanilla Magento installation. There are still issues:

Example 1

  1. Create a Cart
  2. Start checkout, choose POS
  3. Wait for the terminal to initialize the transaction
  4. Close the checkout (browser) to simulate a dropped connection
  5. Finish the payment on the terminal
  6. No order is being created (issue: payment without an order)

Example 2

  1. Create a Cart as Customer
  2. Start checkout, choose POS
  3. Wait for the terminal to initialize the transaction
  4. Update the Customer Cart using another browser, ensure the totals are recalculated
  5. Finish the payment on the terminal
  6. The order is created but the totals don't match

I also noticed, while setting up the vanilla Magento installation with Adyen V9, the module doesn't allow the Magento Setup Install to run. You have to disable to module in order to install Magento and re-enable the module afterwards. Please check your setup scripts / patches, they probably depend on data unavailable during installation.

pmzandbergen avatar Jan 04 '24 21:01 pmzandbergen

Hello @pmzandbergen,

Thanks for testing out the new flow and indeed the steps that you provided were reproduced by us. We will try to create a fix for it, most probably with creating the order in advance as we discussed earlier.

Thanks your informing us about the installation issue. That one has been fixed by an external contributor on #2387 and has been released.

Best Regards, Can

candemiralp avatar Jan 16 '24 12:01 candemiralp

Hi @candemiralp ,

Thanks your informing us about the installation issue. That one has been fixed by an external contributor on #2387 and has been released.

This is a partial fix, your module is still missing dependencies for Magento_Customer:

  • https://github.com/Adyen/adyen-magento2/blob/develop/etc/module.xml

pmzandbergen avatar Feb 13 '24 13:02 pmzandbergen

Hi @pmzandbergen,

Thanks for creating the fix for this issue. We will review and merge it soon.

Best Regards, Can

candemiralp avatar Feb 13 '24 13:02 candemiralp

Hi @pmzandbergen,

Thanks for creating the fix for this issue. We will review and merge it soon.

Best Regards, Can

A colleague is currently testing / reviewing this PR (https://github.com/Adyen/adyen-magento2/pull/2509) and will probably change a few lines since it didn't solve the issue yet. Lets discuss this separate issue in the PR if necessary. :)

pmzandbergen avatar Feb 13 '24 13:02 pmzandbergen

Hi @pmzandbergen, for your interest, I have introduced a PR for supporting the Pos new flow to fix this issue, kindly have a look

hossam-adyen avatar Feb 16 '24 12:02 hossam-adyen

Hello @pmzandbergen and all other contributors,

We've decided to implement this solution configurable to prevent any breaking change on other merchants. The PR #2565 will solve it and the PR #2566 makes it configurable.

I am re-openning this Github issue to inform all the stakeholders and merging feature branch feature/asyncPosFlow will close this issue automatically.

Best Regards, Can

candemiralp avatar Apr 03 '24 07:04 candemiralp