diff --git a/plugins/woocommerce/changelog/fix-37660 b/plugins/woocommerce/changelog/fix-37660 new file mode 100644 index 00000000000..19479c9b9bf --- /dev/null +++ b/plugins/woocommerce/changelog/fix-37660 @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Use first meta value for HPOS migration when there are duplicates for flat column. diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostMetaToOrderMetaMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostMetaToOrderMetaMigrator.php index a10720d2b4f..85a8ea5555f 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostMetaToOrderMetaMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostMetaToOrderMetaMigrator.php @@ -52,6 +52,7 @@ class PostMetaToOrderMetaMigrator extends MetaToMetaTableMigrator { 'meta' => array( 'table_name' => $wpdb->postmeta, 'entity_id_column' => 'post_id', + 'meta_id_column' => 'meta_id', 'meta_key_column' => 'meta_key', 'meta_value_column' => 'meta_value', ), diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderAddressTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderAddressTableMigrator.php index a9aba4bf6dd..526bd0a7da0 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderAddressTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderAddressTableMigrator.php @@ -56,6 +56,7 @@ class PostToOrderAddressTableMigrator extends MetaToCustomTableMigrator { ), 'meta' => array( 'table_name' => $wpdb->postmeta, + 'meta_id_column' => 'meta_id', 'meta_key_column' => 'meta_key', 'meta_value_column' => 'meta_value', 'entity_id_column' => 'post_id', diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderOpTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderOpTableMigrator.php index 05f22f6d1d0..995540baaf8 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderOpTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderOpTableMigrator.php @@ -40,6 +40,7 @@ class PostToOrderOpTableMigrator extends MetaToCustomTableMigrator { ), 'meta' => array( 'table_name' => $wpdb->postmeta, + 'meta_id_column' => 'meta_id', 'meta_key_column' => 'meta_key', 'meta_value_column' => 'meta_value', 'entity_id_column' => 'post_id', diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderTableMigrator.php index e21085c62ba..8635eb36f7b 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostToOrderTableMigrator.php @@ -39,6 +39,7 @@ class PostToOrderTableMigrator extends MetaToCustomTableMigrator { ), 'meta' => array( 'table_name' => $wpdb->postmeta, + 'meta_id_column' => 'meta_id', 'meta_key_column' => 'meta_key', 'meta_value_column' => 'meta_value', 'entity_id_column' => 'post_id', diff --git a/plugins/woocommerce/src/Database/Migrations/MetaToCustomTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/MetaToCustomTableMigrator.php index 71b0d3022a4..55c3e9d1973 100644 --- a/plugins/woocommerce/src/Database/Migrations/MetaToCustomTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/MetaToCustomTableMigrator.php @@ -543,7 +543,11 @@ WHERE private function processs_and_sanitize_meta_data( array &$sanitized_entity_data, array &$error_records, array $meta_data ): void { foreach ( $meta_data as $datum ) { $column_schema = $this->meta_column_mapping[ $datum->meta_key ]; - $value = $this->validate_data( $datum->meta_value, $column_schema['type'] ); + if ( isset( $sanitized_entity_data[ $datum->entity_id ][ $column_schema['destination'] ] ) ) { + // We pick only the first meta if there are duplicates for a flat column, to be consistent with WP core behavior in handing duplicate meta which are marked as unique. + continue; + } + $value = $this->validate_data( $datum->meta_value, $column_schema['type'] ); if ( is_wp_error( $value ) ) { $error_records[ $datum->entity_id ][ $column_schema['destination'] ] = "{$value->get_error_code()}: {$value->get_error_message()}"; } else { @@ -610,7 +614,7 @@ WHERE $query = $this->build_verification_query( $source_ids ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $query should already be prepared. $results = $wpdb->get_results( $query, ARRAY_A ); - + $results = $this->fill_source_metadata( $results, $source_ids ); return $this->verify_data( $results ); } @@ -623,19 +627,13 @@ WHERE */ protected function build_verification_query( $source_ids ) { $source_table = $this->schema_config['source']['entity']['table_name']; - $meta_table = $this->schema_config['source']['meta']['table_name']; $destination_table = $this->schema_config['destination']['table_name']; - $meta_entity_id_column = $this->schema_config['source']['meta']['entity_id_column']; - $meta_key_column = $this->schema_config['source']['meta']['meta_key_column']; - $meta_value_column = $this->schema_config['source']['meta']['meta_value_column']; $destination_source_rel_column = $this->schema_config['destination']['source_rel_column']; $source_destination_rel_column = $this->schema_config['source']['entity']['destination_rel_column']; - $source_meta_rel_column = $this->schema_config['source']['entity']['meta_rel_column']; $source_destination_join_clause = "$destination_table ON $destination_table.$destination_source_rel_column = $source_table.$source_destination_rel_column"; $meta_select_clauses = array(); - $meta_join_clauses = array(); $source_select_clauses = array(); $destination_select_clauses = array(); @@ -646,31 +644,85 @@ WHERE } foreach ( $this->meta_column_mapping as $meta_key => $schema ) { - $meta_table_alias = "meta_source_{$schema['destination']}"; - $meta_select_clauses[] = "$meta_table_alias.$meta_value_column AS $meta_table_alias"; - $meta_join_clauses[] = " -$meta_table $meta_table_alias ON - $meta_table_alias.$meta_entity_id_column = $source_table.$source_meta_rel_column AND - $meta_table_alias.$meta_key_column = '$meta_key' -"; $destination_select_clauses[] = "$destination_table.{$schema['destination']} as {$destination_table}_{$schema['destination']}"; } $select_clause = implode( ', ', array_merge( $source_select_clauses, $meta_select_clauses, $destination_select_clauses ) ); - $meta_join_clause = implode( ' LEFT JOIN ', $meta_join_clauses ); - $where_clause = $this->get_where_clause_for_verification( $source_ids ); return " SELECT $select_clause FROM $source_table LEFT JOIN $source_destination_join_clause - LEFT JOIN $meta_join_clause WHERE $where_clause "; } + /** + * Fill source metadata for given IDs for verification. This will return filled data in following format: + * [ + * { + * $source_table_$source_column: $value, + * ..., + * $destination_table_$destination_column: $value, + * ... + * meta_source_{$destination_column_name1}: $meta_value, + * ... + * }, + * ... + * ] + * + * @param array $results Entity data from both source and destination table. + * @param array $source_ids List of source IDs. + * + * @return array Filled $results param with source metadata. + */ + private function fill_source_metadata( $results, $source_ids ) { + global $wpdb; + $meta_table = $this->schema_config['source']['meta']['table_name']; + $meta_entity_id_column = $this->schema_config['source']['meta']['entity_id_column']; + $meta_key_column = $this->schema_config['source']['meta']['meta_key_column']; + $meta_value_column = $this->schema_config['source']['meta']['meta_value_column']; + $meta_id_column = $this->schema_config['source']['meta']['meta_id_column']; + $meta_columns = array_keys( $this->meta_column_mapping ); + + $meta_columns_placeholder = implode( ', ', array_fill( 0, count( $meta_columns ), '%s' ) ); + $source_ids_placeholder = implode( ', ', array_fill( 0, count( $source_ids ), '%d' ) ); + + $query = $wpdb->prepare( + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + "SELECT $meta_entity_id_column as entity_id, $meta_key_column as meta_key, $meta_value_column as meta_value + FROM $meta_table + WHERE $meta_entity_id_column IN ($source_ids_placeholder) + AND $meta_key_column IN ($meta_columns_placeholder) + ORDER BY $meta_id_column ASC", + array_merge( $source_ids, $meta_columns ) + ); + //phpcs:enable + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $meta_data = $wpdb->get_results( $query, ARRAY_A ); + $source_metadata_rows = array(); + foreach ( $meta_data as $meta_datum ) { + if ( ! isset( $source_metadata_rows[ $meta_datum['entity_id'] ] ) ) { + $source_metadata_rows[ $meta_datum['entity_id'] ] = array(); + } + $destination_column = $this->meta_column_mapping[ $meta_datum['meta_key'] ]['destination']; + $alias = "meta_source_{$destination_column}"; + if ( isset( $source_metadata_rows[ $meta_datum['entity_id'] ][ $alias ] ) ) { + // Only process first value, duplicate values mapping to flat columns are ignored to be consistent with WP core. + continue; + } + $source_metadata_rows[ $meta_datum['entity_id'] ][ $alias ] = $meta_datum['meta_value']; + } + foreach ( $results as $index => $result_row ) { + $source_id = $result_row[ $this->schema_config['source']['entity']['table_name'] . '_' . $this->schema_config['source']['entity']['primary_key'] ]; + $results[ $index ] = array_merge( $result_row, ( $source_metadata_rows[ $source_id ] ?? array() ) ); + } + return $results; + } + /** * Helper function to generate where clause for fetching data for verification. * @@ -777,6 +829,9 @@ WHERE $where_clause * @return array Processed row. */ private function pre_process_row( $row, $schema, $alias, $destination_alias ) { + if ( ! isset( $row[ $alias ] ) ) { + $row[ $alias ] = $this->get_type_defaults( $schema['type'] ); + } if ( in_array( $schema['type'], array( 'int', 'decimal' ), true ) ) { if ( '' === $row[ $alias ] || null === $row[ $alias ] ) { $row[ $alias ] = 0; // $wpdb->prepare forces empty values to 0. @@ -798,10 +853,6 @@ WHERE $where_clause $row[ $destination_alias ] = null; } } - if ( is_null( $row[ $alias ] ) ) { - $row[ $alias ] = $this->get_type_defaults( $schema['type'] ); - } - return $row; } diff --git a/plugins/woocommerce/tests/php/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationControllerTest.php b/plugins/woocommerce/tests/php/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationControllerTest.php index 739d518724f..072125136fb 100644 --- a/plugins/woocommerce/tests/php/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationControllerTest.php +++ b/plugins/woocommerce/tests/php/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationControllerTest.php @@ -747,6 +747,34 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in $this->assertEmpty( $errors ); } + /** + * @testDox When there are mutli meta values for a supposed unique meta key, the first one is picked. + */ + public function test_first_value_is_picked_when_multi_value() { + global $wpdb; + $order = wc_get_order( OrderHelper::create_complex_wp_post_order() ); + $original_order_key = $order->get_order_key(); + + $this->assertNotEmpty( $original_order_key ); + + // Add a second order key. + add_post_meta( $order->get_id(), '_order_key', 'second_order_key_should_be_ignored' ); + + $this->sut->migrate_order( $order->get_id() ); + + $migrated_order_key = $wpdb->get_var( + $wpdb->prepare( + "SELECT order_key FROM {$wpdb->prefix}wc_order_operational_data WHERE order_id = %d", + $order->get_id() + ) + ); + + $this->assertEquals( $original_order_key, $migrated_order_key ); + + $errors = $this->sut->verify_migrated_orders( array( $order->get_id() ) ); + $this->assertEmpty( $errors ); + } + /** * @testDox Test migration for multiple null order_key meta value. */ @@ -760,7 +788,6 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in $this->sut->migrate_order( $order2->get_id() ); $errors = $this->sut->verify_migrated_orders( array( $order1->get_id(), $order2->get_id() ) ); - $this->assertEmpty( $errors ); } }