Refresh cart right after cart_change event (fix #716) #791
Conversation
|
@pjedrzejewski ping, I need it to move forward on other promotion topics (revert, freeProduct). |
| */ | ||
| class RefreshCartListener | ||
| { | ||
| public function onCartChange(GenericEvent $event) |
There was a problem hiding this comment.
Because it's a GenericEvent that is dispatched on the cart_change event.
There was a problem hiding this comment.
Then maybe it should be changed to be consistent =)
There was a problem hiding this comment.
Actually you're right, it could be solved in a better way. I was thinking of doing this in CartEvent:
public function __construct(CartInterface $cart)
{
$cart->calculateTotal();
$this->cart = $cart;
}
So that no need to have another listener, and we ensure that the cart is fresh for all other listeners.
There was a problem hiding this comment.
Just add some tests to prevent BC break.
There was a problem hiding this comment.
@stloyd do you know what's the aim of �the isFresh method of CartEvent? https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CartBundle/Event/CartEvent.php#L63
If I refresh the cart directly in the constructor, we should assume that every listener has a fresh cart in its hands. And if it changes something, it's its responsibility to refresh the cart. What do you think?
There was a problem hiding this comment.
@winzou TBH. I don't remember what was the point of using non-fresh cart, I guess it was just because of move the code from contollers into listeners.
@pjedrzejewski Do you remember that case?
There was a problem hiding this comment.
Not really... I'd love to simplify this, especially that when we dispatch cart change event, it always means that something has changed and everything should be recalculated.
There was a problem hiding this comment.
Let's address this in another PR.
Refresh cart right after cart_change event (fix #716)
|
Thanks Alexandre! I'd like to make little changes to the cart listeners and events, but BC should be kept so do not worry. |
|
So, @pjedrzejewski you're on it? |
|
Hi guys, I am facing this issue. I am using sylius 1.6 and trying to update same product item in the cart but it's not working. can anyone please help me to sort this? |
Refresh the cart with high priority after CART_CHANGE event, so that other listeners are dealing with an updated cart. Fix #716.