First pass at themes endpoint. #41836
Conversation
|
@spacedmonkey, I think it seems more appropriate to discuss and review this in Core. What do you think? |
|
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. |
t-hamano
added
[Type] Enhancement
xavier-lc
left a comment
xavier-lc
left a comment
There was a problem hiding this comment.
I tried merging your branch with trunk locally but git got stuck because of the huge amount of changes. I ended up starting from trunk and cherry-picking the commits, here's the resulting branch: https://github.com/xavier-lc/gutenberg/tree/try/theme-api
I tested the endpoints and left some comments. Do you think you'll be able to address them? If not, I could take over your PR. Also, I'd be glad to help moving this forward; I could add tests, for example.
| array( | ||
| 'methods' => WP_REST_Server::READABLE, | ||
| 'callback' => array( $this, 'get_items' ), | ||
| 'permission_callback' => array( $this, 'get_items_permissions_check' ), | ||
| 'args' => $this->get_collection_params(), | ||
| ), |
There was a problem hiding this comment.
I think this could be removed, since it's doing the same as the base /wp/v2/themes route.
| $language_packs = array_map( | ||
| static function( $item ) { | ||
| return (object) $item; | ||
| }, | ||
| $api->language_packs | ||
| ); |
There was a problem hiding this comment.
I'm getting an error here because $api->language_packs is null instead of the expected array.
| array( | ||
| 'methods' => WP_REST_Server::READABLE, | ||
| 'callback' => array( $this, 'get_item' ), | ||
| 'permission_callback' => array( $this, 'get_item_permissions_check' ), | ||
| ), |
There was a problem hiding this comment.
I think this could be removed, since it's doing the same as the base /wp/v2/theme/{stylesheet} route.
| * @return true|WP_Error True if the request has access to update the item, WP_Error object otherwise. | ||
| */ | ||
| public function update_item_permissions_check( $request ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable | ||
| if ( current_user_can( 'switch_themes' ) ) { |
There was a problem hiding this comment.
A check for current_user_can( 'manage_network_themes' ) is probably needed as well.
| 'previous' => $prepared->get_data(), | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Deleting the active theme leaves the site in a broken state, the default should be set as is done in handle_theme_status:
| $theme_controller->register_routes(); | ||
| } | ||
| add_action( 'rest_api_init', 'gutenberg_register_theme_controller' ); | ||
|
|
There was a problem hiding this comment.
There's a broken comment block below this code.
|
@spacedmonkey @xavier-lc I was interested in pushing this work forward, but I wasn't sure if this one or #70847 is where the canonical work happens. @xavier-lc I understand there are some rebase issues - if you wish to continue in #70847, perhaps it's reasonable to pull any reasonable work from here into your PR ( give @spacedmonkey the credits of course)? Otherwise, #70847 seems a bit stuck too. Happy to help with some reviews on either PR, btw. |
What?
First pass at a REST API for themes management.
To do
Why?
How?
Testing Instructions
Screenshots or screencast