Return all themes into the themes REST API

[spacedmonkey]

Currently only the current theme is returned in the themes REST API api. The theme api should be about to list all active themes installed.

Add all themes to API

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

[spacedmonkey]

There is now PR / patch on github - ​https://github.com/WordPress/wordpress-develop/pull/264

#222 has landed so it should be possible to add tests now. We should also add a status field to the prepared item.

#222 has landed so it should be possible to add tests now. We should also add a status field to the prepared item.

Status was added. I added the ability to filter by inactive themes.

👍 Test failure looks legit.

@TimothyBJacobs Can you review again.

Small note, seems like lints failing for unrelated reason

that's a very odd lint issue.

What happens if I do ?status=active,inactive? It seems like it'll only return the active theme?

that's a very odd lint issue.

What happens if I do ?status=active,inactive? It seems like it'll only return the active theme?

Good catch. I didn't see that you could pass an array there. I have made the logic much more simple.

The new logic looks great. I think we also need to adjust the permissions check so that you can't bypass the fetching inactive caps check by doing a active,inactive. Let's also get unit tests for permissions with those different scenarios.

[spacedmonkey]

CC @ockham

The new logic looks great. I think we also need to adjust the permissions check so that you can't bypass the fetching inactive caps check by doing a active,inactive. Let's also get unit tests for permissions with those different scenarios.

I have added some tests, using a data_provider, to pass different status to check permissions. This should be enough.

I think we should introduce a get_item route at the same time here.

Do you have any idea for what it would look like to switch themes? Even if we don't implement it now, I think we should consider what that API call would look like.

@TimothyBJacobs I have added the get_item method with tests.

I am not sure what switch_theme might look like right now. Do this PR have to tackle this?

I don't think this PR needs to implement it, but I want to make sure we could with this format. Would a PUT status=active work here?

I don't think this PR needs to implement it, but I want to make sure we could with this format. Would a PUT status=active work here?

I would like that format / design of the API the same as ​https://github.com/WordPress/wordpress-develop/pull/359 .

• For WP_REST_Server::EDITABLE, you can to pass status of active and inactive. Active, will switch theme to this theme. Inactive, will switch the theme to the default theme.
• For WP_REST_Server::DELETABLE this would delete theme.
• Any allow for installing themes from the WP.org repo.

We could even create a abstration for dependencies like themes and plugins, something like WP_REST_Dependiens_Controller or similar. Just an idea.

As discussed in #core-restapi, let's switch out the hardcoded twentytwenty for the more future proof WP_DEFAULT_THEME constant.

Additionally, we should definitely make sure that if a user has permission to access the current theme in the list view, that they can access the theme directly as wp/v2/themes/<name>.

We could then explore adding a link to the index response in WP_REST_Server::get_index that points to the currently active theme for the site. We should probably only expose that link if the current user has permission to view the resource.

Let's also make sure that a user can't discover what themes a user has installed by pinging the single theme route when they don't have permission. So it should do a general caps check first, before returning a 404.

We could then explore adding a link to the index response in WP_REST_Server::get_index that points to the currently active theme for the site. We should probably only expose that link if the current user has permission to view the resource.

I am not sure what this means or how to do it?

I am not sure what this means or how to do it?

Adding a link here, ​https://github.com/WordPress/wordpress-develop/blob/3785439c8bf01ad18727433f3586dc2cbf641e2a/src/wp-includes/rest-api/class-wp-rest-server.php#L1125.

Adding a link here,

​https://github.com/WordPress/wordpress-develop/blob/3785439c8bf01ad18727433f3586dc2cbf641e2a/src/wp-includes/rest-api/class-wp-rest-server.php#L1125

I don't know what this does and why we would need to do this? Never touched added one these link. Can you explain the change a little more.

It is the same as any other link in the REST API and is served at the index of wp-json. The index is meant to describe the site, so we would link to the active theme so users can programmatically access that instead of fetching the collection.

[davidbaumwald]

With Beta 1 for 5.5 landing in just a few hours, this is being moved to 5.6. If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[helen]

We're well into 5.6 now, so somebody needs to pick this up and get the PR refreshed with the review suggestions in the next week or so if we still want to keep this in consideration for the release.

[johnbillion]

Is this a Multisite-specific change?

[hellofromTonya]

Adding the refresh keyword for the PR merge conflict.

[TimothyBlynJacobs]

This hasn't seen activity since last week's checkin. Going to go ahead and bump to future release. Feel free to remilestone if someone wants to own this in the next few days.

@TimothyBJacobs Can you re-review? I think this can land early 5.7 thanks @lukaspawlik !

Thanks for the refresh @lukaspawlik!

We need to update the @since versions to 5.7.0.

This still needs to be addressed:

We could then explore adding a link to the index response in WP_REST_Server::get_index that points to the currently active theme for the site. We should probably only expose that link if the current user has permission to view the resource.

@TimothyBJacobs Your feedback has been addressed in ​https://github.com/WordPress/wordpress-develop/pull/264/commits/c2855ea53a2173ab2f643571ec1c3e0724e21af7

@lukaspawlik Thanks! To clarify, only the new functions should have an @since of 5.7.0, the existing function should stay the same.

​https://developer.wordpress.org/coding-standards/inline-documentation-standards/php/#since-section-changelogs

Thanks @TimothyBJacobs for the feedback. My mistake with versioning - updating it as per recommendation.

@TimothyBJacobs thanks for overall feedback. It's been addressed and awaiting your review.

This looks good, thanks for your contribution! We're still in 5.6, so this won't be merged for a few weeks.

Hey @lukaspawlik! Thanks again for working on this. I'm going to plan on merging this next week.

[TimothyBlynJacobs]

In 49925:

REST API: Expose all themes in the themes controller.

Previously, only the active theme was made available. This commit allows for all themes to be queried if the user has the switch_themes or manage_network_themes capabilities.

This commit also no longer exposes the page, per_page, search and context query parameters since they are not supported by this controller.

Props spacedmonkey, lpawlik, TimothyBlynJacobs.
Fixes #50152.

Committed in https://core.trac.wordpress.org/changeset/49925, thanks again for your work on this @lukaspawlik!

[audrasjb]

This enhancement needs a dev note :)

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2021/02/23/rest-api-changes-in-wordpress-5-7/