[API] handle shipping without addressing #12746
Conversation
SirDomin
commented
Jun 23, 2021
SirDomin
commented
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Related tickets | |
| License | MIT |
SirDomin
force-pushed
the
handle-shipping-without-addressing
branch
from
6179567
to
3cb78c1
Compare
src/Sylius/Bundle/ApiBundle/Exception/OrderCannotBeShippedWithoutAddressing.php
Outdated
Show resolved
Hide resolved
| @@ -66,6 +67,10 @@ public function __invoke(ChooseShippingMethod $chooseShippingMethod): OrderInter | |||
|
|
|||
| $stateMachine = $this->stateMachineFactory->get($cart, OrderCheckoutTransitions::GRAPH); | |||
|
|
|||
| if ($cart->getShippingAddress() === null) { | |||
| throw new OrderCannotBeShippedWithoutAddressing(); | |||
There was a problem hiding this comment.
I wonder, is it something we would like to do with a custom exception? Or maybe it should be a guard on state machine? 🤔 We're checking the possibility of transitioning order below nonetheless. cc @GSadee @lchrusciel
There was a problem hiding this comment.
state machine is check in the next lines, it throws 500 error if state cant be applied, this way we capture error we expect before and throw custom error to tell what caused this error.
There was a problem hiding this comment.
I totally understand that 😄 I just wonder shouldn't we keep the order validation in one place rather than have it done with two fashions (custom exception + state machine checking). Still, it's just an improvement 🖖
There was a problem hiding this comment.
Validation should be handled mostly with the validation component. That should be possible with the command, that we are having here. I'm wondering if we should start moving some protecting rules into the state machine, as we don't have any yet. Exception itself is not a big deal in the current implementation tho
There was a problem hiding this comment.
#12746 okay, added another approach (this time with validations)
SirDomin
force-pushed
the
handle-shipping-without-addressing
branch
from
762cf3b
to
df21ee5
Compare
|
Thank you, @SirDomin! 🥇 |
This PR was merged into the 1.11-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | 1.10 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | #12746 | License | MIT <!-- - Bug fixes must be submitted against the 1.9 or 1.10 branch (the lowest possible) - Features and deprecations must be submitted against the master branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> Commits ------- 85eafce [API] change error to violation 7be565f [API] add specs
This PR was merged into the 1.11-dev branch. Discussion ---------- | Q | A | --------------- | ----- | Branch? | 1.10 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | Sylius/Sylius#12746 | License | MIT <!-- - Bug fixes must be submitted against the 1.9 or 1.10 branch (the lowest possible) - Features and deprecations must be submitted against the master branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> Commits ------- 85eafce99fd5e2a71b92d8bb854bba966615f2ba [API] change error to violation 7be565f357d1b49f11279901b7970e134bb3127f [API] add specs
