Skip to content

Rename sync_status top wp_sync_status and move to top level field of wp_block post type #4761

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

Closed
glendaviesnz wants to merge 6 commits into from
Closed

Rename sync_status top wp_sync_status and move to top level field of wp_block post type #4761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39975b1
Rename sync_status top wp_sync_status and move to top level field of …
glendaviesnz Jun 30, 2023
4a8245f
Changes from review
glendaviesnz Jun 30, 2023
cbb6450
Switch to wp_pattern_sync_status
glendaviesnz Jul 4, 2023
24cc9ca
Check that wp_pattern_sync_status is set before assigning
glendaviesnz Jul 4, 2023
bf2ccc2
Add unit test
glendaviesnz Jul 4, 2023
ce0491b
Linting
glendaviesnz Jul 4, 2023
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
4 changes: 2 additions & 2 deletions src/wp-includes/post.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8040,14 +8040,14 @@ function use_block_editor_for_post_type( $post_type ) {
/**
* Registers any additional post meta fields.
*
* @since 6.3.0 Adds sync_status meta field to the wp_block post type so an unsynced option can be added.
* @since 6.3.0 Adds `wp_pattern_sync_status` meta field to the wp_block post type so an unsynced option can be added.
*
* @link https://github.com/WordPress/gutenberg/pull/51144
*/
function wp_create_initial_post_meta() {
register_post_meta(
'wp_block',
'sync_status',
'wp_pattern_sync_status',
array(
'sanitize_callback' => 'sanitize_text_field',
'single' => true,
Expand Down
4 changes: 4 additions & 0 deletions src/wp-includes/rest-api/endpoints/class-wp-rest-blocks-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public function check_read_permission( $post ) {
* Filters a response based on the context defined in the schema.
*
* @since 5.0.0
* @since 6.3 Adds the `wp_pattern_sync_status` postmeta property to the top level of response.
Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc Jul 7, 2023

Choose a reason for hiding this comment

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

Suggested change
* @since 6.3 Adds the `wp_pattern_sync_status` postmeta property to the top level of response.
* @since 6.3.0 Adds the `wp_pattern_sync_status` postmeta property to the top level of response.

Either Tim or @spacedmonkey will have a better idea than I as to whether this is the best approach for moving the value to the top level.

Am I correct in thinking this endpoint is read only?

*
* @param array $data Response data to filter.
* @param string $context Context defined in the schema.
Expand All @@ -56,6 +57,9 @@ public function filter_response_by_context( $data, $context ) {
unset( $data['title']['rendered'] );
unset( $data['content']['rendered'] );

// Add the core wp_pattern_sync_status meta as top level property to the response.
$data['wp_pattern_sync_status'] = isset( $data['meta']['wp_pattern_sync_status'] ) ? $data['meta']['wp_pattern_sync_status'] : '';
unset( $data['meta']['wp_pattern_sync_status'] );
return $data;
}

Expand Down
35 changes: 35 additions & 0 deletions tests/phpunit/tests/rest-api/rest-blocks-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,39 @@ public function test_content() {
$data['content']
);
}

/**
* Check that the `wp_pattern_sync_status` postmeta is moved from meta array to top
* level of response.
*
* @ticket 58677
*/
public function test_wp_patterns_sync_status_post_meta() {
register_post_meta(
'wp_block',
'wp_pattern_sync_status',
array(
'single' => true,
'type' => 'string',
'show_in_rest' => array(
'schema' => array(
'type' => 'string',
'properties' => array(
'sync_status' => array(
'type' => 'string',
),
),
Comment on lines +240 to +244
Copy link
Copy Markdown

@aaronrobertshaw aaronrobertshaw Jul 5, 2023

Choose a reason for hiding this comment

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

Suggested change
'properties' => array(
'sync_status' => array(
'type' => 'string',
),
),
'enum' => array( 'partial', 'unsynced' ),

Should this be matching the actual post meta registration?

Copy link
Copy Markdown
Author

@glendaviesnz glendaviesnz Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was just mocking the minimum required to make the test valid, we are not testing the post meta registration, just that the meta field is moved so the only thing that matters in terms of this test is that a field of type string called wp_pattern_sync_status is set.

I don't have a strong opinion on this though, can just copy and paste the full registration if that is seen as better practice.

),
),
)
);
wp_set_current_user( self::$user_ids['author'] );

$request = new WP_REST_Request( 'GET', '/wp/v2/blocks/' . self::$post_id );
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();

$this->assertArrayHasKey( 'wp_pattern_sync_status', $data );
$this->assertArrayNotHasKey( 'wp_pattern_sync_status', $data['meta'] );
}
}