Merge pull request #741 from coreshop/carrier-shipping-rules-no-rule

[Shipping] shipping is valid when no shipping rule is configured
coreshop · Dec 10, 2018 · a1f7dad · a1f7dad
2 parents 6b6d5a4 + 51d2d64
commit a1f7dad
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 6 deletions.
diff --git a/features/shipping/carrier_with_no_shipping_rules_valid.feature b/features/shipping/carrier_with_no_shipping_rules_valid.feature
@@ -0,0 +1,27 @@
+@shipping @shipping_carrier_with_no_rules
+Feature: Add a new carrier without rules should be valid
+
+  Background:
+    Given the site operates on a store in "Austria"
+    And the site has a currency "Euro" with iso "EUR"
+    And I am in country "Austria"
+    And the site has a tax rate "AT" with "20%" rate
+    And the site has a tax rule group "AT"
+    And the tax rule group has a tax rule for country "Austria" with tax rate "AT"
+    And the site has a product "Shoe" priced at 10000
+    And the product has the tax rule group "AT"
+    And I add the product "Shoe" to my cart
+    And the site has a carrier "Post"
+
+  Scenario: Carrier with not shipping rule should be valid
+    Given I add the product "Shoe" to my cart
+    And the carrier "Post" should be valid for my cart
+
+  Scenario: Carrier with one shipping rule should be valid
+    Given adding a shipping rule named "product"
+    And the shipping rule is active
+    And the shipping rule has a condition products with product "Shoe"
+    And the shipping rule belongs to carrier "Post"
+    And I add the product "Shoe" to my cart
+    And the carrier "Post" should be valid for my cart
+
diff --git a/src/CoreShop/Behat/Context/Domain/ShippingContext.php b/src/CoreShop/Behat/Context/Domain/ShippingContext.php
@@ -20,6 +20,8 @@
 use CoreShop\Component\Resource\Factory\FactoryInterface;
 use CoreShop\Component\Rule\Condition\RuleValidationProcessorInterface;
 use CoreShop\Component\Shipping\Calculator\CarrierPriceCalculatorInterface;
+use CoreShop\Component\Shipping\Checker\CarrierShippingRuleCheckerInterface;
+use CoreShop\Component\Shipping\Model\ShippingRuleGroupInterface;
 use CoreShop\Component\Shipping\Model\ShippingRuleInterface;
 use Webmozart\Assert\Assert;
 
@@ -51,24 +53,32 @@ final class ShippingContext implements Context
     private $carrierPriceCalculator;
 
     /**
-     * @param SharedStorageInterface           $sharedStorage
-     * @param CarrierRepositoryInterface       $carrierRepository
-     * @param RuleValidationProcessorInterface $ruleValidationProcessor
-     * @param FactoryInterface                 $addressFactory
-     * @param CarrierPriceCalculatorInterface  $carrierPriceCalculator
+     * @var CarrierShippingRuleCheckerInterface
+     */
+    private $carrierShippingRuleChecker;
+
+    /**
+     * @param SharedStorageInterface              $sharedStorage
+     * @param CarrierRepositoryInterface          $carrierRepository
+     * @param RuleValidationProcessorInterface    $ruleValidationProcessor
+     * @param FactoryInterface                    $addressFactory
+     * @param CarrierPriceCalculatorInterface     $carrierPriceCalculator
+     * @param CarrierShippingRuleCheckerInterface $carrierShippingRuleChecker
      */
     public function __construct(
         SharedStorageInterface $sharedStorage,
         CarrierRepositoryInterface $carrierRepository,
         RuleValidationProcessorInterface $ruleValidationProcessor,
         FactoryInterface $addressFactory,
-        CarrierPriceCalculatorInterface $carrierPriceCalculator
+        CarrierPriceCalculatorInterface $carrierPriceCalculator,
+        CarrierShippingRuleCheckerInterface $carrierShippingRuleChecker
     ) {
         $this->sharedStorage = $sharedStorage;
         $this->carrierRepository = $carrierRepository;
         $this->ruleValidationProcessor = $ruleValidationProcessor;
         $this->addressFactory = $addressFactory;
         $this->carrierPriceCalculator = $carrierPriceCalculator;
+        $this->carrierShippingRuleChecker = $carrierShippingRuleChecker;
     }
 
     /**
@@ -122,4 +132,23 @@ public function shippingShouldBePriced(CartInterface $cart, CarrierInterface $ca
 
         Assert::same(intval($price), $this->carrierPriceCalculator->getPrice($carrier, $cart, $address));
     }
+
+    /**
+     * @Then /^the (carrier "[^"]+") should be valid for (my cart)$/
+     */
+    public function carrierShouldBeValidForMyCart(CarrierInterface $carrier, CartInterface $cart)
+    {
+        $address = $cart->getShippingAddress() ?: $this->addressFactory->createNew();
+
+        $ruleResult = $this->carrierShippingRuleChecker->isShippingRuleValid($carrier, $cart, $address);
+
+        if ($ruleResult instanceof ShippingRuleGroupInterface) {
+            $ruleResult = true;
+        }
+
+        Assert::true(
+            $ruleResult,
+            sprintf('Asserted that the Carrier %s is valid for my cart, but it is not', $carrier->getTitle('en'))
+        );
+    }
 }
diff --git a/src/CoreShop/Behat/Resources/config/services/contexts/domain.yml b/src/CoreShop/Behat/Resources/config/services/contexts/domain.yml
@@ -98,6 +98,7 @@ services:
             - '@__symfony__.coreshop.shipping_rule.processor'
             - '@__symfony__.coreshop.factory.address'
             - '@__symfony__.coreshop.carrier.price_calculator'
+            - '@__symfony__.coreshop.carrier.shipping_rule.checker'
         tags:
             - { name: fob.context_service }
 

diff --git a/src/CoreShop/Component/Shipping/Checker/CarrierShippingRuleChecker.php b/src/CoreShop/Component/Shipping/Checker/CarrierShippingRuleChecker.php
@@ -39,6 +39,10 @@ public function isShippingRuleValid(CarrierInterface $carrier, ShippableInterfac
     {
         $shippingRules = $carrier->getShippingRules();
 
+        if (count($shippingRules) === 0) {
+            return true;
+        }
+
         foreach ($shippingRules as $rule) {
             $isValid = $this->ruleValidationProcessor->isValid($carrier, $rule->getShippingRule(), [
                 $carrier,