From 7b5b6420fe651c440bb848762e4c1aa501203251 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Wed, 10 May 2023 12:47:59 +0200 Subject: [PATCH 1/4] Fix the child order upshifting when parent order is deleted PR 36218 implemented child order upshifting (setting their parent order id to 0) when the parent order is deleted, but this needs to happen only when the post type of the parent order is hierarchical, otherwise child orders need to just be deleted. This commit fixes that. --- .../Orders/OrdersTableDataStore.php | 49 ++++++++++++++----- .../Orders/OrdersTableDataStoreTests.php | 47 ++++++++++++++++-- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index f9d15c1e53a..91260fe4d3d 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -118,6 +118,13 @@ class OrdersTableDataStore extends \Abstract_WC_Order_Data_Store_CPT implements */ private $error_logger; + /** + * The instance of the LegacyProxy object to use. + * + * @var LegacyProxy + */ + private $legacy_proxy; + /** * Initialize the object. * @@ -131,6 +138,7 @@ class OrdersTableDataStore extends \Abstract_WC_Order_Data_Store_CPT implements final public function init( OrdersTableDataStoreMeta $data_store_meta, DatabaseUtil $database_util, LegacyProxy $legacy_proxy ) { $this->data_store_meta = $data_store_meta; $this->database_util = $database_util; + $this->legacy_proxy = $legacy_proxy; $this->error_logger = $legacy_proxy->call_function( 'wc_get_logger' ); $this->internal_meta_keys = $this->get_internal_meta_keys(); } @@ -1785,7 +1793,7 @@ FROM $order_meta_table */ do_action( 'woocommerce_before_delete_order', $order_id, $order ); - $this->upshift_child_orders( $order ); + $this->upshift_or_delete_child_orders( $order ); $this->delete_order_data_from_custom_order_tables( $order_id ); $this->delete_items( $order ); @@ -1823,23 +1831,40 @@ FROM $order_meta_table } /** - * Helper method to set child orders to the parent order's parent. + * Set the parent id of child orders to the parent order's parent if the post type + * for the order is hierarchical, just delete the child orders otherwise. * * @param \WC_Abstract_Order $order Order object. * * @return void */ - private function upshift_child_orders( $order ) { + private function upshift_or_delete_child_orders( $order ) { global $wpdb; - $order_table = self::get_orders_table_name(); - $order_parent = $order->get_parent_id(); - $wpdb->update( - $order_table, - array( 'parent_order_id' => $order_parent ), - array( 'parent_order_id' => $order->get_id() ), - array( '%d' ), - array( '%d' ) - ); + + $order_table = self::get_orders_table_name(); + $order_parent_id = $order->get_parent_id(); + + if ( $this->legacy_proxy->call_function( 'is_post_type_hierarchical', $order->get_type() ) ) { + $wpdb->update( + $order_table, + array( 'parent_order_id' => $order_parent_id ), + array( 'parent_order_id' => $order->get_id() ), + array( '%d' ), + array( '%d' ) + ); + } else { + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared + $child_order_ids = $wpdb->get_col( + $wpdb->prepare( + "SELECT id FROM $order_table WHERE parent_order_id=%d", + $order->get_id() + ) + ); + foreach ( $child_order_ids as $child_order_id ) { + $this->delete_order_data_from_custom_order_tables( $child_order_id ); + } + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared + } } /** diff --git a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php index 913f40bc591..539d3fb6679 100644 --- a/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php +++ b/plugins/woocommerce/tests/php/src/Internal/DataStores/Orders/OrdersTableDataStoreTests.php @@ -50,6 +50,7 @@ class OrdersTableDataStoreTests extends WC_Unit_Test_Case { * Initializes system under test. */ public function setUp(): void { + $this->reset_legacy_proxy_mocks(); $this->original_time_zone = wp_timezone_string(); //phpcs:ignore WordPress.DateTime.RestrictedFunctions.timezone_change_date_default_timezone_set -- We need to change the timezone to test the date sync fields. update_option( 'timezone_string', 'Asia/Kolkata' ); @@ -58,8 +59,10 @@ class OrdersTableDataStoreTests extends WC_Unit_Test_Case { $this->setup_cot(); $this->cot_state = OrderUtil::custom_orders_table_usage_is_enabled(); $this->toggle_cot( false ); - $this->sut = wc_get_container()->get( OrdersTableDataStore::class ); - $this->migrator = wc_get_container()->get( PostsToOrdersMigrationController::class ); + $container = wc_get_container(); + $container->reset_all_resolved(); + $this->sut = $container->get( OrdersTableDataStore::class ); + $this->migrator = $container->get( PostsToOrdersMigrationController::class ); $this->cpt_data_store = new WC_Order_Data_Store_CPT(); } @@ -2020,9 +2023,17 @@ class OrdersTableDataStoreTests extends WC_Unit_Test_Case { } /** - * @testDox When parent order is deleted, child orders should be upshifted. + * @testDox When parent order is deleted, and the post order type is hierarchical, child orders should be upshifted. */ - public function test_child_orders_are_promoted_when_parent_is_deleted() { + public function test_child_orders_are_promoted_when_parent_is_deleted_if_order_type_is_hierarchical() { + $this->register_legacy_proxy_function_mocks( + array( + 'is_post_type_hierarchical' => function( $post_type ) { + return 'shop_order' === $post_type || is_post_type_hierarchical( $post_type ); + }, + ) + ); + $this->toggle_cot( true ); $order = new WC_Order(); $order->save(); @@ -2038,6 +2049,34 @@ class OrdersTableDataStoreTests extends WC_Unit_Test_Case { $this->assertEquals( 0, $child_order->get_parent_id() ); } + /** + * @testDox When parent order is deleted, and the post order type is NOT hierarchical, child orders should be deleted. + */ + public function test_child_orders_are_promoted_when_parent_is_deleted_if_order_type_is_not_hierarchical() { + $this->register_legacy_proxy_function_mocks( + array( + 'is_post_type_hierarchical' => function( $post_type ) { + return 'shop_order' === $post_type ? false : is_post_type_hierarchical( $post_type ); + }, + ) + ); + + $this->toggle_cot( true ); + $order = new WC_Order(); + $order->save(); + + $child_order = new WC_Order(); + $child_order->set_parent_id( $order->get_id() ); + $child_order->save(); + + $this->assertEquals( $order->get_id(), $child_order->get_parent_id() ); + $this->sut->delete( $order, array( 'force_delete' => true ) ); + $child_order = wc_get_order( $child_order->get_id() ); + + $this->assertFalse( $child_order ); + } + + /** * @testDox Make sure get_order return false when checking an order of different order types without warning. */ From 2ff1c70f9d2607948ea749351f5e9bdab28c941d Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Wed, 10 May 2023 12:55:14 +0200 Subject: [PATCH 2/4] Add changelog file --- plugins/woocommerce/changelog/fix-order-upshifting | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 plugins/woocommerce/changelog/fix-order-upshifting diff --git a/plugins/woocommerce/changelog/fix-order-upshifting b/plugins/woocommerce/changelog/fix-order-upshifting new file mode 100644 index 00000000000..42ea7acfddb --- /dev/null +++ b/plugins/woocommerce/changelog/fix-order-upshifting @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Fix: when order is deleted child orders should be deleted too, not set to parent id 0, unless the post type for the order is hierarchical From a0a2390d9e1bc252f8497924943fe36ee41dc28a Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Thu, 11 May 2023 12:48:21 +0200 Subject: [PATCH 3/4] Delete child orders using existing functions instead of direct db access. Additionally, add get/set_verify_parent_id methods to the order object. --- .../includes/abstracts/abstract-wc-order.php | 35 +++++++++++++++++-- .../Orders/OrdersTableDataStore.php | 8 ++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php index 7ba29926df4..98ef3d07af6 100644 --- a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php +++ b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php @@ -103,6 +103,14 @@ abstract class WC_Abstract_Order extends WC_Abstract_Legacy_Order { */ protected $object_type = 'order'; + /** + * Indicates if set_parent_id will throw an error if no order exists with the supplied id. + * + * @since 7.8.0 + * @var bool + */ + private $verify_parent_id = true; + /** * Get the order if ID is passed, otherwise the order is new and empty. * This class should NOT be instantiated, but the wc_get_order function or new WC_Order_Factory @@ -608,15 +616,38 @@ abstract class WC_Abstract_Order extends WC_Abstract_Legacy_Order { * * @since 3.0.0 * @param int $value Value to set. - * @throws WC_Data_Exception Exception thrown if parent ID does not exist or is invalid. + * @throws WC_Data_Exception Exception thrown if parent ID does not exist or is invalid (only if $verify_parent_id is true). */ public function set_parent_id( $value ) { - if ( $value && ( $value === $this->get_id() || ! wc_get_order( $value ) ) ) { + if ( $this->verify_parent_id && $value && ( $value === $this->get_id() || ! wc_get_order( $value ) ) ) { $this->error( 'order_invalid_parent_id', __( 'Invalid parent ID', 'woocommerce' ) ); } $this->set_prop( 'parent_id', absint( $value ) ); } + /** + * Sets the value of the $verify_parent_id flag. + * + * If the flag is set, set_parent_id will throw an error if no order exists with the supplied id. + * + * @param bool $value The value to set. + * @return void + */ + public function set_verify_parent_id( bool $value ) { + $this->verify_parent_id = $value; + } + + /** + * Gets the value of the $verify_parent_id flag. + * + * If the flag is set, set_parent_id will throw an error if no order exists with the supplied id. + * + * @return bool + */ + public function get_verify_parent_id(): bool { + return $this->verify_parent_id; + } + /** * Set order status. * diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 91260fe4d3d..46cb5b807cc 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -1860,8 +1860,14 @@ FROM $order_meta_table $order->get_id() ) ); + foreach ( $child_order_ids as $child_order_id ) { - $this->delete_order_data_from_custom_order_tables( $child_order_id ); + // We can't use wc_get_order here because we would get a "Invalid parent ID" error + // (and we don't need to load the full order from the database anyway). + $child_order = new \WC_Order(); + $child_order->set_verify_parent_id( false ); + $child_order->set_id( $child_order_id ); + $child_order->delete( true ); } // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared } From 6ed8fb4e4c8e7e5107debd666ebe3443a69fd438 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 12 May 2023 15:32:41 +0200 Subject: [PATCH 4/4] Delete child orders using wc_get_order->delete Also remove get/set_verify_parent_id methods as in the end they don't seem to be needed. --- .../includes/abstracts/abstract-wc-order.php | 35 ++----------------- .../Orders/OrdersTableDataStore.php | 12 +++---- 2 files changed, 7 insertions(+), 40 deletions(-) diff --git a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php index 98ef3d07af6..7ba29926df4 100644 --- a/plugins/woocommerce/includes/abstracts/abstract-wc-order.php +++ b/plugins/woocommerce/includes/abstracts/abstract-wc-order.php @@ -103,14 +103,6 @@ abstract class WC_Abstract_Order extends WC_Abstract_Legacy_Order { */ protected $object_type = 'order'; - /** - * Indicates if set_parent_id will throw an error if no order exists with the supplied id. - * - * @since 7.8.0 - * @var bool - */ - private $verify_parent_id = true; - /** * Get the order if ID is passed, otherwise the order is new and empty. * This class should NOT be instantiated, but the wc_get_order function or new WC_Order_Factory @@ -616,38 +608,15 @@ abstract class WC_Abstract_Order extends WC_Abstract_Legacy_Order { * * @since 3.0.0 * @param int $value Value to set. - * @throws WC_Data_Exception Exception thrown if parent ID does not exist or is invalid (only if $verify_parent_id is true). + * @throws WC_Data_Exception Exception thrown if parent ID does not exist or is invalid. */ public function set_parent_id( $value ) { - if ( $this->verify_parent_id && $value && ( $value === $this->get_id() || ! wc_get_order( $value ) ) ) { + if ( $value && ( $value === $this->get_id() || ! wc_get_order( $value ) ) ) { $this->error( 'order_invalid_parent_id', __( 'Invalid parent ID', 'woocommerce' ) ); } $this->set_prop( 'parent_id', absint( $value ) ); } - /** - * Sets the value of the $verify_parent_id flag. - * - * If the flag is set, set_parent_id will throw an error if no order exists with the supplied id. - * - * @param bool $value The value to set. - * @return void - */ - public function set_verify_parent_id( bool $value ) { - $this->verify_parent_id = $value; - } - - /** - * Gets the value of the $verify_parent_id flag. - * - * If the flag is set, set_parent_id will throw an error if no order exists with the supplied id. - * - * @return bool - */ - public function get_verify_parent_id(): bool { - return $this->verify_parent_id; - } - /** * Set order status. * diff --git a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php index 46cb5b807cc..26329e90a71 100644 --- a/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php +++ b/plugins/woocommerce/src/Internal/DataStores/Orders/OrdersTableDataStore.php @@ -1860,16 +1860,14 @@ FROM $order_meta_table $order->get_id() ) ); + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared foreach ( $child_order_ids as $child_order_id ) { - // We can't use wc_get_order here because we would get a "Invalid parent ID" error - // (and we don't need to load the full order from the database anyway). - $child_order = new \WC_Order(); - $child_order->set_verify_parent_id( false ); - $child_order->set_id( $child_order_id ); - $child_order->delete( true ); + $child_order = wc_get_order( $child_order_id ); + if ( $child_order ) { + $child_order->delete( true ); + } } - // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared } }