From ed511dbb76bd26527548b3a9fd504f3d2383facc Mon Sep 17 00:00:00 2001 From: barryhughes <3594411+barryhughes@users.noreply.github.com> Date: Tue, 24 Jan 2023 17:34:13 -0800 Subject: [PATCH 1/5] Synchronize order meta data (between HPOS and CPT stores) upon a call to $order->save_meta_data. --- .../changelog/fix-35703-order-meta-backfill | 4 ++ .../DataStores/CustomMetaDataStore.php | 4 ++ .../Orders/OrdersTableDataStore.php | 32 ++++++++++-- .../Orders/DataSynchronizerTests.php | 50 +++++++++++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 plugins/woocommerce/changelog/fix-35703-order-meta-backfill diff --git a/plugins/woocommerce/changelog/fix-35703-order-meta-backfill b/plugins/woocommerce/changelog/fix-35703-order-meta-backfill new file mode 100644 index 00000000000..d6b48db0887 --- /dev/null +++ b/plugins/woocommerce/changelog/fix-35703-order-meta-backfill @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +When order meta data is saved via HPOS, it should be backfilled to the CPT data store. diff --git a/plugins/woocommerce/src/Internal/DataStores/CustomMetaDataStore.php b/plugins/woocommerce/src/Internal/DataStores/CustomMetaDataStore.php index 3a082eeb289..f9c13e2f9bc 100644 --- a/plugins/woocommerce/src/Internal/DataStores/CustomMetaDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/CustomMetaDataStore.php @@ -78,6 +78,8 @@ abstract class CustomMetaDataStore { * * @param WC_Data $object WC_Data object. * @param stdClass $meta (containing at least ->id). + * + * @return bool */ public function delete_meta( &$object, $meta ) { global $wpdb; @@ -127,6 +129,8 @@ abstract class CustomMetaDataStore { * * @param WC_Data $object WC_Data object. * @param stdClass $meta (containing ->id, ->key and ->value). + * + * @return bool */ public function update_meta( &$object, $meta ) { global $wpdb; diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 72dda5adbec..410b6faa6cd 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -11,6 +11,7 @@ use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil; use Automattic\WooCommerce\Proxies\LegacyProxy; use Automattic\WooCommerce\Utilities\ArrayUtil; use Exception; +use WC_Abstract_Order; use WC_Data; use WC_Order; @@ -2457,9 +2458,17 @@ CREATE TABLE $meta_table ( * * @param WC_Data $object WC_Data object. * @param stdClass $meta (containing at least ->id). + * + * @return bool */ public function delete_meta( &$object, $meta ) { - return $this->data_store_meta->delete_meta( $object, $meta ); + $delete_meta = $this->data_store_meta->delete_meta( $object, $meta ); + + if ( $object instanceof WC_Abstract_Order ) { + $this->maybe_backfill_post_record( $object ); + } + + return $delete_meta; } /** @@ -2467,10 +2476,17 @@ CREATE TABLE $meta_table ( * * @param WC_Data $object WC_Data object. * @param stdClass $meta (containing ->key and ->value). - * @return int meta ID + * + * @return int|bool meta ID or false on failure */ public function add_meta( &$object, $meta ) { - return $this->data_store_meta->add_meta( $object, $meta ); + $add_meta = $this->data_store_meta->add_meta( $object, $meta ); + + if ( $object instanceof WC_Abstract_Order ) { + $this->maybe_backfill_post_record( $object ); + } + + return $add_meta; } /** @@ -2478,8 +2494,16 @@ CREATE TABLE $meta_table ( * * @param WC_Data $object WC_Data object. * @param stdClass $meta (containing ->id, ->key and ->value). + * + * @return bool */ public function update_meta( &$object, $meta ) { - return $this->data_store_meta->update_meta( $object, $meta ); + $update_meta = $this->data_store_meta->update_meta( $object, $meta ); + + if ( $object instanceof WC_Abstract_Order ) { + $this->maybe_backfill_post_record( $object ); + } + + return $update_meta; } } diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php index eb76417d9a0..be046c21c5f 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -223,4 +223,54 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { 'After the COT order record was deleted, the order was also deleted from the posts table.' ); } + + /** + * When sync is enabled, changes to meta data should propoagate from the Custom Orders Table to + * the post meta table whenever the order object's save_meta_data() method is called. + * + * @return void + */ + public function test_meta_data_changes_propagate_from_cot_to_cpt(): void { + // Sync enabled and COT authoritative. + update_option( $this->sut::ORDERS_DATA_SYNC_ENABLED_OPTION, 'yes' ); + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); + + $order = OrderHelper::create_order(); + $order->add_meta_data( 'foo', 'bar' ); + $order->save_meta_data(); + + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'no' ); + $refreshed_order = wc_get_order( $order->get_id() ); + + $this->assertEquals( + $refreshed_order->get_meta( 'foo' ), + 'bar', + 'Meta data persisted via the HPOS datastore is accessible via the CPT datastore' + ); + } + + /** + * When sync is enabled, changes to meta data should propoagate from the post meta table to + * the Custom Orders Table whenever the order object's save_meta_data() method is called. + * + * @return void + */ + public function test_meta_data_changes_propagate_from_cpt_to_cot(): void { + // Sync enabled and COT authoritative. + update_option( $this->sut::ORDERS_DATA_SYNC_ENABLED_OPTION, 'yes' ); + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'no' ); + + $order = OrderHelper::create_order(); + $order->add_meta_data( 'foo', 'bar' ); + $order->save_meta_data(); + + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); + $refreshed_order = wc_get_order( $order->get_id() ); + + $this->assertEquals( + $refreshed_order->get_meta( 'foo' ), + 'bar', + 'Meta data persisted via the CPT datastore is accessible via the HPOS datastore' + ); + } } From e15b942054fc8906bae07948f304c2b62d07e3c4 Mon Sep 17 00:00:00 2001 From: barryhughes <3594411+barryhughes@users.noreply.github.com> Date: Tue, 24 Jan 2023 20:52:16 -0800 Subject: [PATCH 2/5] Fix typos. --- .../Internal/DataStores/Orders/DataSynchronizerTests.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php index be046c21c5f..d7902685f21 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -225,7 +225,7 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { } /** - * When sync is enabled, changes to meta data should propoagate from the Custom Orders Table to + * When sync is enabled, changes to meta data should propagate from the Custom Orders Table to * the post meta table whenever the order object's save_meta_data() method is called. * * @return void @@ -250,13 +250,13 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { } /** - * When sync is enabled, changes to meta data should propoagate from the post meta table to + * When sync is enabled, changes to meta data should propagate from the post meta table to * the Custom Orders Table whenever the order object's save_meta_data() method is called. * * @return void */ public function test_meta_data_changes_propagate_from_cpt_to_cot(): void { - // Sync enabled and COT authoritative. + // Sync enabled and CPT authoritative. update_option( $this->sut::ORDERS_DATA_SYNC_ENABLED_OPTION, 'yes' ); update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'no' ); From fd3749d5c2a896c73b54d0080b5c53c6cb3de476 Mon Sep 17 00:00:00 2001 From: barryhughes <3594411+barryhughes@users.noreply.github.com> Date: Tue, 14 Mar 2023 00:12:20 +0000 Subject: [PATCH 3/5] Add assertions covering synchronization of order meta data deletions. --- .../Orders/DataSynchronizerTests.php | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php index d7902685f21..fbe5e011fa0 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -237,6 +237,10 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { $order = OrderHelper::create_order(); $order->add_meta_data( 'foo', 'bar' ); + $order->add_meta_data( 'bar', 'baz' ); + $order->save_meta_data(); + + $order->delete_meta_data( 'bar' ); $order->save_meta_data(); update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'no' ); @@ -245,7 +249,13 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { $this->assertEquals( $refreshed_order->get_meta( 'foo' ), 'bar', - 'Meta data persisted via the HPOS datastore is accessible via the CPT datastore' + 'Meta data persisted via the HPOS datastore is accessible via the CPT datastore.' + ); + + $this->assertEquals( + $refreshed_order->get_meta( 'bar' ), + '', + 'Meta data deleted from the HPOS datastore should also be deleted from the CPT datastore.' ); } @@ -262,6 +272,10 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { $order = OrderHelper::create_order(); $order->add_meta_data( 'foo', 'bar' ); + $order->add_meta_data( 'bar', 'baz' ); + $order->save_meta_data(); + + $order->delete_meta_data( 'bar' ); $order->save_meta_data(); update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); @@ -270,7 +284,13 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { $this->assertEquals( $refreshed_order->get_meta( 'foo' ), 'bar', - 'Meta data persisted via the CPT datastore is accessible via the HPOS datastore' + 'Meta data persisted via the CPT datastore is accessible via the HPOS datastore.' + ); + + $this->assertEquals( + $refreshed_order->get_meta( 'bar' ), + '', + 'Meta data deleted from the CPT datastore should also be deleted from the HPOS datastore.' ); } } From c76a999ae979e957004b76cec15293eccfa2c188 Mon Sep 17 00:00:00 2001 From: "Jorge A. Torres" Date: Wed, 22 Mar 2023 21:19:42 -0500 Subject: [PATCH 4/5] Propagate metadata deletion --- .../abstract-wc-order-data-store-cpt.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php b/plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php index b26a0875462..6d29543da62 100644 --- a/plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php +++ b/plugins/woocommerce/includes/data-stores/abstract-wc-order-data-store-cpt.php @@ -613,14 +613,28 @@ abstract class Abstract_WC_Order_Data_Store_CPT extends WC_Data_Store_WP impleme foreach ( $order->get_meta_data() as $meta_data ) { if ( isset( $existing_meta_data[ $meta_data->key ] ) ) { if ( $existing_meta_data[ $meta_data->key ] === $meta_data->value ) { + unset( $existing_meta_data[ $meta_data->key ] ); continue; } - delete_post_meta( $order->get_id(), $meta_data->key ); + unset( $existing_meta_data[ $meta_data->key ] ); + delete_post_meta( $order->get_id(), $meta_data->key ); } add_post_meta( $order->get_id(), $meta_data->key, $meta_data->value, false ); } + // Find remaining meta that was deleted from the order but still present in the associated post. + // Post meta corresponding to order props is excluded (as it shouldn't be deleted). + $keys_to_delete = array_diff( + array_keys( $existing_meta_data ), + $this->internal_meta_keys, + array_keys( $this->get_internal_data_store_key_getters() ) + ); + + foreach ( $keys_to_delete as $meta_key ) { + delete_post_meta( $order->get_id(), $meta_key ); + } + $this->update_post_meta( $order ); } } From 41c2549f2087032347beade453eb9b80951646ef Mon Sep 17 00:00:00 2001 From: Vedanshu Jain Date: Thu, 23 Mar 2023 16:06:16 +0530 Subject: [PATCH 5/5] Remove manual meta handling code since custom meta is synced now. --- .../DataStores/Orders/OrdersTableDataStore.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 410b6faa6cd..4050de796f1 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -1964,21 +1964,6 @@ FROM $order_meta_table $order->delete_meta_data( '_wp_trash_meta_comments_status' ); $order->save_meta_data(); - $data_synchronizer = wc_get_container()->get( DataSynchronizer::class ); - if ( $data_synchronizer->data_sync_is_enabled() ) { - // The previous $order->save() will have forced a sync to the posts table, - // this implies that the post status is not "trash" anymore, and thus - // wp_untrash_post would do nothing. - wp_update_post( - array( - 'ID' => $id, - 'post_status' => 'trash', - ) - ); - - wp_untrash_post( $id ); - } - return true; }