Skip to content

ISSUE-20004: Refactoring minimum order amount including tax value in the calculate #33327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: 2.4-develop
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@
<element name="total" type="text" selector="//*[@id='cart-totals']//tr[@class='grand totals']//td//span[@class='price']"/>
<element name="totalAmount" type="text" selector="//*[@id='cart-totals']//tr[@class='grand totals']//td//span[@class='price' and contains(text(), '{{amount}}')]" parameterized="true"/>
<element name="proceedToCheckout" type="button" selector=".action.primary.checkout span" timeout="30"/>
<element name="proceedToCheckoutDisabled" type="button" selector=".action.primary.checkout.disabled" timeout="30"/>
<element name="discountAmount" type="text" selector="td[data-th='Discount']"/>
<element name="shippingHeading" type="button" selector="#block-shipping-heading" timeout="10"/>
<element name="postcode" type="input" selector="input[name='postcode']" timeout="10"/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
<test name="StorefrontVerifyGuestCartMinimumAmountTest">
<annotations>
<features value="Checkout"/>
<stories value="Minimum amount order on checkout page"/>
<title value="Verify minimum amount on checkout page"/>
<description value="When the minimum order amount is set it must consider the tax value"/>
<severity value="BLOCKER"/>
<testCaseId value="MC-28285"/>
<group value="checkout"/>
<group value="tax"/>
</annotations>
<before>
<createData entity="FlatRateShippingMethodConfig" stepKey="enableFlatRate"/>
<createData entity="FreeShippingMethodsSettingConfig" stepKey="freeShippingMethodsSettingConfig"/>
<magentoCLI command="config:set sales/minimum_order/active 1" stepKey="enableMinimumOrderAmount"/>
<magentoCLI command="config:set sales/minimum_order/amount 100" stepKey="setMinimumOrderAmount100"/>
<createData entity="taxRate_US_NY_8_1" stepKey="createTaxRateUSNY"/>
<createData entity="DefaultTaxRuleWithCustomTaxRate" stepKey="createTaxRuleUSNY">
<requiredEntity createDataKey="createTaxRateUSNY" />
</createData>
<createData entity="ApiCategory" stepKey="createCategory"/>
<createData entity="defaultSimpleProduct" stepKey="simpleProduct">
<requiredEntity createDataKey="createCategory"/>
<field key="price">93.00</field>
</createData>
<actionGroup ref="CliCacheCleanActionGroup" stepKey="cleanInvalidatedCaches">
<argument name="tags" value="config full_page"/>
</actionGroup>
</before>
<after>
<deleteData createDataKey="createCategory" stepKey="deleteCategory"/>
<deleteData createDataKey="simpleProduct" stepKey="deleteProduct"/>
<deleteData createDataKey="createTaxRuleUSNY" stepKey="deleteTaxRuleUSNY"/>
<deleteData createDataKey="createTaxRateUSNY" stepKey="deleteTaxRateUSNY"/>
<createData entity="DefaultShippingMethodsConfig" stepKey="defaultShippingMethodsConfig"/>
<createData entity="DefaultMinimumOrderAmount" stepKey="defaultMinimumOrderAmount"/>
</after>
<actionGroup ref="AssertProductNameAndSkuInStorefrontProductPageByCustomAttributeUrlKeyActionGroup" stepKey="openProductPageAndVerifyProduct">
<argument name="product" value="$simpleProduct$"/>
</actionGroup>
<actionGroup ref="StorefrontAddProductToCartWithQtyActionGroup" stepKey="addSimpleProductToTheCart">
<argument name="productQty" value="1"/>
</actionGroup>
<actionGroup ref="ClickViewAndEditCartFromMiniCartActionGroup" stepKey="clickMiniCart"/>
<waitForElementVisible selector="{{CheckoutCartSummarySection.proceedToCheckoutDisabled}}" stepKey="goToCheckoutDisabled"/>
<actionGroup ref="AssertMessageCustomerChangeAccountInfoActionGroup" stepKey="assertMinimumAmountOrderMessage">
<argument name="message" value="Minimum order amount is $100.00"/>
<argument name="messageType" value="notice"/>
</actionGroup>
<actionGroup ref="CheckoutFillEstimateShippingAndTaxActionGroup" stepKey="fillEstimateShippingAndTaxFields">
<argument name="address" value="US_Address_NY_Default_Shipping"/>
</actionGroup>
<click selector="{{CheckoutCartSummarySection.shippingMethodElementId('freeshipping', 'freeshipping')}}" stepKey="selectShippingMethod"/>
<scrollTo selector="{{CheckoutCartSummarySection.proceedToCheckout}}" stepKey="scrollToProceedToCheckout" />
<actionGroup ref="StorefrontClickProceedToCheckoutActionGroup" stepKey="goToCheckout"/>
<waitForPageLoad stepKey="waitForPageToLoad"/>
<waitForElementVisible selector="{{CheckoutShippingMethodsSection.next}}" stepKey="waitForNextButton"/>
</test>
</tests>
Original file line number Diff line number Diff line change
@@ -6,8 +6,9 @@
define([
'jquery',
'Magento_Customer/js/model/authentication-popup',
'Magento_Customer/js/customer-data'
], function ($, authenticationPopup, customerData) {
'Magento_Customer/js/customer-data',
'Magento_Checkout/js/model/quote'
], function ($, authenticationPopup, customerData, quote) {
'use strict';

return function (config, element) {
@@ -26,5 +27,16 @@ define([
location.href = config.checkoutUrl;
});

quote.totals.subscribe(function (totals) {
if (totals['is_minimum_order_amount']) {
$(element).prop('disabled', false);
$(element).removeClass('disabled');

return;
}

$(element).prop('disabled', true);
$(element).addClass('disabled');
});
};
});
4 changes: 4 additions & 0 deletions app/code/Magento/Quote/Model/Cart/CartTotalRepository.php
Original file line number Diff line number Diff line change
@@ -117,6 +117,10 @@ public function get($cartId): QuoteTotalsInterface
$quoteTotals->setItemsQty($quote->getItemsQty());
$quoteTotals->setBaseCurrencyCode($quote->getBaseCurrencyCode());
$quoteTotals->setQuoteCurrencyCode($quote->getQuoteCurrencyCode());
$isMinimumOrderAmount = $quote->validateMinimumAmount();
$extensionAttributes = $quoteTotals->getExtensionAttributes();
$extensionAttributes->setIsMinimumOrderAmount($isMinimumOrderAmount);
$quoteTotals->setExtensionAttributes($extensionAttributes);
return $quoteTotals;
}
}
2 changes: 2 additions & 0 deletions app/code/Magento/Quote/Model/Quote.php
Original file line number Diff line number Diff line change
@@ -2298,6 +2298,8 @@ public function validateMinimumAmount($multishipping = false)
$storeId
);

