-
Notifications
You must be signed in to change notification settings - Fork 50
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
Check if transition can be applied before apply it during order creation #134
Check if transition can be applied before apply it during order creation #134
Conversation
b2c23d2
to
caf95fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Maxime :)
Thanks for contributing!
Anyway, I'm thinking if we shouldn't provide some way to allow for customizations of these transitions.
$stateMachine->can(OrderCheckoutTransitions::TRANSITION_ADDRESS)->shouldBeCalled(); | ||
$stateMachine->can(OrderCheckoutTransitions::TRANSITION_SELECT_SHIPPING)->shouldBeCalled(); | ||
$stateMachine->can(OrderCheckoutTransitions::TRANSITION_SELECT_PAYMENT)->shouldBeCalled(); | ||
$stateMachine->can(OrderCheckoutTransitions::TRANSITION_COMPLETE)->shouldBeCalled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe apply(...)->shouldBeCalled()
should still be there. More, all of these should be $stateMachine->can(...)->willReturn(true);
We should also create at least one spec when some of the transition cannot be called (and check that apply(...)->shouldNotBeCalled()
@maximehuran can you also take a look at the failing build? And I agree with @lchrusciel, but it can be done as an improvement, let's focus on fixing a bug 🚀 |
Hi, I don't get how to avoid having this error. Is is still a bug or what should I do? |
…i, GSadee) This PR was merged into the 1.1-dev branch. Discussion ---------- This is similar to already existing PR, but i think original poster stopped responding so I can try to get that working. Issue: #134 However, the issue also appears when product is not virtual, but skipping_shipping_step_allowed in channel PR: #132 Commits ------- c1f47db Fix state machine exception on order creation bc25387 added phpspec tests 10bf4b5 Remove PHPUnit cache file
fixes #132
Apply transition only if we can apply it.
During virtual order creation, this avoid the exception :
Transition "select_shipping" cannot be applied on state "shipping_skipped" of object "App\Entity\Order\Order" with graph "sylius_order_checkout"