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
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
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"/>
<comment userInput="Adding the comment to replace waitForPageToLoad action for preserving Backward Compatibility" stepKey="waitForPageToLoad"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment block used on refactored mftf tests to preserve backward compatibility and don't needed in new mftf tests

Copy link
Author

Choose a reason for hiding this comment

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

Done. Please review again when you have a chance.

<waitForElementVisible selector="{{CheckoutShippingMethodsSection.next}}" stepKey="waitForNextButton"/>
</test>
</tests>
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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');
});
};
});
2 changes: 1 addition & 1 deletion app/code/Magento/Quote/Api/Data/TotalsInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ public function getTotalSegments();
* @return $this
*/
public function setTotalSegments($totals = []);

Copy link
Contributor

Choose a reason for hiding this comment

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

Please rollback

Copy link
Author

Choose a reason for hiding this comment

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

Done.

/**
* Retrieve existing extension attributes object or create a new one.
*
Expand Down
4 changes: 4 additions & 0 deletions app/code/Magento/Quote/Model/Cart/CartTotalRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -108,7 +109,8 @@ protected function setUp(): void
'getBillingAddress',
'getAllVisibleItems',
'getItemsQty',
'collectTotals'
'collectTotals',
'validateMinimumAmount'
]
)
->disableOriginalConstructor()
Expand Down Expand Up @@ -138,6 +140,10 @@ protected function setUp(): void
TotalsConverter::class
);

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

$this->model = new CartTotalRepository(
$this->totalsFactoryMock,
$this->quoteRepositoryMock,
Expand Down Expand Up @@ -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));
}

Expand Down
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
Expand Up @@ -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;
Expand Down Expand Up @@ -190,6 +191,11 @@ class QuoteTest extends TestCase
*/
private $orderIncrementIdChecker;

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

/**
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
Expand Down Expand Up @@ -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(
Expand All @@ -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,
],
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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());
}

Expand Down
1 change: 1 addition & 0 deletions app/code/Magento/Quote/etc/extension_attributes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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];
Expand Down