$this->collectTotals()->save();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that triggering recollect totals in this method is good solution.
I believe that we can extend QuoteRepository with additional param triggerRecollect that will set triggerRecollect data at Quote object.
https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Quote/Model/Quote.php#L2461-L2470

@ihor-sviziev what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@viniciusbord9 Could you pay attention to this comment? Extending of QuoteRepository allow to reuse recollect functional in a future.
Or may be you have another points for this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Den4ik

I was waiting for an answer from @ihor-sviziev to whom you asked for sooner. I am going to try to implement your idea and check if it is viable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Den4ik I think it's a good idea

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Den4ik @ihor-sviziev

I was developing the new repository extending the QuoteRepository as suggested and due to a necessity of injecting the repository for two classes(TotalsInformationManagement, CartTotalRepository) , I have realized that maybe the current flow was not working well.

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Model/TotalsInformationManagement.php

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Quote/Model/Cart/CartTotalRepository.php

The class TotalsInformationManagement depends on the CartTotalRepository and apply changes to the quote but do not save the data

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Model/TotalsInformationManagement.php#L48-L61

I have only change the line 61 from $quote->collectTotals(); to $quote->collectTotals()->save(); and then my changes worked as expected.

I have finished the implementation of the QuoteRepository supporting an additional parameter $triggerRecollect with default value 0. If you still think that change is the best one I will proceed, but to be honest I think it is too big that fix.


$addresses = $this->getAllAddresses();

if (!$multishipping) {
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
use Magento\Quote\Api\CartRepositoryInterface;
use Magento\Quote\Api\CouponManagementInterface;
use Magento\Quote\Api\Data\TotalSegmentInterface;
use Magento\Quote\Api\Data\TotalsExtensionInterface;
use Magento\Quote\Api\Data\TotalsInterface as QuoteTotalsInterface;
use Magento\Quote\Api\Data\TotalsInterfaceFactory;
use Magento\Quote\Model\Cart\CartTotalRepository;
@@ -108,7 +109,8 @@ protected function setUp(): void
'getBillingAddress',
'getAllVisibleItems',
'getItemsQty',
'collectTotals'
'collectTotals',
'validateMinimumAmount'
]
)
->disableOriginalConstructor()
@@ -138,6 +140,10 @@ protected function setUp(): void
TotalsConverter::class
);

$this->totalsConverterMock = $this->createMock(
TotalsConverter::class
);

