[COT] Effectively synchronize orders from the posts table to the custom orders table #32817

Konamiman · 2022-04-28T15:21:35Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

TLDR:

This pull request combines the the settings UI/scheduled actions for orders sync work that was implemented in #32062 (and adjusted in #32706) with the posts/meta tables to custom tables migration classes that were implemented in #32034 and #32640. Additionally, creating and updating orders will now cause a synchronization process to be started, provided that synchronization is enabled. It also fixes a bug related to the automatic authoritative table switch and does some significant refactor on the migration classes.

More precisely:

The scheduled action handler implemented in the DataSynchronizer class will now effectively synchronize orders from the posts table to the custom orders table (but not the other way around! That part will be implemented separately).
The order creation and update process will be captured by the DataSynchronizer class (via woocommerce_after_order_object_save hook) and will start a data synchronization process (only if data synchronization is enabled in settings).
- Note/remember that another way to start a data synchronization process is to go to Settings - Advanced - Custom data stores and hitting "Save", even without actually modifying any settings.
- Also, remember that for now, actual sync will only happen when the posts table is the authoritative data source for orders.
Fixes the handling of the "Switch to using the other table when sync finishes" option handling, which was reversed (also, option cleanup was happening before that option was actually read which made it useless).
Classes in the Database\Migrations\CustomOrderTable namespace have been refactored:
- Some have been renamed.
- More descriptive doc comments have been added.
- (Now) redundant methods used during the development of the classes have been removed.
- Type hints for parameters and return values have been added to all the methods.
Changes have been implemented to ease testing:
- The code that throws an exception if a switch of the authoritative table is attempted while there are out of sync orders has been temporarily disabled. Thus it's now possible to run wp option update woocommerce_custom_orders_table_enabled yes|no.
- Saving the settings will always restart the sync process, even if it's in progress already (this is currently the only way to recover from an error thrown inside the scheduled action that performs the sync).
- Added a new filter, woocommerce_orders_cot_and_posts_sync_step_size, to customize the count of orders that will be synchronized in each scheduled action run.

NOTE: Given the scope of this pull request it's recommended to review the individual commits separately.

How to test the changes in this Pull Request:

Prerrequisite: having some orders in the posts table.

Enable the feature by adding the following code snippet somewhere, for example at the end of woocommerce.php:

wc_get_container()
   ->get('Automattic\WooCommerce\Internal\DataStores\Orders\CustomOrdersTableController')
   ->show_feature();

