[wp-trac] [WordPress Trac] #30480: wp_get_archives() should support a secondary ORDER BY column

[WordPress Trac]

#30480: wp_get_archives() should support a secondary ORDER BY column
---------------------------------+-----------------------------
 Reporter:  boonebgorges         |       Owner:
     Type:  defect (bug)         |      Status:  new
 Priority:  normal               |   Milestone:  Future Release
Component:  General              |     Version:
 Severity:  normal               |  Resolution:
 Keywords:  4.2-early has-patch  |     Focuses:
---------------------------------+-----------------------------
Changes (by boonebgorges):

 * keywords:  needs-unit-tests good-first-bug has-patch => 4.2-early has-
               patch


Comment:

 Hi herbmillerjr - Thanks for the patch.

 I spent a bit more time looking at this issue, and while the fix is very
 simple, testing it (and writing unit tests for it) is not simple at all.
 The bug *only* seems to occur when all of the following conditions hold
 (and all of which are demonstrated in the failing test at https://travis-
 ci.org/pento/develop.wordpress/jobs/41917241):
 * using 'type=postbypost'
 * using a LIMIT clause
 * running MySQL 5.1.x

 There seems to be something in this series of MySQL that causes the LIMIT
 clause to trigger a different set of rules for breaking the 'post_date'
 tie than all other versions of MySQL than I tested - in 5.1.x, 'post_date
 DESC' implies 'post_date DESC, ID ASC'.

 So, the fix that I've put in [attachment:30480.2.patch] fixes the problem.
 The clauses that you added in [attachment:30480.patch] don't actually make
 a difference in the case where there are ties with 'post_date', since the
 results are grouped by month/year/day etc anyway.

 My apologies for the misleading description that I gave above, where I
 suggested that it was necessary to use `$order` dynamically - since this
 only arises in the case of 'postbypost', which hardcodes 'post_date DESC'
 (ignoring `$order`), we can hardcode the secondary ORDER BY clause there
 too. And I don't think another unit test here would help us - the existing
 (failing) `test_wp_get_archives_limit()` is pretty much a duplicate of the
 test I'd have to write.

 Thanks again for the patch. If you come across another ticket with a
 needs-unit-tests flag, feel free to ping me and I'll be happy to point you
 in the right direction :)

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