Refactor and update the datapoint route registration to make use of WordPress' REST schema capabilities
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 1.4k
- Forks
- 384
- Avg merge
- 4d 14h
- Merged PRs (30d)
- 77
Description
## Feature Description
As mentioned [here](https://github.com/google/site-kit-wp/pull/6749#discussion_r1143446469), it would be nice to be able to leverage WordPress' [argument schema validation](https://developer.wordpress.org/rest-api/extending-the-rest-api/schema/#argument-schema) for module datapoint REST requests. Additionally, we could make use of the [resource schema](https://developer.wordpress.org/rest-api/extending-the-rest-api/schema/#resource-schema) capabilities too.
This would require some refactoring in order to register the datapoint routes individually, rather than using a dynamic path parameter.
---------------
_Do not alter or remove anything below. The following sections will be managed by moderators only._
## Acceptance criteria
* A module data endpoint can be registered as its own REST route, with its own argument schema describing the parameters it accepts and its own resource schema describing the data it returns.
* Module data endpoints that do not declare a schema keep being served by the existing catch-all route, with unchanged behaviour.
* An endpoint that declares a schema returns the same data, status codes and error messages as it does today, apart from the new validation errors.
* A request to an endpoint that declares a schema is rejected with a 400 response when a required parameter is missing or a parameter has the wrong type, before the request reaches Google's APIs.
* The argument and resource schemas of endpoints that declare them appear in the site's REST API index.
* No existing module data endpoint is converted in this issue: it only adds the ability to do so, and the conversions are handled separately.
* The Site Kit dashboard, module setup, module settings and shared dashboards continue to work unchanged.
## Implementation Brief
* [ ] In `includes/Core/Modules/Schema_Aware_Datapoint.php` (new file):
* Declare an interface `Schema_Aware_Datapoint`, following `Executable_Datapoint`, with `get_args()` returning the endpoint argument schema and `get_schema()` returning the resource schema.
* [ ] In `includes/Core/Modules/Module.php`:
* Move the lazy memoization of `$datapoint_definitions` out of `get_datapoint_definition()` into a private helper.
* Add `final public function get_schema_aware_datapoints()` returning the memoized definitions whose values implement `Schema_Aware_Datapoint`, keyed by their `METHOD:datapoint` id.
* [ ] In `includes/Core/Modules/REST_Modules_Controller.php`:
* Move the `WP_REST_Server::READABLE` and `WP_REST_Server::EDITABLE` callbacks in `get_modules_data_route()` into private methods taking the module slug and datapoint name, so the catch-all route and the per-datapoint routes run the same code, including the `invalid_module_slug` and `module_not_active` errors.
* Do the same for `resolve_datapoint_permission()`, so a datapoint implementing `Permission_Aware_Datapoint` keeps its own permission check on either route.
* Add `get_datapoint_routes()`: for each module returned by `Modules::get_available_modules()`, call `get_schema_aware_datapoints()` and build one `REST_Route` per datapoint name at `modules//data/`, with the `GET:` and `POST:` definitions of the same name becoming the two endpoints of that route.
* Pass each definition's `get_args()` as the endpoint `args`, and `get_schema()` wrapped in a closure as the route-level `schema` key, matching `get_modules_list_route()`.
* In `get_rest_routes()`, return the `get_datapoint_routes()` entries before `get_modules_data_route()`. `WP_REST_Server` matches routes in registration order, so the catch-all has to stay last.
* A datapoint name needs no special handling when only one of its `GET:` and `POST:` definitions declares a schema: `WP_REST_Server` moves on to the next matching route when the individual route has no handler for the request method, so the other method falls through to the catch-all.
* [ ] Datapoints keep their existing `Missing_Required_Param_Exception` guards. `Module::get_data()` is also called directly from PHP, for example from `Analytics_4::get_google_tag_settings()`, where REST validation does not run.
### Test Coverage
* [ ] Add `tests/phpunit/includes/Core/Modules/Datapoints/FakeModule_Schema_Aware_Request.php`, a fake datapoint implementing `Schema_Aware_Datapoint` with a required string argument and a resource schema, registered on `FakeModule` alongside `FakeModule_Test_Request`.
* [ ] In `tests/phpunit/integration/Core/Modules/ModuleTest.php`:
* `get_schema_aware_datapoints()` returns only the definitions implementing the interface, keyed by `METHOD:datapoint`.
* [ ] In `tests/phpunit/integration/Core/Modules/REST_Modules_ControllerTest.php`:
* the schema-aware fake datapoint is served by its own route and returns the same data as before.
* a request to that route without the required parameter returns a 400 and the datapoint is never executed.
* a fake datapoint that does not implement the interface is still served by the catch-all route.
* the individual route wins over the catch-all when both would match the path.
* requesting the schema-aware datapoint on a deactivated module returns `module_not_active`.
* a schema-aware datapoint that also implements `Permission_Aware_Datapoint` uses its own permission check on the individual route.
* the route, its argument schema and its resource schema are listed in the `google-site-kit/v1` REST index.
## QA Brief
*
## Changelog entry
*
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with includes/Core/Modules/Module.php and REST_Modules_Controller.php, then read the new interface requirements in includes/Core/Modules/Schema_Aware_Datapoint.php. Add the fake schema-aware datapoint and update ModuleTest.php and REST_Modules_ControllerTest.php to cover route precedence, validation, permissions, errors, and REST index schemas. Done means schema-aware endpoints use individual routes while other datapoints retain the catch-all behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- api, backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100