[wp-trac] [WordPress Trac] #47642: Order by comment count - posts list tables

[WordPress Trac]

#47642: Order by comment count - posts list tables
-------------------------------------------------+-------------------------
 Reporter:  alektabor                            |       Owner:
                                                 |  johnbillion
     Type:  defect (bug)                         |      Status:  reviewing
 Priority:  normal                               |   Milestone:  5.8
Component:  Posts, Post Types                    |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-screenshots has-patch needs-     |     Focuses:
  unit-tests early early-like-actually-early     |  administration
-------------------------------------------------+-------------------------

Comment (by msea55):

 Hello people working on this ticket,

 I recently came across this issue in a
 [https://core.trac.wordpress.org/ticket/53201 slightly different context
 ]. I see that
 [https://core.trac.wordpress.org/attachment/ticket/47642/patch.47642.20201030.1
 the patch] submitted by @ramon-fincken only looks for specific order
 criteria which are likely to produce duplicates. It is my opinion that the
 fix of adding an ID sort is useful no matter the order criteria passed in.
 For example, while post_title is unlikely to produce duplicates, it is
 still possible, and I see no harm in adding ID to that as well, similarly
 @gwwar mentioned the case of `menu_order` which also may not be unique.

 The context in which I came across this issue
 https://core.trac.wordpress.org/ticket/53201 was in the admin ui on the
 page that displays posts. On this page, if no sort is selected, a sort of
 "post_date" is assigned on what is [https://github.com/WordPress
 /wordpress-develop/blob/c4fd5626638bd13c890c9cf6fe6bc50c4e4a1ff3/src/wp-
 includes/class-wp-query.php#L2328 here] line 2328. This is inside the "if"
 block of `if ( empty( $q['orderby'] ) )`, for whichn @ramon-fincken 's
 patch edits the `else` block. Which is to say, the patch does not work for
 my case because it is too restrictive in where it applies.

 I think adding `ID` is useful no matter what the passed in criteria, which
 is why I propose making the change a bit further down on what is
 [https://github.com/WordPress/wordpress-
 develop/blob/c4fd5626638bd13c890c9cf6fe6bc50c4e4a1ff3/src/wp-includes
 /class-wp-query.php#L2393 here] line 2393, after the order params, default
 order, and relevance order are all accounted for but before the filters at
 the end of the query, by adding the line `$orderby = $orderby ? $orderby .
 ', ID' : ' ID';`. I don't know how to attach a diff like you guys did but
 it'd look like:

 {{{#!php
 ...
                         if ( $search_orderby ) {
                                 $orderby = $orderby ? $search_orderby . ',
 ' . $orderby : $search_orderby;
                         }
                 }

 +               //Add additional sort by order to break ties consistently
 for pagination https://core.trac.wordpress.org/ticket/47642
 +               $orderby = $orderby ? $orderby . ', ID' : ' ID';

                 if ( is_array( $post_type ) && count( $post_type ) > 1 ) {
                         $post_type_cap = 'multiple_post_type';
                 } else {
 ...
 }}}

 This will resolve the issue for all sort orders except `rand`. For `rand`
 this addition doesn't hurt, but it also doesn't prevent this issue.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47642#comment:37>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform