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

With HPOS + compatibility mode enabled, manually created subscriptions don't have a start date in the new order tables #514

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

mattallan
Copy link
Contributor

Fixes https://github.com/woocommerce/woocommerce-subscriptions/issues/4560

Description

When a store has both HPOS and compatibility mode enabled, there is some odd behaviour where new subscriptions are getting created with _scheduled_start meta stored as 0 in the wp_wc_orders_meta table and the correct start date in the wp_postmeta.

WP Posts Table WC Orders Table
image image

With HPOS enabled and compatibility mode disabled, the correct _scheduled_start is stored in wp_wc_orders_meta so this issue just impacts those with the compatibility mode feature.

The approach I've taken in this PR is to fix this by updating the persist_order_to_db() function inside our Subscriptions OrdersTable datastore class so that if the schedule_start prop needs to be updated and it's empty, use the subscription's created date instead.

We do similar behavior already in init_order_record() however, this function doesn't write the dates to the database as it's surrounded by $subscription->set_object_read( false );.

How to test this PR

Not sure if this is necessary but to flick between the different HPOS and compatibility mode settings, rather than waiting for the order data sync to finish, I just wipe my orders and subscriptions from the DB using the following queries:

TRUNCATE TABLE `wp_wc_orders`;
TRUNCATE TABLE `wp_wc_orders_meta`;
TRUNCATE TABLE `wp_wc_order_addresses`;
TRUNCATE TABLE `wp_wc_order_operational_data`;


DELETE FROM wp_posts WHERE post_type = 'shop_subscription' OR post_type = 'shop_order';
DELETE pm
FROM wp_postmeta pm
LEFT JOIN wp_posts wp ON wp.ID = pm.post_id
WHERE wp.ID IS NULL;

DELETE FROM wp_usermeta WHERE meta_key = '_wcs_subscription_ids_cache';
  1. Enable HPOS and sync (compatibility mode) in WC > Settings > Advanced > Features
  2. Go to WC > Subscriptions and click Add Subscription
  3. Enter any details but do not change the status from "Pending"
  4. Click Update
  5. Using phpMyAdmin or some other DB tool:
    1. On trunk verify that _scheduled_start is set to 0 in the wc_orders_meta table but it is set to the correct value in the wp_postmeta table for that same subscription
    2. On this branch create a new subscription and verify that _scheduled_start has the same value in wc_orders_meta and wp_postmeta tables

Other tests:

    1. Keep HPOS on but with compatibility mode disabled and confirm new subscriptions have the correct _schedule_start
    1. With HPOS + compatibility mode enabled/disabled, purchase a subscription from the checkout.
    1. With HPOS + compatibility mode enabled/disabled, manually create a subscription using the following snippet. On trunk you'll notice there's no start date, on this branch there will be:
$subscription = new WC_Subscription();
$subscription = $subscription->save();

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
  • Added deprecated functions, hooks or classes to the spreadsheet

@mattallan mattallan changed the title With HPOS + compatibility mode enabled, subscriptions get created without a start date in the new order tables but present in the WP posts table With HPOS + compatibility mode enabled, manually created subscriptions don't have a start date in the new order tables Sep 22, 2023
@james-allan
Copy link
Contributor

@mattallan I was testing this and trying to understand what the origins of this issue is.
When I run the following code using wp posts tables, the subscription doesn't have a start date stored in the database.

$subscription = new WC_Subscription();
$subscription = $subscription->save();
Screenshot 2023-09-28 at 5 14 27 pm

On this branch when using wc orders table, it does have a start date so I'm thinking that the issue here isn't that the subscriptions doesn't have a date saved in the hpos tables, I believe the issue is that it does have a start date in the wp-posts tables when compatibility mode is on.

I'll have to dig into it some more tomorrow because I don't understand what is causing the post table to get a start date set when hpos and sycning is enabled.

@james-allan
Copy link
Contributor

I think I've found the cause of the discrepancy here.

When you save a subscription using HPOS, in persist_order_to_db() it gets a 0 start date. After it is saved in the orders table with a 0 start date, the init_order_record() function runs and in our Subscriptions datastore it sets the start date if it's empty. After this, the subscription is then back-ported to the posts table and it gets this version of the subscription with a start date set and so it is saved.

So I think to untangle this, there'd need to be some work done to understand how to separate this "on load" we set a start date but don't save it.


Regarding this comment I made above. If you run this code snippet on just wp-posts tables, it doesn't save a start date.

$subscription = new WC_Subscription();
$subscription = $subscription->save();

To resolve that I think we could update the WCS_Subscription_Data_Store_CPT::update_post_meta() function to behave similarly to the change made in this PR to persist_order_to_db() change and if the start date is empty, set it to the date created.

Whether this is the "right" approach to set start dates even if it's not specified. At first I thought that would be the wrong approach but looking at wcs_create_subscription() it does does this exact same sort of thing. It sets the start date to the date created or current time when creating a subscription.

So imo there are currently just discrepancies between loading and saving subscriptions in the different datastores.

Method HPOS WP posts
Read Does default the start date Does default the start date
Save In this PR, it does now default the start date Doesn't default the start date

The problem arises in this HPOS with syncing scenario because there's a read in between the two saves. IMO all the reads and writes need to be consistant.

… data

Prior to this change there were discrepancies between saving and reading that
caused issues when hpos and syncing is enabled.
@james-allan
Copy link
Contributor

@mattallan I submitted a small change in 5998319 brings the saving of subscription post meta inline with the existing changes on this branch so the hpos and post table experience is the same.

Let me know what you think. :)

@mattallan
Copy link
Contributor Author

I submitted a small change in 5998319 brings the saving of subscription post meta inline with the existing changes on this branch so the hpos and post table experience is the same.

Thanks @james-allan, I tested non-HPOS sites and confirmed the start date is now added.
I also added a changelog entry to this PR.

Merging

@mattallan mattallan merged commit df7d71b into trunk Oct 3, 2023
12 checks passed
@mattallan mattallan deleted the fix/hpos-subscriptions-not-syncing-start-dates branch October 3, 2023 04:57
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.

2 participants