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 user's shipping & billing address interaction with order #11660

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mrbrdo
Copy link
Contributor

@mrbrdo mrbrdo commented Mar 12, 2022

There are several issues regarding this, see the individual commits.

This also makes legacy frontend work correctly:

  • when order is created, the billing/shipping address will correctly be assigned to user if he doesn't have an address assigned
  • when a new order is started, it will correctly use the user's billing/shipping address if present

This was quite broken with even a security issue with the unscoped Spree::Address query. I suggest to merge asap due to this.

@viezly
Copy link

viezly bot commented Mar 12, 2022

Changes preview:

Legend:

👀 Review pull request on Viezly

@stefnnn
Copy link

stefnnn commented Mar 13, 2022

This is tackling simliar problems to my PR here: #11606, maybe we want to merge this into one PR?

@mrbrdo
Copy link
Contributor Author

mrbrdo commented Mar 13, 2022

@stefnnn yes we could. Although your PR was not merged after 2 months. I am afraid the maintainers are sleeping?
No point in putting in the work if it will never get merged.

stefnnn added a commit to stefnnn/spree that referenced this pull request Mar 13, 2022
@mrbrdo
Copy link
Contributor Author

mrbrdo commented Apr 17, 2022

@damianlegawiec ping, this is kinda important. Been using these fixes in production for a while now, no issues at all.

…, fix this

It would be more proper to put this into an Order after_save hook, but the same can be said for
calling the update_or_create_address method, so I will leave that to a future refactor
The Order should use_billing == true if ship_address is missing, is empty, or is the same as bill_address
@mrbrdo
Copy link
Contributor Author

mrbrdo commented May 12, 2022

Updated to correctly determine use_billing? (fixes issue if ship is same as bill and gets updated in checkout form)

@mrbrdo
Copy link
Contributor Author

mrbrdo commented May 26, 2022

@damianlegawiec what do you think?

@mrbrdo
Copy link
Contributor Author

mrbrdo commented Oct 25, 2022

@damianlegawiec well??

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

Successfully merging this pull request may close these issues.

None yet

2 participants