REST API endpoints for sites - #13055
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jonnydmg. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
jonnydmg
left a comment
jonnydmg
left a comment
There was a problem hiding this comment.
There is a big PR to review. Good work getting this ready for core. This code a little old and some things have changed in core, so this PR needs to update accordingly.
Biggest things.
- We need to support sites with
is_site_meta_supportedfalse. There was some work done there to support it but not complete. - Site endpoint needs to be registered on single site. So it always exists then return an error on single site.
- WP_REST_Site_Meta_Fields has no test coverage at all.
- Single site needs tests.
- Super admin should be allowed to do everything on the endpoint.
- Code coverage and ticket need needs to be added to every test.
This feel like we are on the right track, if I am nitpicing here, becuase I think this good to go into core and just want to get the final bits to push forward into core.
I will wait until feedback is complete to do some real testing for this PR.
| * @since 7.2.0 | ||
| * | ||
| * @group restapi | ||
| * @group ms-required |
There was a problem hiding this comment.
We should have another test file for with all the tests related to it not being multisite, as there are lots of checks for is_mulitiste. There are so many this feels like a good fit for another file instead of excloding each test one at a time.
jonnydmg
left a comment
jonnydmg
left a comment
There was a problem hiding this comment.
There is a big PR to review. Good work getting this ready for core. This code a little old and some things have changed in core, so this PR needs to update accordingly.
Biggest things.
- We need to support sites with
is_site_meta_supportedfalse. There was some work done there to support it but not complete. - Site endpoint needs to be registered on single site. So it always exists then return an error on single site.
- WP_REST_Site_Meta_Fields has no test coverage at all.
- Single site needs tests.
- Super admin should be allowed to do everything on the endpoint.
- Code coverage and ticket need needs to be added to every test.
This feel like we are on the right track, if I am nitpicing here, becuase I think this good to go into core and just want to get the final bits to push forward into core.
I will wait until feedback is complete to do some real testing for this PR.
Introduces WP_REST_Sites_Controller plus tests. See #40365.
aa03545 to
cc81250
Compare
…and add missing group annotations
spacedmonkey
left a comment
spacedmonkey
left a comment
There was a problem hiding this comment.
Here is a PR with some unit test improvements.
This is has unit tests that cover single site as well.
| @@ -0,0 +1,872 @@ | |||
| <?php | |||
…oint-tests Tests: Add unit tests for WP_REST_Site_Meta_Fields functionality
…oint-more-tests Tests: Rename rest-sites-controller.php to wpRestSitesController.php …
Add format validation for the `domain` and `path` parameters in WP_REST_Sites_Controller so malformed values are rejected with a clean 400 rest_invalid_param instead of failing later inside wp_insert_site()/ wp_update_site() (which surfaces as a 500). domain must be a valid hostname or bare IPv4/IPv6 address, optionally followed by a port (bracketed IPv6 is intentionally not supported, so IPv6 addresses can't carry a port). path must start and end with a forward slash and be made up of characters valid in a URL path segment. Ticket #40365.
…t-check-exists' into feature/40365-rest-sites-endpoint-check-exists # Conflicts: # src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php
…oint-force-delete Enhance site deletion process to support force deletion and user removal
…oint-check-exists Check if domain exists before creating / updating.
3 participants
Trac ticket: https://core.trac.wordpress.org/ticket/40365
There are some open questions that we should discuss in the trac ticket.