Fix obscure issue when choosing payment method in checkout #13197
+52
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
It didn't catch the error I was looking for. I'm not sure if it is a valid use case, but it still seems helpful to add coverage for current functionality.
I tried to build it within context "with existing invalid payments", but couldn't get it to work so created a different one.
Note that in the real world, this avoids a crash, but still requires the user to click the button two more times before it will work, with no hints as to why. So not a great help.
dacook
force-pushed
the
reload-payments-12693
branch
from
0d938b7 to
55cbe51
Compare
dacook
changed the title
Co-authored-by: Gaetan Craig-Riou <[email protected]>
rioug
added
the
user facing changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What? Why?
When you perform a very particular set of actions, the checkout fails to proceed, with no feedback to the user at all (see comment on issue) I'd consider this a bug but haven't bothered to classify it.
This was caused by a clever pre-fetch optimisation of Turbo, when you hover over a link. It makes sense if the web app follows the rule that GET is a safe HTTP method. But our web app breaks that rule here 🤦
I've added a controller test to cover this, because I wasn't sure at first where to start. I now think that it would have been better to add a system test (eg checkout/payment_spec) and/or unit test (workflow_service_spec), but am not sure it's worth the effort. But I'm happy to be told otherwise by reviewers.
What should we test?
Repeat for both cases "i" and "ii":
For method "i", this should now work seamlessly.
For method "ii", you will have to click "next" a few times without getting feedback, but it does work eventually. We can probably improve that but I think it's pretty obscure and not worth bothering.
Release notes
It's probably worth mentioning as a user-facing change. Users may have been reporting strange issues with checkout to their instance managers, but they probably didn't figure out how to replicate it so didn't escalate further.
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.