From ddd4865f070ead05100ac7e7f176292c7650765d Mon Sep 17 00:00:00 2001 From: Steve Grunwell Date: Mon, 24 Sep 2018 17:03:11 +0000 Subject: [PATCH] Set an explicit fallback ORDER BY for the migrate command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a test creates multiple test orders within the same second, the resulting products cause MySQL to try to guess which order should come first rather than — as I had assumed — falling back to sorting by ID. As a result, ordering products by p.post_date alone cannot reliably keep two orders sorted the same way, since MySQL may decide to give precedence to any one of the rows with matching timestamps. Further details are [available in MySQL's "ORDER BY Optimization" documentation](https://dev.mysql.com/doc/refman/5.5/en/order-by-optimization.html). This commit ensures that the `migrate` command explicitly falls back to sorting by `wp_posts.ID` in descending order, which matches the intent of "migrate the most recent orders (which are presumably more likely to be active) first, working backwards historically". Additionally, this commit cleans up instances where tests were passing integers instead of arrays to `TestCase::count_orders_in_table_with_ids()`, relying on type coercion. Fixes #83. --- includes/class-woocommerce-custom-orders-table-cli.php | 2 +- tests/test-cli.php | 6 +++--- tests/test-order-data-store.php | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/includes/class-woocommerce-custom-orders-table-cli.php b/includes/class-woocommerce-custom-orders-table-cli.php index 3fab82c..0390845 100644 --- a/includes/class-woocommerce-custom-orders-table-cli.php +++ b/includes/class-woocommerce-custom-orders-table-cli.php @@ -110,7 +110,7 @@ public function migrate( $args = array(), $assoc_args = array() ) { "SELECT p.ID FROM {$wpdb->posts} p LEFT JOIN " . esc_sql( $order_table ) . ' o ON p.ID = o.order_id WHERE p.post_type IN (' . implode( ', ', array_fill( 0, count( $order_types ), '%s' ) ) . ') AND o.order_id IS NULL - ORDER BY p.post_date DESC + ORDER BY p.post_date DESC, p.ID DESC LIMIT %d', array_merge( $order_types, array( $assoc_args['batch-size'] ) ) ); diff --git a/tests/test-cli.php b/tests/test-cli.php index 482c3ea..c7e12f8 100644 --- a/tests/test-cli.php +++ b/tests/test-cli.php @@ -227,11 +227,11 @@ public function test_migrate_with_duplicate_ids() { $order = wc_get_order( $order_id ); $order->get_total(); - $this->assertEquals( 1, $this->count_orders_in_table_with_ids( $order_id )); + $this->assertEquals( 1, $this->count_orders_in_table_with_ids( array( $order_id ) ) ); $this->cli->migrate(); - $this->assertEquals( 1, $this->count_orders_in_table_with_ids( $order_id )); + $this->assertEquals( 1, $this->count_orders_in_table_with_ids( array( $order_id ) ) ); } public function test_migrate_aborts_if_no_orders_require_migration() { @@ -263,7 +263,7 @@ public function test_migrate_output_when_items_were_skipped() { $this->assertEquals( 2, - $this->count_orders_in_table_with_ids( array( $order1->get_id(), $order3->get_id() ) ), + $this->count_orders_in_table_with_ids( array( $order2->get_id(), $order3->get_id() ) ), 'Expected to only see two orders in the custom table.' ); diff --git a/tests/test-order-data-store.php b/tests/test-order-data-store.php index 137aa60..857617c 100644 --- a/tests/test-order-data-store.php +++ b/tests/test-order-data-store.php @@ -13,11 +13,11 @@ public function test_loading_a_product_can_automatically_populate_from_meta() { $order_id = WC_Helper_Order::create_order()->get_id(); $this->toggle_use_custom_table( true ); - $this->assertEquals( 0, $this->count_orders_in_table_with_ids( $order_id ) ); + $this->assertEquals( 0, $this->count_orders_in_table_with_ids( array( $order_id ) ) ); $order = wc_get_order( $order_id ); - $this->assertEquals( 1, $this->count_orders_in_table_with_ids( $order->get_id() ) ); + $this->assertEquals( 1, $this->count_orders_in_table_with_ids( array( $order->get_id() ) ) ); } /** @@ -34,7 +34,7 @@ public function test_wc_custom_order_table_automatic_migration_filter() { $order = wc_get_order( $order_id ); $this->assertEmpty( $order->get_total() ); - $this->assertEquals( 0, $this->count_orders_in_table_with_ids( $order_id ) ); + $this->assertEquals( 0, $this->count_orders_in_table_with_ids( array( $order_id ) ) ); } public function test_delete() {