Configurable order state machine and reduce the number of possible transitions #3542
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
Mar 6, 2020
kennyadsl
reviewed
Mar 6, 2020
|
@mamhoff it would be awesome to get this in 2.11, which is the last release of the 2.x branch. The idea is to merge this in 2.11 and deprecate the old state machine module, then remove it completely in 3.0. |
|
I can try to move this forward, if @mamhoff agrees. |
|
Sure, go ahead! I'm really happy this is moving forward.
Andrea Longhi <[email protected]> schrieb am Fr., 29. Mai 2020,
16:40:
… I can try to move this forward, if @mamhoff <https://github.com/mamhoff>
agrees.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3542 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFLXKMHQWUAIXVWTQVK3UTRT7CMZANCNFSM4LDDAQFA>
.
|
|
Quick note here, to reflect discussion on the issue: When this is merged, we can close #1576 |
spaghetticode
force-pushed
the
configurable-order-state-machine
branch
from
0f1ea10 to
90e6fcf
Compare
spaghetticode
added
type:enhancement
spaghetticode
force-pushed
the
configurable-order-state-machine
branch
3 times, most recently
from
2ece35b to
2e7c5db
Compare
spaghetticode
changed the title
spaghetticode
changed the title
5 tasks
aldesantis
approved these changes
Jun 5, 2020
jarednorman
approved these changes
Jun 5, 2020
|
Putting this on hold because @kennyadsl had some opinions about potentially deprecating and then removing |
spaghetticode
changed the title
|
I hope to be able to talk about this in our Core Team meeting this week. |
In 7c4f82c, all relevant state machines except for the order one have been extracted into modules. This performs the same exercise for the order state machine. A lot of this code is deeply interlinked with the rest of the system, so you'd have to take lots of care when replacing this; but it should be possible nonetheless. The new state machine is named Spree::Core::StateMachines::Order and includes the same code of the old one. The newly named state machine is active on new stores generated from scratch and in Solidus tests, but old stores will still use the old one, unless the preference `Spree::Config.use_legacy_order_state_machine` is explicitly set to `false`. When using the old named state machine a deprecation warning will be printed at boot time, as Solidus 3.0 will start using the new order state machine by default.
This removes many, many transitions from the Order state machine definitions included in Spree::Core::StateMachines::Order, allowing us to reason more clearly about what the system is supposed to be doing and when. This new state machine definition is not active yet on existing stores, but will become standard from Solidus 3.0. The legacy state machine definitions from Spree::Order::Checkout are left untouched, so old stores can continue to use them. A bunch of transitions have been removed, and now an order can transition to: * Canceled only when Complete; * Awaiting Return only when Complete; * Returned only when Complete or Awaiting Return.
spaghetticode
force-pushed
the
configurable-order-state-machine
branch
from
41bf48b to
98099f0
Compare
|
I explored the idea of adding a deprecation to the constant |
aldesantis
added a commit
to solidusio/solidus_subscriptions
that referenced
this pull request
Sep 24, 2020
In Solidus 2.11, it isn't possible to cancel an order that hasn't been completed anymore[1], which causes our failure dispatchers to fail when they call `order.cancel!`. To work around the issue, we're now only cancelling if the state machine allows us to (i.e., on Solidus <= 2.10 and in customized state machines). If the order cannot be cancelled, we skip the transition silently. [1]: solidusio/solidus#3542
aldesantis
added a commit
to solidusio/solidus_subscriptions
that referenced
this pull request
Sep 24, 2020
In Solidus 2.11, it isn't possible to cancel an order that hasn't been completed anymore[1], which causes our failure dispatchers to fail when they call `order.cancel!`. To work around the issue, we're now only cancelling if the state machine allows us to (i.e., on Solidus <= 2.10 and in customized state machines). If the order cannot be cancelled, we skip the transition silently. [1]: solidusio/solidus#3542
aldesantis
added a commit
to solidusio/solidus_subscriptions
that referenced
this pull request
Sep 25, 2020
In Solidus 2.11, it isn't possible to cancel an order that hasn't been completed anymore[1], which causes our failure dispatchers to fail when they call `order.cancel!`. To work around the issue, we're now only cancelling if the state machine allows us to (i.e., on Solidus <= 2.10 and in customized state machines). If the order cannot be cancelled, we skip the transition silently. [1]: solidusio/solidus#3542
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.
This PR can be divided into two logical parts that are interdependent and
breaking each one in their way. These parts are described in detail later.
Make Order state machine configurable
In 7c4f82c, all relevant state machines except for the order one have
been extracted into modules. This performs the same exercise for the
order state machine. A lot of this code is deeply interlinked with the
rest of the system, so you'd have to take lots of care when replacing
this; but it should be possible nonetheless.
The new state machine is named
Spree::Core::StateMachines::Orderandincludes the same code of the old one.
The newly named state machine is active on new stores generated from
scratch and in Solidus tests, but old stores will still use the old one,
unless the preference
Spree::Config.use_legacy_order_state_machineisexplicitly set to
false.When using the old named state machine a deprecation warning will be
printed at boot time, as Solidus 3.0 will start using the new order state
machine by default.
Reduce the number of new order state machine transitions
This removes many, many transitions from the Order state machine definitions
included in
Spree::Core::StateMachines::Order, allowing us to reason moreclearly about what the system is supposed to be doing and when.
This new state machine definition is not active yet on existing stores, but
will become standard from Solidus 3.0.
The legacy state machine definitions from
Spree::Order::Checkoutare leftuntouched, so old stores can continue to use them.
A bunch of transitions have been removed, and now an order can transition to:
This change was initially proposed with #3532 but for convenience it
has been included directly here as the original PR, being a breaking change, was
not mergeable in isolation without the premise of the first part of this PR. Please
consider taking a look at the original PR for the interesting discussion.
Checklist: