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.

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

Fix PHP notice when outside of orders screens #38641

Merged
merged 4 commits into from Jun 7, 2023

Conversation

jorgeatorres
Copy link
Member

Submission Review Guidelines:

Changes proposed in this Pull Request:

After merging #38617 I noticed a PHP notice started appearing on all screens:

Screenshot 2023-06-07 at 16 15 49

While #38617 made this evident, this is IMHO a failure in our orders page controller, since its logic is executed on every page even when not viewing an orders-related screen.

This PR addresses this by returning early if we're not on an admin page with wc-orders in its slug.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Check out trunk.
  2. Visit any page outside of the Orders screen.
  3. Confirm that you see the PHP notice mentioned above:
  4. Visit WC > Orders and confirm that the notice no longer appears.
  5. Repeat the above steps on this branch, and confirm that the notice does not appear at any point.

@jorgeatorres jorgeatorres requested review from a team and rrennick and removed request for a team June 7, 2023 19:20
@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Jun 7, 2023
@jorgeatorres jorgeatorres added focus: custom order tables / HPOS Issues related to High-Performance Order Storage (HPOS) née Custom Order Tables. and removed focus: react admin [team:Ghidorah] labels Jun 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Hi @coreymckrill,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Test Results Summary

Commit SHA: d21cd0b

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 49s
E2E Tests1950010020515m 54s

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.

@coreymckrill coreymckrill requested review from coreymckrill and removed request for rrennick June 7, 2023 20:41
coreymckrill
coreymckrill previously approved these changes Jun 7, 2023
Copy link
Collaborator

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

👍 Approving, but the extraneous files should be removed before merging. Thanks again for catching this notice @jorgeatorres

I also noticed from the PR screenshot that I forgot to update the version placeholder in the two wc_doing_it_wrong calls (currently x.x.x). Would it make sense to update those here? If not, I'll just open a separate PR.

plugins/woocommerce/phpstan.neon.dist Outdated Show resolved Hide resolved
plugins/woocommerce/phpstan.txt Outdated Show resolved Hide resolved
@jorgeatorres
Copy link
Member Author

jorgeatorres commented Jun 7, 2023

Hey @coreymckrill! Thanks for the review. I did push some files by mistake, but those have been removed now. I've also updated the 'x.x.x' version numbers per your request.

You'll have to review again, though, as the change dismissed the previous review. Thanks again!

@coreymckrill coreymckrill merged commit ef3168a into trunk Jun 7, 2023
17 of 18 checks passed
@coreymckrill coreymckrill deleted the fix/order-type-notice-outside-orders branch June 7, 2023 23:49
@github-actions github-actions bot added this to the 7.9.0 milestone Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: custom order tables / HPOS Issues related to High-Performance Order Storage (HPOS) née Custom Order Tables. plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants