Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global Styles: Update REST controller override method and backport changes from Core #65259

Merged
merged 11 commits into from
Sep 17, 2024
Merged

Global Styles: Update REST controller override method and backport changes from Core #65259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
457daea
Global Styles: Update REST controller override method
Mamaduka Sep 12, 2024
04e462b
Update unit tests
Mamaduka Sep 12, 2024
992719a
Try string value
Mamaduka Sep 12, 2024
7d7cd38
Try: update class with Core's WP_REST_Global_Styles_Controller. Inher…
ramonjd Sep 16, 2024
d812e00
Register the route
ramonjd Sep 16, 2024
b75834d
Restore tests
ramonjd Sep 16, 2024
01bf6be
Restore removal of register_routes and add revisions controller class…
ramonjd Sep 16, 2024
c917442
formatting,yo
ramonjd Sep 16, 2024
f2b8dcd
Add an error_log
ramonjd Sep 16, 2024
43d70f9
Add 6.6+ post args
ramonjd Sep 16, 2024
2e75d56
Complete making Gutenberg global styles controller match core
aaronrobertshaw Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 57 additions & 106 deletions lib/class-wp-rest-global-styles-controller-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,30 @@
/**
* Base Global Styles REST API Controller.
*/
class WP_REST_Global_Styles_Controller_Gutenberg extends WP_REST_Controller {
class WP_REST_Global_Styles_Controller_Gutenberg extends WP_REST_Posts_Controller {

/**
* Post type.
* Whether the controller supports batching.
*
* @since 5.9.0
* @var string
* @since 6.6.0
* @var array
*/
protected $post_type;
protected $allow_batch = array( 'v1' => false );

/**
* Constructor.
*
* @since 5.9.0
*/
public function __construct() {
$this->namespace = 'wp/v2';
$this->rest_base = 'global-styles';
$this->post_type = 'wp_global_styles';
/**
* Constructor.
*
* @since 6.6.0
*
* @param string $post_type Post type.
*/
public function __construct( $post_type = 'wp_global_styles' ) {
parent::__construct( $post_type );
Comment on lines +16 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in Core in WordPress/wordpress-develop@6fc019a as part of inheriting WP_REST_Posts_Controller

}

/**
Expand All @@ -54,8 +59,14 @@ public function register_routes() {
'type' => 'string',
),
),
'allow_batch' => $this->allow_batch,
),
)
),
/*
* $override is set to true to avoid conflicts with the core endpoint.
* Do not sync to WordPress core.
*/
true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required to overwrite Core routes when this endpoint is registered.

);

// List themes global styles.
Expand All @@ -65,8 +76,10 @@ public function register_routes() {
sprintf(
'/%s/themes/(?P<stylesheet>%s)',
$this->rest_base,
// Matches theme's directory: `/themes/<subdirectory>/<theme>/` or `/themes/<theme>/`.
// Excludes invalid directory name characters: `/:<>*?"|`.
/*
* Matches theme's directory: `/themes/<subdirectory>/<theme>/` or `/themes/<theme>/`.
* Excludes invalid directory name characters: `/:<>*?"|`.
*/
'[^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?'
),
array(
Expand All @@ -81,8 +94,14 @@ public function register_routes() {
'sanitize_callback' => array( $this, '_sanitize_global_styles_callback' ),
),
),
'allow_batch' => $this->allow_batch,
),
)
),
/*
* $override is set to true to avoid conflicts with the core endpoint.
* Do not sync to WordPress core.
*/
true
);

// Lists/updates a single global style variation based on the given id.
Expand All @@ -108,8 +127,14 @@ public function register_routes() {
'permission_callback' => array( $this, 'update_item_permissions_check' ),
'args' => $this->get_endpoint_args_for_item_schema( WP_REST_Server::EDITABLE ),
),
'schema' => array( $this, 'get_public_item_schema' ),
)
'schema' => array( $this, 'get_public_item_schema' ),
'allow_batch' => $this->allow_batch,
),
/*
* $override is set to true to avoid conflicts with the core endpoint.
* Do not sync to WordPress core.
*/
true
);
}

