WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 5 months ago

Last modified 3 months ago

#49906 closed enhancement (fixed)

REST API: Add basic fields to /themes endpoint

Reported by: Bernhard Reiter Owned by: TimothyBlynJacobs
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.0
Component: REST API Keywords: has-dev-note
Focuses: rest-api Cc:

Description

The /wp/v2/themes endpoint currently only exposes each theme's theme_supports field, but nothing else -- not even theme name or title.

I need a few more theme properties for full-site editing, for which I'll file a PR against the wordpress-develop git repo. I'm filing this ticket to have something to link to :)

Change History (31)

This ticket was mentioned in PR #222 on WordPress/wordpress-develop by ockham.


7 months ago

The /wp/v2/themes [endpoint](https://developer.wordpress.org/rest-api/reference/themes/) currently only exposes each theme's theme_supports field, but nothing else -- not even theme name or title.

See https://core.trac.wordpress.org/ticket/49906.

Needed for https://github.com/WordPress/gutenberg/pull/21578 -- you can use that PR to test this one :slightly_smiling_face:

#2 @prbot
7 months ago

ockham commented on PR #222:

/cc @kadamwhite

#3 @prbot
7 months ago

TimothyBJacobs commented on PR #222:

Looks great! Are you interested in adding tests for this?

#4 @prbot
7 months ago

ockham commented on PR #222:

Looks great! Are you interested in adding tests for this?

Thanks! Sure, I'll add some tomorrow :smile:

#5 @prbot
7 months ago

ockham commented on PR #222:

Update, I've fixed the failing unit tests and lint.

I haven't yet added new tests, since I'm still struggling with setting up the tests environment on my Linux box. I asked on Core Slack: https://wordpress.slack.com/archives/C02RQBWTW/p1586985925240400

#6 @prbot
7 months ago

ockham commented on PR #222:

I managed to get unit tests to run locally, and added a few more fields and corresponding tests (see updated PR description). I'd say this is ready for review :slightly_smiling_face:

#7 @prbot
6 months ago

ockham commented on PR #222:

cc/ @mcsf for review. Because it's a themes endpoint! :tada:

#8 @prbot
6 months ago

mcsf commented on PR #222:

Because it's a themes endpoint! 🎉

Good times!

Looks great, as far as I'm concerned. I'd defer to Timothy or K Adam for the final green light, though.

#9 @prbot
6 months ago

ockham commented on PR #222:

Looks great, as far as I'm concerned. I'd defer to Timothy or K Adam for the final green light, though.

@kadamwhite @TimothyBJacobs Pretty pleaaase? :blush:

#10 @prbot
6 months ago

ockham commented on PR #222:

Addressed all feedback. Should be ready for another look!

#11 @prbot
6 months ago

TimothyBJacobs commented on PR #222:

Thanks, looking good!

One small thing, we should figure out some better language for as it exists in the database. These values aren't stored in the database, they are pulled from the theme's files.

The values are also sanitized and translated, so I'm not sure we could say as it exists in the theme's style.css file either. Not sure ¯\_(ツ)_/¯.

Also tagging @spacedmonkey for a general review as he's expressed interest in expanding the themes endpoint.

#12 @prbot
6 months ago

ockham commented on PR #222:

Thanks, looking good!

One small thing, we should figure out some better language for as it exists in the database. These values aren't stored in the database, they are pulled from the theme's files.

Ah yeah, good point. Lazy copy-pasta from another endpoint :grimacing:

The values are also sanitized and translated, so I'm not sure we could say as it exists in the theme's style.css file either. Not sure ¯_(ツ)_/¯.

I'll think of something :+1:

Also tagging @spacedmonkey for a general review as he's expressed interest in expanding the themes endpoint.

:+1:

#13 @prbot
6 months ago

TimothyBJacobs commented on PR #222:

That wording sounds good to me!

#14 @prbot
6 months ago

ockham commented on PR #222:

:wave: @TimothyBJacobs @spacedmonkey

Just checking in if there's anything else I should do or if mayyybe I could get y'all's approval :blush:

#15 @prbot
6 months ago

TimothyBJacobs commented on PR #222:

This still looks good to me @ockham! Will commit later today/tomorrow unless I hear different from @spacedmonkey.

#16 @prbot
6 months ago

spacedmonkey commented on PR #222:

At the moment, these fields can only display data on the currently active theme.

With a very small tweak, to get_items, we could display this data for all themes.
`

public function get_items( $request ) {

Retrieve the list of registered collection query parameters.
$registered = $this->get_collection_params();
$themes = array();

if ( isset( $registeredstatus?, $requeststatus? ) && in_array( 'active', $requeststatus?, true ) ) {

$active_theme = wp_get_theme();
$active_theme = $this->prepare_item_for_response( $active_theme, $request );
$themes[] = $this->prepare_response_for_collection( $active_theme );

} else {

$active_themes = wp_get_themes();
foreach( $active_themes as $theme ) {

$active_theme = $this->prepare_item_for_response( $theme, $request );
$themes[] = $this->prepare_response_for_collection( $active_theme );

}

}

$response = rest_ensure_response( $themes );

$response->header( 'X-WP-Total', count( $themes ) );
$response->header( 'X-WP-TotalPages', count( $themes ) );

return $response;

}

`

The only issue with this, is that theme supports can not be displayed on themes that are not active.

#17 @prbot
6 months ago

TimothyBJacobs commented on PR #222:

At the moment, these fields can only display data on the currently active theme.

With a very small tweak, to get_items, we could display this data for all themes.

I think it makes sense to expand that in 5.5, but let's make it a separate ticket.

#18 @prbot
6 months ago

spacedmonkey commented on PR #222:

I have created another PR for showing all themes in the API - https://github.com/WordPress/wordpress-develop/pull/264

This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.


5 months ago

#20 @prbot
5 months ago

TimothyBJacobs commented on PR #222:

Hey @ockham just checking in on this!

#21 @prbot
5 months ago

ockham commented on PR #222:

Hey @ockham just checking in on this!

Thanks! I'm back today from 2 weeks of maintenance rotation, picking this up again :+1:

#22 @prbot
5 months ago

TimothyBJacobs commented on PR #222:

@ockham To clarify, the type should still be string but have a format => uri.

#23 @prbot
5 months ago

ockham commented on PR #222:

@ockham To clarify, the type should still be string but have a format => uri.

Fixed by 7f5db79 :slightly_smiling_face:

#24 @TimothyBlynJacobs
5 months ago

  • Milestone changed from Awaiting Review to 5.5
  • Owner set to TimothyBlynJacobs
  • Severity changed from minor to normal
  • Status changed from new to assigned
  • Version set to 5.0

#25 @TimothyBlynJacobs
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 47921:

REST API: Add additional fields to the themes controller.

When the themes controller was introduced it only returned a theme's supported features. This adds the majority of a theme's header information to the response.

Props ockham, spacedmonkey.
Fixes #49906.

#27 @afragen
5 months ago

@TimothyBlynJacobs in a quick glance at r47921, it seems that Requires_WP should only be Requires. I think that’s how it’s listed in the theme headers.

#28 @TimothyBlynJacobs
5 months ago

Do you mean how we should be fetching that data or returning that data?

If fetching, WP_Theme indicates it as RequiresWP and we have a test for it here.

#29 @afragen
5 months ago

Just ignore me. I’m at work and was going by memory. 🤦🏻‍♂️

#30 @TimothyBlynJacobs
5 months ago

@afragen no worries, thanks for taking a look!

Note: See TracTickets for help on using tickets.