Skip to content
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 broken admin order after emptying order and readding items #21051

Merged

Conversation

driskell
Copy link
Contributor

@driskell driskell commented Feb 7, 2019

Description (*)

After filling in a backend Magento Admin new order and then removing all items from the order creation screen, when you then add an item back in you cannot select any shipping rates and "Sorry, no quotes are available" appears until you modify the address to cause it to re-save the address.

Investigation shows the following commit causes this regression by wiping the address from the database when last item is removed from the quote, even though in Magento Admin the address should still remain.
4c5372d

Manual testing scenarios (*)

  1. Login to Magento Admin and go to Sales, Orders and click Create New Order
  2. Click Create New Customer
  3. Enter a product into the order and complete the email and address fields, unticking shipping same as billing and filling in another address in the shipping address
  4. Shipping methods will be visible
  5. Now empty the product list and then add another product back in
  6. Click get shipping in the shipping area, if there is a link to do so
  7. Note it says Sorry, no quotes available even though the address is filled in above
  8. Modify address such as telephone number and retry - the shipping methods now appear

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

…roken quote process until address is updated
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Feb 7, 2019

CLA assistant check
All committers have signed the CLA.

@driskell driskell changed the base branch from 2.3-develop to 2.2-develop February 7, 2019 22:25
@magento-engcom-team
Copy link
Contributor

Hi @driskell. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@driskell
Copy link
Contributor Author

driskell commented Feb 7, 2019

I changed the PR to 2.2-develop and may have confused the contributor checks...

@dmytro-ch dmytro-ch self-requested a review February 11, 2019 19:30
@dmytro-ch dmytro-ch self-assigned this Feb 11, 2019
@dmytro-ch
Copy link
Contributor

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, here is your new Magento instance.
Admin access: https://pr-21051.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@dmytro-ch
Copy link
Contributor

@magento-engcom-team give me 2.2-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch. Thank you for your request. I'm working on Magento 2.2-develop instance for you

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, here is your Magento instance.
Admin access: https://i-21051-2-2-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@dmytro-ch
Copy link
Contributor

Hi @driskell,
thank you for your contribution.
I could not reproduce the issue on test 2.2-develop instance.
Could you please check if the issue still reproducible?
If yes, please create the corresponding issue.

Thank you!

@driskell
Copy link
Contributor Author

@dmytro-ch I tracked a change that sends billing address data when you click get shipping rates.
It does not send shipping address though. So if you untick "Same as billing" in the shipping address section, then follow the steps, the issue still reproduces. So it's just narrowed down now. I will update the reproduction steps now and I will attempt to track that change that sends the billing address and compare it to my 2.2.7 installation.

@driskell
Copy link
Contributor Author

I also restricted shipping to UK only in my latest reproduction case, just in case the default Magento shipping rules mean a blank address still gets the shipping option. So I can now reproduce again. Will now track down the change that changed things.

@driskell
Copy link
Contributor Author

8232358

The above improved a fair few things. With this PR it will fix the issue I describe above where when you untick the use billing as shipping then empty cart, addresses are lost on server and not repopulated (the do not use billing as shipping flag is lost on server - so even though we send new address data now on shipping rates collection, it's ignored as it resets to using the billing address which is empty and not resent)

Kind of really like the new flow - it's much much smoother! Loving it. If this can be merged it'll be awesome 👍

@driskell
Copy link
Contributor Author

Tested and reproduced successfully even without changing shipping configuration to UK only. So updated.

Also tested the PR fix and it fixes the issue.

(I used those machines generated above - please feel free to create new ones to ensure clean tests.)

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-4499 has been created to process this Pull Request

@soleksii
Copy link

Hi @driskell ! Can you please port the pull request to 2.3-develop as we'd like to merge bug fixes to 2.3 before 2.2?

@soleksii
Copy link

Hi @driskell! The pull request to 2.3-develop was created - #21785

@soleksii
Copy link

✔️ QA Passed

@driskell
Copy link
Contributor Author

Thanks!

@p-bystritsky p-bystritsky self-assigned this Mar 18, 2019
@magento-engcom-team magento-engcom-team merged commit fe02088 into magento:2.2-develop Mar 20, 2019
@ghost
Copy link

ghost commented Mar 20, 2019

Hi @driskell, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants