From f229c3d7656f11ea14045cb295ab8ac23ae87af0 Mon Sep 17 00:00:00 2001 From: "Jorge A. Torres" Date: Thu, 7 Mar 2024 19:24:36 -0500 Subject: [PATCH] Prevent backup post from being deleted when HPOS is authoritative (#45330) This PR short-circuits wp_delete_post() so that when HPOS is authoritative, any attempts to delete the backup post are not successful (which could also end up deleting the HPOS order). This only applies to non placeholder posts, as placeholder posts won't trigger the cascade of operations that might be problematic here. Fixes #42746 --- plugins/woocommerce/changelog/fix-42746 | 4 ++++ .../DataStores/Orders/DataSynchronizer.php | 21 +++++++++++++++++++ .../Orders/DataSynchronizerTests.php | 15 +++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 plugins/woocommerce/changelog/fix-42746 diff --git a/plugins/woocommerce/changelog/fix-42746 b/plugins/woocommerce/changelog/fix-42746 new file mode 100644 index 00000000000..0024c8d17be --- /dev/null +++ b/plugins/woocommerce/changelog/fix-42746 @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Add some safeguards against programmatic removal of orders due to sync when HPOS is active. diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php index 4ceebfaf4ca..c1a3edc61aa 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/DataSynchronizer.php @@ -105,6 +105,7 @@ class DataSynchronizer implements BatchProcessorInterface { * Class constructor. */ public function __construct() { + self::add_filter( 'pre_delete_post', array( $this, 'maybe_prevent_deletion_of_post' ), 10, 2 ); self::add_action( 'deleted_post', array( $this, 'handle_deleted_post' ), 10, 2 ); self::add_action( 'woocommerce_new_order', array( $this, 'handle_updated_order' ), 100 ); self::add_action( 'woocommerce_refund_created', array( $this, 'handle_updated_order' ), 100 ); @@ -901,6 +902,26 @@ ORDER BY orders.id ASC return 'Synchronizes orders between posts and custom order tables.'; } + /** + * Prevents deletion of order backup posts (regardless of sync setting) when HPOS is authoritative and the order + * still exists in HPOS. + * This should help with edge cases where wp_delete_post() would delete the HPOS record too or backfill would sync + * incorrect data from an order with no metadata from the posts table. + * + * @since 8.8.0 + * + * @param WP_Post|false|null $delete Whether to go forward with deletion. + * @param WP_Post $post Post object. + * @return WP_Post|false|null + */ + private function maybe_prevent_deletion_of_post( $delete, $post ) { + if ( self::PLACEHOLDER_ORDER_POST_TYPE !== $post->post_type && $this->custom_orders_table_is_authoritative() && $this->data_store->order_exists( $post->ID ) ) { + $delete = false; + } + + return $delete; + } + /** * Handle the 'deleted_post' action. * 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 52a4eeab8c6..177138d071d 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/DataSynchronizerTests.php @@ -206,13 +206,24 @@ class DataSynchronizerTests extends HposTestCase { * change should propagate across to the other table. */ public function test_order_deletions_propagate_with_sync_enabled(): void { - // Sync enabled and COT authoritative. + // Sync enabled. update_option( $this->sut::ORDERS_DATA_SYNC_ENABLED_OPTION, 'yes' ); - update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); + // COT authoritative. + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); + $order = OrderHelper::create_order(); + + // Deleting the backup post while COT is authoritative shouldn't delete the COT version. + wp_delete_post( $order->get_id(), true ); + $this->assertNotFalse( wc_get_order( $order->get_id() ) ); + + // Deleting the backup post while posts is authoritative should delete everything. + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'no' ); $order = OrderHelper::create_order(); wp_delete_post( $order->get_id(), true ); + $this->assertFalse( wc_get_order( $order->get_id() ) ); + update_option( CustomOrdersTableController::CUSTOM_ORDERS_TABLE_USAGE_ENABLED_OPTION, 'yes' ); $this->assertFalse( wc_get_order( $order->get_id() ), 'After the order post record was deleted, the order was also deleted from COT.'