-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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(core) incorrect quantity adjustment #983
Fix(core) incorrect quantity adjustment #983
Conversation
I pulled down your branch and played around with it locally. One thing I noticed is that the new test seems to pass event when I revert your changes to If so, could you see if you can create a test which fails before the changes, and passes after them? |
The old service is passed with new test is understandable. The incorrect quantity is only returned when we make another query to get the Active order, which is not included in the test case. @WilliamMilne: Thanks for the fix, I have been looking for the solution for couple days and tried to fix it myself, but clearly it's not covered enough comparing to yours. Personally, I found this issue is a critical one and need to be fix asap. Would be great if @michaelbromley could help us out, the PR owner has not given new update for more than a month. |
Hi @quangpdt - I'm actually taking a look at this today, hopefully will have it resolved soon! Sorry that things have been so busy lately! |
@quangpdt and @michaelbromley - I was able to get a test to fail on @michaelbromley do you have any suggestions on how to fix these issues so I can push the latest changes? Thanks!! Also thank you so much for your suggestion @quangpdt! Excited to see if this helps us wrap up the issue! |
@michaelbromley - it appears that the customers[0].email issue was solved by deleting the existing e2e data in the folder. I was also using an outdated version of node, which was causing the asset server test to crash. I've pushed the latest change now! Sorry about the multiple comments! |
Thanks for the great fix @WilliamMilne! |
Thank you @quangpdt for your input and @WilliamMilne for your effort to wrap this up. I will review again and all being well, merge this tomorrow. |
Looks good, thanks all! Will do a patch release this week including this fix. |
Fixed some edge cases for quantity adjustment. Addresses issue #931.