Do not allow successful checkout when order has only a void payment #3123
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
houndci-bot
reviewed
Feb 26, 2019
When a payment has been marked for some reason as `void`, the customer should not be able to complete the checkout process successfully. The customer is now redirected to the payment page, where they can add another payment to the order in order to complete it successfully.
spaghetticode
force-pushed
the
spaghetticode/void-payments-checkout
branch
from
f8f8d9a to
5eb0bdd
Compare
When a single payment exists for the order, and the payment is marked as void, then the order payment state changes to `Failed`. When there are multiple pending payments for the order, if only one is marked as void the order payment state is still `Balance Due`. If all the payments are marked as void then the order payment state changes to `Failed`.
|
After some input from @aitbw about the possible behavior regarding the failing spec, it seems reasonable that when a single payment exists and it is marked as On the other hand, when there are other pending payments, the state remains as So I added one more spec (and did some refactoring later) in order to properly expose this behavior. |
aitbw
approved these changes
Feb 26, 2019
kennyadsl
reviewed
Feb 28, 2019
There was a problem hiding this comment.
I'm ok with this solution, I'd just change the last commit message with something more descriptive. I know you added some lines in the commit description but sometimes you only have the commit message visible and it would be great to have some more information about what it does.
Utility method `create_payment` was extracted in order to DRY up the code a bit.
spaghetticode
force-pushed
the
spaghetticode/void-payments-checkout
branch
from
a4ec39a to
e663b24
Compare
kennyadsl
approved these changes
Feb 28, 2019
jacobherrington
approved these changes
Mar 1, 2019
3 tasks
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.
When a payment has been marked for some reason as
void, the customer shouldnot be able to complete the checkout process successfully.
The customer is now redirected to the payment page, where they can add another
payment to the order in order to complete it successfully.
This scope for
Spree::Paymentmodel:now includes also
void, first because adding it there fixes the issue during the checkout, second because the wordvoidmeans quite the opposite ofvalid.One spec in the backend is currently failing (
spec/features/admin/orders/payments_spec.rb:65) so I'd like to understand from the community if its behavior can be changed there or other solutions make more sense.The reasoning behind this change is that, when an order with payment gets stale, admins may want to set the payment as
voidfor some reason. When the customer resumes the checkout after some days they should not be able to complete it with that voided payment.