MC-35065: Catalog pricerules are not working with custom options as expected in Magento 2.3.0 product details page

OlgaVasyltsun · OlgaVasyltsun · commit a7ee62f69f6f · 2020-10-05T12:17:32.000+03:00
diff --git a/app/code/Magento/Catalog/Block/Product/View/Options/AbstractOptions.php b/app/code/Magento/Catalog/Block/Product/View/Options/AbstractOptions.php
@@ -60,7 +60,7 @@ abstract class AbstractOptions extends \Magento\Framework\View\Element\Template
      * @param \Magento\Framework\Pricing\Helper\Data $pricingHelper
      * @param \Magento\Catalog\Helper\Data $catalogData
      * @param array $data
-     * @param CalculateCustomOptionCatalogRule $calculateCustomOptionCatalogRule
+     * @param CalculateCustomOptionCatalogRule|null $calculateCustomOptionCatalogRule
      */
     public function __construct(
         \Magento\Framework\View\Element\Template\Context $context,
@@ -174,14 +174,15 @@ protected function _formatPrice($value, $flag = true)
         $priceStr = $sign;
 
         $customOptionPrice = $this->getProduct()->getPriceInfo()->getPrice('custom_option_price');
+        $isPercent = (bool) $value['is_percent'];
 
-        if (!$value['is_percent']) {
+        if (!$isPercent) {
             $catalogPriceValue = $this->calculateCustomOptionCatalogRule->execute(
                 $this->getProduct(),
                 (float)$value['pricing_value'],
-                (bool)$value['is_percent']
+                $isPercent
             );
-            if ($catalogPriceValue!==null) {
+            if ($catalogPriceValue !== null) {
                 $value['pricing_value'] = $catalogPriceValue;
             }
         }
diff --git a/app/code/Magento/Catalog/Model/Product/Option.php b/app/code/Magento/Catalog/Model/Product/Option.php
@@ -146,7 +146,7 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter
      * @param ProductCustomOptionValuesInterfaceFactory|null $customOptionValuesFactory
      * @param array $optionGroups
      * @param array $optionTypesToGroups
-     * @param CalculateCustomOptionCatalogRule $calculateCustomOptionCatalogRule
+     * @param CalculateCustomOptionCatalogRule|null $calculateCustomOptionCatalogRule
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -474,19 +474,21 @@ public function afterSave()
      */
     public function getPrice($flag = false)
     {
-        if ($flag && $this->getPriceType() == self::$typePercent) {
+        if ($flag && $this->getPriceType() === self::$typePercent) {
             $price = $this->calculateCustomOptionCatalogRule->execute(
                 $this->getProduct(),
                 (float)$this->getData(self::KEY_PRICE),
                 $this->getPriceType() === Value::TYPE_PERCENT
             );
 
-            if ($price == null) {
+            if ($price === null) {
                 $basePrice = $this->getProduct()->getPriceInfo()->getPrice(BasePrice::PRICE_CODE)->getValue();
                 $price = $basePrice * ($this->_getData(self::KEY_PRICE) / 100);
             }
+
             return $price;
         }
+
         return $this->_getData(self::KEY_PRICE);
     }
 
diff --git a/app/code/Magento/Catalog/Model/Product/Option/Type/DefaultType.php b/app/code/Magento/Catalog/Model/Product/Option/Type/DefaultType.php
@@ -73,7 +73,7 @@ class DefaultType extends \Magento\Framework\DataObject
      * @param \Magento\Checkout\Model\Session $checkoutSession
      * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
      * @param array $data
-     * @param CalculateCustomOptionCatalogRule $calculateCustomOptionCatalogRule
+     * @param CalculateCustomOptionCatalogRule|null $calculateCustomOptionCatalogRule
      */
     public function __construct(
         \Magento\Checkout\Model\Session $checkoutSession,
@@ -357,12 +357,12 @@ public function getOptionPrice($optionValue, $basePrice)
             (float)$option->getPrice(),
             $option->getPriceType() === Value::TYPE_PERCENT
         );
-        if ($catalogPriceValue!==null) {
+        if ($catalogPriceValue !== null) {
             return $catalogPriceValue;
         } else {
             return $this->_getChargeableOptionPrice(
                 $option->getPrice(),
-                $option->getPriceType() == 'percent',
+                $option->getPriceType() === Value::TYPE_PERCENT,
                 $basePrice
             );
         }
diff --git a/app/code/Magento/Catalog/Model/Product/Option/Type/Select.php b/app/code/Magento/Catalog/Model/Product/Option/Type/Select.php
@@ -10,6 +10,7 @@
 use Magento\Catalog\Pricing\Price\CalculateCustomOptionCatalogRule;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\LocalizedException;
+use Magento\Catalog\Model\Product\Option;
 
 /**
  * Catalog product option select type
@@ -52,7 +53,7 @@ class Select extends \Magento\Catalog\Model\Product\Option\Type\DefaultType
      * @param \Magento\Framework\Escaper $escaper
      * @param array $data
      * @param array $singleSelectionTypes
-     * @param CalculateCustomOptionCatalogRule $calculateCustomOptionCatalogRule
+     * @param CalculateCustomOptionCatalogRule|null $calculateCustomOptionCatalogRule
      */
     public function __construct(
         \Magento\Checkout\Model\Session $checkoutSession,
@@ -350,24 +351,24 @@ protected function _isSingleSelection()
     /**
      * Returns calculated price of option
      *
-     * @param \Magento\Catalog\Model\Product\Option $option
-     * @param \Magento\Catalog\Model\Product\Option\Value $result
+     * @param Option $option
+     * @param Option\Value $result
      * @param float $basePrice
-     * @return float|null
+     * @return float
      */
-    protected function getCalculatedOptionValue($option, $result, $basePrice)
+    protected function getCalculatedOptionValue(Option $option, Value $result, float $basePrice) : float
     {
         $catalogPriceValue = $this->calculateCustomOptionCatalogRule->execute(
             $option->getProduct(),
             (float)$result->getPrice(),
             $result->getPriceType() === Value::TYPE_PERCENT
         );
-        if ($catalogPriceValue!==null) {
+        if ($catalogPriceValue !== null) {
             $optionCalculatedValue = $catalogPriceValue;
         } else {
             $optionCalculatedValue = $this->_getChargeableOptionPrice(
                 $result->getPrice(),
-                $result->getPriceType() == 'percent',
+                $result->getPriceType() === Value::TYPE_PERCENT,
                 $basePrice
             );
         }
diff --git a/app/code/Magento/Catalog/Pricing/Price/CalculateCustomOptionCatalogRule.php b/app/code/Magento/Catalog/Pricing/Price/CalculateCustomOptionCatalogRule.php
@@ -27,7 +27,6 @@ class CalculateCustomOptionCatalogRule
     private $priceModifier;
 
     /**
-     * CalculateCustomOptionCatalogRule constructor.
      * @param PriceCurrencyInterface $priceCurrency
      * @param PriceModifierInterface $priceModifier
      */
@@ -51,7 +50,7 @@ public function execute(
         Product $product,
         float $optionPriceValue,
         bool $isPercent
-    ) {
+    ): ?float {
         $regularPrice = (float)$product->getPriceInfo()
             ->getPrice(RegularPrice::PRICE_CODE)
             ->getValue();
@@ -69,6 +68,7 @@ public function execute(
             $finalOptionPrice = $totalCatalogRulePrice - $catalogRulePrice;
             return $this->priceCurrency->convertAndRound($finalOptionPrice);
         }
+
         return null;
     }
 
diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/ValueTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/ValueTest.php
@@ -32,14 +32,14 @@ class ValueTest extends TestCase
     /**
      * @var CalculateCustomOptionCatalogRule|MockObject
      */
-    private $CalculateCustomOptionCatalogRule;
+    private $calculateCustomOptionCatalogRule;
 
     protected function setUp(): void
     {
         $mockedResource = $this->getMockedResource();
         $mockedCollectionFactory = $this->getMockedValueCollectionFactory();
 
-        $this->CalculateCustomOptionCatalogRule = $this->createMock(
+        $this->calculateCustomOptionCatalogRule = $this->createMock(
             CalculateCustomOptionCatalogRule::class
         );
 
@@ -49,7 +49,7 @@ protected function setUp(): void
             [
                 'resource' => $mockedResource,
                 'valueCollectionFactory' => $mockedCollectionFactory,
-                'CalculateCustomOptionCatalogRule' => $this->CalculateCustomOptionCatalogRule
+                'CalculateCustomOptionCatalogRule' => $this->calculateCustomOptionCatalogRule
             ]
         );
         $this->model->setOption($this->getMockedOption());
@@ -77,7 +77,7 @@ public function testGetPrice()
         $this->assertEquals($price, $this->model->getPrice(false));
 
         $percentPrice = 100.0;
-        $this->CalculateCustomOptionCatalogRule->expects($this->atLeastOnce())
+        $this->calculateCustomOptionCatalogRule->expects($this->atLeastOnce())
             ->method('execute')
             ->willReturn($percentPrice);
         $this->assertEquals($percentPrice, $this->model->getPrice(true));
diff --git a/app/code/Magento/CatalogRule/Model/Rule.php b/app/code/Magento/CatalogRule/Model/Rule.php
@@ -898,6 +898,7 @@ public function getIdentities()
 
     /**
      * Clear price rules cache.
+     * @return void;
      */
     public function clearPriceRulesData(): void
     {
diff --git a/app/code/Magento/CatalogRule/Test/Mftf/Test/ApplyCatalogRuleForSimpleProductAndFixedMethodTest.xml b/app/code/Magento/CatalogRule/Test/Mftf/Test/ApplyCatalogRuleForSimpleProductAndFixedMethodTest.xml
@@ -7,7 +7,7 @@
 -->
 
 <tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
-    <test name="ApplyCatalogRuleForSimpleProductAndFixedMethodTest" deprecated="Use ApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest instead">
+    <test name="ApplyCatalogRuleForSimpleProductAndFixedMethodTest" deprecated="Use StorefrontApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest instead">
         <annotations>
             <features value="CatalogRule"/>
             <stories value="Apply catalog price rule"/>
@@ -18,7 +18,7 @@
             <group value="CatalogRule"/>
             <group value="mtf_migrated"/>
             <skip>
-                <issueId value="DEPRECATED">Use ApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest instead</issueId>
+                <issueId value="DEPRECATED">Use StorefrontApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest instead</issueId>
             </skip>
         </annotations>
         <before>
diff --git a/app/code/Magento/CatalogRule/Test/Mftf/Test/ApplyCatalogRuleForSimpleProductWithCustomOptionsTest.xml b/app/code/Magento/CatalogRule/Test/Mftf/Test/ApplyCatalogRuleForSimpleProductWithCustomOptionsTest.xml
@@ -7,7 +7,7 @@
 -->
 
 <tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
-    <test name="ApplyCatalogRuleForSimpleProductWithCustomOptionsTest" deprecated="Use ApplyCatalogRuleForSimpleProductsWithCustomOptionsTest instead">
+    <test name="ApplyCatalogRuleForSimpleProductWithCustomOptionsTest" deprecated="Use StorefrontApplyCatalogRuleForSimpleProductsWithCustomOptionsTest instead">
         <annotations>
             <features value="CatalogRule"/>
             <stories value="Apply catalog price rule"/>
@@ -18,7 +18,7 @@
             <group value="CatalogRule"/>
             <group value="mtf_migrated"/>
             <skip>
-                <issueId value="DEPRECATED">Use ApplyCatalogRuleForSimpleProductsWithCustomOptionsTest instead</issueId>
+                <issueId value="DEPRECATED">Use StorefrontApplyCatalogRuleForSimpleProductsWithCustomOptionsTest instead</issueId>
             </skip>
         </annotations>
         <before>
diff --git a/app/code/Magento/CatalogRule/Test/Mftf/Test/StorefrontApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest.xml b/app/code/Magento/CatalogRule/Test/Mftf/Test/StorefrontApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest.xml
@@ -7,7 +7,7 @@
 -->
 
 <tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
-    <test name="ApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest">
+    <test name="StorefrontApplyCatalogRuleForSimpleProductWithSelectFixedMethodTest">
         <annotations>
             <features value="CatalogRule"/>
             <stories value="Apply catalog price rule"/>
@@ -17,6 +17,7 @@
             <testCaseId value="MC-14771"/>
             <group value="catalogRule"/>
             <group value="mtf_migrated"/>
+            <group value="catalog"/>
         </annotations>
         <before>
             <!-- Create category -->
@@ -29,45 +30,45 @@
             </createData>
 
             <!-- Update all products to have custom options -->
-            <updateData createDataKey="createProduct1" entity="productWithFixedOptions" stepKey="updateProductWithOptions1"/>
-            <actionGroup ref="CliIndexerReindexActionGroup" stepKey="reindex">
-                <argument name="indices" value=""/>
-            </actionGroup>
+            <updateData createDataKey="createProduct1" entity="productWithFixedOptions" stepKey="updateFirstProductWithOptions"/>
 
             <!-- Login as Admin -->
             <actionGroup ref="AdminLoginActionGroup" stepKey="loginAsAdmin"/>
+
+            <!-- Clear all catalog price rules and reindex before test -->
+            <actionGroup ref="AdminCatalogPriceRuleDeleteAllActionGroup" stepKey="deleteAllCatalogRulesBeforeTest"/>
+            <actionGroup ref="CliIndexerReindexActionGroup" stepKey="reindex">
+                <argument name="indices" value=""/>
+            </actionGroup>
         </before>
         <after>
             <!-- Delete products and category -->
             <deleteData createDataKey="createProduct1" stepKey="deleteProduct1"/>
             <deleteData createDataKey="createCategory" stepKey="deleteCategory"/>
 
             <!-- Delete the catalog price rule -->
-            <actionGroup ref="AdminOpenCatalogPriceRulePageActionGroup" stepKey="goToPriceRulePage"/>
-            <actionGroup ref="deleteEntitySecondaryGrid" stepKey="deletePriceRule">
-                <argument name="name" value="{{CatalogRuleByFixed.name}}"/>
-                <argument name="searchInput" value="{{AdminSecondaryGridSection.catalogRuleIdentifierSearch}}"/>
-            </actionGroup>
-
+            <actionGroup ref="AdminCatalogPriceRuleDeleteAllActionGroup" stepKey="deleteAllCatalogRulesAfterTest"/>
             <!-- Logout -->
             <actionGroup ref="AdminLogoutActionGroup" stepKey="logoutFromAdmin"/>
         </after>
         <!-- 1. Begin creating a new catalog price rule -->
-        <actionGroup ref="NewCatalogPriceRuleByUIWithConditionIsCategoryActionGroup" stepKey="newCatalogPriceRuleByUIWithConditionIsCategory">
-            <argument name ="categoryId" value="$createCategory.id$"/>
-            <argument name ="catalogRule" value="CatalogRuleByFixed"/>
+        <actionGroup ref="AdminOpenNewCatalogPriceRuleFormPageActionGroup" stepKey="startCreatingFirstPriceRule"/>
+        <actionGroup ref="AdminCatalogPriceRuleFillMainInfoActionGroup" stepKey="fillMainInfoForFirstPriceRule">
+            <argument name="groups" value="'NOT LOGGED IN'"/>
         </actionGroup>
-
-        <!-- Select not logged in customer group -->
-        <actionGroup ref="SelectNotLoggedInCustomerGroupActionGroup" stepKey="selectNotLoggedInCustomerGroup"/>
-
-        <!-- Save and apply the new catalog price rule -->
-        <conditionalClick selector="{{AdminNewCatalogPriceRule.active}}" dependentSelector="{{AdminNewCatalogPriceRule.activeIsEnabled}}" visible="false" stepKey="enableActiveBtn"/>
-        <click selector="{{AdminNewCatalogPriceRule.save}}" stepKey="clickSave"/>
-        <waitForPageLoad stepKey="waitForSave"/>
+        <actionGroup ref="AdminFillCatalogRuleConditionActionGroup" stepKey="createCatalogPriceRule">
+            <argument name="conditionValue" value="$createCategory.id$"/>
+        </actionGroup>
+        <actionGroup ref="AdminCatalogPriceRuleFillActionsActionGroup" stepKey="fillActionsForCatalogPriceRule">
+            <argument name="apply" value="by_fixed"/>
+            <argument name="discountAmount" value="12.3"/>
+        </actionGroup>
+        <actionGroup ref="AdminCatalogPriceRuleSaveAndApplyActionGroup" stepKey="saveAndApplyCatalogPriceRule"/>
 
         <!-- Navigate to category on store front -->
-        <amOnPage url="{{StorefrontProductPage.url($createCategory.name$)}}" stepKey="goToStorefrontCategoryPage"/>
+        <actionGroup ref="StorefrontNavigateCategoryPageActionGroup" stepKey="goToStorefrontCategoryPage">
+            <argument name="category" value="$createCategory$"/>
+        </actionGroup>
 
         <!-- Check product 1 name on store front category page -->
         <actionGroup ref="AssertProductDetailsOnStorefrontActionGroup" stepKey="storefrontProduct1Name">
@@ -87,8 +88,10 @@
             <argument name="productNumber" value="1"/>
         </actionGroup>
 
-        <!-- Navigate to product 1 on store front -->
-        <amOnPage url="{{StorefrontProductPage.url($createProduct1.name$)}}" stepKey="goToProductPage1"/>
+        <!-- Navigate to product on store front -->
+        <actionGroup ref="OpenStoreFrontProductPageActionGroup" stepKey="goToProductPage1">
+            <argument name="productUrlKey" value="$createProduct1.custom_attributes[url_key]$"/>
+        </actionGroup>
 
         <!-- Assert regular and special price after selecting ProductOptionValueDropdown1 -->
         <actionGroup ref="StorefrontSelectCustomOptionRadioAndAssertPricesActionGroup" stepKey="storefrontSelectCustomOptionAndAssertPrices1">
@@ -99,8 +102,8 @@
         </actionGroup>
 
         <!-- Add product 1 to cart -->
-        <actionGroup ref="StorefrontAddProductToCartWithQtyActionGroup" stepKey="cartAddSimpleProduct1ToCart">
-            <argument name="productQty" value="1"/>
+        <actionGroup ref="AddToCartFromStorefrontProductPageActionGroup" stepKey="addToCartFromStorefrontProductPage1">
+            <argument name="productName" value="$createProduct1.name$"/>
         </actionGroup>
 
         <!-- Assert sub total on mini shopping cart -->
diff --git a/app/code/Magento/CatalogRule/Test/Mftf/Test/StorefrontApplyCatalogRuleForSimpleProductsWithCustomOptionsTest.xml b/app/code/Magento/CatalogRule/Test/Mftf/Test/StorefrontApplyCatalogRuleForSimpleProductsWithCustomOptionsTest.xml

Original file line number	Diff line number	Diff line change
`@@ -898,6 +898,7 @@ public function getIdentities()`
`898`	`898`
`899`	`899`	`/**`
`900`	`900`	`* Clear price rules cache.`
	`901`	`+ * @return void;`
`901`	`902`	`*/`
`902`	`903`	`public function clearPriceRulesData(): void`
`903`	`904`	`{`