REST API: Extend orderby parameter to support mime_type for attachments.

[ramonjd]

This update allows the orderby request argument in the attachments controller to include mime_type, enhancing the sorting capabilities of media items.

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

Testing

npm run test:php -- --filter WP_Test_REST_Attachments_Controller

In the block editor:

// desc by default, e.g., video, audio
await wp.data.resolveSelect( 'core' ).getEntityRecords( 'postType', 'attachment', { orderby: 'mime_type'  } );

// asc, e.g., audio, video
await wp.data.resolveSelect( 'core' ).getEntityRecords( 'postType', 'attachment', { orderby: 'mime_type', order: 'asc'  } );

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props ramonopoly, mukesh27, timothyblynjacobs, andrewserong.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[ramonjd]

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}";
}

[ramonjd]

@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.

[andrewserong]

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.

[andrewserong]

This is testing well for me, and the code change looks consistent with how other controllers handle the orderby mapping 👍

I'll leave it up to you to decide whether or not this should land before or after fixing https://core.trac.wordpress.org/ticket/44349. From my perspective, given that other fields where duplicate values are common (like author) are already supported for orderby, I don't think it's necessarily a blocker for landing.

That said, it'd be a great bug to get fixed!

This LGTM.

[ramonjd]

and the code change looks consistent with how other controllers handle the orderby mapping

Thanks for the retest @andrewserong

I'll also see about test coverage for the WP_Query change.

I'll leave it up to you to decide whether or not this should land before or after fixing core.trac.wordpress.org/ticket/44349

that would be a good one to pick up, it's old and might have baggage so I'm not confident it would be smooth sailing.

Agree that it shouldn't block this change. Thanks!

[ramonjd]

that would be a good one to pick up, it's old and might have baggage so I'm not confident it would be smooth sailing.

I'll see if I can have a crack.