From 1d91a8e2e30cf0f21d72a9b92a69eb81b7693dc1 Mon Sep 17 00:00:00 2001 From: Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Thu, 15 Dec 2022 00:38:15 -0800 Subject: [PATCH] HPOS sync/deleted twins (#35723) --- .../changelog/fix-35590-deleted-twins | 4 +++ .../DataStores/Orders/DataSynchronizer.php | 2 +- .../Orders/OrdersTableDataStore.php | 26 +++++++++++++--- .../rest-api/Helpers/OrderHelper.php | 15 ++++++++- .../Orders/DataSynchronizerTests.php | 31 ++++++++++++++++++- 5 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 plugins/woocommerce/changelog/fix-35590-deleted-twins diff --git a/plugins/woocommerce/changelog/fix-35590-deleted-twins b/plugins/woocommerce/changelog/fix-35590-deleted-twins new file mode 100644 index 00000000000..94624181dfc --- /dev/null +++ b/plugins/woocommerce/changelog/fix-35590-deleted-twins @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Address HPOS synchronization issues relating to the deletion of orders. diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php index cf2b2174f21..efb50a8940c 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php @@ -426,7 +426,7 @@ WHERE * @param WP_Post $post The deleted post. */ private function handle_deleted_post( $postid, $post ): void { - if ( 'shop_order' === $post->post_type && ! $this->custom_orders_table_is_authoritative() && $this->data_sync_is_enabled() ) { + if ( 'shop_order' === $post->post_type && $this->data_sync_is_enabled() ) { $this->data_store->delete_order_data_from_custom_order_tables( $postid ); } } diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index a2ba703c0ca..f2543f037f6 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -9,6 +9,7 @@ use Automattic\Jetpack\Constants; use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil; use Automattic\WooCommerce\Proxies\LegacyProxy; use Automattic\WooCommerce\Utilities\ArrayUtil; +use Exception; use WC_Data; use WC_Order; @@ -1150,9 +1151,26 @@ WHERE foreach ( $cpt_store_orders[ $cpt_store_name ] as $order_id => $order ) { $cpt_order_class_name = wc_get_order_type( $order->get_type() )['class_name']; $cpt_order = new $cpt_order_class_name(); - $cpt_order->set_id( $order_id ); - $cpt_store->read( $cpt_order ); - $cpt_orders[ $order_id ] = $cpt_order; + + try { + $cpt_order->set_id( $order_id ); + $cpt_store->read( $cpt_order ); + $cpt_orders[ $order_id ] = $cpt_order; + } catch ( Exception $e ) { + // If the post record has been deleted (for instance, by direct query) then an exception may be thrown. + $this->error_logger->warning( + sprintf( + /* translators: %1$d order ID. */ + __( 'Unable to load the post record for order %1$d', 'woocommerce' ), + $order_id + ), + array( + 'exception_code' => $e->getCode(), + 'exception_msg' => $e->getMessage(), + 'origin' => __METHOD__, + ) + ); + } } } return $cpt_orders; @@ -1752,7 +1770,7 @@ FROM $order_meta_table $order->set_id( 0 ); // If this datastore method is called while the posts table is authoritative, refrain from deleting post data. - if ( ! is_a( $order->get_data_store(), self::class ) ) { + if ( $order->get_data_store()->get_current_class_name() !== self::class ) { return; } diff --git a/plugins/woocommerce/tests/legacy/unit-tests/rest-api/Helpers/OrderHelper.php b/plugins/woocommerce/tests/legacy/unit-tests/rest-api/Helpers/OrderHelper.php index 60edc223091..6ed934b6dfb 100644 --- a/plugins/woocommerce/tests/legacy/unit-tests/rest-api/Helpers/OrderHelper.php +++ b/plugins/woocommerce/tests/legacy/unit-tests/rest-api/Helpers/OrderHelper.php @@ -14,6 +14,7 @@ use Automattic\WooCommerce\Internal\DataStores\Orders\CustomOrdersTableControlle use Automattic\WooCommerce\Internal\DataStores\Orders\DataSynchronizer; use Automattic\WooCommerce\Internal\DataStores\Orders\OrdersTableDataStore; use Automattic\WooCommerce\Internal\Features\FeaturesController; +use WC_Data_Store; use WC_Mock_Payment_Gateway; use WC_Order; use WC_Product; @@ -249,8 +250,20 @@ class OrderHelper { */ public static function switch_data_store( $order, $data_store ) { $update_data_store_func = function ( $data_store ) { - $this->data_store = $data_store; + // Each order object contains a reference to its data store, but this reference is itself + // held inside of an instance of WC_Data_Store, so we create that first. + $data_store_wrapper = WC_Data_Store::load( 'order' ); + + // Bind $data_store to our WC_Data_Store. + ( function ( $data_store ) { + $this->current_class_name = get_class( $data_store ); + $this->instance = $data_store; + } )->call( $data_store_wrapper, $data_store ); + + // Finally, update the $order object with our WC_Data_Store( $data_store ) instance. + $this->data_store = $data_store_wrapper; }; + $update_data_store_func->call( $order, $data_store ); } 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 e64999ec1f9..c02026c4952 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -148,7 +148,7 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { } /** - * When sync is enbabled and the posts store is authoritative, creating an order and updating the status from + * When sync is enabled and the posts store is authoritative, creating an order and updating the status from * draft to some non-draft status (as happens when an order is manually created in the admin environment) should * result in the same status change being made in the duplicate COT record. */ @@ -192,4 +192,33 @@ class DataSynchronizerTests extends WC_Unit_Test_Case { 'When the order status is updated, the change should be observed by the DataSynhronizer and a matching update will take place in the COT table.' ); } + + /** + * When sync is enabled, and an order is deleted either from the post table or the COT table, the + * change should propagate across to the other table. + * + * @return void + */ + public function test_order_deletions_propagate(): 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(); + wp_delete_post( $order->get_id(), true ); + + $this->assertFalse( + wc_get_order( $order->get_id() ), + 'After the order post record was deleted, the order was also deleted from COT.' + ); + + $order = OrderHelper::create_order(); + $order_id = $order->get_id(); + $order->delete( true ); + + $this->assertNull( + get_post( $order_id ), + 'After the COT order record was deleted, the order was also deleted from the posts table.' + ); + } }