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
This commit is contained in:
Jorge A. Torres 2024-03-07 19:24:36 -05:00 committed by GitHub
parent ed6c7ea1e9
commit f229c3d765
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 38 additions and 2 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: fix
Add some safeguards against programmatic removal of orders due to sync when HPOS is active.

View File

@ -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.
*

View File

@ -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.'