Skip to content

Font Library: add wp_font_face post type and scaffold font face REST API controller #57656

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

Merged
creativecoder merged 9 commits into from
Jan 9, 2024
Merged

Font Library: add wp_font_face post type and scaffold font face REST API controller #57656

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
11af824
Adds wp_font_face post type and REST API controller
creativecoder Jan 8, 2024
3c2fc75
Adds a basic set of controller tests for WP_REST_Font_Faces_Controller
creativecoder Jan 9, 2024
a3e494c
Override GET collection args
creativecoder Jan 9, 2024
894f198
Adds missing property comment
creativecoder Jan 9, 2024
c0739dc
Tweaks and cleanup
creativecoder Jan 9, 2024
56823bc
Fix phpcs error and warnings
creativecoder Jan 9, 2024
88eb462
Correct typo
creativecoder Jan 9, 2024
1de34f4
Fix remaining phpcs warnings
creativecoder Jan 9, 2024
ee7a0b8
Add error for invalid font face id
creativecoder Jan 9, 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
Prev Previous commit
Next Next commit
Tweaks and cleanup
  • Loading branch information
@creativecoder
creativecoder committed Jan 9, 2024
commit c0739dcf2b13a9230d7444735501860aec4dadf3
41 changes: 38 additions & 3 deletions lib/experimental/fonts/font-library/class-wp-rest-font-faces-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function register_routes() {
'methods' => WP_REST_Server::READABLE,
'callback' => array( $this, 'get_items' ),
'permission_callback' => array( $this, 'get_font_faces_permissions_check' ),
'args' => array(),
'args' => $this->get_collection_params(),
),
array(
'methods' => WP_REST_Server::CREATABLE,
Expand Down Expand Up @@ -144,7 +144,7 @@ protected function get_parent( $parent_post_id ) {
return $parent_post;
}

/**
/**
* Checks if a given request has access to read posts.
*
* @since 6.5.0
Expand All @@ -158,7 +158,7 @@ public function get_font_faces_permissions_check( $request ) {
if ( ! current_user_can( $post_type->cap->edit_posts ) ) {
Copy link
Contributor Author

@creativecoder creativecoder Jan 9, 2024

Choose a reason for hiding this comment

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

I'm using the capabilities registered with the post type to check permissions here, so that creating font families in PHP will also be limited by the appropriate user caps.

return new WP_Error(
'rest_cannot_read',
__( 'Sorry, you are not allowed to do that.' ),
__( 'Sorry, you are not allowed to access font faces.' ),
array( 'status' => rest_authorization_required_code() )
);
}
Expand Down Expand Up @@ -415,4 +415,39 @@ public function get_item_schema() {

return $this->add_additional_fields_schema( $this->schema );
}

/**
* Retrieves the query params for the font face collection.
*
* @since 6.5.0
*
* @return array Collection parameters.
*/
public function get_collection_params() {
return array(
'page' => array(
'description' => __( 'Current page of the collection.' ),
'type' => 'integer',
'default' => 1,
'sanitize_callback' => 'absint',
'validate_callback' => 'rest_validate_request_arg',
'minimum' => 1,
),
'per_page' => array(
'description' => __( 'Maximum number of items to be returned in result set.' ),
'type' => 'integer',
'default' => 10,
'minimum' => 1,
'maximum' => 100,
'sanitize_callback' => 'absint',
'validate_callback' => 'rest_validate_request_arg',
),
'search' => array(
'description' => __( 'Limit results to those matching a string.' ),
'type' => 'string',
'sanitize_callback' => 'sanitize_text_field',
'validate_callback' => 'rest_validate_request_arg',
),
);
}
}
19 changes: 9 additions & 10 deletions lib/experimental/fonts/font-library/font-library.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,17 @@ function gutenberg_init_font_library_routes() {
register_post_type(
'wp_font_face',
array(
'labels' => array(
'labels' => array(
'name' => __( 'Font Faces', 'gutenberg' ),
'singular_name' => __( 'Font Face', 'gutenberg' ),
),
'public' => false,
'_builtin' => true, /* internal use only. don't use this when registering your own post type. */
'hierarchical' => false,
'show_in_rest' => false,
'rest_base' => 'font-faces',
'public' => false,
'_builtin' => true, /* internal use only. don't use this when registering your own post type. */
'hierarchical' => false,
'show_in_rest' => false,
Copy link
Contributor

@matiasbenedetto matiasbenedetto Jan 10, 2024

Choose a reason for hiding this comment

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

Not sure why show_in_rest is set to false. It seems contradictory with the function defined after

	protected function check_is_post_type_allowed( $post_type ) {
	     return true;
	}

Copy link
Contributor Author

@creativecoder creativecoder Jan 10, 2024

Choose a reason for hiding this comment

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

I'm thinking of it this way: we don't want to manage the wp_font_face post type through the REST API, in the way that one usually would for post types that store user created content. From the perspective of the API consumer, posts for font faces and font families is more of an implementation detail. For example, none of the fields used in other post related endpoints (post_title, post_content, etc) are used in the REST API interface for fonts. We don't want autosaves or revisions, which are typically used with 'show_in_rest' => true,.

This also matches how the wp_global_styles post type is registered, which I think is the one most similar to wp_font_face and wp_font_family. These 3 post types store settings, rather than content.

Another problem is that a side effect of 'show_in_rest' => true, is that the post type is added to the wp/v2/types endpoint, including links that will be incorrect for font faces (wp/v2/font-faces rather than wp/v2/font-families/<id>/font-faces). I don't see a way to correct that with filters, currently.

'rest_base' => 'font-faces',
Copy link
Member

@spacedmonkey spacedmonkey Jan 17, 2024

Choose a reason for hiding this comment

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

'rest_controller_class' => "WP_REST_Font_Faces_Controller"

This should have been used here. Not manaul registeration

// TODO: Add custom font capability
'capability_type' => 'post',
'capabilities' => array(
'capabilities' => array(
'read' => 'edit_theme_options',
'read_post' => 'edit_theme_options',
'read_private_posts' => 'edit_theme_options',
Expand All @@ -61,8 +60,8 @@ function gutenberg_init_font_library_routes() {
'edit_others_posts' => 'edit_theme_options',
'delete_others_posts' => 'edit_theme_options',
),
'map_meta_cap' => false,
'query_var' => false,
'map_meta_cap' => false,
'query_var' => false,
)
);

Expand Down
42 changes: 39 additions & 3 deletions phpunit/tests/fonts/font-library/wpRestFontFacesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ public function test_context_param() {
// Controller does not use get_context_param().
}

/**
* @covers WP_REST_Font_Faces_Controller::get_items
*/
public function test_get_items() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' );
Expand All @@ -91,10 +94,13 @@ public function test_get_items() {

$this->assertSame( 200, $response->get_status() );
$this->assertCount( 2, $data );
$this->assertSame( self::$font_face_id2, $data[0]['id'] );
$this->assertSame( self::$font_face_id1, $data[1]['id'] );
$this->assertSame( self::$font_face_id1, $data[0]['id'] );
$this->assertSame( self::$font_face_id2, $data[1]['id'] );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_items
*/
public function test_get_items_no_permission() {
wp_set_current_user( 0 );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' );
Expand All @@ -106,13 +112,19 @@ public function test_get_items_no_permission() {
$this->assertErrorResponse( 'rest_cannot_read', $response, 403 );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_items
*/
public function test_get_items_missing_parent() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . REST_TESTS_IMPOSSIBLY_HIGH_NUMBER . '/font-faces' );
$response = rest_get_server()->dispatch( $request );
$this->assertErrorResponse( 'rest_post_invalid_parent', $response, 404 );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_item
*/
public function test_get_item() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 );
Expand All @@ -127,6 +139,9 @@ public function test_get_item() {
$this->assertSameSets( $fields, array_keys( $data ) );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_item
*/
public function test_get_item_no_permission() {
wp_set_current_user( 0 );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 );
Expand All @@ -139,6 +154,9 @@ public function test_get_item_no_permission() {
$this->assertErrorResponse( 'rest_cannot_read', $response, 403 );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_item
*/
public function test_get_item_missing_parent() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . REST_TESTS_IMPOSSIBLY_HIGH_NUMBER . '/font-faces/' . self::$font_face_id1 );
Expand All @@ -147,6 +165,9 @@ public function test_get_item_missing_parent() {
$this->assertErrorResponse( 'rest_post_invalid_parent', $response, 404 );
}

/**
* @covers WP_REST_Font_Faces_Controller::get_item
*/
public function test_get_item_valid_parent_id() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'GET', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces/' . self::$font_face_id1 );
Expand All @@ -157,7 +178,7 @@ public function test_get_item_valid_parent_id() {
}

/**
* @ticket 59875
* @covers WP_REST_Font_Faces_Controller::get_item
*/
public function test_get_item_invalid_parent_id() {
wp_set_current_user( self::$admin_id );
Expand All @@ -169,6 +190,9 @@ public function test_get_item_invalid_parent_id() {
$this->assertSame( $expected_message, $response->as_error()->get_error_messages()[0], 'The message must contain the correct parent ID.' );
}

/**
* @covers WP_REST_Font_Faces_Controller::create_item
*/
public function test_create_item() {
wp_set_current_user( self::$admin_id );
$request = new WP_REST_Request( 'POST', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' );
Expand All @@ -191,6 +215,9 @@ public function test_update_item() {
$this->assertErrorResponse( 'rest_no_route', $response, 404 );
}

/**
* @covers WP_REST_Font_Faces_Controller::delete_item
*/
public function test_delete_item() {
wp_set_current_user( self::$admin_id );
$font_face_id = self::factory()->post->create(
Expand All @@ -207,6 +234,9 @@ public function test_delete_item() {
$this->assertNull( get_post( $font_face_id ) );
}

/**
* @covers WP_REST_Font_Faces_Controller::delete_item
*/
public function test_delete_item_no_trash() {
wp_set_current_user( self::$admin_id );
$font_face_id = self::factory()->post->create(
Expand All @@ -230,6 +260,9 @@ public function test_delete_item_no_trash() {
$this->assertNotEmpty( $post );
}

/**
* @covers WP_REST_Font_Faces_Controller::delete_item
*/
public function test_delete_item_invalid_delete_permissions() {
wp_set_current_user( self::$editor_id );
$font_face_id = self::factory()->post->create(
Expand All @@ -251,6 +284,9 @@ public function test_prepare_item() {
// Not yet using the prepare_item method for font faces.
}

/**
* @covers WP_REST_Font_Faces_Controller::get_item_schema
*/
public function test_get_item_schema() {
$request = new WP_REST_Request( 'OPTIONS', '/wp/v2/font-families/' . self::$font_family_id . '/font-faces' );
$response = rest_get_server()->dispatch( $request );
Expand Down