fix: defer failed Requirement messages until translation functions are available#453
Conversation
justlevine
requested review from
Copilot and
dkotter
and removed request for
dkotter
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an initialization edge case in the plugin’s requirements checks by deferring failed-requirement message generation until {network_}admin_notices, when translation functions/text domain are available, and updates integration tests accordingly.
Changes:
- Defer requirement error-message callback invocation to admin notice render time (instead of during early checks).
- Refactor
get_admin_notice_message_html()into a static method that accepts resolved requirement results. - Update/expand
RequirementsTestto validate notice output behavior (single vs multiple failures) and the deferral behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
includes/Requirements.php |
Defers error-message generation to admin notice hooks; makes notice-message builder static and parameterized. |
tests/Integration/Includes/RequirementsTest.php |
Adjusts tests to validate notice output and ensures error-message callbacks aren’t invoked until notices render. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #453 +/- ##
=============================================
- Coverage 67.41% 66.87% -0.54%
Complexity 907 907
=============================================
Files 59 59
Lines 4352 4695 +343
=============================================
+ Hits 2934 3140 +206
- Misses 1418 1555 +137
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
github-project-automation
Bot
moved this from Needs review
to Done
in WordPress AI Planning & Roadmap
What?
wp_supports_ai()before initializing experiments #268 (comment)This PR moves the failed requirement message generation so they're triggered inside
{network_}admin_notices.RequirementsTest has been cleaned up to cover the full public API (mocking the `::$requirements) instead of testing individual methods), and to cover this particular edge case.
Why?
While we need to resolve the checks as early as possible, messages can only be generated after translation functions are loaded.
How?
::get_admin_notice_message_html()is now a static method that takes the array of::$requirementspassed inside the{network_}admin_noticescallback.Use of AI Tools
GitHub Copilot autocomplete
Testing Instructions
Screenshots or screencast
(This screenshot is with #268 applied
)
Changelog Entry
Fixed - Defer failed
Requirementsmessages until translation functions are available.