If you hadn't created the orders table previously, go to Status - Tools and run the "Create and fill custom orders table" tool. If you had, make sure that the table is empty (truncate table wp_wc_orders is fine).
Go to WooCommerce - Settings - Advanced - Custom data stores, you should see a message saying that there are out of sync orders (all your orders, actually) but no mention to sync being in progress.
Make sure that the "Use the WordPress table" radio is selected and that "Keep the posts table and the orders table synchronized" is checked, and save (save even if you didn't need to change anything).
Once the page reloads, you'll see that the message has changed to indicate that the sync process is in progress. Reload again repeatedly, until the message disappears.
Verify that the wp_wc_orders table now contains proper entries for all the existing orders.
Repeat the process (manually truncate the table, save settings with "Keep the posts table and the orders table synchronized" checked) but this time, once the first reload of the settings page check "Switch to using the orders table as the authoritative data store when sync finishes" and save. Now, after the sync finishes you should see that the "Use the WooCommerce orders table" is selected instead.
Make the posts table authoritative again.
Modify any of the existing orders, for example change its status.
Immediately after the previous step reload the settings page. You should see that the message appears again saying that there's one order pending sync, and that the sync is in progress.
Once the sync has happened verify that the corresponding order (same value for the id field) has been updated accordingly in wp_wc_orders.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully run tests with your changes locally?
Have you created a changelog file by running pnpm nx affected --target=changelog?

FOR PR REVIEWER ONLY:

I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

This constant contains the name of the setting that is used to determine if the roles of the custom orders table and the posts table (authoritative and backup) are reversed once synchronization of orders between both tables finishes. The logic was mistakenly implemented backwards. Also, as part of the fix the constant is moved to the CustomOrdersTableController class.

It's now maybe_start_synchronizing_pending_orders. Also, the method now contains all the logic needed to decide if a synchronization needs to actually be started.

This commit connects the logic of the DataSyncrhonizer class with the actual posts to COT synchronization implemented in the WPPostToCOTMigrator class, so orders are actually syncrhonized using scheduled actions. Additionally, a synchronization process is triggered whenever an order is created or modified, using the woocommerce_after_order_object_save hook.

- Rename to PostsToOrdersMigrationController (same for tests class) - Rename process_next_migration_batch method to migrate_orders - Rename process_single to migrate_order - Remove batch and checkpoint related methods (batch management is performed by DataSynchronizer class) - Adjust unit tests

…Table - Add meaningful docblock comments for the classes - Add type ints for parameters and return values in all mehtods

vedanshujain

Seems like there is some issue with order counting logic, it is always showing me 1815 orders even when everything is synced. Investigating a bit more

vedanshujain · 2022-05-04T10:37:29Z

plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php

+	public const PENDING_SYNCHRONIZATION_FINISHED_ACTION   = 'woocommerce_orders_sync_finished';
+	public const PLACEHOLDER_ORDER_POST_TYPE               = 'shop_order_placehold';
+
+	private const ORDERS_SYNC_BATCH_SIZE      = 50;


this can be increased to 250 as a default

I have added the woocommerce_orders_cot_and_posts_sync_step_size filter to customize this if needed.

let's still increase the step size to at least 250. 50 per batch is really low

vedanshujain · 2022-05-05T06:49:39Z

Looks like the issue was that I had COT already enabled and then had 1815 orders out of sync. Given that backfill isn't implemented, the count of orders was not going down. Should we allow to switch data stores when COT is authoritative, even with pending orders, till the time backfill is merged?

Also, we can actually increase the batch size to 500 or 1000, I want folks during the testing time to run into issues so that we can calibrate to a more optimum number as default. WDYT?

- The woocommerce_custom_orders_table_enabled option can now be manually altered even if there are orders out of sync - Saving settings will always restart the sync process, even if one is already in progress - Add the woocommerce_orders_cot_and_posts_sync_step_size filter

vedanshujain

Left a minor comment, looks good otherwise 👍

vedanshujain · 2022-05-09T10:50:30Z

plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php

+	public const PENDING_SYNCHRONIZATION_FINISHED_ACTION   = 'woocommerce_orders_sync_finished';
+	public const PLACEHOLDER_ORDER_POST_TYPE               = 'shop_order_placehold';
+
+	private const ORDERS_SYNC_BATCH_SIZE      = 50;


let's still increase the step size to at least 250. 50 per batch is really low

github-actions · 2022-05-09T15:03:04Z

Hi @vedanshujain, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

Add the release: add testing instructions label

Konamiman added 4 commits April 28, 2022 15:25

Rename DataSynchronizer::start_synchronizing_pending_orders

7a08924

It's now maybe_start_synchronizing_pending_orders. Also, the method now contains all the logic needed to decide if a synchronization needs to actually be started.

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Apr 28, 2022

Konamiman added 3 commits April 29, 2022 10:15

Remove 'WP' class name prefixes in Database\Migrations\CustomOrderTable

d9fb2e3

Improve the readability of classes in Database\Migrations\CustomOrder…

57b1bd2

…Table - Add meaningful docblock comments for the classes - Add type ints for parameters and return values in all mehtods

Add changelog file

e7a4ff2

Konamiman marked this pull request as ready for review April 29, 2022 14:32

Konamiman requested a review from vedanshujain April 29, 2022 14:32

vedanshujain reviewed May 4, 2022

View reviewed changes

vedanshujain reviewed May 9, 2022

View reviewed changes

Increase orders sync default batch size to 250

dcb2f45

Konamiman mentioned this pull request May 9, 2022

Admin list table [COT] #32864

Merged

vedanshujain approved these changes May 9, 2022

View reviewed changes

vedanshujain merged commit 53a6202 into trunk May 9, 2022

vedanshujain deleted the cot/effectively_sync_posts_to_cot branch May 9, 2022 15:02

github-actions bot added this to the 6.6.0 milestone May 9, 2022

github-actions bot mentioned this pull request Jun 15, 2022

Test WooCommerce 6.6 UVLabs/location-picker-at-checkout-for-woocommerce#35

Closed

Konamiman mentioned this pull request Aug 1, 2022

[COT] Add unique key constraints for the addresses and operational data tables #34144

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[COT] Effectively synchronize orders from the posts table to the custom orders table #32817

[COT] Effectively synchronize orders from the posts table to the custom orders table #32817

Uh oh!

Konamiman commented Apr 28, 2022 •

edited

Loading

Uh oh!

vedanshujain left a comment

Uh oh!

vedanshujain May 4, 2022

Uh oh!

Konamiman May 9, 2022

Uh oh!

vedanshujain May 9, 2022

Uh oh!

Konamiman May 9, 2022

Uh oh!

vedanshujain commented May 5, 2022

Uh oh!

vedanshujain left a comment

Uh oh!

vedanshujain May 9, 2022

Uh oh!

github-actions bot commented May 9, 2022 •

edited by vedanshujain

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[COT] Effectively synchronize orders from the posts table to the custom orders table #32817

[COT] Effectively synchronize orders from the posts table to the custom orders table #32817

Uh oh!

Conversation

Konamiman commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

TLDR:

More precisely:

How to test the changes in this Pull Request:

Other information:

FOR PR REVIEWER ONLY:

Uh oh!

vedanshujain left a comment

Choose a reason for hiding this comment

Uh oh!

vedanshujain May 4, 2022

Choose a reason for hiding this comment

Uh oh!

Konamiman May 9, 2022

Choose a reason for hiding this comment

Uh oh!

vedanshujain May 9, 2022

Choose a reason for hiding this comment

Uh oh!

Konamiman May 9, 2022

Choose a reason for hiding this comment

Uh oh!

vedanshujain commented May 5, 2022

Uh oh!

vedanshujain left a comment

Choose a reason for hiding this comment

Uh oh!

vedanshujain May 9, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 9, 2022 • edited by vedanshujain Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Konamiman commented Apr 28, 2022 •

edited

Loading

github-actions bot commented May 9, 2022 •

edited by vedanshujain

Loading