Skip to content

Commit 9b23170

Browse files
aplapanaaplapana
committed
ACP2E-3498: Incorrect discount value when multiple cart price rules are applied simultaneously with discounted/special priced products
1 parent 99908ad commit 9b23170

File tree

4 files changed

+217
-14
lines changed

4 files changed

+217
-14
lines changed

‎app/code/Magento/SalesRule/Model/Rule/Action/Discount/CartFixed.php

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,33 +30,41 @@ class CartFixed extends AbstractDiscount
3030
/**
3131
* @var DeltaPriceRound
3232
*/
33-
private $deltaPriceRound;
33+
private DeltaPriceRound $deltaPriceRound;
3434

3535
/**
3636
* @var CartFixedDiscount
3737
*/
38-
private $cartFixedDiscountHelper;
38+
private CartFixedDiscount $cartFixedDiscountHelper;
3939

4040
/**
4141
* @var string
4242
*/
4343
private static $discountType = 'CartFixed';
4444

45+
/**
46+
* @var ExistingDiscountRuleCollector
47+
*/
48+
private ExistingDiscountRuleCollector $existingDiscountRuleCollector;
49+
4550
/**
4651
* @param Validator $validator
4752
* @param DataFactory $discountDataFactory
4853
* @param PriceCurrencyInterface $priceCurrency
4954
* @param DeltaPriceRound $deltaPriceRound
55+
* @param ExistingDiscountRuleCollector $existingDiscountRuleCollector
5056
* @param CartFixedDiscount|null $cartFixedDiscount
5157
*/
5258
public function __construct(
5359
Validator $validator,
5460
DataFactory $discountDataFactory,
5561
PriceCurrencyInterface $priceCurrency,
5662
DeltaPriceRound $deltaPriceRound,
63+
ExistingDiscountRuleCollector $existingDiscountRuleCollector,
5764
?CartFixedDiscount $cartFixedDiscount = null
5865
) {
5966
$this->deltaPriceRound = $deltaPriceRound;
67+
$this->existingDiscountRuleCollector = $existingDiscountRuleCollector;
6068
$this->cartFixedDiscountHelper = $cartFixedDiscount ?:
6169
ObjectManager::getInstance()->get(CartFixedDiscount::class);
6270
parent::__construct($validator, $discountDataFactory, $priceCurrency);
@@ -76,18 +84,7 @@ public function __construct(
7684
*/
7785
public function calculate($rule, $item, $qty)
7886
{
79-
/** @var Data $discountData */
80-
$discountData = $this->discountFactory->create();
81-
8287
$ruleTotals = $this->validator->getRuleItemTotalsInfo($rule->getId());
83-
if ($rule->getExistingDiscounts() === null) {
84-
$existingRuleDiscount = 0;
85-
/** @var Item $ruleItem */
86-
foreach ($ruleTotals['affected_items'] as $ruleItem) {
87-
$existingRuleDiscount += $ruleItem->getBaseDiscountAmount();
88-
}
89-
$rule->setExistingDiscounts($existingRuleDiscount);
90-
}
9188
$baseRuleTotals = $ruleTotals['base_items_price'] ?? 0.0;
9289
$ruleItemsCount = $ruleTotals['items_count'] ?? 0;
9390

@@ -111,6 +108,8 @@ public function calculate($rule, $item, $qty)
111108
$availableDiscountAmount = (float) $cartRules[$rule->getId()];
112109
$discountType = self::$discountType . $rule->getId();
113110

111+
/** @var Data $discountData */
112+
$discountData = $this->discountFactory->create();
114113
if ($availableDiscountAmount > 0) {
115114
$store = $quote->getStore();
116115
$shippingPrice = $this->cartFixedDiscountHelper->applyDiscountOnPricesIncludedTax()
@@ -142,7 +141,8 @@ public function calculate($rule, $item, $qty)
142141
$qty,
143142
$baseItemPrice,
144143
$baseItemDiscountAmount,
145-
$baseRuleTotals - $rule->getExistingDiscounts(),
144+
$baseRuleTotals -
145+
$this->getItemsTotalDiscount($rule->getId(), $ruleTotals['affected_items']),
146146
$discountType
147147
);
148148

@@ -200,6 +200,25 @@ public function calculate($rule, $item, $qty)
200200
return $discountData;
201201
}
202202

203+
/**
204+
* @param int $ruleId
205+
* @param array $affectedItems
206+
* @return float
207+
*/
208+
private function getItemsTotalDiscount(int $ruleId, array $affectedItems): float
209+
{
210+
if ($this->existingDiscountRuleCollector->getExistingRuleDiscount($ruleId) === null) {
211+
$existingRuleDiscount = 0;
212+
/** @var Item $ruleItem */
213+
foreach ($affectedItems as $ruleItem) {
214+
$existingRuleDiscount += $ruleItem->getBaseDiscountAmount();
215+
}
216+
$this->existingDiscountRuleCollector->setExistingRuleDiscount($ruleId, $existingRuleDiscount);
217+
}
218+
219+
return $this->existingDiscountRuleCollector->getExistingRuleDiscount($ruleId);
220+
}
221+
203222
/**
204223
* Set information about usage cart fixed rule by quote address
205224
*

‎app/code/Magento/SalesRule/Model/Rule/Action/Discount/ExistingDiscountRuleCollector.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
/**
3+
* Copyright 2024 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\SalesRule\Model\Rule\Action\Discount;
9+
10+
class ExistingDiscountRuleCollector
11+
{
12+
/**
13+
* @var array
14+
*/
15+
private array $ruleDiscounts = [];
16+
17+
/**
18+
* @param int $ruleId
19+
* @param float $discountAmount
20+
* @return void
21+
*/
22+
public function setExistingRuleDiscount(int $ruleId, float $discountAmount): void
23+
{
24+
$this->ruleDiscounts[$ruleId] = $discountAmount;
25+
}
26+
27+
/**
28+
* @param int $ruleId
29+
* @return float|null
30+
*/
31+
public function getExistingRuleDiscount(int $ruleId): ?float
32+
{
33+
return $this->ruleDiscounts[$ruleId] ?? null;
34+
}
35+
}

‎app/code/Magento/SalesRule/etc/di.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,4 +211,5 @@
211211
<argument name="scopeConfig" xsi:type="object">Magento\Framework\App\Config\ScopeConfigInterface\Proxy</argument>
212212
</arguments>
213213
</type>
214+
<type name="Magento\SalesRule\Model\Rule\Action\Discount\ExistingDiscountRuleCollector" shared="true" />
214215
</config>

‎dev/tests/integration/testsuite/Magento/SalesRule/Model/Rule/Action/Discount/CartFixedTest.php

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
use Magento\SalesRule\Api\RuleRepositoryInterface;
4444
use Magento\SalesRule\Model\Rule;
4545
use Magento\SalesRule\Model\RuleFactory;
46+
use Magento\SalesRule\Test\Fixture\ProductCondition as ProductConditionFixture;
47+
use Magento\SalesRule\Test\Fixture\ProductFoundInCartConditions as ProductFoundInCartConditionsFixture;
48+
use Magento\SalesRule\Test\Fixture\ProductSubselectionInCartConditions as ProductSubselectionInCartConditionsFixture;
4649
use Magento\SalesRule\Test\Fixture\Rule as RuleFixture;
4750
use Magento\Store\Model\StoreManagerInterface;
4851
use Magento\TestFramework\Fixture\DataFixture;
@@ -601,6 +604,151 @@ public function testCarFixedDiscountWithApplyToShippingAmountAfterADiscount(): v
601604
$this->assertEquals(-70, $totals->getDiscountAmount());
602605
}
603606

