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.
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
Handle posts > HPOS redirect more gracefully when backup post is missing #45605
Conversation
Test Results SummaryCommit SHA: 46727c7
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. |
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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left couple of minor comments, lgtm otherwise
plugins/woocommerce/src/Internal/Admin/Orders/PostsRedirectionController.php
Outdated
Show resolved
Hide resolved
@vedanshujain: Thanks for the review! I've implemented your suggestion and simplified the logic a bit. Please let me know what you think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, LGTM!
Submission Review Guidelines:
Changes proposed in this Pull Request:
When HPOS is authoritative, we automatically redirect admin URLs for order posts to the corresponding HPOS one. For example,
/wp-admin/post.php?post=123&action=edit
gets turned into/wp-admin/admin.php?page=wc-orders&action=edit&id=123
.This works correctly when the order post exists, but was producing a 500 error when not.
This is sort of an edge case as we don't endorse removal of the backup post / placeholder post, but we've seen some users doing that after migrating to HPOS anyways.
This PR adds an additional check so that the redirect succeeds in those cases (as long as the HPOS order exists).
Closes #44177.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
https://yoursite.test/wp-admin/post.php?post=<order_id>&action=edit
and confirm that you're redirected to the (HPOS) order edit screen.https://yoursite.test/wp-admin/post.php?post=<post_id>&action=edit
with a<post_id>
that has no corresponding post and confirm that you see an error.Changelog entry
Significance
Type
Message
Comment