-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
viniciusbord9
wants to merge
8
commits into
magento:2.4-develop
Choose a base branch
from
viniciusbord9:bugfix/ISSUE-20004-minimum-order-amount-cart-page
base: 2.4-develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+161
−5
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6438a06
ISSUE-20004: Refactoring minimum order amount including tax value in …
viniciusbord9 f7a3b09
ISSUE-20004: Resolving issues with static tests for B2B and Unit test…
viniciusbord9 535a60c
ISSUE-20004: Resolving Unit and API functional tests
viniciusbord9 1b476be
ISSUE-20004: Fixing API tests
viniciusbord9 3529fbd
ISSUE-20004: Moving the logic to the extension attributes instead of …
viniciusbordinhao-blueacorn c100ffc
ISSUE-20004: Adjusting the functional test to make sure that default …
viniciusbordinhao-blueacorn 7710b5e
ISSUE-20004: Adjusting the functional test to make sure minimum order…
viniciusbordinhao-blueacorn 77600ef
ISSUE-20004: Reverting changes not needed for the interface and adjus…
viniciusbordinhao-blueacorn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
app/code/Magento/Checkout/Test/Mftf/Test/StorefrontVerifyGuestCartMinimumAmountTest.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 paramtriggerRecollect
that will settriggerRecollect
data atQuote
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 theCartTotalRepository
and apply changes to the quote but do not save the datahttps://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.