607+
#[
608+
DataFixture(ProductFixture::class, ['price' => 10], 'p1'),
609+
DataFixture(ProductFixture::class, ['price' => 10], 'p2'),
610+
DataFixture(
611+
ProductConditionFixture::class,
612+
['attribute' => 'sku', 'value' => '$p1.sku$'],
613+
'cond1'
614+
),
615+
DataFixture(
616+
ProductFoundInCartConditionsFixture::class,
617+
['conditions' => ['$cond1$']],
618+
'cond11'
619+
),
620+
DataFixture(
621+
ProductConditionFixture::class,
622+
['attribute' => 'sku', 'value' => '$p1.sku$'],
623+
'applyCond1'
624+
),
625+
DataFixture(
626+
RuleFixture::class,
627+
[
628+
'simple_action' => Rule::BY_PERCENT_ACTION,
629+
'discount_amount' => 50,
630+
'apply_to_shipping' => 0,
631+
'stop_rules_processing' => 0,
632+
'sort_order' => 1,
633+
'conditions' => ['$cond11$'],
634+
'actions' => ['$applyCond1$']
635+
]
636+
),
637+
DataFixture(
638+
ProductConditionFixture::class,
639+
['attribute' => 'sku', 'value' => '$p2.sku$'],
640+
'cond2'
641+
),
642+
DataFixture(
643+
ProductFoundInCartConditionsFixture::class,
644+
['conditions' => ['$cond2$']],
645+
'cond22'
646+
),
647+
DataFixture(
648+
ProductConditionFixture::class,
649+
['attribute' => 'sku', 'value' => '$p2.sku$'],
650+
'applyCond2'
651+
),
652+
DataFixture(
653+
RuleFixture::class,
654+
[
655+
'simple_action' => Rule::CART_FIXED_ACTION,
656+
'discount_amount' => 12,
657+
'apply_to_shipping' => 0,
658+
'stop_rules_processing' => 0,
659+
'sort_order' => 2,
660+
'conditions' => ['$cond22$']
661+
]
662+
),
663+
DataFixture(GuestCartFixture::class, as: 'cart'),
664+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$p1.id$', 'qty' => 1]),
665+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$p2.id$', 'qty' => 1])
666+
]
667+
public function testFixedAmountToWholeCart2productsAfterPercent1Product(): void
668+
{
669+
$cart = DataFixtureStorageManager::getStorage()->get('cart');
670+
$totals = $this->getTotals((int) $cart->getId());
671+
$this->assertEquals(-17, $totals->getDiscountAmount());
672+
}
673+
674+
#[
675+
DataFixture(ProductFixture::class, ['price' => 850, 'special_price' => 765], 'p1'),
676+
DataFixture(ProductFixture::class, ['price' => 85, 'special_price' => 68], 'p2'),
677+
DataFixture(
678+
ProductConditionFixture::class,
679+
['attribute' => 'sku', 'value' => '$p1.sku$'],
680+
'cond1'
681+
),
682+
DataFixture(
683+
ProductSubselectionInCartConditionsFixture::class,
684+
['attribute' => 'qty', 'operator' => '==', 'value' => 2, 'conditions' => ['$cond1$']],
685+
'cond11'
686+
),
687+
DataFixture(
688+
ProductFoundInCartConditionsFixture::class,
689+
['conditions' => ['$cond11$']],
690+
'cond111'
691+
),
692+
DataFixture(
693+
ProductConditionFixture::class,
694+
['attribute' => 'sku', 'value' => '$p1.sku$'],
695+
'applyCond1'
696+
),
697+
DataFixture(
698+
RuleFixture::class,
699+
[
700+
'simple_action' => Rule::CART_FIXED_ACTION,
701+
'discount_amount' => 153,
702+
'apply_to_shipping' => 0,
703+
'stop_rules_processing' => 0,
704+
'sort_order' => 1,
705+
'conditions' => ['$cond111$'],
706+
'actions' => ['$applyCond1$']
707+
]
708+
),
709+
DataFixture(
710+
ProductConditionFixture::class,
711+
['attribute' => 'sku', 'value' => '$p2.sku$'],
712+
'cond2'
713+
),
714+
DataFixture(
715+
ProductSubselectionInCartConditionsFixture::class,
716+
['attribute' => 'qty', 'operator' => '==', 'value' => 2, 'conditions' => ['$cond2$']],
717+
'cond22'
718+
),
719+
DataFixture(
720+
ProductFoundInCartConditionsFixture::class,
721+
['conditions' => ['$cond22$']],
722+
'cond222'
723+
),
724+
DataFixture(
725+
ProductConditionFixture::class,
726+
['attribute' => 'sku', 'value' => '$p2.sku$'],
727+
'applyCond2'
728+
),
729+
DataFixture(
730+
RuleFixture::class,
731+
[
732+
'simple_action' => Rule::CART_FIXED_ACTION,
733+
'discount_amount' => 14,
734+
'apply_to_shipping' => 0,
735+
'stop_rules_processing' => 0,
736+
'sort_order' => 2,
737+
'conditions' => ['$cond222$'],
738+
'actions' => ['$applyCond2$']
739+
]
740+
),
741+
DataFixture(GuestCartFixture::class, as: 'cart'),
742+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$p1.id$', 'qty' => 2]),
743+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart.id$', 'product_id' => '$p2.id$', 'qty' => 2])
744+
]
745+
public function testFixedAmountToWholeCart1ProductPerRule2ProductsWithSpecialPriceTotal(): void
746+
{
747+
$cart = DataFixtureStorageManager::getStorage()->get('cart');
748+
$totals = $this->getTotals((int) $cart->getId());
749+
$this->assertEquals(-167, $totals->getDiscountAmount());
750+
}
751+
604752
/**
605753
* Get list of orders by quote id.
606754
*

0 commit comments

Comments
 (0)