Fix and improve admin child pages redirections logic #9400
Merged
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.33 MB ℹ️ View Unchanged
|
dmallory42
approved these changes
Sep 6, 2024
mordeth
approved these changes
Sep 6, 2024
There was a problem hiding this comment.
Thanks for working on it, Vlad!
The $force_refresh parameter was introduced with the Test-drive PR, and I have thoroughly tested it through the following scenarios:
- Onboarding a test-drive account locally with both Jetpack connected and Dev Tools enabled.
- Onboarding a test-drive account locally with Jetpack disconnected and Dev Tools enabled.
- Onboarding a test-drive account on a JN site with Jetpack connected and without Dev Tools.
- Onboarding a test-drive account on a JN site with Jetpack disconnected and without Dev Tools.
✅ Code changes are looking good!
vladolaru
deleted the
fix/9395-account-data-force-refresh-on-wcpay-pages
branch
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.
Fixes #9395
Changes proposed in this Pull Request
We match the logic from
WC_Payments_Admin::maybe_redirect_from_payments_admin_child_pages()with the one inWC_Payments_Account::maybe_redirect_from_overview_page()for a more robust logic and consistent UX. Themaybe_redirect_from_payments_admin_child_pages()action hook is now called after the redirections action hooks inWC_Payments_Account(those have a priority of 15, we use 16).At the same time, we eliminate the erroneous
is_stripe_connected()call with the$force_refreshparameter set totrue.Testing instructions
Pre-testing setup
US)npm run tube:start) so you will be able to set up your Jetpack connectiontrunkof the WCPay Dev ToolsTesting broken setup
/wp-admin/admin.php?page=wc-admin&path=/payments/overviewand you should be redirected to the Connect page with an error notice at the top:/wp-admin/admin.php?page=wc-admin&path=/payments/depositsand you should be redirected to the Connect page with an error notice at the top:/wp-admin/admin.php?page=wc-admin&path=/payments/transactionsand you should be redirected to the Connect page with an error notice at the top./wp-admin/admin.php?page=wc-admin&path=/payments/disputesand you should be redirected to the Connect page with an error notice at the top./wp-admin/admin.php?page=wc-admin&path=/payments/overviewand you should be redirected to the Connect page with an error notice at the top:/wp-admin/admin.php?page=wc-admin&path=/payments/depositsand you should be redirected to the Connect page with an error notice at the top:/wp-admin/admin.php?page=wc-admin&path=/payments/transactionsand you should be redirected to the Connect page with an error notice at the top./wp-admin/admin.php?page=wc-admin&path=/payments/disputesand you should be redirected to the Connect page with an error notice at the top.Testing working setup
enabled/completeaccount on our Payments > Overview pagenpm 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