Fix issues when the Stripe Billing is_migrating() function would return false if the one of the actions was actively running
#7227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.41 MB ℹ️ View Unchanged
|
mattallan
approved these changes
Sep 17, 2023
There was a problem hiding this comment.
Thanks @james-allan for the quick fix.
I replicated the issue on trunk and confirmed the notices are correct while a migration is in-progress on this branch.
LGTM 💯
Because the release branch is already opened. I think we'll need to:
- Merge this into
develop - Cherry-pick the merge commit into
release/6.5.0 - Manually delete the changelog file from the release branch
james-allan
deleted the
fix/subscription-migration-in-progress-check
branch
james-allan
added a commit
that referenced
this pull request
Sep 17, 2023
…turn false if the one of the actions was actively running (#7227)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request
This PR fixes an issue I came across late last week after testing the Stripe Billing subscription migrator.
After I pressed the Begin migration button on the WooPayments Settings screen. I hit refresh a couple of minutes later and even though the migration hadn't finished yet, the Begin migration notice had returned.
After doing a deep dive I discovered it was because one of the
wcpay_migrate_subscriptionactions had thein-progressstatus (ie was actively running at the time the check was made) which causedas_next_scheduled_action()returntruerather than a timestamp which caused theis_numeric()checks inis_migrating()to return false.This PR fixes that by making sure we check for
true(is running) or an int (is scheduled).Testing instructions
sleep( 120 )into theWC_Payments_Subscriptions_Migrator::get_items_to_repair()function.developyou should see the Begin migration notice appear. Despite the migration is still technically running.pendingorin-progress.Video showcasing the bug:
https://github.com/Automattic/woocommerce-payments/assets/8490476/e75188cb-c202-4e6c-8bdf-bca0d18580db
You can also use this code snippet to observe the bug. When the action is
in-progress, this will echotrueon this branch andfalseon develop.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge