Make WordPress Core

Opened 5 weeks ago

Last modified 3 weeks ago

#64073 new enhancement

Attachments REST API: extend to support ordering by `mime_type`.

Reported by: ramonopoly's profile ramonopoly Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: REST API Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:

Description

To support dataviews tables it would be helpful to be able to sort a set of attachment results by mime_type.

The means that the response results could be ordered ASC/DESC, eg.,

[
{ "mime_type": "image/gif", ... },
{ "mime_type": "video/mp4", ... },
]

This could be done by extending the $params in WP_REST_Attachments_Controller::get_collection_params with $params['orderby']['enum'][] = 'mime_type';

Change History (9)

This ticket was mentioned in PR #10135 on WordPress/wordpress-develop by @ramonopoly.


5 weeks ago
#1

  • Keywords has-patch has-unit-tests added

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

#2 @TimothyBlynJacobs
5 weeks ago

I'm totally failing to find this ticket. But WP_Query has an issue where doing an order by by a non-unique field results in posts being returned in an unpredictable order. That means, as a user paginates over the query, they may end up not seeing all posts and instead see duplicates depending on how MySQL decides to order posts with the same mime_type.

So we may not want to ship this as a feature until that issue is resolved.

#3 @ramonopoly
5 weeks ago

@TimothyBlynJacobs Thanks for the note here.

Are these the ones you're referring to?

See:

https://core.trac.wordpress.org/ticket/47642

https://core.trac.wordpress.org/ticket/52907

Looks like it's related to a long-standing Core issue where posts share the non-unique fields as you say - I came across it working with post_date

I think the solution has something to do with adding deterministic ordering, e.g., ordering also by ID

#47642 has a patch, it's a bit old.

Maybe a filter could work to avoid that blocker?

/*
 * Ensure deterministic ordering to prevent duplicate records across pages.
 * When multiple attachments have the same mime_type, add ID as secondary sort to guarantee consistent ordering.
 * This prevents the same attachment from appearing on multiple pages when filtering by media_type.
 */
if ( empty( $query_args['orderby'] ) || 'post_mime_type' === $query_args['orderby'] ) {
    $query_args['orderby'] = array(
        'post_mime_type' => $query_args['order'] ?? 'DESC',
        'ID'             => 'DESC',
    );
}

#4 @ramonopoly
5 weeks ago

So we may not want to ship this as a feature until that issue is resolved.

I was just wondering - this bug is already noticeable for any post type and has been around for years.

You can recreate it for posts with the same post_date.

I agree it should be fixed, but I'm not convinced it should be a blocker for this change.

The only argument I can glean is that this change might make that bug more obvious.

@ramonopoly commented on PR #10135:


4 weeks ago
#5

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!

@ramonopoly commented on PR #10135:


4 weeks ago
#6

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.

#7 @ramonopoly
3 weeks ago

I'm totally failing to find this ticket. But WP_Query has an issue where doing an order by by a non-unique field results in posts being returned in an unpredictable order. That means, as a user paginates over the query, they may end up not seeing all posts and instead see duplicates depending on how MySQL decides to order posts with the same mime_type.

Seems like this bug, or a version of it, has existed for more than 17 years

https://core.trac.wordpress.org/ticket/8107

#8 @TimothyBlynJacobs
3 weeks ago

Yep, those are exactly the tickets I was thinking of. Thanks for digging them up!

I think the difference, and why I'd consider waiting to ship this until the bug is resolved, is that MIME types are basically never going to be unique. Most folks will probably only have a few. So users are almost certainly going to run into broken behavior when trying to use this feature.

#9 @ramonopoly
3 weeks ago

I think the difference, and why I'd consider waiting to ship this until the bug is resolved, is that MIME types are basically never going to be unique. Most folks will probably only have a few. So users are almost certainly going to run into broken behavior when trying to use this feature.

Agree it needs to be fixed. Every patch that has proposed to do so in the last 20 years has fizzled out.

I've optimistically started a new one!

https://github.com/WordPress/wordpress-develop/pull/10262

Note: See TracTickets for help on using tickets.