Move Site Health async checks to a REST API endpoint

[Clorith]

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.

[Clorith]

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]

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.

[TimothyBlynJacobs]

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

[Clorith]

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.

[afragen]

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

[esoj]

Namespace decision is found here: ​https://make.wordpress.org/core/2020/04/23/rest-api-decision-on-introducing-a-dashboard-namespace/

[Clorith]

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.

[TimothyBlynJacobs]

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

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

[whyisjake]

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.

[hellofromTonya]

Looks like this one needs a refresh.

Thanks for refreshing @Clorith! Can we make the REST API calls use wp.apiRequest?

Sorry, I think the last thing. Do we need each REST API endpoint to wrap its results in the data property? We don't typically do this. Looking at the JS it looks like it passes on response.data, can we then just change that to response?

Sorry, I think the last thing. Do we need each REST API endpoint to wrap its results in the data property? We don't typically do this. Looking at the JS it looks like it passes on response.data, can we then just change that to response?

Good catch, in my head I was trying to maintain the same format as the admin-ajax calls had when using wp_send_json_success for the sake of the filters in place, but the filters take just the response data any way, and not the whole response object, so this shouldn't need ot be mimiced.

@Clorith Can you rebase this please?

Well, that rebase was an absolute horror, let's try that again, this time with feeling (and less branch-bleed) 😔 !

Thanks @Clorith!

Looks like there is a relevant test failure:

1) Tests_User_Capabilities::testMetaCapsTestsAreCorrect
These meta capabilities are not tested
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 ()
+Array &0 (
+    30 => 'view_site_health_check'
+    31 => 'view_site_health_debug_info'
+)

Thanks @Clorith!

Looks like there is a relevant test failure:

Yeah, the rebase went a bit nuts on me and added some ide configs, and commits from a different branch and I've no idea what exploded, but exploded it did, just doing a reset and re-rebase here to clean it up :)

See trac ticket comments for changes in the initial PR

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

This is a clean branch, after a rather large rebase went bonkers and is unable to revert it self in #381. (Live and learn).

For sanity sake, referencing the ticket above as the origin, as it has some history included, also making note of contributions to this feature by @chrisvanpatten

Closing this in favor of #592 (unrecoverable rebase issue, so just reworked it instead).

[TimothyBlynJacobs]

In 49154:

Site Health, REST API: Move async tests to REST API endpoints.

This provides more flexibility when writing tests and benefits from running in a front-end context which is necessary for some tests like checking that updates are supported. Additionally, this provides a more robust interface for developers who want to integrate with Site Health tests.

Because the wp/v2 endpoint is reserved for modeling core entities, site health is registered in its own wp-site-health/v1 namespace.

The existing ajax actions have been maintained for backward compatibility.

Props Clorith, chrisvanpatten, afragen, pokhriyal, TimothyBlynJacobs.
Fixes #48105.

Merged in efe06cdceac62f928f1adf008bb55a4293f9a3c2.

[TimothyBlynJacobs]

In 49155:

REST API: Grant super admin to site health test user.

The current user needs to be a super admin to access Site Health on multisite.

Follow up to [49154].
See #48105.

[TimothyBlynJacobs]

In 49156:

REST API: Exclude custom site health capability check on multisite.

Super admins will always pass capability checks.

Follow up to [49154], [49155].
See #48105.

[TimothyBlynJacobs]

In 49917:

Site Health: Use a front-end URL for loopback tests.

In [49154] the async Site Health tests were changed to use the REST API instead of admin-ajax. An unintended side effect of this change was that the loopback tests which tried to ping the site's admin_url() were no longer authenticated because admin-cookies aren't provided to the REST API.

This commit adjusts the loopback test to use the front-end site_url which checks that cron will function properly. A follow-up ticket will focus on tests that will cover the file editor checks.

Props Clorith.
Fixes #52097.
See #48105.

[SergeyBiryukov]

In 49997:

Site Health: Use a front-end URL for loopback tests.

In [49154] the async Site Health tests were changed to use the REST API instead of admin-ajax. An unintended side effect of this change was that the loopback tests which tried to ping the site's admin_url() were no longer authenticated because admin-cookies aren't provided to the REST API.

This commit adjusts the loopback test to use the front-end site_url which checks that cron will function properly. A follow-up ticket will focus on tests that will cover the file editor checks.

Props Clorith.
Merges [49917] to the 5.6 branch.
Fixes #52097.
See #48105.