-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIx a stale data issue when using Spree::OrderCapturing. #407
Conversation
👍 |
This needs a test. |
This shouldn't happen if I thought that was automatic in our version of Rails but it doesn't seem to be happening for me. Maybe it's because of one of the options we put on our When I test out the current state of the code: >> o = Spree::Order.complete.last; o.object_id
=> 70196650722200
>> o.payments.pending.to_a.first.order.object_id
=> 70196674897280 We do have an inverse_of set up here on Payment: https://github.com/solidusio/solidus/blob/157365b/core/app/models/spree/payment.rb#L9 If I change the Order relationship to: >> o = Spree::Order.complete.last; o.object_id
=> 70151364663080
>> o.payments.pending.to_a.first.order.object_id
=> 70151364663080 Can we add a test and then see if correcting the inverse_of relationship will fix this without a reload? |
I agree with @jordan-brough, but that needs to be tested thoroughly. We don't want to re-introduce this spree 2.2 era regression. Either way, we should get a test first 😉 |
@jhawthorn wow, great memory! If adding inverse_of becomes a huge effort then I'm 👍 on this PR as-is and then opening an issue around figuring out how to add inverse_of separately. |
87430ac
to
fbb572b
Compare
As described in the comment, calling `payment.capture!` triggers an after_save on Spree::Payment. In this after_save, things can happen, one of those things is an `order.update!`. That means if the `order.update!` happens within a payment after save, the order we have stored in `@order` is now out of date, and has to be reloaded. If we don't reload it, the hooks in the order updater will be called twice. If they make decisions around fresh state changes, idempotency could be compromised.
fbb572b
to
d3c402a
Compare
👍 I'm really interested in seeing |
FIx a stale data issue when using Spree::OrderCapturing.
Note: inverse_of investigation PR here: #414 |
As described in the comment, calling
payment.capture!
triggers anafter_save on Spree::Payment. In this after_save, things can happen, one
of those things is an
order.update!
.That means if the
order.update!
happens within a payment after save, the order we have stored in@order
is now out of date, and has to be reloaded.If we don't reload it, the hooks in the order updater will be called
twice. If they make decisions around fresh state changes, idempotency
could be compromised. cc @athal7