Surface checks that failed to run - #1457
Conversation
|
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.
🟢 Approval recommended
The change is small, localized to admin UI messaging, and addresses the reported false-positive completion state without introducing risky behavior.
Pull request overview
This PR updates the Plugin Check admin UI completion notice so that when one or more check AJAX requests fail (e.g., returning a non-JSON 0 response), the final notice explicitly lists the checks that did not complete instead of incorrectly implying a clean pass.
Changes:
- Adds a localized “failed checks” message string to the admin page script data.
- Tracks check slugs whose
runCheckrequest fails and surfaces them in the final results notice (forcing error severity when present).
File summaries
| File | Description |
|---|---|
| includes/Admin/Admin_Page.php | Adds a translated message template for listing checks that could not be completed. |
| assets/js/plugin-check-admin.js | Collects failed check slugs and appends them to the completion notice so “No errors found” is not shown on partial/incomplete runs. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } catch { | ||
| // Ignore for now. | ||
| failedChecks.push( checks[ i ] ); | ||
| } |
| * @param {string} template The translated format string. | ||
| * @param {...string} args Replacement values. | ||
| * @return {string} Formatted string with placeholders replaced. | ||
| */ |
| * @param {boolean} isSuccessMessage Whether the message is a success message. | ||
| * @param {Object} aiStats AI statistics. | ||
| * @param {Array} failedChecks Slugs of checks whose request did not complete. |
There was a problem hiding this comment.
Fixed in 745f4a8 — aiStats is {?Object}, failedChecks is {string[]}.
|
Thanks for your contribution. You need to fix comments and tests before we can review. |
|
Comments fixed in 745f4a8: the caught error is now logged, and the CI was red on infrastructure, not tests — The change is JS-only and the repo has no JS test setup; happy to add one if you want coverage here. |
|
Re-pushed and CI is still red, but on a repo-wide break rather than this branch: the |
A failed check request is no longer swallowed: the completion notice names the checks that did not run, instead of reporting "No errors found".
/wp-admin/tools.php?page=plugin-check— Performance category on an active plugin: notice lists the five runtime checks whose requests return 400npm run lint-jsPart of #1445.