-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Fix minicart not cleared after order completion #5807
Conversation
Fixes magento#4460 Fixes magento#4416 Fixes magento#1461 Fixes magento#4969 Fixes MAGETWO-52593
when is this going to be merged? |
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.
great, it works
@ajpevers thank you for taking part in development. All the issues you mentioned seem to be fixed by internal changes, aren't they? Is any of them still reproducible for you on develop branch or 2.0.9 release? |
@SerhiyShkolyarenko Are you sure? I don't think so. If you compare https://github.com/magento/magento2/blob/develop/app/code/Magento/Checkout/view/frontend/templates/success.phtml (develop branch) with this PR (https://github.com/ajpevers/magento2/blob/9093ce9c2f5a4ba8c60ef0dd0769e620b4ce6f18/app/code/Magento/Checkout/view/frontend/templates/success.phtml). The problem still exists in 2.1.2. |
@PieterCappelle is correct. These changes do not appear in 2.1.2. |
There might be another solution to this problem, so I think only a functional test can tell. I haven't had the time yet to do this. |
Running 2.1.2, the minicart does not clear on order place. Testing this fix. |
Running 2.1.2, the minicart issue is still there and its not clearing after success checkout |
Nobody mentioned before, but looks like the issue happens with guest checkout only (v2.1.2) |
@mamsincl No, the issue is also occurring for registered customers. I got it at magento version 2.1.2 |
This bug still occurs in M2.1.3. Can anyone confirm that this patch also solves this issue there? |
@SerhiyShkolyarenko Any plans to still merge this pull request? Any idea what release version it will be in? |
@SerhiyShkolyarenko Any update on this? Issue still exists in 2.1.5. |
Still having the issue in 2.1.6 |
I can confirm that this fix works for Magento 2.1.5. I am unsure what is the hold up in merging this into the repo. |
@surfer190 This is what hold up this merging:
|
That error implies that the js/customer-data constructor was called with null settings. Some kind of requirejs or config failure? I have tried to work around this cart problem by overriding |
@dnadle That is what exactly I'm doing for my own troubles. Instead of insert new inline javascript to
Reload section may leads to some potential issue with cache (!) |
I think a more elegant solution is to invalidate sections the same way as in https://github.com/magento/magento2/blob/develop/app/code/Magento/Customer/view/frontend/web/js/customer-data.js#L346 and https://github.com/magento/magento2/blob/develop/app/code/Magento/Customer/view/frontend/web/js/customer-data.js#L368. These invalidations work based on the sections.xml files in modules but they only apply to form submits and post|put|delete calls. |
UPDATE 10/11/2017 - It appears that this fix actually did fix the issue. Disregard comment below. I applied the contents of this patch to a Magento EE 2.1.5 site and pushed it to a server running in production mode. The fix only works intermittently, but for the majority of checkouts, the minicart still shows items on the checkout confirmation page. So I would say that this PR does NOT work and an alternative solution should be used, such as the one that @ajpevers just suggested. |
The actual placement of the order and redirect after success happens in payment/default.js, and there is an afterPlaceOrder callback method defined. I don't understand the resistance to putting the cache invalidation there rather than listening for a get on the success page. |
@ajpevers Is this PR still valid? Internal tickets show it as fixed in the |
@ishakhsuvarov close this PR please. I cannot reproduce this issue in the |
Anybody advise the status of this issue MAGETWO-52593? |
I am also getting it on 2.1.7 CE |
It's not working on 2.1.7 EE |
Still not works on 2.1.8 |
These changes are not in 2.1.9 either. |
Changes are not in 2.1.8 EE, bug still exists |
[MPI]MC-31375: Catalog graphQl - position sort doesn't work properly
Fixes #4460 Fixes #4416 Fixes #1461 Fixes #4969 Fixes MAGETWO-52593