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.

Sign up for GitHub

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

Jump to bottom

[HPOS CLI] Allow --re-migrate to be used even when --verbose is not set #44669

Merged
merged 4 commits into from Mar 4, 2024
Merged

[HPOS CLI] Allow --re-migrate to be used even when --verbose is not set #44669

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2810ced
Allow `—re-migrate` to work even when `—verbose` is not set
jorgeatorres Feb 15, 2024
7b2cae7
Add changelog
jorgeatorres Feb 15, 2024
ed85320
Docblock update
jorgeatorres Feb 23, 2024
19dc888
Indicate that remigration was attempted in errors array
jorgeatorres Feb 29, 2024
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-41248
View file
Open in desktop
@@ -0,0 +1,4 @@
Significance: minor
Type: add

Allow --re-migrate to work without --verbose in HPOS CLI verification tool.
112 changes: 60 additions & 52 deletions plugins/woocommerce/src/Database/Migrations/CustomOrderTable/CLIRunner.php
View file
Open in desktop
Expand Up @@ -308,7 +308,6 @@ public function migrate( $args = array(), $assoc_args = array() ) {
*
* [--re-migrate]
* : Attempt to re-migrate orders that failed verification. You should only use this option when you have never run the site with HPOS as authoritative source of order data yet, or you have manually checked the reported errors, otherwise, you risk stale data overwriting the more recent data.
* This option can only be enabled when --verbose flag is also set.
* default: false
*
* ## EXAMPLES
Expand Down Expand Up @@ -371,6 +370,8 @@ public function verify_cot_data( $args = array(), $assoc_args = array() ) {

$progress = WP_CLI\Utils\make_progress_bar( 'Order Data Verification', $order_count / $batch_size );

$error_processing = false;

if ( ! $order_count ) {
return WP_CLI::warning( __( 'There are no orders to verify, aborting.', 'woocommerce' ) );
}
Expand Down Expand Up @@ -404,53 +405,64 @@ public function verify_cot_data( $args = array(), $assoc_args = array() ) {
$failed_ids_in_current_batch = $this->post_to_cot_migrator->verify_migrated_orders( $order_ids );
$failed_ids_in_current_batch = $this->verify_meta_data( $order_ids, $failed_ids_in_current_batch );
$failed_ids = $verbose ? array() : $failed_ids + $failed_ids_in_current_batch;
$error_processing = $error_processing || ! empty( $failed_ids_in_current_batch );
$processed += count( $order_ids );
$batch_total_time = microtime( true ) - $batch_start_time;
$batch_count ++;
$total_time += $batch_total_time;

if ( $verbose && count( $failed_ids_in_current_batch ) > 0 ) {
$errors = wp_json_encode( $failed_ids_in_current_batch, JSON_PRETTY_PRINT );
WP_CLI::warning(
sprintf(
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s. Please review the error above.',
'%1$d errors found: %2$s. Please review the errors above.',
count( $failed_ids_in_current_batch ),
'woocommerce'
),
count( $failed_ids_in_current_batch ),
$errors
)
);
if ( $remigrate ) {
if ( count( $failed_ids_in_current_batch ) > 0 ) {
if ( $verbose ) {
$errors = wp_json_encode( $failed_ids_in_current_batch, JSON_PRETTY_PRINT );
WP_CLI::warning(
sprintf(
__( 'Attempting to remigrate...', 'woocommerce' )
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s. Please review the error above.',
'%1$d errors found: %2$s. Please review the errors above.',
count( $failed_ids_in_current_batch ),
'woocommerce'
),
count( $failed_ids_in_current_batch ),
$errors
)
);
$failed_ids = array_keys( $failed_ids_in_current_batch );
$this->synchronizer->process_batch( $failed_ids );
$errors_in_remigrate_batch = $this->post_to_cot_migrator->verify_migrated_orders( $failed_ids );
$errors_in_remigrate_batch = $this->verify_meta_data( $failed_ids, $errors_in_remigrate_batch );
}

if ( $remigrate ) {
$failed_ids = $failed_ids ? array_diff_key( $failed_ids, $failed_ids_in_current_batch ) : array();
$error_processing = ( ! $verbose ) && $failed_ids;

$verbose && WP_CLI::warning( sprintf( __( 'Attempting to remigrate...', 'woocommerce' ) ) );

$failed_ids_in_current_batch_keys = array_keys( $failed_ids_in_current_batch );
$this->synchronizer->process_batch( $failed_ids_in_current_batch_keys );
$errors_in_remigrate_batch = $this->post_to_cot_migrator->verify_migrated_orders( $failed_ids_in_current_batch_keys );
$errors_in_remigrate_batch = $this->verify_meta_data( $failed_ids_in_current_batch_keys, $errors_in_remigrate_batch );

if ( count( $errors_in_remigrate_batch ) > 0 ) {
$error_processing = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, when re-migration fails, perhaps we should still show a message that re-migration was attempted but was failed. wdyt? currently, based on messages it looks like re-migration was never attempted for failed cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a remigrate_failed => true line to the errors when that happens (see 19dc888). I believe this conveys the same without us having to split errors between failed and failed after remigration, but please let me know what you think.

Example:

Screenshot 2024-02-29 at 16 14 50

$formatted_errors = wp_json_encode( $errors_in_remigrate_batch, JSON_PRETTY_PRINT );
WP_CLI::warning(
sprintf(
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s when re-migrating order. Please review the error above.',
'%1$d errors found: %2$s when re-migrating orders. Please review the errors above.',

if ( $verbose ) {
WP_CLI::warning(
sprintf(
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s when re-migrating order. Please review the error above.',
'%1$d errors found: %2$s when re-migrating orders. Please review the errors above.',
count( $errors_in_remigrate_batch ),
'woocommerce'
),
count( $errors_in_remigrate_batch ),
'woocommerce'
),
count( $errors_in_remigrate_batch ),
$formatted_errors
)
);
$formatted_errors
)
);
} else {
$failed_ids = $failed_ids + $errors_in_remigrate_batch;
}
} else {
WP_CLI::warning( 'Re-migration successful.', 'woocommerce' );
$verbose && WP_CLI::warning( 'Re-migration successful.', 'woocommerce' );
}
}
}
Expand Down Expand Up @@ -478,11 +490,7 @@ public function verify_cot_data( $args = array(), $assoc_args = array() ) {
$progress->finish();
WP_CLI::log( __( 'Verification completed.', 'woocommerce' ) );

if ( $verbose ) {
return;
}

if ( 0 === count( $failed_ids ) ) {
if ( ! $error_processing ) {
return WP_CLI::success(
sprintf(
/* Translators: %1$d is the number of migrated orders and %2$d is time taken. */
Expand All @@ -497,8 +505,6 @@ public function verify_cot_data( $args = array(), $assoc_args = array() ) {
)
);
} else {
$errors = wp_json_encode( $failed_ids, JSON_PRETTY_PRINT );

return WP_CLI::error(
sprintf(
'%1$s %2$s',
Expand All @@ -513,17 +519,19 @@ public function verify_cot_data( $args = array(), $assoc_args = array() ) {
$processed,
$total_time
),
sprintf(
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s. Please review the error above.',
'%1$d errors found: %2$s. Please review the errors above.',
$failed_ids
? sprintf(
/* Translators: %1$d is number of errors and %2$s is the formatted array of order IDs. */
_n(
'%1$d error found: %2$s. Please review the error above.',
'%1$d errors found: %2$s. Please review the errors above.',
count( $failed_ids ),
'woocommerce'
),
count( $failed_ids ),
'woocommerce'
),
count( $failed_ids ),
$errors
)
wp_json_encode( $failed_ids, JSON_PRETTY_PRINT )
)
: __( 'Please review the errors above.', 'woocommerce' )
)
);
}
Expand Down