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.

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

Backport #2576 to Gutenberg #40902

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

anton-vlasenko
Copy link
Contributor

@anton-vlasenko anton-vlasenko commented May 6, 2022

What?

This PR aims to backport changes from WordPress/wordpress-develop#2576 to Gutenberg.

Fixes #40824.

Why?

This is needed to keep Core and Gutenberg as compatible as possible.

Testing Instructions

  1. Make sure you are using WordPress 5.9.x
  2. Enable Gutenberg.
  3. Install this plugin to allow basic authorization for REST API requests.
  4. Activate the plugin.
  5. Send a GET request to your WordPress instance, e.g.:
curl --user username:password "http://wordpress.test/wp-json/wp/v2/block-patterns/patterns"

Replace

username:user_password

with the actual credentials.
Replace

http://your.wordpress.instance

with the actual URL of your instance.
6. Note the response. It must contain block patterns.

@anton-vlasenko anton-vlasenko added Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended labels May 6, 2022
@anton-vlasenko anton-vlasenko added this to the Gutenberg 13.3 milestone May 6, 2022
@anton-vlasenko anton-vlasenko changed the title Backport https://github.com/WordPress/wordpress-develop/pull/2576 to … Backport https://github.com/WordPress/wordpress-develop/pull/2576 to Gutenberg May 6, 2022
@ndiego ndiego modified the milestones: Gutenberg 13.3, Gutenberg 13.5 Jun 2, 2022
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @anton-vlasenko,
By accident on #41791, I needed another change to the pattern endpoint and ended up doing the objective of this PR.

I think we need a new file for the patterns endpoint. The changes in lib/compat/wordpress-6.0/class-wp-rest-block-patterns-controller.php don't have a practical effect because the file is not used at all if the core already includes the class:

	if ( ! class_exists( 'WP_REST_Block_Patterns_Controller' ) ) {
		require_once __DIR__ . '/compat/wordpress-6.0/class-wp-rest-block-patterns-controller.php';
	}

So when we need to change the endpoint in the plugin we will not be able to do so because this file is ignored.

In I'm creating a new file lib/compat/wordpress-6.1/class-gutenberg-rest-block-patterns-controller.php that contains a new class Gutenberg_REST_Block_Patterns_Controller overwriting the core one during the backports we just need to pass the class to the core and change its name. So in the plugin, we are free to apply the multiple changes we will need on the endpoint. If you prefer I can extract these changes to the endpoint in a separate PR so we can review the approach.

@TimothyBJacobs
Copy link
Member

I think we need a new file for the patterns endpoint. The changes in lib/compat/wordpress-6.0/class-wp-rest-block-patterns-controller.php don't have a practical effect because the file is not used at all if the core already includes the class:

I believe that's the desired behavior here. This PR adjusts the included compat controller to behave the same way that Core behaves, so users running the plugin on WP 5.9 have identical behavior.

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

LGTM 👍 We can and should change lib/compat/wordpress-6.0/class-wp-rest-block-patterns-controller.php to match core although on #41791 I'm adding a new Gutenberg class so we can extend the core. I guess both PRs can be merged. And after both are merged we can just remove this file and start relying on the Gutenberg one.

@jorgefilipecosta jorgefilipecosta changed the title Backport https://github.com/WordPress/wordpress-develop/pull/2576 to Gutenberg Backport #2576 to Gutenberg Jun 23, 2022
@jorgefilipecosta jorgefilipecosta merged commit be111df into trunk Jun 23, 2022
@jorgefilipecosta jorgefilipecosta deleted the fix/dont-load-remote-patterns-twice branch June 23, 2022 14:12
@anton-vlasenko
Copy link
Contributor Author

Hi @jorgefilipecosta
Thank you for getting to the bottom of the issue.
I haven't had a chance to review your PR yet, but the reasoning to change the class name looks completely valid to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backport fixes for wp/v2/block-patterns/patterns and /wp/v2/pattern-directory/patterns endpoints to Gutenberg
4 participants