Skip to content

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Oct 3, 2025

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'  } );

Copy link

github-actions bot commented Oct 3, 2025

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.

Copy link

github-actions bot commented Oct 3, 2025

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.

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

ramonjd and others added 4 commits October 7, 2025 16:37
…hments.

This update allows the `orderby` request argument in the attachments controller to include `mime_type`, enhancing the sorting capabilities of media items. Additionally, a new test has been added to verify the correct functionality of this feature.

Fixes trac 64073.
combine isset

Co-authored-by: Timothy Jacobs <timothy@ironbounddesigns.com>
period

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@ramonjd ramonjd force-pushed the update/attachment-controller-orderby-mime-type branch from 5369643 to 0800c1d Compare October 7, 2025 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants