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

[glendaviesnz]

As discussed here this moves the sync_status postmeta to a top level field of wp_block, and also renames it to wp_pattern_sync_status as discussed here.

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

Testing Instructions

• This change will break the sync status of unsynced patterns added in previous betas. All previous unsycned patterns will display as synced. Deleting these and adding new unsynced patterns is the easiest way to test
• Switch to this branch with 16.2 RC or trunk GB plugin installed and in the post editor add some new synced and unsynced patterns - make sure the synced ones appear in the Sync Patterns inserter tab and that the unsynced ones appear in Patterns tab under Custom patterns
• Follow the Manage All Patterns link in the Editors top right settings menu
• Edit each pattern and make sure sync status displays correctly in right post info panel
• When editing an unsycned pattern check rest endpoint (/wp/v2/blocks?) return to see that post meta field is empty but wp_pattern_status field is set
• In the Site editor library check that the patterns appear in the correct Synced and unsynced sections
• In the Site editor add new synced and unsynced patterns and make sure the appear in the correct sections

Screenshots or screencast

Before:

After:

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[glendaviesnz]

@TimothyBJacobs is this the sort of thing you were thinking in this comment?

[mukeshpanchal27]

Thanks @glendaviesnz for the PR, Left some quick feedback.

[glendaviesnz]

This will need to be merged alongside the next lot of editor updates to be pulled in as it is dependent on those.

[glendaviesnz]

It would be good to have a test for the postmeta field being moved up to top level field in rest response - will try and get that added tomorrow.

[aaronrobertshaw]

Thanks for this one 👍

✅ wp_pattern_sync_status registration matches Gutenberg PR
✅ Gutenberg-only filter hasn't been backported
✅ Move of sync status to top level property works

As per your last comment, an extra test would be great 🙇

[glendaviesnz]

@aaronrobertshaw I have added that unit test. The e2e failures seem unrelated but I do not have the option to restart them.

[aaronrobertshaw]

Thanks for the test @glendaviesnz

I just have one question regarding the register_post_meta call in the test. FWIW the test still passes when I update it to match the current registration of the wp_pattern_sync_status post meta.

The e2e failures seem unrelated but I do not have the option to restart them

Unfortunately, I do not have that ability either. Perhaps @tellthemachines or @ramonjd could help here?

[aaronrobertshaw]

Suggested change

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

Should this be matching the actual post meta registration?

[glendaviesnz]

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.

[ramonjd]

Just to confirm that this PR also covers the changes in:

• Patterns: rename sync_status postmeta to pattern_post_sync_status gutenberg#52232

Unfortunately, I do not have that ability either. Perhaps @tellthemachines or @ramonjd could help here?

I too am not a member of the exalted group 😄

[glendaviesnz]

Just to confirm that this PR also covers the changes in:

• Patterns: rename sync_status postmeta to pattern_post_sync_status gutenberg#52232

Yes, except for this bit which is only needed in plugin, not core.

[tellthemachines]

I restarted the tests. They've been super flaky lately so failures are unlikely to be related to this PR.

[peterwilsoncc]

The name change looks good to me, deferring to others re the changes to the rest api controller.

Unless you hear otherwise, I reckon this is good to go for the next beta. If anything needs to change, that will allow people to pick it up.

[peterwilsoncc]

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?

[tellthemachines]

Committed in r56160 / 7035b61.