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

WordPress 6.0 backports amendments for PR 2488 #2567

Closed
wants to merge 29 commits into from

Conversation

ironprogrammer
Copy link

Continued amendments to @hellofromtonya's backports for #2488.

This PR builds off the original PR, and addresses reviewer commentary from April 8 and onward. Please use this more recent PR going forward.

Original PR 2488:

Backports changes from Gutenberg to add support for custom taxonomies filtering [Block Library - Query Loop].

Also being tracked in WordPress/gutenberg#39889.

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


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.

hellofromtonya and others added 24 commits March 31, 2022 11:48
* Update to stable namespace.
* Register routes for WP_REST_Block_Pattern_Categories_Controller
* Require class file for WP_REST_Block_Pattern_Categories_Controller
* Update expected route for WP_REST_Block_Patterns_Controller
* Require class file for WP_REST_Block_Patterns_Controller
* Register routes for WP_REST_Block_Patterns_Controller
* Girl scouting annotations and formatting
Resolves the `ReflectionException: Class [name of test class] does not have a property named instance`.

Error occurred as `getProperty()` is not available on `private` or `protected` properties.

This commit uses `ReflectionProperty` to change the `instance` property to `public` for the test.
The property is restored to `private` when done.
@ironprogrammer ironprogrammer changed the title WordPress 6.0 backports amendments for PR 2488, Part 2 WordPress 6.0 backports amendments for PR 2488 Apr 12, 2022
@ironprogrammer ironprogrammer marked this pull request as ready for review April 12, 2022 00:44
'terms' => array( 3, 11, 10 ),
'include_children' => false,
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this test be duplicated with the new taxQuery set up.

@ntsekouras
Copy link

Don't we need the updates of packages in order to test this properly? I mean the patterns fetch request is triggered through js here: https://github.com/WordPress/gutenberg/pull/39185/files#diff-278a4a275170c4215a2f4e9d5c0c6d831f9328258220a76cda04b8d9f39f01a5R71. And also in the packages we use experimental namespace for the API.

Sorry if I'm missing something, but I'm trying to figure this out and test it properly.

@gziolo
Copy link
Member

gziolo commented Apr 12, 2022

@ntsekouras, we decided to split backport commits into as many individual pieces as possible so it's expected that some changes depend on each other. We can test everything once all backports are in place. This is also why we have 3 weeks dedicated to beta testing so we could detect integration issues and fix them.

if ( ! is_dir( $dirpath ) || ! is_readable( $dirpath ) ) {
continue;
}
if ( file_exists( $dirpath ) ) {

Choose a reason for hiding this comment

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

We can skip this check now: WordPress/gutenberg#40259 (comment), since we do it in the above line.

Copy link
Member

Choose a reason for hiding this comment

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

@ntsekouras, can you move this comment to #2488?

@gziolo
Copy link
Member

gziolo commented Apr 12, 2022

Committed with https://core.trac.wordpress.org/changeset/53152. Let's address the remaing issues listed in #2488 seperately.

@gziolo gziolo closed this Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants