[HPOS CLI] Allow --re-migrate to be used even when --verbose is not set
#44669
Conversation
github-actions
bot
added
the
plugin: woocommerce
jorgeatorres
force-pushed
the
fix/41248
branch
from
1e629eb
to
9eabd33
Compare
Test Results SummaryCommit SHA: 19dc888
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
jorgeatorres
requested review from
a team and
vedanshujain
and removed request for
a team
|
Hi @vedanshujain, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
jorgeatorres
force-pushed
the
fix/41248
branch
from
49b2531
to
6f4480d
Compare
jorgeatorres
force-pushed
the
fix/41248
branch
from
6f4480d
to
ed85320
Compare
| if ( count( $errors_in_remigrate_batch ) > 0 ) { | ||
| $error_processing = true; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
|
Hey @vedanshujain! Thanks for the review. I've added a quick fix for what you mentioned. Let me know if that works. |
github-actions
bot
added
the
needs: analysis
…ot set (#44669) * Allow `—re-migrate` to work even when `—verbose` is not set * Add changelog * Docblock update * Indicate that remigration was attempted in errors array
alvarothomas
added
status: analysis complete
…ot set (#44669) * Allow `—re-migrate` to work even when `—verbose` is not set * Add changelog * Docblock update * Indicate that remigration was attempted in errors array
Submission Review Guidelines:
Changes proposed in this Pull Request:
Currently, while verifying order data via the CLI tool
wp cot verify_cot_data, the--re-migrateflag (which attempts to sync the order again if verification fails) requires the use of--verbosetoo.While this isn't necessarily problematic, it'd be best to support
--re-migrateregardless of the--verboseflag to reduce confusion. This PR makes that possible and also allows the tool to display completion messages ("M orders were verified in N seconds") even when verbosity is enabled.Closes #41248.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
wp wc cot syncto sync orders.wp wc cot verify_cot_dataand make sure no verification errors are indicated. This way we make sure we're starting at a "no verification errors" state.If there are, in fact, errores reported for any order, you should be able to address those manually by running
wp hpos backfill <order_id> --from=posts --to=hpos.wp wc cot verify_cot_data --verboseand make sure the completion message appears ("Success: X orders were verified in Y seconds.")wp post meta update <order id> some_random_meta_name 123wp post meta update <anothe order id> _billing_first_name "Kip Stephen Thorne"wp wc cot verify_cot_dataand confirm that the differences introduced above are reported as errors.wp wc cot verify_cot_data --re-migrateand confirm that no errores are reported. Note that we didn't have to use--verbosefor it to work.Changelog entry
Significance
Type
Message
Comment