Skip to content

Remove ability to void invalid payments#3858

Merged
kennyadsl merged 2 commits intosolidusio:masterfrom
vl3:remove_ability_to_void_invalid_payments
Dec 14, 2020
Merged

Remove ability to void invalid payments#3858
kennyadsl merged 2 commits intosolidusio:masterfrom
vl3:remove_ability_to_void_invalid_payments

Conversation

@vl3
Copy link
Copy Markdown
Contributor

@vl3 vl3 commented Dec 9, 2020

Description

My reasoning behind this is that attempts to void invalid payments will fail because response_code is not set for invalid payments. There are several things that suggests that transitioning from invalid to void should be forbidden (although that the payment could be reattempted, but that's another story).

So everything suggests that invalid state should be final and, other than reattempting the payment , we should block this transition. Attached is an screenshot of how the admin panel will understand transitions.

Before

invalid_payment_before

After
invalid_payment_after

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Copy link
Copy Markdown
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

I think it makes sense, thanks! I left a question about the implementation that I think we can DRY a bit, let me know!

Comment thread core/app/models/spree/payment_source.rb Outdated
Voiding invalid payments will result in an error due to the lack of
response_code. Additionally, this transition is not allowed in the
payment state machine.
@vl3 vl3 force-pushed the remove_ability_to_void_invalid_payments branch from ff27f7f to f4637c9 Compare December 9, 2020 18:09
@vl3 vl3 requested a review from kennyadsl December 9, 2020 19:24
Copy link
Copy Markdown
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

Copy link
Copy Markdown
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍

@kennyadsl kennyadsl added this to the 3.1.0 milestone Dec 10, 2020
@kennyadsl kennyadsl added release:major Breaking change on hold until next major release Needs Backport and removed release:major Breaking change on hold until next major release labels Dec 10, 2020
@kennyadsl kennyadsl modified the milestones: 3.1.0, 2.11 Dec 10, 2020
@kennyadsl kennyadsl merged commit ff43626 into solidusio:master Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants