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

Add support for the format property in query #7314

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7b34ec9
Add support for the `format` property in query
carolinan Sep 9, 2024
24efb21
Tests: Add format as a valid parameter in WP_Test_REST_Posts_Controll…
carolinan Sep 9, 2024
8e44597
build_query_vars_from_query_block: Fix incorrect variable name and br…
carolinan Sep 11, 2024
d7ab338
Tests: Update the qunit fixture in `wp-api-generated.js`
carolinan Sep 11, 2024
a4fb1b4
`WP_REST_Posts_Controller`: remove redundant array conversion for the…
carolinan Sep 11, 2024
7eec23a
`WP_REST_Posts_Controller`: Tidy up the inline comments.
carolinan Sep 11, 2024
83eefcb
`WP_REST_Posts_Controller` `get_collection_params`: add `uniqueItems`…
carolinan Sep 11, 2024
5fb2c49
Tests: Update the qunit fixtures in wp-api-generated.js
carolinan Sep 11, 2024
5058604
Update the format of inline comments
carolinan Sep 12, 2024
1b974aa
Remove unnecessary `terms` from the query for the standard format
carolinan Sep 17, 2024
3244361
Change the `relation` from `OR` to `AND` when querying for the `format`
carolinan Sep 17, 2024
5ee2750
In build_query_vars_from_query_block: validate the formats
carolinan Sep 17, 2024
c71f92f
WP_REST_Posts_Controller: Change the `relation` from `AND` to `OR` wh…
carolinan Sep 19, 2024
19ba793
build_query_vars_from_query_block: Change the `relation` when queryin…
carolinan Sep 19, 2024
ef44367
Formats: address feedback from code review and PHPCS issues
carolinan Sep 19, 2024
47e1581
build_query_vars_from_query_block: Try to resolve the nested relations
carolinan Sep 19, 2024
ad5ea9f
Merge branch 'WordPress:trunk' into try/post-formats-62014
carolinan Sep 20, 2024
0d3d4dc
Merge branch 'WordPress:trunk' into try/post-formats-62014
carolinan Sep 23, 2024
4e69a77
Merge branch 'WordPress:trunk' into try/post-formats-62014
carolinan Sep 25, 2024
6a78361
build_query_vars_from_query_block: Update variable names and if/else …
carolinan Sep 25, 2024
290a106
Oops
carolinan Sep 25, 2024
8ae0075
Tests: Tests_Blocks_wpBlock: Update arrays used in `assertSame`
carolinan Sep 25, 2024
53e45f4
Merge branch 'WordPress:trunk' into try/post-formats-62014
carolinan Sep 27, 2024
54f6b67
Merge branch 'trunk' into try/post-formats-62014
peterwilsoncc Sep 29, 2024
16223d6
Merge branch 'trunk' into try/post-formats-62014
peterwilsoncc Sep 29, 2024
4aa0b29
Add tests for the rest api format getter.
peterwilsoncc Sep 30, 2024
f1508b7
Add tests for post formats in query block builder.
peterwilsoncc Sep 30, 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
47 changes: 47 additions & 0 deletions src/wp-includes/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -2330,6 +2330,7 @@ function wp_migrate_old_typography_shape( $metadata ) {
*
* @since 5.8.0
* @since 6.1.0 Added `query_loop_block_query_vars` filter and `parents` support in query.
* @since 6.7.0 Added support for the `format` property in query.
*
* @param WP_Block $block Block instance.
* @param int $page Current query's page.
Expand Down Expand Up @@ -2420,6 +2421,52 @@ function build_query_vars_from_query_block( $block, $page ) {
}
}
}
if ( ! empty( $block->context['query']['format'] ) && is_array( $block->context['query']['format'] ) ) {
$formats = $block->context['query']['format'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Validation: Ensure that the formats come from the supported values only provided by get_post_format_slugs().

⚠️ Untested but I think you want something along the lines of:

$formats = array_intersect( $formats, get_post_format_slugs() );

$tax_query = array( 'relation' => 'OR' );
Copy link
Contributor

Choose a reason for hiding this comment

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

My reading of this code is that if someone sets up a query for both formats and taxomomies, this will return posts that contain either.

Eg a query for post format: link; category: rest-api will return:

  • Any posts with the post format link, regardless of category
  • Any posts in the category rest-api, regardless of format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think something is not working correctly.
If I use OR or AND, the result is the same: Posts that have both the category and the format.
I think the correct use is AND.
But then why is not OR returning all posts that have either the format or the category?

Copy link
Contributor Author

@carolinan carolinan Sep 16, 2024

Choose a reason for hiding this comment

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

Here is an example query loop for listing posts in category 4, with either the gallery or link format. The category id needs to be updated to match a category available on the installation.

<!-- wp:query {"queryId":1,"query":{"perPage":10,"pages":0,"offset":0,"postType":"post","order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":false,"format":["link","gallery"],"taxQuery":{"category":[4]}}} -->
<div class="wp-block-query"><!-- wp:post-template -->
<!-- wp:post-title /-->
<!-- /wp:post-template --></div>
<!-- /wp:query -->

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it to AND though it bothers me why OR is working the way I am seeing it in testing 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 In the WP_REST_Posts_Controller class, there is also:

// Enable filtering by both post formats and other taxonomies by combining them with `AND`.
			if ( isset( $args['tax_query'] ) ) {
				$args['tax_query'][] = array(
					'relation' => 'AND',
					$tax_query,
				);
			} else {
				$args['tax_query'] = $tax_query;
			}

https://github.com/WordPress/wordpress-develop/pull/7314/files#diff-48cfdad3b747164ac4b6857186399519dabf913026235575b00ebe5b0dc7169bR383

Copy link
Contributor Author

@carolinan carolinan Sep 18, 2024

Choose a reason for hiding this comment

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

I am badly regretting not documenting why I added this relation in the first place.

Once I get back to work I will continue doing more logging of the different queries:
But I am now thinking that I meant for this to be the relation between the formats.
That I choose 'OR' because the user can query for both 'video' and 'link' and it needs to be 'OR' because a post can only have one format. It always have to list posts that has either 'video' or 'link', never both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I think $tax_query was not a good name for the variable, it too can cause confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After more testing and logging of $args['tax_query'] in the WP_REST_Posts_Controller class, I found that:

  • Setting the relation on the array to either OR or AND works when the query has two formats such as link and gallery. Because there is only one condition; the relation is not actually used, there are no conditions to create a relation between.

  • Using OR is needed when the query includes standard and a non-default format such as link and gallery, because now there are two conditions.

So that is why the testing earlier showed the same results: I was not testing with a combination of standard.


/*
* The default post format, `standard`, is not stored in the database.
* If `standard` is part of the request, the query needs to exclude all post items that
* have a format assigned.
*/
if ( in_array( 'standard', $formats, true ) ) {
$tax_query[] = array(
'taxonomy' => 'post_format',
'field' => 'slug',
'terms' => array(),
carolinan marked this conversation as resolved.
Show resolved Hide resolved
'operator' => 'NOT EXISTS',
);
// Remove the `standard` format, since it cannot be queried.
unset( $formats[ array_search( 'standard', $formats, true ) ] );
}
// Add any remaining formats to the tax query.
if ( ! empty( $formats ) ) {
carolinan marked this conversation as resolved.
Show resolved Hide resolved
// Add the `post-format-` prefix.
$terms = array_map(
static function ( $format ) {
return 'post-format-' . $format;
carolinan marked this conversation as resolved.
Show resolved Hide resolved
},
$formats
);
$tax_query[] = array(
'taxonomy' => 'post_format',
'field' => 'slug',
'terms' => $terms,
'operator' => 'IN',
);
}
if ( ! isset( $query['tax_query'] ) ) {
$query['tax_query'] = array();
}
/*
* This condition is intended to prevent `$tax_query` from being added to `$query`
* if it only contains the relation.
*/
if ( count( $tax_query ) > 1 ) {
$query['tax_query'][] = $tax_query;
}
}
if (
isset( $block->context['query']['order'] ) &&
in_array( strtoupper( $block->context['query']['order'] ), array( 'ASC', 'DESC' ), true )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,55 @@ public function get_items( $request ) {

$args = $this->prepare_tax_query( $args, $request );

if ( ! empty( $request['format'] ) ) {
$formats = $request['format'];
$tax_query = array( 'relation' => 'OR' );

/*
* The default post format, `standard`, is not stored in the database.
* If `standard` is part of the request, the query needs to exclude all post items that
* have a format assigned.
*/
if ( in_array( 'standard', $formats, true ) ) {
$tax_query[] = array(
'taxonomy' => 'post_format',
'field' => 'slug',
'terms' => array(),
carolinan marked this conversation as resolved.
Show resolved Hide resolved
'operator' => 'NOT EXISTS',
);
// Remove the `standard` format, since it cannot be queried.
unset( $formats[ array_search( 'standard', $formats, true ) ] );
}

// Add any remaining formats to the tax query.
if ( ! empty( $formats ) ) {
carolinan marked this conversation as resolved.
Show resolved Hide resolved
// Add the `post-format-` prefix.
$terms = array_map(
static function ( $format ) {
return 'post-format-' . $format;
},
$formats
);

$tax_query[] = array(
'taxonomy' => 'post_format',
'field' => 'slug',
'terms' => $terms,
'operator' => 'IN',
);
}

// Enable filtering by both post formats and other taxonomies by combining them with `AND`.
if ( isset( $args['tax_query'] ) ) {
$args['tax_query'][] = array(
'relation' => 'AND',
$tax_query,
);
} else {
$args['tax_query'] = $tax_query;
}
}

// Force the post_type argument, since it's not a user input variable.
$args['post_type'] = $this->post_type;

Expand Down Expand Up @@ -2979,6 +3028,18 @@ public function get_collection_params() {
);
}

if ( post_type_supports( $this->post_type, 'post-formats' ) ) {
$query_params['format'] = array(
'description' => __( 'Limit result set to items assigned one or more given formats.' ),
'type' => 'array',
'uniqueItems' => true,
'items' => array(
'enum' => array_values( get_post_format_slugs() ),
'type' => 'string',
),
);
}

/**
* Filters collection parameters for the posts controller.
*
Expand Down
1 change: 1 addition & 0 deletions tests/phpunit/tests/rest-api/rest-posts-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ public function test_registered_query_params() {
'categories_exclude',
'context',
'exclude',
'format',
'include',
'modified_after',
'modified_before',
Expand Down
21 changes: 21 additions & 0 deletions tests/qunit/fixtures/wp-api-generated.js
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,27 @@ mockedApiResponse.Schema = {
"description": "Limit result set to items that are sticky.",
"type": "boolean",
"required": false
},
"format": {
"description": "Limit result set to items assigned one or more given formats.",
"type": "array",
"uniqueItems": true,
"items": {
"enum": [
"standard",
"aside",
"chat",
"gallery",
"link",
"image",
"quote",
"status",
"video",
"audio"
],
"type": "string"
},
"required": false
}
}
},
Expand Down
Loading