Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent backup post from being deleted when HPOS is authoritative #45330

Merged
merged 5 commits into from Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions 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.
Expand Up @@ -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 );
Expand Down Expand Up @@ -901,6 +902,26 @@ public function get_description(): string {
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.
*
Expand Down
Expand Up @@ -206,13 +206,24 @@ public function test_status_syncs_correctly_after_order_creation() {
* 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' );

// 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.'
Expand Down