Expand Down Expand Up @@ -196,28 +221,10 @@ public function get_item_permissions_check( $request ) {
* @param WP_Post $post Post object.
* @return bool Whether the post can be read.
*/
protected function check_read_permission( $post ) {
public function check_read_permission( $post ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in Core in WordPress/wordpress-develop@6fc019a as part of inheriting WP_REST_Posts_Controller

return current_user_can( 'read_post', $post->ID );
}

/**
* Returns the given global styles config.
*
* @since 5.9.0
*
* @param WP_REST_Request $request The request instance.
*
* @return WP_REST_Response|WP_Error
*/
public function get_item( $request ) {
$post = $this->get_post( $request['id'] );
if ( is_wp_error( $post ) ) {
return $post;
}

return $this->prepare_item_for_response( $post, $request );
}

/**
* Checks if a given request has access to write a single global styles config.
*
Expand All @@ -243,61 +250,12 @@ public function update_item_permissions_check( $request ) {
return true;
}

/**
* Checks if a global style can be edited.
*
* @since 5.9.0
*
* @param WP_Post $post Post object.
* @return bool Whether the post can be edited.
*/
protected function check_update_permission( $post ) {
return current_user_can( 'edit_post', $post->ID );
}

/**
* Updates a single global style config.
*
* @since 5.9.0
* @since 6.2.0 Added validation of styles.css property.
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
*/
public function update_item( $request ) {
$post_before = $this->get_post( $request['id'] );
if ( is_wp_error( $post_before ) ) {
return $post_before;
}

$changes = $this->prepare_item_for_database( $request );
if ( is_wp_error( $changes ) ) {
return $changes;
}

$result = wp_update_post( wp_slash( (array) $changes ), true, false );
if ( is_wp_error( $result ) ) {
return $result;
}

$post = get_post( $request['id'] );
$fields_update = $this->update_additional_fields_for_object( $post, $request );
if ( is_wp_error( $fields_update ) ) {
return $fields_update;
}

wp_after_insert_post( $post, true, $post_before );

$response = $this->prepare_item_for_response( $post, $request );

return rest_ensure_response( $response );
}

/**
* Prepares a single global styles config for update.
*
* @since 5.9.0
* @since 6.2.0 Added validation of styles.css property.
* @since 6.6.0 Added registration of block style variations from theme.json sources (theme.json, user theme.json, partials).
*
* @param WP_REST_Request $request Request object.
* @return stdClass|WP_Error Prepared item on success. WP_Error on when the custom CSS is not valid.
Expand Down Expand Up @@ -394,10 +352,12 @@ public function prepare_item_for_response( $post, $request ) { // phpcs:ignore V
}
if ( rest_is_field_included( 'title.rendered', $fields ) ) {
add_filter( 'protected_title_format', array( $this, 'protected_title_format' ) );
add_filter( 'private_title_format', array( $this, 'protected_title_format' ) );

$data['title']['rendered'] = get_the_title( $post->ID );

remove_filter( 'protected_title_format', array( $this, 'protected_title_format' ) );
remove_filter( 'private_title_format', array( $this, 'protected_title_format' ) );
Comment on lines +355 to +360
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The private_title_format filter add/remove wasn't backported from WordPress/wordpress-develop@edfc2b0

cc @youknowriad to see whether this was intentional

}

if ( rest_is_field_included( 'settings', $fields ) ) {
Expand Down Expand Up @@ -426,7 +386,7 @@ public function prepare_item_for_response( $post, $request ) { // phpcs:ignore V
}
$response->add_links( $links );
if ( ! empty( $links['self']['href'] ) ) {
$actions = $this->get_available_actions();
$actions = $this->get_available_actions( $post, $request );
$self = $links['self']['href'];
foreach ( $actions as $rel ) {
$response->add_link( $rel, $self );
Expand All @@ -450,9 +410,12 @@ protected function prepare_links( $id ) {
$base = sprintf( '%s/%s', $this->namespace, $this->rest_base );

$links = array(
'self' => array(
'self' => array(
'href' => rest_url( trailingslashit( $base ) . $id ),
),
'about' => array(
'href' => rest_url( 'wp/v2/types/' . $this->post_type ),
),
Comment on lines +416 to +418
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in Core in WordPress/wordpress-develop@6fc019a as part of inheriting WP_REST_Posts_Controller

);

if ( post_type_supports( $this->post_type, 'revisions' ) ) {
Expand All @@ -473,13 +436,16 @@ protected function prepare_links( $id ) {
*
* @since 5.9.0
* @since 6.2.0 Added 'edit-css' action.
* @since 6.6.0 Added $post and $request parameters.
*
* @param WP_Post $post Post object.
* @param WP_REST_Request $request Request object.
* @return array List of link relations.
*/
protected function get_available_actions() {
protected function get_available_actions( $post, $request ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in Core in WordPress/wordpress-develop@6fc019a as part of inheriting WP_REST_Posts_Controller

$rels = array();

$post_type = get_post_type_object( $this->post_type );
$post_type = get_post_type_object( $post->post_type );
if ( current_user_can( $post_type->cap->publish_posts ) ) {
$rels[] = 'https://api.w.org/action-publish';
}
Expand All @@ -491,21 +457,6 @@ protected function get_available_actions() {
return $rels;
}

/**
* Overwrites the default protected title format.
*
* By default, WordPress will show password protected posts with a title of
* "Protected: %s", as the REST API communicates the protected status of a post
* in a machine readable format, we remove the "Protected: " prefix.
*
* @since 5.9.0
*
* @return string Protected title format.
*/
public function protected_title_format() {
return '%s';
}

/**
* Retrieves the query params for the global styles collection.
*
Expand Down Expand Up @@ -589,7 +540,7 @@ public function get_theme_item_permissions_check( $request ) { // phpcs:ignore V

/*
* Verify if the current user has edit_theme_options capability.
* This capability is required to edit/view/delete templates.
* This capability is required to edit/view/delete global styles.
*/
if ( ! current_user_can( 'edit_theme_options' ) ) {
return new WP_Error(
Expand Down Expand Up @@ -623,8 +574,8 @@ public function get_theme_item( $request ) {
}

$theme = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data( 'theme' );
$data = array();
$fields = $this->get_fields_for_response( $request );
$data = array();

if ( rest_is_field_included( 'settings', $fields ) ) {
$data['settings'] = $theme->get_settings();
Expand Down Expand Up @@ -669,7 +620,7 @@ public function get_theme_items_permissions_check( $request ) { // phpcs:ignore

/*
* Verify if the current user has edit_theme_options capability.
* This capability is required to edit/view/delete templates.
* This capability is required to edit/view/delete global styles.
*/
if ( ! current_user_can( 'edit_theme_options' ) ) {
return new WP_Error(
Expand Down
22 changes: 16 additions & 6 deletions lib/rest-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,24 @@
}

/**
* Registers the Global Styles REST API routes.
* Overrides the REST controller for the `wp_global_styles` post type.
*
* @param array $args Array of arguments for registering a post type.
* See the register_post_type() function for accepted arguments.
* @param string $post_type Post type key.
*
* @return array Array of arguments for registering a post type.
*/
function gutenberg_register_global_styles_endpoints() {
$global_styles_controller = new WP_REST_Global_Styles_Controller_Gutenberg();
$global_styles_controller->register_routes();
}
add_action( 'rest_api_init', 'gutenberg_register_global_styles_endpoints' );
function gutenberg_override_global_styles_endpoint( array $args ): array {
$args['rest_controller_class'] = 'WP_REST_Global_Styles_Controller_Gutenberg';
$args['revisions_rest_controller_class'] = 'Gutenberg_REST_Global_Styles_Revisions_Controller_6_6';
$args['late_route_registration'] = true;
$args['show_in_rest'] = true;
$args['rest_base'] = 'global-styles';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI tests are running 6.5.5. I think it's computed for CI here.

They should probably be running 6.6, but even so, adding the args for backwards compat just in case.

Compare 6.5 https://github.com/WordPress/wordpress-develop/blob/6.5/src/wp-includes/post.php#L473

With 6.6 https://github.com/WordPress/wordpress-develop/blob/6.6/src/wp-includes/post.php#L476

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should probably be running 6.6

I'm assuming that these tests run on the previous major WP version due to our BC commitment. That said, I did expect we also ran the unit tests on the latest too.


return $args;
}
add_filter( 'register_wp_global_styles_post_type_args', 'gutenberg_override_global_styles_endpoint', 10, 2 );

/**
* Registers the Edit Site Export REST API routes.
Expand Down
8 changes: 6 additions & 2 deletions phpunit/class-wp-rest-global-styles-controller-gutenberg-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,18 @@ public static function wpTearDownAfterClass() {
* @covers WP_REST_Global_Styles_Controller_Gutenberg::register_routes
*/
public function test_register_routes() {
// Register routes so that they overwrite identical Core routes.
$global_styles_controller = new WP_REST_Global_Styles_Controller_Gutenberg();
$global_styles_controller->register_routes();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to occur to ensure we're only testing the Gutenberg-registered endpoints...


$routes = rest_get_server()->get_routes();
$this->assertArrayHasKey(
'/wp/v2/global-styles/(?P<id>[\/\w-]+)',
$routes,
'Single global style based on the given ID route does not exist'
);
$this->assertCount(
4, // Double core because both sets get registered in the plugin.
2,
$routes['/wp/v2/global-styles/(?P<id>[\/\w-]+)'],
'Single global style based on the given ID route does not have exactly two elements'
);
Expand All @@ -96,7 +100,7 @@ public function test_register_routes() {
'Theme global styles route does not exist'
);
$this->assertCount(
2, // Double core because both sets get registered in the plugin.
1,
$routes['/wp/v2/global-styles/themes/(?P<stylesheet>[^\/:<>\*\?"\|]+(?:\/[^\/:<>\*\?"\|]+)?)'],
'Theme global styles route does not have exactly one element'
);
Expand Down
Loading