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

Update wcs_copy_order_address() to use modern APIs for setting address fields #228

Merged
merged 4 commits into from
Oct 28, 2022

Conversation

james-allan
Copy link
Contributor

@james-allan james-allan commented Oct 25, 2022

Fixes #227

Description

While testing #226 I discovered that the billing and presumably the shipping address indexes weren't being saved correctly on checkout.

This issue happens because of the wcs_copy_order_address() function which uses a legacy function (set_address()) to copy the address data.

That function sets the address data directly into post meta. Because of that, when the subscription is eventually saved, the address field updates aren't updated here (since there is nothing changed) which causes the indexes to not be updated.

This PR replaces the set_address() functions with the respective set_shipping_address() and set_billing_address().

How to test this PR

  1. Purchase a subscription on the refactor-create-subscription branch.
    • no HPOS setup is required. Just using the wp post architecture is fine.
  2. In the database look at the subscription and notice the billing address index is incomplete.
    • On trunk the billing index will be incomplete (only include the email).
    • On this branch the index field will be complete.

Screen Shot 2022-10-25 at 3 45 42 pm

^ trunk 👎

Screen Shot 2022-10-25 at 3 45 01 pm

^ this branch. 👍

Product impact

  • Added changelog entry (or does not apply)
  • Will this PR affect WooCommerce Subscriptions? yes/no/tbc, add issue ref
  • Will this PR affect WooCommerce Payments? yes/no/tbc, add issue ref

@mattallan
Copy link
Contributor

I've just added a changelog for this one @james-allan. Feel free to change it :)

Copy link
Contributor

@mattallan mattallan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've confirmed the issue on refactor-create-subscription where creating a subscription resulted in no _shipping_address_index data being saved to the DB and only my WP account email was present in _billing_address_index (not the billing email set on checkout):

3495	27	_billing_address_index	         matt.d.a@live.com 
3496	27	_shipping_address_index	         

I've confirmed the problem is fixed on this branch:

3596	29	_billing_address_index	Matt Allan  959 Market Street  San Francisco CA 94013 US matt@example.com (555) 555-5555
3597	29	_shipping_address_index	Matt Allan  959 Market Street  San Francisco CA 94013 US 

LGTM

Base automatically changed from refactor-create-subscription to trunk October 28, 2022 03:41
@james-allan james-allan merged commit 07a8bc8 into trunk Oct 28, 2022
@james-allan james-allan deleted the issue/227 branch October 28, 2022 04:01
mattallan added a commit that referenced this pull request Apr 17, 2023
…s fields (#228)

* Update wcs_copy_order_address to use modern APIs to copy the billing & shipping address

* Simplify the address type logic

* Yoda conditions

* Add changelog entry

Co-authored-by: mattallan <matt.allan@automattic.com>
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.

HPOS: Make sure billing and shipping address indexes are set on checkout
2 participants