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 4 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
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');
});
};
});
13 changes: 13 additions & 0 deletions app/code/Magento/Quote/Api/Data/TotalsInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ interface TotalsInterface extends \Magento\Framework\Api\ExtensibleDataInterface

const KEY_ITEMS_QTY = 'items_qty';

const KEY_IS_MINIMUM_ORDER_AMOUNT = 'is_minimum_order_amount';

/**#@-*/

/**
Expand Down Expand Up @@ -476,6 +478,17 @@ public function getTotalSegments();
*/
public function setTotalSegments($totals = []);

/**
* @return bool
*/
public function getIsMinimumOrderAmount();

/**
* @param bool $isMinimumAmount
* @return mixed
*/
public function setIsMinimumOrderAmount(bool $isMinimumAmount);

/**
* Retrieve existing extension attributes object or create a new one.
*
Expand Down
2 changes: 2 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,8 @@ public function get($cartId): QuoteTotalsInterface
$quoteTotals->setItemsQty($quote->getItemsQty());
$quoteTotals->setBaseCurrencyCode($quote->getBaseCurrencyCode());
$quoteTotals->setQuoteCurrencyCode($quote->getQuoteCurrencyCode());
$isMinimumOrderAmount = $quote->validateMinimumAmount();
$quoteTotals->setIsMinimumOrderAmount($isMinimumOrderAmount);
return $quoteTotals;
}
}
16 changes: 16 additions & 0 deletions app/code/Magento/Quote/Model/Cart/Totals.php
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,22 @@ public function setTotalSegments($totals = [])
return $this->setData(self::KEY_TOTAL_SEGMENTS, $totals);
}

/**
* {@inheritdoc}
*/
public function getIsMinimumOrderAmount()
{
return $this->getData(self::KEY_IS_MINIMUM_ORDER_AMOUNT);
}

/**
* {@inheritdoc}
*/
public function setIsMinimumOrderAmount(bool $isMinimumAmount)
{
return $this->setData(self::KEY_IS_MINIMUM_ORDER_AMOUNT, $isMinimumAmount);
}

/**
* {@inheritdoc}
*
Expand Down
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 @@ -108,7 +108,8 @@ protected function setUp(): void
'getBillingAddress',
'getAllVisibleItems',
'getItemsQty',
'collectTotals'
'collectTotals',
'validateMinimumAmount'
]
)
->disableOriginalConstructor()
Expand Down Expand Up @@ -138,6 +139,11 @@ 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,10 @@ public function testGetCartTotal($isVirtual, $getAddressType): void
->with(self::STUB_CURRENCY_CODE)
->willReturnSelf();

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

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

Expand Down
44 changes: 44 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,17 @@ protected function setUp(): void
CustomerInterfaceFactory::class,
['create']
);

/*$this->totalCollector = $this->createPartialMock(
TotalsCollector::class,
['collect']
);*/
$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 +354,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 +1101,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 +1141,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
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ private function getData(Quote $quote, Address $shippingAddress) : array
Totals::KEY_QUOTE_CURRENCY_CODE => $quote->getQuoteCurrencyCode(),
Totals::KEY_ITEMS_QTY => $quote->getItemsQty(),
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)],
Totals::KEY_IS_MINIMUM_ORDER_AMOUNT => $quote->validateMinimumAmount()
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public function testGetTotals()
Totals::KEY_QUOTE_CURRENCY_CODE => $quote->getQuoteCurrencyCode(),
Totals::KEY_ITEMS_QTY => $quote->getItemsQty(),
Totals::KEY_ITEMS => [$this->getQuoteItemTotalsData($quote)],
Totals::KEY_IS_MINIMUM_ORDER_AMOUNT => $quote->validateMinimumAmount()
];

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