From 0881623d48294f1f1682718381639ae078835f1e Mon Sep 17 00:00:00 2001 From: TheMilek Date: Mon, 18 Sep 2023 14:22:40 +0200 Subject: [PATCH 1/3] Prevent from removing taxon that is in use by a promotion rule --- ...s_that_are_used_in_promotion_rules.feature | 2 +- .../Api/Admin/RemovingTaxonContext.php | 62 ++++++++++++++++ .../config/services/contexts/api/admin.xml | 6 ++ .../api/promotion/managing_promotions.yml | 3 + .../TaxonDeletionEventSubscriber.php | 22 +++++- .../Exception/TaxonCannotBeRemoved.php | 26 +++++++ .../Resources/config/app/config.yaml | 1 + .../config/services/event_subscribers.xml | 1 + .../TaxonDeletionEventSubscriberSpec.php | 73 ++++++++++++++++++- 9 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 src/Sylius/Behat/Context/Api/Admin/RemovingTaxonContext.php create mode 100644 src/Sylius/Bundle/ApiBundle/Exception/TaxonCannotBeRemoved.php diff --git a/features/promotion/managing_promotions/prevent_from_removing_taxons_that_are_used_in_promotion_rules.feature b/features/promotion/managing_promotions/prevent_from_removing_taxons_that_are_used_in_promotion_rules.feature index b499328dce9..59cc34df3fc 100644 --- a/features/promotion/managing_promotions/prevent_from_removing_taxons_that_are_used_in_promotion_rules.feature +++ b/features/promotion/managing_promotions/prevent_from_removing_taxons_that_are_used_in_promotion_rules.feature @@ -9,7 +9,7 @@ Feature: Preventing from removing taxons that are used in promotion rules And the store has "Mugs" taxonomy And I am logged in as an administrator - @ui @javascript + @ui @javascript @api Scenario: Being prevented from removing taxon that is in use by a promotion rule Given there is a promotion "Christmas sale" with "Total price of items from taxon" rule configured with "Mugs" taxon and $100 amount for "United States" channel When I try to delete taxon named "Mugs" diff --git a/src/Sylius/Behat/Context/Api/Admin/RemovingTaxonContext.php b/src/Sylius/Behat/Context/Api/Admin/RemovingTaxonContext.php new file mode 100644 index 00000000000..e0e1c6a9a38 --- /dev/null +++ b/src/Sylius/Behat/Context/Api/Admin/RemovingTaxonContext.php @@ -0,0 +1,62 @@ +client->delete(Resources::TAXONS, $taxon->getCode()); + $this->sharedStorage->set('taxon', $taxon); + } + + /** + * @Then /^(this taxon) should still exist$/ + */ + public function theTaxonShouldStillExist(TaxonInterface $taxon): void + { + $this->client->show(Resources::TAXONS, $taxon->getCode()); + + Assert::true($this->responseChecker->isShowSuccessful($this->client->getLastResponse())); + } + + /** + * @Then I should be notified that this taxon could not be deleted as it is in use by a promotion rule + */ + public function iShouldBeNotifiedThatThisTaxonCouldNotBeDeleted(): void + { + Assert::contains( + $this->responseChecker->getError($this->client->getLastResponse()), + 'Cannot delete a taxon that is in use by a promotion rule.', + ); + } +} diff --git a/src/Sylius/Behat/Resources/config/services/contexts/api/admin.xml b/src/Sylius/Behat/Resources/config/services/contexts/api/admin.xml index c822f537b47..4c5cf164e34 100644 --- a/src/Sylius/Behat/Resources/config/services/contexts/api/admin.xml +++ b/src/Sylius/Behat/Resources/config/services/contexts/api/admin.xml @@ -181,6 +181,12 @@ + + + + + + diff --git a/src/Sylius/Behat/Resources/config/suites/api/promotion/managing_promotions.yml b/src/Sylius/Behat/Resources/config/suites/api/promotion/managing_promotions.yml index a1ac8a94cff..939ef6c17a7 100644 --- a/src/Sylius/Behat/Resources/config/suites/api/promotion/managing_promotions.yml +++ b/src/Sylius/Behat/Resources/config/suites/api/promotion/managing_promotions.yml @@ -11,15 +11,18 @@ default: - sylius.behat.context.transform.lexical - sylius.behat.context.transform.product - sylius.behat.context.transform.promotion + - sylius.behat.context.transform.taxon - sylius.behat.context.transform.shared_storage - sylius.behat.context.setup.channel - sylius.behat.context.setup.product - sylius.behat.context.setup.promotion + - sylius.behat.context.setup.taxonomy - sylius.behat.context.setup.admin_api_security - sylius.behat.context.api.admin.managing_promotions - Sylius\Behat\Context\Api\Admin\RemovingProductContext + - Sylius\Behat\Context\Api\Admin\RemovingTaxonContext filters: tags: "@managing_promotions&&@api" diff --git a/src/Sylius/Bundle/ApiBundle/EventSubscriber/TaxonDeletionEventSubscriber.php b/src/Sylius/Bundle/ApiBundle/EventSubscriber/TaxonDeletionEventSubscriber.php index 7046756aeb3..fad283be993 100644 --- a/src/Sylius/Bundle/ApiBundle/EventSubscriber/TaxonDeletionEventSubscriber.php +++ b/src/Sylius/Bundle/ApiBundle/EventSubscriber/TaxonDeletionEventSubscriber.php @@ -15,8 +15,10 @@ use ApiPlatform\Core\EventListener\EventPriorities; use Sylius\Bundle\ApiBundle\Exception\CannotRemoveMenuTaxonException; +use Sylius\Bundle\ApiBundle\Exception\TaxonCannotBeRemoved; use Sylius\Component\Channel\Repository\ChannelRepositoryInterface; use Sylius\Component\Core\Model\TaxonInterface; +use Sylius\Component\Core\Promotion\Checker\TaxonInPromotionRuleCheckerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\ViewEvent; @@ -27,13 +29,17 @@ final class TaxonDeletionEventSubscriber implements EventSubscriberInterface { public function __construct( private ChannelRepositoryInterface $channelRepository, + private TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, ) { } public static function getSubscribedEvents(): array { return [ - KernelEvents::VIEW => ['protectFromRemovingMenuTaxon', EventPriorities::PRE_WRITE], + KernelEvents::VIEW => [ + ['protectFromRemovingMenuTaxon', EventPriorities::PRE_WRITE], + ['protectFromRemovingTaxonInUseByPromotionRule', EventPriorities::PRE_WRITE], + ], ]; } @@ -52,4 +58,18 @@ public function protectFromRemovingMenuTaxon(ViewEvent $event): void throw new CannotRemoveMenuTaxonException($taxon->getCode()); } } + + public function protectFromRemovingTaxonInUseByPromotionRule(ViewEvent $event): void + { + $taxon = $event->getControllerResult(); + $method = $event->getRequest()->getMethod(); + + if (!$taxon instanceof TaxonInterface || $method !== Request::METHOD_DELETE) { + return; + } + + if ($this->taxonInPromotionRuleChecker->isInUse($taxon)) { + throw new TaxonCannotBeRemoved('Cannot delete a taxon that is in use by a promotion rule.'); + } + } } diff --git a/src/Sylius/Bundle/ApiBundle/Exception/TaxonCannotBeRemoved.php b/src/Sylius/Bundle/ApiBundle/Exception/TaxonCannotBeRemoved.php new file mode 100644 index 00000000000..76e0ed5f91b --- /dev/null +++ b/src/Sylius/Bundle/ApiBundle/Exception/TaxonCannotBeRemoved.php @@ -0,0 +1,26 @@ + + diff --git a/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/TaxonDeletionEventSubscriberSpec.php b/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/TaxonDeletionEventSubscriberSpec.php index c21262aca51..4f0b4649e38 100644 --- a/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/TaxonDeletionEventSubscriberSpec.php +++ b/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/TaxonDeletionEventSubscriberSpec.php @@ -14,18 +14,22 @@ namespace spec\Sylius\Bundle\ApiBundle\EventSubscriber; use PhpSpec\ObjectBehavior; +use Sylius\Bundle\ApiBundle\Exception\TaxonCannotBeRemoved; use Sylius\Component\Channel\Repository\ChannelRepositoryInterface; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Core\Model\TaxonInterface; +use Sylius\Component\Core\Promotion\Checker\TaxonInPromotionRuleCheckerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\ViewEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; final class TaxonDeletionEventSubscriberSpec extends ObjectBehavior { - function let(ChannelRepositoryInterface $channelRepository): void - { - $this->beConstructedWith($channelRepository); + function let( + ChannelRepositoryInterface $channelRepository, + TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, + ): void { + $this->beConstructedWith($channelRepository, $taxonInPromotionRuleChecker); } function it_allows_to_remove_taxon_if_any_channel_has_not_it_as_a_menu_taxon( @@ -81,4 +85,67 @@ function it_throws_an_exception_if_a_subject_is_menu_taxon( )]) ; } + + function it_does_not_throw_exception_when_taxon_is_not_being_deleted( + TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, + TaxonInterface $taxon, + Request $request, + HttpKernelInterface $kernel, + ): void { + $request->getMethod()->willReturn(Request::METHOD_POST); + + $event = new ViewEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + HttpKernelInterface::MAIN_REQUEST, + $taxon->getWrappedObject(), + ); + + $taxonInPromotionRuleChecker->isInUse($taxon)->shouldNotBeCalled(); + + $this->shouldNotThrow()->during('protectFromRemovingTaxonInUseByPromotionRule', [$event]); + } + + function it_does_not_throw_exception_when_taxon_is_not_in_use_by_a_promotion_rule( + TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, + TaxonInterface $taxon, + Request $request, + HttpKernelInterface $kernel, + ): void { + $request->getMethod()->willReturn(Request::METHOD_POST); + + $event = new ViewEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + HttpKernelInterface::MAIN_REQUEST, + $taxon->getWrappedObject(), + ); + + $taxonInPromotionRuleChecker->isInUse($taxon)->willReturn(false); + + $this->shouldNotThrow()->during('protectFromRemovingTaxonInUseByPromotionRule', [$event]); + } + + function it_throws_an_exception_when_trying_to_delete_taxon_that_is_in_use_by_a_promotion_rule( + TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, + TaxonInterface $taxon, + Request $request, + HttpKernelInterface $kernel, + ): void { + $request->getMethod()->willReturn(Request::METHOD_DELETE); + + $event = new ViewEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + HttpKernelInterface::MAIN_REQUEST, + $taxon->getWrappedObject(), + ); + + $taxonInPromotionRuleChecker->isInUse($taxon)->willReturn(true); + + $this + ->shouldThrow(TaxonCannotBeRemoved::class) + ->during('protectFromRemovingTaxonInUseByPromotionRule', [$event]) + ; + } } From 14081008e0352d4d6d67c3f3402210833dc8ede4 Mon Sep 17 00:00:00 2001 From: TheMilek Date: Mon, 18 Sep 2023 14:25:42 +0200 Subject: [PATCH 2/3] [Upgrade] Update upgrade file --- UPGRADE-API-1.13.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/UPGRADE-API-1.13.md b/UPGRADE-API-1.13.md index dca721b7e76..55d2243889c 100644 --- a/UPGRADE-API-1.13.md +++ b/UPGRADE-API-1.13.md @@ -1,5 +1,15 @@ # UPGRADE FROM `v1.12.x` TO `v1.13.0` +1. The constructor of 'Sylius\Bundle\ApiBundle\EventSubscriber\TaxonDeletionEventSubscriber' has changed: + +````diff + public function __construct( + private ChannelRepositoryInterface $channelRepository, ++ private TaxonInPromotionRuleCheckerInterface $taxonInPromotionRuleChecker, + ) { + } +```` + 1. The signature of constructor of 'Sylius\Bundle\ApiBundle\Command\Cart\ChangeItemQuantityInCart' command changed: ````diff From da9a512667a6c924eacc76fddb282e1fbdaa0c6f Mon Sep 17 00:00:00 2001 From: TheMilek Date: Tue, 19 Sep 2023 08:28:05 +0200 Subject: [PATCH 3/3] Minor improvements --- .../ProductDeletionEventSubscriberSpec.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/ProductDeletionEventSubscriberSpec.php b/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/ProductDeletionEventSubscriberSpec.php index 6df20358c88..3a7a4792f9a 100644 --- a/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/ProductDeletionEventSubscriberSpec.php +++ b/src/Sylius/Bundle/ApiBundle/spec/EventSubscriber/ProductDeletionEventSubscriberSpec.php @@ -43,12 +43,32 @@ function it_does_not_throw_exception_when_product_is_not_being_deleted( $product->getWrappedObject(), ); + $productInPromotionRuleChecker->isInUse($product)->shouldNotBeCalled(); + + $this->shouldNotThrow()->during('protectFromRemovingProductInUseByPromotionRule', [$event]); + } + + function it_does_not_throw_exception_when_product_is_not_in_use_by_a_promotion_rule( + ProductInPromotionRuleCheckerInterface $productInPromotionRuleChecker, + ProductInterface $product, + Request $request, + HttpKernelInterface $kernel, + ): void { + $request->getMethod()->willReturn(Request::METHOD_POST); + + $event = new ViewEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + HttpKernelInterface::MAIN_REQUEST, + $product->getWrappedObject(), + ); + $productInPromotionRuleChecker->isInUse($product)->willReturn(false); $this->shouldNotThrow()->during('protectFromRemovingProductInUseByPromotionRule', [$event]); } - function it_throws_an_exception_when_trying_to_delete_product_assigned_to_promotion_rule( + function it_throws_an_exception_when_trying_to_delete_product_that_is_in_use_by_a_promotion_rule( ProductInPromotionRuleCheckerInterface $productInPromotionRuleChecker, ProductInterface $product, Request $request,