Skip to content

Conversation

@joshheald
Copy link
Contributor

@joshheald joshheald commented Nov 14, 2022

Closes: #8079

Description

The Reader is ready screen used to show immediately when you tap Card on the Payment Method screen, to start the card payment, when a card reader is connected, or immediately after a reader is connected.

This led to a slightly jarring payment experience for the merchant and their customers, as we prompted the merchant to ask their customer to tap their card before the reader is actually ready. There are network-bound operations which happen in this state, so the delay could be tens of seconds in a bad case. On fast Wifi, it's still noticeable at 1-2 seconds of delay

In this PR, we add a Getting ready to collect payment screen with a spinner at this point, and then show the Reader is ready screen when the Payment Intent has been returned.

Testing instructions

Navigate to Menu > Payments > Collect Payment
Enter an amount, tap Next, then Take Payment.
Be ready to tap your card immediately on seeing Reader is ready on the phone
Select Card as the payment method.
Observe that a spinner shows for a few seconds.
Observe that Reader is ready then shows on the phone, at the same time as the green light goes steady on the M2 reader.
Observe that the card can be read immediately.

To see this more clearly, set a breakpoint in Proxyman/Charles on https://api.stripe.com/v1/payment_intents, and observe that Reader is ready displays before that returns, but you can't tap your card until after you let the breakpoint run.

N.B. This affects the Interac refund flow as well, so Interac payment and refunds should be tested. Try both with and without a connected reader.

I noticed that the Tap/Swipe/Insert card alert would not work in the current code, and logged #8116 to fix this. This is not a regression of this PR.

Screenshots

preparing-reader-spinner.mp4

N.B. ignore the tipping screen: it's leftover from the WisePad configuration I added in my HACK week project and won't show for anyone else.


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@joshheald joshheald added type: bug A confirmed bug. feature: mobile payments Related to mobile payments / card present payments / Woo Payments. labels Nov 14, 2022
@joshheald joshheald added this to the 11.3 milestone Nov 14, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 14, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8115-c0b6e12 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@joshheald
Copy link
Contributor Author

@joe-keenan Here's the fix with the spinner I mentioned. Let me know what you think...

@joshheald joshheald requested a review from koke November 14, 2022 20:25
@joshheald joshheald marked this pull request as ready for review November 14, 2022 20:26
@joshheald
Copy link
Contributor Author

@lmischner @jostnes Optional review on this one... it's preparation which will make the Tap on Mobile UI much smoother, so can be reviewed as part of the overall flow later if that's more convenient for you.

Copy link
Member

@koke koke left a comment

Choose a reason for hiding this comment

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

This seems to work fine with the M2 reader. Note that I don't have a WisePad 3 anymore so I can't test Interac. Once thing I noticed is that the cancel button on the new modal will dismiss the modal, but then it will show the reader-is-ready modal again once ready. I'm not sure how much of a blocker it is but we might want to disable that cancel button if it doesn't actually cancel the reader initialization.

cancel-reader-ready

@joshheald
Copy link
Contributor Author

This seems to work fine with the M2 reader. Note that I don't have a WisePad 3 anymore so I can't test Interac.

Thanks for the review @koke

One thing I noticed is that the cancel button on the new modal will dismiss the modal, but then it will show the reader-is-ready modal again once ready.

I saw this but only once, and I thought I'd fixed it with another change. :/ if you're still seeing it, it's a bigger issue than I thought.

I'm not sure how much of a blocker it is but we might want to disable that cancel button if it doesn't actually cancel the reader initialization.

I'll see if I can figure out what it is – I'd rather have the cancel button as a spinner with no way out other than force quitting the app seems bad. That said, there are other steps in the flow which can't be cancelled either, so it's not a lot worse as a solution.

The `ReceiptEmailParameterDeterminer` synchronized plugins every time `receiptEmail` was called, i.e. when setting up the payment parameters at the start of every payment flow. This added an async call to the reader preparation flow, and if the flow was cancelled during that flow, the payment continued.

This happened because `CardPresentPaymentAction.collectPayment` hadn’t been sent yet, so we didn’t have a payment to cancel. This was sent when the plugin sync completed, even if we’d cancelled the flow in the meantime.

We also sync these plugins in the onboarding flow, which is checked at the start of every payment. Syncing again on the payment flow’s critical path is unneccesary.

This change removes the extra sync, and thus fixes the cancellation issue.
@peril-woocommerce
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@joshheald
Copy link
Contributor Author

@koke c0b6e12 fixes the cancellation issue. It was happening due to cancelPayment being called before collectPayment action was dispatched, while we were waiting for an unnecessary syncPlugins action.

Removing the extra sync fixed the cancelation, and makes payment preparation a little quicker.

@joshheald joshheald merged commit 83f1d4c into trunk Nov 15, 2022
@joshheald joshheald deleted the issue/8079-spinner-while-awaiting-reader-readiness branch November 15, 2022 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mobile Payments] Reader is Ready screen shows before it's really ready

4 participants