WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 4 weeks ago

#48105 assigned enhancement

Move Site Health async checks to a REST API endpoint

Reported by: Clorith Owned by: Clorith
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: has-patch
Focuses: rest-api Cc:

Description

Currently various tests are ran asyncronously with a call to admin-ajax.php, which has some limitations, specifically in core filters not being loaded when using old style ajax calls (see the loopback to wp-admin/site-health.php?health-check-test-wp_version_check to verify the existence of the wp_version_check filter).

Moving things out into REST API endpoints would provide more flexibility, and better handling of permission checks as well. It also means that any code extending tests and adding their own async checks would have a much ore robust platform to test against, if they choose to utilize it.

There's some backwards compatibility considerations to be kept in mind, the current implementation of calling an admin-ajax call needs to be maintained, likely by introducing a has_rest => true flag to the async test registration to decide where to forward the call.

Attachments (4)

48105.patch (12.5 KB) - added by Clorith 11 months ago.
48105.2.patch (20.4 KB) - added by Clorith 8 months ago.
48105.3.patch (22.0 KB) - added by Clorith 6 weeks ago.
test-unavailable.PNG (43.7 KB) - added by Clorith 6 weeks ago.

Download all attachments as: .zip

Change History (28)

@Clorith
11 months ago

#1 @Clorith
11 months ago

48105.patch is a first iteration of the above, it retains backwards compatibility for async tests created with admin-ajax.php in mind, but switches to using a REST endpoint if has_rest is defined and set to true.

It also adds failure handling to the async tests, so that if a test fails, it shouldn't break the JS from continuing with the next available check.

There's still some cleaning needed before this is ready (removing references to the old ajax hooks)

@Clorith
8 months ago

#2 @Clorith
8 months ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 5.4

48105.2.patch provides REST endpoints for the previous ajax calls for the core provided asynchronous Site Health tests.

It also removed the now unused ajax handlers, and provides deprecation notices, and _doing_it_wrong calls to the callback functions from those ajax calls. I don't believe anyone is calling those functions directly, but better safe than sorry, as the saying goes.

It also improves on the failure handling for those tests, if a test fails, a notice will now be displayed in the recommended section, the badge has the label Unavailable, with a red outline, as opposed to the normal blue used by core for all other tests.

The URL invoked to perform the test is provided, and either the string No details available (as a fallback), or the error returned by the site.

#3 @TimothyBlynJacobs
7 months ago

I think this would be a great time to take a look at using a dashboard/v1 namespace for these dashboard endpoints that aren't modeling WP data objects. https://github.com/WP-API/proposals/pull/1

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-site-health by timothybjacobs. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-restapi by clorith. View the logs.


6 months ago

This ticket was mentioned in Slack in #core-site-health by clorith. View the logs.


6 months ago

#9 @Clorith
6 months ago

  • Milestone changed from 5.4 to 5.5

Bumping this to the 5.5. milestone, there's still discussions happening in relation to namespaces on the REST API, which I don't think it's realistic there will be a decision on before beta-1 for 5.4, and this enhancement is nice to have, but not critical.

#10 @afragen
5 months ago

@Clorith there's a typo in the translators comment taht. Copy/pasted several times. ;)

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


3 months ago

#14 @afragen
3 months ago

  • Keywords needs-refresh added

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


3 months ago

#17 @TimothyBlynJacobs
2 months ago

  • Owner set to Clorith
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


6 weeks ago

@Clorith
6 weeks ago

#20 @Clorith
6 weeks ago

  • Keywords needs-refresh removed

48105.3.patch is what I hope is the final iteration here.

It removes the now redundant REST API test, as the asynchronous tests use the REST endpoint and will display a warning if they fail (see comment:2 and test-unavailable.PNG), this makes the original test no longer useful in this scenario (although I know there's a ticket to introduce JavaScript-only tests to account for edge cases here, but that's out of scope for the upcoming release).

A new field is introduces to the site_status_tests filter, named $has_rest, if this is a truthy value, then the $test check is fired off as a REST call instead of passing it as an action to admin-ajax.

The space-calculation from the Info screen has also been moved to use this, to remove _all_ Site Health dependencies on admin-ajax for consistency.

A new filter also comes along with this named site_health_test_rest_capability_{$check}, this will allow developers to change what capability is needed to access the various tests, using the existing view_site_health_checks as a default.

#21 @Clorith
5 weeks ago

  • Keywords commit added

#22 @TimothyBlynJacobs
5 weeks ago

@Clorith can you turn this into a PR? I have some line-to-line feedback.

This ticket was mentioned in PR #381 on WordPress/wordpress-develop by Clorith.


5 weeks ago

See [trac ticket comments](https://core.trac.wordpress.org/ticket/48105#comment:20) for changes in the initial PR

Trac ticket: https://core.trac.wordpress.org/ticket/48105

#24 @whyisjake
4 weeks ago

  • Keywords commit removed
  • Milestone changed from 5.5 to 5.6

Would love to get this into the 5.5 beta, but not sure about the timing here. Checked in with @TimothyBlynJacobs, and going to punt. There is a lot of feedback to be addressed in the PR.

Note: See TracTickets for help on using tickets.