Re-use Payments in Checkout - #6564
Open
adammathys wants to merge 5 commits into
Open
Conversation
github-actions
Bot
added
the
changelog:solidus_core
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6564 +/- ##
=======================================
Coverage 92.26% 92.27%
=======================================
Files 1037 1037
Lines 21211 21229 +18
=======================================
+ Hits 19570 19588 +18
Misses 1641 1641 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adammathys
force-pushed
the
feature/reuse-payments-in-checkout
branch
from
August 31, 2026 18:21
3b53200 to
146bc7c
Compare
jarednorman
requested changes
Aug 31, 2026
|
|
||
| # Payments | ||
| has_many :payments, dependent: :destroy, inverse_of: :order | ||
| has_many :payments, dependent: :destroy, inverse_of: :order, autosave: true |
Member
There was a problem hiding this comment.
I worry about this. Feels like it could be considered a breaking change.
Member
Author
There was a problem hiding this comment.
Same. That's why I called it out in the pull-request description.
jarednorman
requested changes
Aug 31, 2026
5 tasks
Member
|
We want to update the wallet source fieldsets too, right? Saved cards won't reused payments methods the way you ahve this PR, if I understand correctly. |
adammathys
force-pushed
the
feature/reuse-payments-in-checkout
branch
from
September 1, 2026 17:32
146bc7c to
050aa3e
Compare
Member
Author
Yeah, that's correct. This change does not work for wallet sources because of how we completely override the |
Member
|
Are you going to do that in this PR or make that separate? |
Member
Author
|
I'll add that to this pull-request. |
When persisting an order, we want to persist changes made to the associated payments as well if there are any. This is a breaking change from previous behaviour, but will make it easier for us to reuse existing payment records in checkout instead of creating a new one and invalidating all of the existing ones every time the customer navigates through that step. Co-authored-by: Noah Silvera <noah@super.gd>
This will allow for updating an existing payment instead of always creating new ones. Which will lead to us being able to cut down on the number of invalid payments created on orders when customers navigated through the payment step repeatedly. Co-authored-by: Noah Silvera <noah@super.gd>
Changes the behaviour of OrderUpdateAttributes to update an existing payment if an ID is passed through for that payment instead of always creating a new one. Co-authored-by: Noah Silvera <noah@super.gd>
adammathys
force-pushed
the
feature/reuse-payments-in-checkout
branch
from
September 2, 2026 17:57
050aa3e to
cbe4262
Compare
If we've already got a unprocessed payment record for a specific payment method, then let's just re-use it and update the source instead of creating a brand new payment record and invalidating the old one leading to a bunch of junk invalidated payment data we don't care about. Co-authored-by: Noah Silvera <noah@super.gd> Co-authored-by: Senem <senem@super.gd>
Updates Order#validate_payments_attributes to also check if a passed payment ID param matches an ID of a payment that belongs to the order. This will ensure we don't create the possibility to modify a payment that does not belong to this order. Co-authored-by: Noah Silvera <noah@super.gd>
adammathys
force-pushed
the
feature/reuse-payments-in-checkout
branch
from
September 2, 2026 18:04
cbe4262 to
f288281
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
While repeatedly going through the payment step of checkout, we would create a new payment record each time and invalidate all of the old ones. This could eventually lead to a long list of invalidated payments which is confusing for admins and provides no real value. (It also causes issues for gateways like Stripe that expect there to be a single payment intent for a customer checkout session, and we typically tie the intent to the payment record.)
Instead, we now check for an existing valid payment record and reuse it if it exists. This means that the payment step of checkout will only ever create a single unprocessed payment record for a given payment method on each order.
This change is largely backwards compatible. Existing stores will not benefit from the new change without updating their storefront code to pass the payment ID when appropriate. If the ID is omitted, there's no functional change to behaviour.
We considered updating the admin and API uses, but felt both of those were clearer leaving the existing behaviour because they involve either an explicit call to create a payment (API) or a "New Payment" button (admin).
Possibly Breaking Changes
This updates the
Order#paymentsassociation to haveautosave: trueenabled so that we can persist the payment and its source when callingsave!on the order itself.Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs: