[CatalogPromotion] Refactor scope validators to do an actual validation

Sylius · Oct 6, 2021 · b6efb9a · b6efb9a
1 parent 9efc29a
commit b6efb9a
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 71 deletions.
diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/validators.xml
@@ -66,8 +66,8 @@
 
         <service id="Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScopeValidator">
             <argument type="collection">
-                <argument>for_taxons</argument>
-                <argument>for_variants</argument>
+                <argument type="constant">Sylius\Component\Core\Model\CatalogPromotionScopeInterface::TYPE_FOR_TAXONS</argument>
+                <argument type="constant">Sylius\Component\Core\Model\CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS</argument>
             </argument>
             <argument type="tagged_iterator" tag="sylius.catalog_promotion.scope_validator" index-by="key" />
             <tag name="validator.constraint_validator" alias="sylius_catalog_promotion_scope" />
@@ -78,7 +78,6 @@
             <tag name="sylius.catalog_promotion.scope_validator" key="for_taxons"/>
         </service>
 
-
         <service id="Sylius\Bundle\CoreBundle\Validator\CatalogPromotionScope\ForVariantsScopeValidator">
             <argument type="service" id="sylius.repository.product_variant" />
             <tag name="sylius.catalog_promotion.scope_validator" key="for_variants"/>

diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForTaxonsScopeValidator.php
@@ -16,6 +16,7 @@
 use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope;
 use Sylius\Component\Taxonomy\Repository\TaxonRepositoryInterface;
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Webmozart\Assert\Assert;
 
 final class ForTaxonsScopeValidator implements ScopeValidatorInterface
@@ -27,21 +28,23 @@ public function __construct(TaxonRepositoryInterface $taxonRepository)
         $this->taxonRepository = $taxonRepository;
     }
 
-    public function validate(array $configuration, Constraint $constraint): array
+    public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void
     {
         /** @var CatalogPromotionScope $constraint */
         Assert::isInstanceOf($constraint, CatalogPromotionScope::class);
 
         if (!isset($configuration['taxons']) || empty($configuration['taxons'])) {
-            return ['configuration.taxons' => $constraint->taxonsNotEmpty];
+            $context->buildViolation($constraint->taxonsNotEmpty)->atPath('configuration.taxons')->addViolation();
+
+            return;
         }
 
         foreach ($configuration['taxons'] as $taxonCode) {
             if (null === $this->taxonRepository->findOneBy(['code' => $taxonCode])) {
-                return ['configuration.taxons' => $constraint->invalidTaxons];
+                $context->buildViolation($constraint->invalidTaxons)->atPath('configuration.taxons')->addViolation();
+
+                return;
             }
         }
-
-        return [];
     }
 }
diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ForVariantsScopeValidator.php
@@ -16,6 +16,7 @@
 use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope;
 use Sylius\Component\Core\Repository\ProductVariantRepositoryInterface;
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Webmozart\Assert\Assert;
 
 final class ForVariantsScopeValidator implements ScopeValidatorInterface
@@ -27,21 +28,23 @@ public function __construct(ProductVariantRepositoryInterface $variantRepository
         $this->variantRepository = $variantRepository;
     }
 
-    public function validate(array $configuration, Constraint $constraint): array
+    public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void
     {
         /** @var CatalogPromotionScope $constraint */
         Assert::isInstanceOf($constraint, CatalogPromotionScope::class);
 
         if (!array_key_exists('variants', $configuration) || empty($configuration['variants'])) {
-            return ['configuration.variants' => $constraint->variantsNotEmpty];
+            $context->buildViolation($constraint->variantsNotEmpty)->atPath('configuration.variants')->addViolation();
+
+            return;
         }
 
         foreach ($configuration['variants'] as $variantCode) {
             if (null === $this->variantRepository->findOneBy(['code' => $variantCode])) {
-                return ['configuration.variants' => $constraint->invalidVariants];
+                $context->buildViolation($constraint->invalidVariants)->atPath('configuration.variants')->addViolation();
+
+                return;
             }
         }
-
-        return [];
     }
 }
diff --git a/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php b/src/Sylius/Bundle/CoreBundle/Validator/CatalogPromotionScope/ScopeValidatorInterface.php
@@ -14,8 +14,9 @@
 namespace Sylius\Bundle\CoreBundle\Validator\CatalogPromotionScope;
 
 use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 
 interface ScopeValidatorInterface
 {
-    public function validate(array $configuration, Constraint $constraint): array;
+    public function validate(array $configuration, Constraint $constraint, ExecutionContextInterface $context): void;
 }
diff --git a/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php b/src/Sylius/Bundle/CoreBundle/Validator/Constraints/CatalogPromotionScopeValidator.php
@@ -43,14 +43,15 @@ public function validate($value, Constraint $constraint): void
             return;
         }
 
+        $type = $value->getType();
+        if (!key_exists($type, $this->scopeValidators)) {
+            return;
+        }
+
         $configuration = $value->getConfiguration();
 
         /** @var ScopeValidatorInterface $validator */
-        $validator = $this->scopeValidators[$value->getType()];
-        $violations = $validator->validate($configuration, $constraint);
-
-        foreach ($violations as $path => $message) {
-            $this->context->buildViolation($message)->atPath($path)->addViolation();
-        }
+        $validator = $this->scopeValidators[$type];
+        $validator->validate($configuration, $constraint, $this->context);
     }
 }
diff --git a/...us/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php b/...us/Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForTaxonsScopeValidatorSpec.php
@@ -18,6 +18,8 @@
 use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope;
 use Sylius\Component\Core\Model\TaxonInterface;
 use Sylius\Component\Taxonomy\Repository\TaxonRepositoryInterface;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
+use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface;
 
 final class ForTaxonsScopeValidatorSpec extends ObjectBehavior
 {
@@ -31,31 +33,41 @@ function it_is_a_scope_validator(): void
         $this->shouldHaveType(ScopeValidatorInterface::class);
     }
 
-    function it_prepares_array_with_violation_if_catalog_promotion_scope_does_not_have_taxons_key_configured(): void
-    {
-        $this
-            ->validate([], new CatalogPromotionScope())
-            ->shouldReturn(['configuration.taxons' => 'sylius.catalog_promotion_scope.for_taxons.not_empty'])
-        ;
+    function it_adds_violation_if_catalog_promotion_scope_does_not_have_taxons_key_configured(
+        ExecutionContextInterface $executionContext,
+        ConstraintViolationBuilderInterface $constraintViolationBuilder
+    ): void {
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.not_empty')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->atPath('configuration.taxons')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+
+        $this->validate([], new CatalogPromotionScope(), $executionContext);
     }
 
-    function it_prepares_array_with_violation_if_catalog_promotion_scope_has_not_existing_taxons_configured(
-        TaxonRepositoryInterface $taxonRepository
+    function it_adds_violation_if_catalog_promotion_scope_has_not_existing_taxons_configured(
+        TaxonRepositoryInterface $taxonRepository,
+        ExecutionContextInterface $executionContext,
+        ConstraintViolationBuilderInterface $constraintViolationBuilder
     ): void {
         $taxonRepository->findOneBy(['code' => 'not_existing_taxon'])->willReturn(null);
 
-        $this
-            ->validate(['taxons' => ['not_existing_taxon']], new CatalogPromotionScope())
-            ->shouldReturn(['configuration.taxons' => 'sylius.catalog_promotion_scope.for_taxons.invalid_taxons'])
-        ;
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.invalid_taxons')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->atPath('configuration.taxons')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+
+        $this->validate(['taxons' => ['not_existing_taxon']], new CatalogPromotionScope(), $executionContext);
     }
 
-    function it_returns_an_empty_array_if_catalog_promotion_scope_is_valid(
+    function it_does_nothing_if_catalog_promotion_scope_is_valid(
         TaxonRepositoryInterface $taxonRepository,
+        ExecutionContextInterface $executionContext,
         TaxonInterface $taxon
     ): void {
         $taxonRepository->findOneBy(['code' => 'taxon'])->willReturn($taxon);
 
-        $this->validate(['taxons' => ['taxon']], new CatalogPromotionScope())->shouldReturn([]);
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.not_empty')->shouldNotBeCalled();
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_taxons.invalid_taxons')->shouldNotBeCalled();
+
+        $this->validate(['taxons' => ['taxon']], new CatalogPromotionScope(), $executionContext);
     }
 }
diff --git a/.../Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php b/.../Bundle/CoreBundle/spec/Validator/CatalogPromotionScope/ForVariantsScopeValidatorSpec.php
@@ -18,6 +18,9 @@
 use Sylius\Bundle\CoreBundle\Validator\Constraints\CatalogPromotionScope;
 use Sylius\Component\Core\Model\ProductVariantInterface;
 use Sylius\Component\Core\Repository\ProductVariantRepositoryInterface;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
+use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface;
 
 final class ForVariantsScopeValidatorSpec extends ObjectBehavior
 {
@@ -31,41 +34,54 @@ function it_is_a_scope_validator(): void
         $this->shouldHaveType(ScopeValidatorInterface::class);
     }
 
-    function it_prepares_array_with_violation_if_catalog_promotion_scope_does_not_have_variants_key_configured(): void
-    {
-        $this
-            ->validate([], new CatalogPromotionScope())
-            ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty'])
-        ;
+    function it_adds_violation_if_catalog_promotion_scope_does_not_have_variants_key_configured(
+        ExecutionContextInterface $executionContext,
+        ConstraintViolationBuilderInterface $constraintViolationBuilder
+    ): void {
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+
+        $this->validate([], new CatalogPromotionScope(), $executionContext);
     }
 
-    function it_prepares_array_with_violation_if_catalog_promotion_scope_has_empty_variants_configured(): void
-    {
-        $this
-            ->validate(['variants' => []], new CatalogPromotionScope())
-            ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty'])
-        ;
+    function it_adds_violation_if_catalog_promotion_scope_has_empty_variants_configured(
+        ExecutionContextInterface $executionContext,
+        ConstraintViolationBuilderInterface $constraintViolationBuilder
+    ): void {
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+
+        $this->validate(['variants' => []], new CatalogPromotionScope(), $executionContext);
     }
 
-    function it_prepares_array_with_violation_if_catalog_promotion_scope_has_not_existing_variants_configured(
-        ProductVariantRepositoryInterface $variantRepository
+    function it_adds_violation_if_catalog_promotion_scope_has_not_existing_variants_configured(
+        ProductVariantRepositoryInterface $variantRepository,
+        ExecutionContextInterface $executionContext,
+        ConstraintViolationBuilderInterface $constraintViolationBuilder
     ): void {
         $variantRepository->findOneBy(['code' => 'not_existing_variant'])->willReturn(null);
 
-        $this
-            ->validate(['variants' => ['not_existing_variant']], new CatalogPromotionScope())
-            ->shouldReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.invalid_variants'])
-        ;
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.invalid_variants')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder);
+        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+
+        $this->validate(['variants' => ['not_existing_variant']], new CatalogPromotionScope(), $executionContext);
     }
 
-    function it_returns_an_empty_array_if_catalog_promotion_scope_is_valid(
+    function it_does_nothing_if_catalog_promotion_scope_is_valid(
         ProductVariantRepositoryInterface $variantRepository,
+        ExecutionContextInterface $executionContext,
         ProductVariantInterface $firstVariant,
         ProductVariantInterface $secondVariant
     ): void {
         $variantRepository->findOneBy(['code' => 'first_variant'])->willReturn($firstVariant);
         $variantRepository->findOneBy(['code' => 'second_variant'])->willReturn($secondVariant);
 
-        $this->validate(['variants' => ['first_variant', 'second_variant']], new CatalogPromotionScope())->shouldReturn([]);
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->shouldNotBeCalled();
+        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.invalid_variants')->shouldNotBeCalled();
+
+        $this->validate(['variants' => ['first_variant', 'second_variant']], new CatalogPromotionScope(), $executionContext);
     }
 }
diff --git a/...ylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php b/...ylius/Bundle/CoreBundle/spec/Validator/Constraints/CatalogPromotionScopeValidatorSpec.php
@@ -50,37 +50,29 @@ function it_is_a_constraint_validator(): void
     function it_adds_violation_if_catalog_promotion_scope_has_invalid_type(
         ExecutionContextInterface $executionContext,
         ConstraintViolationBuilderInterface $constraintViolationBuilder,
-        CatalogPromotionScopeInterface $rule
+        CatalogPromotionScopeInterface $scope
     ): void {
-        $rule->getType()->willReturn('wrong_type');
+        $scope->getType()->willReturn('wrong_type');
 
         $executionContext->buildViolation('sylius.catalog_promotion_scope.invalid_type')->willReturn($constraintViolationBuilder);
         $constraintViolationBuilder->atPath('type')->willReturn($constraintViolationBuilder);
         $constraintViolationBuilder->addViolation()->shouldBeCalled();
 
-        $this->validate($rule, new CatalogPromotionScope());
+        $this->validate($scope, new CatalogPromotionScope());
     }
 
-    function it_adds_violation_if_scope_validator_returns_one(
+    function it_calls_a_proper_validator_to_validate_the_configuration(
         ExecutionContextInterface $executionContext,
-        ConstraintViolationBuilderInterface $constraintViolationBuilder,
-        CatalogPromotionScopeInterface $rule,
+        CatalogPromotionScopeInterface $scope,
         ScopeValidatorInterface $forVariantsValidator
     ): void {
         $constraint = new CatalogPromotionScope();
 
-        $rule->getType()->willReturn(CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS);
-        $rule->getConfiguration()->willReturn([]);
-
-        $forVariantsValidator
-            ->validate([], $constraint)
-            ->willReturn(['configuration.variants' => 'sylius.catalog_promotion_scope.for_variants.not_empty'])
-        ;
+        $scope->getType()->willReturn(CatalogPromotionScopeInterface::TYPE_FOR_VARIANTS);
+        $scope->getConfiguration()->willReturn([]);
 
-        $executionContext->buildViolation('sylius.catalog_promotion_scope.for_variants.not_empty')->willReturn($constraintViolationBuilder);
-        $constraintViolationBuilder->atPath('configuration.variants')->willReturn($constraintViolationBuilder);
-        $constraintViolationBuilder->addViolation()->shouldBeCalled();
+        $forVariantsValidator->validate([], $constraint, $executionContext)->shouldBeCalled();
 
-        $this->validate($rule, $constraint);
+        $this->validate($scope, $constraint);
     }
 }