Changes between Initial Version and Version 1 of Ticket #47642, comment 37
- Timestamp:
- 05/23/2021 06:37:26 PM (5 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #47642, comment 37
initial v1 3 3 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. 4 4 5 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 which n @ramon-fincken 's patch edits the `else` block. Which is to say, the patch does not work for my casebecause it is too restrictive in where it applies.5 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 which @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. 6 6 7 7 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: