Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/wp-includes/class-wp-query.php
Original file line number Diff line number Diff line change
Expand Up @@ -1679,6 +1679,7 @@ protected function parse_search_order( &$q ) {
* Converts the given orderby alias (if allowed) to a properly-prefixed value.
*
* @since 4.0.0
* @since 6.9.0 Extends allowed_keys to support ordering by `post_mime_type`.
*
* @global wpdb $wpdb WordPress database abstraction object.
*
Expand All @@ -1695,6 +1696,7 @@ protected function parse_orderby( $orderby ) {
'post_date',
'post_title',
'post_modified',
'post_mime_type',
Copy link
Member Author

Choose a reason for hiding this comment

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

Before I add tests (in Tests_Query_Results I think(, I'd like to check if adding post_mime_type to this whitelist is okay?

The alternative is to add a filter in the attachments controller I guess, e.g.,

// In prepare_items_query method, after the orderby mapping:
if ( isset( $query_args['orderby'] ) && isset( $request['orderby'] ) && 'mime_type' === $request['orderby'] ) {
    // Use posts_orderby filter to add custom ordering
    add_filter( 'posts_orderby', array( $this, 'orderby_mime_type' ), 10, 2 );
    // Set orderby to something that won't conflict
    $query_args['orderby'] = 'date';
}

public function orderby_mime_type( $orderby, $query ) {
    global $wpdb;
    $order = $query->get( 'order' );
    return "{$wpdb->posts}.post_mime_type {$order}";
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@TimothyBJacobs made a good point about a long-standing pagination bug

This change might make that bug more obvious, not convinced it's a blocker thought.

It does make me lean into using a filter in the controller thought as we can set a deterministic field like id to get around the pagination bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

Before I add tests (in Tests_Query_Results I think(, I'd like to check if adding post_mime_type to this whitelist is okay?

IMO because post_mime_type is a field on the wp_posts table, I think including it in this allow list makes sense, rather than adding a filter to the attachments controller. But I'm not very experienced with the WP_Query class, so feel free to get a second opinion!

made a good point about a long-standing pagination bug

That is a good point. I think it's a really important bug to fix as there's going to be lots of situations where it'll crop up (i.e. order by post_author). But as you mention, this issue exists already, and there are lots of other fields where this situation can come up, so I wouldn't think of it as a blocker to this particular PR, either.

'post_parent',
'post_type',
'name',
Expand Down Expand Up @@ -1748,6 +1750,7 @@ protected function parse_orderby( $orderby ) {
case 'post_author':
case 'post_date':
case 'post_title':
case 'post_mime_type':
case 'post_modified':
case 'post_parent':
case 'post_type':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public function register_routes() {
* prepares for WP_Query.
*
* @since 4.7.0
* @since 6.9.0 Extends the `orderby` request argument to support `mime_type`.
*
* @param array $prepared_args Optional. Array of prepared arguments. Default empty array.
* @param WP_REST_Request $request Optional. Request to prepare items for.
Expand Down Expand Up @@ -100,6 +101,17 @@ protected function prepare_items_query( $prepared_args = array(), $request = nul
add_filter( 'wp_allow_query_attachment_by_filename', '__return_true' );
}

// Map to proper WP_Query orderby param - this needs to happen AFTER parent class.
if ( isset( $query_args['orderby'], $request['orderby'] ) ) {
$orderby_mappings = array(
'mime_type' => 'post_mime_type',
);

if ( isset( $orderby_mappings[ $request['orderby'] ] ) ) {
$query_args['orderby'] = $orderby_mappings[ $request['orderby'] ];
}
}

return $query_args;
}

Expand Down Expand Up @@ -1342,6 +1354,7 @@ public static function get_filename_from_disposition( $disposition_header ) {
* Retrieves the query params for collections of attachments.
*
* @since 4.7.0
* @since 6.9.0 Extends the `orderby` request argument to support `mime_type`.
*
* @return array Query parameters for the attachment collection as an array.
*/
Expand All @@ -1364,6 +1377,8 @@ public function get_collection_params() {
'type' => 'string',
);

$params['orderby']['enum'][] = 'mime_type';

return $params;
}

Expand Down
65 changes: 65 additions & 0 deletions tests/phpunit/tests/rest-api/rest-attachments-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -2827,4 +2827,69 @@ public function test_edit_image_vertical_flip_only() {
// The controller converts the integer values to booleans: 0 !== (int) 1 = true.
$this->assertSame( array( true, false ), WP_Image_Editor_Mock::$spy['flip'][0], 'Vertical flip of the image is not identical.' );
}

/**
* Test that the `orderby` parameter works with the `mime_type` parameter.
*
* @ticket 64073
*/
public function test_get_items_orderby_mime_type() {
$jpeg_id = self::factory()->attachment->create_object(
self::$test_file,
0,
array(
'post_mime_type' => 'image/jpeg',
'post_excerpt' => 'A sample caption',
)
);

$png_id = self::factory()->attachment->create_object(
self::$test_file2,
0,
array(
'post_mime_type' => 'image/png',
'post_excerpt' => 'A sample caption',
)
);

$avif_id = self::factory()->attachment->create_object(
self::$test_avif_file,
0,
array(
'post_mime_type' => 'video/avif',
'post_excerpt' => 'A sample caption',
)
);

$svg_id = self::factory()->attachment->create_object(
self::$test_svg_file,
0,
array(
'post_mime_type' => 'image/svg+xml',
'post_excerpt' => 'A sample caption',
)
);

$request = new WP_REST_Request( 'GET', '/wp/v2/media' );
$request->set_param( '_fields', 'id,mime_type' );

// Check ordering. Default ORDER is DESC.
$request->set_param( 'orderby', 'mime_type' );
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();
$this->assertCount( 4, $data, 'Response count for orderby DESC mime_type is not 4' );
// Check that ordering is working by verifying the mime types are in order
$mime_types = array_column( $data, 'mime_type' );
$expected_desc = array( 'video/avif', 'image/svg+xml', 'image/png', 'image/jpeg' );
$this->assertSame( $expected_desc, $mime_types, 'MIME types not in expected DESC order' );

// ASC order.
$request->set_param( 'order', 'asc' );
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();

$mime_types = array_column( $data, 'mime_type' );
$expected_asc = array( 'image/jpeg', 'image/png', 'image/svg+xml', 'video/avif' );
$this->assertSame( $expected_asc, $mime_types, 'MIME types not in expected ASC order' );
}
}
3 changes: 2 additions & 1 deletion tests/qunit/fixtures/wp-api-generated.js
Original file line number Diff line number Diff line change
Expand Up @@ -2903,7 +2903,8 @@ mockedApiResponse.Schema = {
"relevance",
"slug",
"include_slugs",
"title"
"title",
"mime_type"
],
"required": false
},
Expand Down
Loading