Restricted zones, fixes #624 #665
Conversation
| @@ -15,7 +15,7 @@ | |||
| use Sylius\Bundle\PromotionsBundle\Processor\PromotionProcessorInterface; | |||
| use Sylius\Bundle\PromotionsBundle\SyliusPromotionEvents; | |||
| use Symfony\Component\EventDispatcher\GenericEvent; | |||
| use Symfony\Component\HttpFoundation\Session\SessionInterface; | |||
There was a problem hiding this comment.
What's the point of replacing interface with implementation?
There was a problem hiding this comment.
@stloyd getFlashBag() is not defined in interface, used in https://github.com/umpirsky/Sylius/blob/7efea6634df8a0c3f33b456dd98f94cd2dc74452/src/Sylius/Bundle/CoreBundle/EventListener/OrderPromotionListener.php#L99.
There was a problem hiding this comment.
Then change implementation to use what is in interface & not use shortcut:
$session->getBag('flashes')->add();There was a problem hiding this comment.
You are right.
On 28 Nov 2013 20:19, "Joseph Bielawski" notifications@github.com wrote:
In src/Sylius/Bundle/CoreBundle/EventListener/OrderPromotionListener.php:
@@ -15,7 +15,7 @@
use Sylius\Bundle\PromotionsBundle\Processor\PromotionProcessorInterface;
use Sylius\Bundle\PromotionsBundle\SyliusPromotionEvents;
use Symfony\Component\EventDispatcher\GenericEvent;
-use Symfony\Component\HttpFoundation\Session\SessionInterface;Then change implementation to use what is in interface & not use shortcut:
$session->getBag('flashes')->add();
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/665/files#r7991055
.
|
@umpirsky Here is IMO better approach umpirsky#1 it uses |
|
I prefer @stloyd's approach. |
ghost
assigned 
|
Yes, it is better @stloyd, thanks. But now I get generic flash message "Error occurred while adding item to cart.", and it was more precise before with "This product is not available in your country.". |
|
We need anyway a serious improvement on error feedback from ItemResolver. It's currently way too generic (what if form has errors? We only show "error occurred" in cart summary and don't even get the user back to the product page!) |
|
@umpirsky If you see that error, not message from exception, than it looks like a bug in @winzou I tend to agree, but let's stop messing with stuff that not belongs to current PR, and just create new issue describing the problem & maybe possible solutions to avoid mud & unnecessary blocking of PR fixing totally different issue =) |
|
@stloyd Yes, I assumed same. In https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CartBundle/Controller/CartItemController.php#L60 event subject is error message, but in https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CartBundle/EventListener/FlashListener.php#L85 message is used, and not subject. I'm not sure what is the idea there, but obviously same thing should be used on both places. Is it message or subject? cc @pjedrzejewski |
|
@umpirsky @pjedrzejewski Here is an fix for Cart flash messages: #670 |
|
|
||
| foreach ($cart->getItems() as $item) { | ||
| if ($this->restrictedZoneChecker->isRestricted($product = $item->getProduct())) { | ||
| $cart->removeItem($item); |
There was a problem hiding this comment.
@stloyd Do I need to:
$this->dispatchEvent(SyliusCartEvents::ITEM_REMOVE_INITIALIZE, $event);or
$this->cartManager->persist($cart);
$this->cartManager->flush();here in order to persist cart?
Because now it's not persisted.
|
@stloyd @pjedrzejewski Good to merge? |
Restricted zones, fixes #624
|
Thank you Sasha! 👍 |
No description provided.