-
Notifications
You must be signed in to change notification settings - Fork 121
Complete card payment only after alert presentation is done #15967
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.
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
Complete card payment only after alert presentation is done #15967
Conversation
isEligibleForBackendReceipts is async resulting in onPaymentCompletion called before alert presentation is done. This causes CardPresentPaymentCollectOrderPaymentUseCaseAdaptor to deinitialize CollectOrderPaymentUseCase before payment success is shown resulting in card payment stuck at processing step.
|
|
jaclync
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks a lot for looking into the root cause and fixing it properly. I tested in a simulator with simulated card reader enabled, and a physical device with a card reader, for successful payments in both POS and IPP without getting stuck.
| } | ||
|
|
||
| // Then ensure payment completion happens after alert presentation to avoid CollectOrderPaymentUseCase deinit before alert presentation | ||
| XCTAssertEqual(eventOrder, [.receiptEligibilityCheck, .alertPresented, .paymentCompletion]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice way of testing this 👍
|
Merging this now to post about releasing a new beta, cc @RafaelKayumov. |

WOOMOB-953
Description
As @jaclync has noticed when doing PR Review for #15956 the card payments would succeed, but POS was left stuck in "Processing..." screen.
Investigation
I tested TestFlight versions and confirmed that the issue wasn't reproducible on 22.8 but was reproducible on the current beta, 22.9.
I couldn't pinpoint an issue by looking at the changes so I did a
git bisectthat pointed to this commit.What this small change did was expose a slight timing issue when
onPaymentCompletion()was called.onPaymentCompletion()was relying onisEligibleForBackendReceiptscheck being synchronous, so alert presentation would be called before payment completion. However, after this commit,isEligibleForBackendReceiptsbecame asynchronous, and alert presentation was called afteronPaymentCompletion().It didn't cause issues for IPP because of the different ways
CollectOrderPaymentUseCasewas used. For POS,CardPresentPaymentCollectOrderPaymentUseCaseAdaptorwould deinitializeCollectOrderPaymentUseCaseafter a successful payment. Therefore, alert presenters were never called to notify about successful paymentSolution
The solution is straightforward. Move
onPaymentCompletion()intoisEligibleForBackendReceiptsto ensure alert presenters are called before completing the payment.I also added a test to ensure the expected event order.
Steps to reproduce
Testing information
RELEASE-NOTES.txtif necessary.