-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Order] Implementation of OrderItemQuantityModifier #3795
[Order] Implementation of OrderItemQuantityModifier #3795
Conversation
@@ -15,6 +15,7 @@ | |||
use Doctrine\Common\Collections\Collection; | |||
use Sylius\Component\Cart\Model\Cart; | |||
use Sylius\Component\Channel\Model\ChannelInterface as BaseChannelInterface; | |||
use Sylius\Component\Inventory\Model\InventoryUnitInterface; |
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.
Is it used?
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.
yes, it is. My bad.
a0e45d8
to
aa0949e
Compare
} else if ($targetQuantity < $itemQuantity) { | ||
$this->decreaseUnitsNumber($orderItem, $itemQuantity - $targetQuantity); | ||
} | ||
|
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.
Redundant blank line
OrderItemUnit recalculations enhancements OrderItem recalculations enhancements Added get adjustments total recursively method Conflicts: src/Sylius/Component/Order/Model/OrderItem.php src/Sylius/Component/Order/spec/Model/OrderItemSpec.php
Also deleted unnecessary methods (setTotal and setItemsTotal in Order, setQuantity in OrderItem).
46f951c
to
b8a65d0
Compare
@@ -410,6 +410,7 @@ | |||
<service id="sylius.promotion_action.add_product" class="%sylius.promotion_action.add_product.class%"> | |||
<argument type="service" id="sylius.factory.order_item" /> | |||
<argument type="service" id="sylius.repository.product_variant" /> | |||
<argument type="service" id="sylius.modifier.order_item_quantity" /> |
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 think we should name it just sylius.order_item_quantity_modifier
. Why? Because we should only group services, which really do the same thing. It is true for factory, repository, manager, controller etc. But Modifier in Sylius terms is very vague. We don't have any other modifiers and I doubt we will have anytime soon and if yes, then there is a low chance that they will be doing similar thing to this one. :) WDYT? /cc @michalmarcinkowski
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 totally agree with this argumentation.
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.
👍
[Order] Implementation of OrderItemQuantityModifier
Thank you Mateusz! 👍 We are rocking this thing! ;) |
…ifier [Order] Implementation of OrderItemQuantityModifier
…ifier [Order] Implementation of OrderItemQuantityModifier
…ifier [Order] Implementation of OrderItemQuantityModifier
…ifier [Order] Implementation of OrderItemQuantityModifier
This PR is based on #3771 and correspond with @michalmarcinkowski work in #3786.
In this PR there is
OrderItemQuantityModifier
implemented, that handles item units management. The idea is, to changeOrderItem
quantity only by adding/removing units.Possible improvements/modifications
UPDATE
I've applied commits from #3808, as they're necessary to make behat scenarios green.
UPDATE vol.2
Docs PR mentioned.