Skip to content

Commit 97c0418

Browse files
bubasumabubasuma
committed
ACP2E-3973: [Cloud] Free Shipping discount not correctly removed when cart no longer meets requirements
1 parent 76e2a92 commit 97c0418

File tree

2 files changed

+166
-0
lines changed

2 files changed

+166
-0
lines changed

‎app/code/Magento/SalesRule/Model/Quote/Discount.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ public function collect(
192192
/** @var Rule $rule */
193193
foreach ($rules as $rule) {
194194
$ruleTotalDiscount = 0;
195+
$ruleDiscountAmount = 0;
195196
/** @var Item $item */
196197
foreach ($itemsToApplyRules as $key => $item) {
197198
if ($item->getNoDiscount() || !$this->calculator->canApplyDiscount($item) || $item->getParentItem()) {
@@ -224,11 +225,15 @@ public function collect(
224225
if ($item->getChildren() && $item->isChildrenCalculated()) {
225226
foreach ($item->getChildren() as $child) {
226227
$ruleTotalDiscount += $child->getBaseDiscountAmount();
228+
$ruleDiscountAmount += $child->getDiscountAmount();
227229
}
228230
}
229231
$ruleTotalDiscount += $item->getBaseDiscountAmount();
232+
$ruleDiscountAmount += $item->getDiscountAmount();
230233
}
231234
$address->setBaseDiscountAmount($ruleTotalDiscount);
235+
$address->setBaseSubtotalWithDiscount($address->getBaseSubtotal() - $ruleTotalDiscount);
236+
$address->setSubtotalWithDiscount($address->getSubtotal() - $ruleDiscountAmount);
232237
}
233238
$this->calculator->initTotals($items, $address);
234239
foreach ($items as $item) {

‎dev/tests/integration/testsuite/Magento/SalesRule/Model/Quote/DiscountTest.php

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use Magento\Bundle\Test\Fixture\Product as BundleProductFixture;
1515
use Magento\Catalog\Test\Fixture\Category as CategoryFixture;
1616
use Magento\Catalog\Test\Fixture\Product as ProductFixture;
17+
use Magento\Checkout\Test\Fixture\SetDeliveryMethod;
18+
use Magento\Checkout\Test\Fixture\SetShippingAddress;
1719
use Magento\ConfigurableProduct\Test\Fixture\AddProductToCart as AddConfigurableProductToCartFixture;
1820
use Magento\ConfigurableProduct\Test\Fixture\Attribute as AttributeFixture;
1921
use Magento\ConfigurableProduct\Test\Fixture\Product as ConfigurableProductFixture;
@@ -32,6 +34,7 @@
3234
use Magento\SalesRule\Model\Rule;
3335
use Magento\SalesRule\Model\Rule\Condition\Combine as CombineCondition;
3436
use Magento\SalesRule\Model\Rule\Condition\Product as ProductCondition;
37+
use Magento\SalesRule\Test\Fixture\AddressCondition;
3538
use Magento\SalesRule\Test\Fixture\ProductCondition as ProductConditionFixture;
3639
use Magento\SalesRule\Test\Fixture\Rule as RuleFixture;
3740
use Magento\TestFramework\Fixture\AppIsolation;
@@ -862,4 +865,162 @@ public function testDiscountOnSimpleProductWhenBuyXGetYRuleHasDiscountQtyStepSpe
862865
$this->assertEquals(-123, $this->total->getDiscountAmount());
863866
$this->assertEqualsCanonicalizing([$rule1Id], explode(',', $quote1->getAppliedRuleIds()));
864867
}
868+
869+
#[
870+
DataFixture(ProductFixture::class, ['price' => 100], 'p1'),
871+
DataFixture(
872+
AddressCondition::class,
873+
['attribute' => 'base_subtotal_with_discount', 'operator' => '>', 'value' => 75],
874+
'cond1'
875+
),
876+
DataFixture(
877+
RuleFixture::class,
878+
[
879+
'coupon_code' => '%uniqid%',
880+
'stop_rules_processing'=> 0,
881+
'discount_amount' => 30,
882+
'simple_action' => Rule::BY_PERCENT_ACTION,
883+
'sort_order' => 1
884+
],
885+
'rule1'
886+
),
887+
DataFixture(
888+
RuleFixture::class,
889+
[
890+
'stop_rules_processing'=> 0,
891+
'discount_amount' => 10,
892+
'simple_action' => Rule::BY_FIXED_ACTION,
893+
'sort_order' => 2,
894+
'apply_to_shipping' => 1,
895+
'conditions' => ['$cond1$'],
896+
],
897+
'rule2'
898+
),
899+
DataFixture(GuestCartFixture::class, as: 'cart1'),
900+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart1.id$', 'product_id' => '$p1.id$']),
901+
DataFixture(SetShippingAddress::class, ['cart_id' => '$cart1.id$']),
902+
DataFixture(SetDeliveryMethod::class, ['cart_id' => '$cart1.id$']),
903+
]
904+
public function testSubtotalWithDiscountShouldReflectPreviouslyAppliedRules(): void
905+
{
906+
$rule1Id = (int)$this->fixtures->get('rule1')->getId();
907+
$rule2Id = (int)$this->fixtures->get('rule2')->getId();
908+
$coupon = $this->fixtures->get('rule1')->getCouponCode();
909+
$cart = $this->quoteRepository->get((int)$this->fixtures->get('cart1')->getId());
910+
911+
// Check that rule2 is applied before the coupon code is applied
912+
// Note: the subtotal with discount includes the shipping discount
913+
$this->assertEquals(85, $cart->getSubtotalWithDiscount());
914+
$this->assertEquals(85, $cart->getBaseSubtotalWithDiscount());
915+
// Bypass getter methods for subtotal_with_discount and base_subtotal_with_discount to retrieve stored values
916+
$this->assertEquals(85, $cart->getShippingAddress()->getData('subtotal_with_discount'));
917+
$this->assertEquals(85, $cart->getShippingAddress()->getData('base_subtotal_with_discount'));
918+
$this->assertEquals(-15, $cart->getShippingAddress()->getBaseDiscountAmount());
919+
$this->assertEquals(-15, $cart->getShippingAddress()->getDiscountAmount());
920+
$this->assertEquals(5, $cart->getShippingAddress()->getShippingDiscountAmount());
921+
$this->assertEquals(5, $cart->getShippingAddress()->getBaseShippingDiscountAmount());
922+
$this->assertEquals([$rule2Id], explode(',', $cart->getAppliedRuleIds()));
923+
$this->assertEquals([$rule2Id], explode(',', current($cart->getItems())->getAppliedRuleIds()));
924+
925+
// Now apply the coupon code to ensure that the subtotal with discount is recalculated correctly
926+
$cart->setCouponCode($coupon);
927+
// This is needed because applied_rule_ids is not properly reset in the code for subsequent recollect.
928+
$cart->setAppliedRuleIds('');
929+
$cart->collectTotals();
930+
931+
// Check that rule1 is applied after the coupon code is applied and rule2 is no longer applied
932+
$this->assertEquals(70, $cart->getSubtotalWithDiscount());
933+
$this->assertEquals(70, $cart->getBaseSubtotalWithDiscount());
934+
$this->assertEquals(70, $cart->getShippingAddress()->getData('subtotal_with_discount'));
935+
$this->assertEquals(70, $cart->getShippingAddress()->getData('base_subtotal_with_discount'));
936+
$this->assertEquals(-30, $cart->getShippingAddress()->getBaseDiscountAmount());
937+
$this->assertEquals(-30, $cart->getShippingAddress()->getDiscountAmount());
938+
$this->assertEquals(0, $cart->getShippingAddress()->getShippingDiscountAmount());
939+
$this->assertEquals(0, $cart->getShippingAddress()->getBaseShippingDiscountAmount());
940+
$this->assertEquals([$rule1Id], explode(',', $cart->getAppliedRuleIds()));
941+
$this->assertEquals([$rule1Id], explode(',', current($cart->getItems())->getAppliedRuleIds()));
942+
}
943+
944+
#[
945+
DataFixture(ProductFixture::class, ['price' => 100], 'p1'),
946+
DataFixture(
947+
AddressCondition::class,
948+
['attribute' => 'base_subtotal_with_discount', 'operator' => '>', 'value' => 75],
949+
'cond1'
950+
),
951+
DataFixture(
952+
ProductConditionFixture::class,
953+
// Ensures that the discount is applied to shipping only
954+
['attribute' => 'quote_item_price', 'operator' => '<', 'value' => 0],
955+
'cond2'
956+
),
957+
DataFixture(
958+
RuleFixture::class,
959+
[
960+
'coupon_code' => '%uniqid%',
961+
'stop_rules_processing'=> 0,
962+
'discount_amount' => 30,
963+
'simple_action' => Rule::BY_PERCENT_ACTION,
964+
'sort_order' => 1,
965+
],
966+
'rule1'
967+
),
968+
DataFixture(
969+
RuleFixture::class,
970+
[
971+
'stop_rules_processing'=> 0,
972+
'discount_amount' => 10,
973+
'simple_action' => Rule::BY_FIXED_ACTION,
974+
'sort_order' => 2,
975+
'apply_to_shipping' => 1,
976+
'conditions' => ['$cond1$'],
977+
'actions' => ['$cond2$'],
978+
],
979+
'rule2'
980+
),
981+
DataFixture(GuestCartFixture::class, as: 'cart1'),
982+
DataFixture(AddProductToCartFixture::class, ['cart_id' => '$cart1.id$', 'product_id' => '$p1.id$']),
983+
DataFixture(SetShippingAddress::class, ['cart_id' => '$cart1.id$']),
984+
DataFixture(SetDeliveryMethod::class, ['cart_id' => '$cart1.id$']),
985+
]
986+
public function testSubtotalWithDiscountShouldReflectPreviouslyAppliedRulesForShippingDiscountOnlyRule(): void
987+
{
988+
$rule1Id = (int)$this->fixtures->get('rule1')->getId();
989+
$rule2Id = (int)$this->fixtures->get('rule2')->getId();
990+
$coupon = $this->fixtures->get('rule1')->getCouponCode();
991+
$cart = $this->quoteRepository->get((int)$this->fixtures->get('cart1')->getId());
992+
993+
// Check that rule2 is applied before the coupon code is applied
994+
// Note: the subtotal with discount includes the shipping discount
995+
$this->assertEquals(95, $cart->getSubtotalWithDiscount());
996+
$this->assertEquals(95, $cart->getBaseSubtotalWithDiscount());
997+
// Bypass getter methods for subtotal_with_discount and base_subtotal_with_discount to retrieve stored values
998+
$this->assertEquals(95, $cart->getShippingAddress()->getData('subtotal_with_discount'));
999+
$this->assertEquals(95, $cart->getShippingAddress()->getData('base_subtotal_with_discount'));
1000+
$this->assertEquals(-5, $cart->getShippingAddress()->getBaseDiscountAmount());
1001+
$this->assertEquals(-5, $cart->getShippingAddress()->getDiscountAmount());
1002+
$this->assertEquals(5, $cart->getShippingAddress()->getShippingDiscountAmount());
1003+
$this->assertEquals(5, $cart->getShippingAddress()->getBaseShippingDiscountAmount());
1004+
$this->assertEquals([$rule2Id], explode(',', $cart->getAppliedRuleIds()));
1005+
// rule2 is applied shipping only
1006+
$this->assertEmpty(current($cart->getItems())->getAppliedRuleIds());
1007+
1008+
// Now apply the coupon code to ensure that the subtotal with discount is recalculated correctly
1009+
$cart->setCouponCode($coupon);
1010+
// This is needed because applied_rule_ids is not properly reset in the code for subsequent recollect.
1011+
$cart->setAppliedRuleIds('');
1012+
$cart->collectTotals();
1013+
1014+
// Check that rule1 is applied after the coupon code is applied and rule2 is no longer applied
1015+
$this->assertEquals(70, $cart->getSubtotalWithDiscount());
1016+
$this->assertEquals(70, $cart->getBaseSubtotalWithDiscount());
1017+
$this->assertEquals(70, $cart->getShippingAddress()->getData('subtotal_with_discount'));
1018+
$this->assertEquals(70, $cart->getShippingAddress()->getData('base_subtotal_with_discount'));
1019+
$this->assertEquals(-30, $cart->getShippingAddress()->getBaseDiscountAmount());
1020+
$this->assertEquals(-30, $cart->getShippingAddress()->getDiscountAmount());
1021+
$this->assertEquals(0, $cart->getShippingAddress()->getShippingDiscountAmount());
1022+
$this->assertEquals(0, $cart->getShippingAddress()->getBaseShippingDiscountAmount());
1023+
$this->assertEquals([$rule1Id], explode(',', $cart->getAppliedRuleIds()));
1024+
$this->assertEquals([$rule1Id], explode(',', current($cart->getItems())->getAppliedRuleIds()));
1025+
}
8651026
}

0 commit comments

Comments
 (0)