$this->model = new CartTotalRepository(
$this->totalsFactoryMock,
$this->quoteRepositoryMock,
@@ -247,6 +253,29 @@ public function testGetCartTotal($isVirtual, $getAddressType): void
->with(self::STUB_CURRENCY_CODE)
->willReturnSelf();

$totalExtensionInterfaceMock = $this->getMockBuilder(TotalsExtensionInterface::class)
->onlyMethods(['setIsMinimumOrderAmount'])
->disableOriginalConstructor()
->getMockForAbstractClass();

$totalsMock->expects($this->once())
->method('getExtensionAttributes')
->willReturn($totalExtensionInterfaceMock);

$totalExtensionInterfaceMock->expects($this->once())
->method('setIsMinimumOrderAmount')
->with(true)
->willReturnSelf();

$totalsMock->expects($this->once())
->method('setExtensionAttributes')
->with($totalExtensionInterfaceMock)
->willReturnSelf();

$this->quoteMock->expects($this->once())
->method('validateMinimumAmount')
->willReturn(true);

$this->assertEquals($totalsMock, $this->model->get(self::STUB_CART_ID));
}

39 changes: 39 additions & 0 deletions app/code/Magento/Quote/Test/Unit/Model/QuoteTest.php
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@
use Magento\Quote\Model\Quote\AddressFactory;
use Magento\Quote\Model\Quote\Item;
use Magento\Quote\Model\Quote\Item\Processor;
use Magento\Quote\Model\Quote\TotalsCollector;
use Magento\Quote\Model\Quote\Payment;
use Magento\Quote\Model\Quote\PaymentFactory;
use Magento\Quote\Model\ResourceModel\Quote\Address\Collection;
@@ -190,6 +191,11 @@ class QuoteTest extends TestCase
*/
private $orderIncrementIdChecker;

/**
* @var TotalsCollector|MockObject
*/
private $totalsCollector;

/**
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
@@ -311,6 +317,12 @@ protected function setUp(): void
CustomerInterfaceFactory::class,
['create']
);

$this->totalsCollector = $this->getMockBuilder(TotalsCollector::class)
->onlyMethods(['collect'])
->disableOriginalConstructor()
->getMock();

$this->orderIncrementIdChecker = $this->createMock(OrderIncrementIdChecker::class);
$this->quote = (new ObjectManager($this))
->getObject(
@@ -337,6 +349,7 @@ protected function setUp(): void
'customerDataFactory' => $this->customerDataFactoryMock,
'itemProcessor' => $this->itemProcessor,
'orderIncrementIdChecker' => $this->orderIncrementIdChecker,
'totalsCollector' => $this->totalsCollector,
'data' => [
'reserved_order_id' => 1000001,
],
@@ -1083,6 +1096,19 @@ public function testValidateMinimumAmount()
->method('setQuoteFilter')
->willReturn([$this->quoteAddressMock]);

$totalsMock = $this->getMockBuilder(DataObject::class)
->onlyMethods(['getData'])
->disableOriginalConstructor()
->getMock();

$totalsMock->expects($this->once())
->method('getData')
->willReturn([]);

$this->totalsCollector->expects($this->once())
->method('collect')
->willReturn($totalsMock);

$this->assertTrue($this->quote->validateMinimumAmount());
}

@@ -1110,6 +1136,19 @@ public function testValidateMinimumAmountNegative()
->method('setQuoteFilter')
->willReturn([$this->quoteAddressMock]);

$totalsMock = $this->getMockBuilder(DataObject::class)
->onlyMethods(['getData'])
->disableOriginalConstructor()
->getMock();

$totalsMock->expects($this->once())
->method('getData')
->willReturn([]);

$this->totalsCollector->expects($this->once())
->method('collect')
->willReturn($totalsMock);

$this->assertFalse($this->quote->validateMinimumAmount());
}

1 change: 1 addition & 0 deletions app/code/Magento/Quote/etc/extension_attributes.xml
Original file line number Diff line number Diff line change
@@ -12,5 +12,6 @@

<extension_attributes for="Magento\Quote\Api\Data\TotalsInterface">
<attribute code="coupon_label" type="string" />
<attribute code="is_minimum_order_amount" type="int" />
</extension_attributes>
</config>
Original file line number Diff line number Diff line change
@@ -240,7 +240,7 @@ private function getData(Quote $quote, Address $shippingAddress) : array
Totals::KEY_BASE_CURRENCY_CODE => $quote->getBaseCurrencyCode(),
Totals::KEY_QUOTE_CURRENCY_CODE => $quote->getQuoteCurrencyCode(),
Totals::KEY_ITEMS_QTY => $quote->getItemsQty(),
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)],
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)]
];
}
}
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ public function testGetTotals()
Totals::KEY_BASE_CURRENCY_CODE => $quote->getBaseCurrencyCode(),
Totals::KEY_QUOTE_CURRENCY_CODE => $quote->getQuoteCurrencyCode(),
Totals::KEY_ITEMS_QTY => $quote->getItemsQty(),
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)],
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)]
];

$requestData = ['cartId' => $cartId];