WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 6 weeks ago

#8107 new defect (bug)

get_next_post, get_previous_post do not work for posts posted within same second

Reported by: whoismanu Owned by:
Milestone: Future Release Priority: low
Severity: minor Version: 2.7
Component: Posts, Post Types Keywords: needs-testing dev-feedback
Focuses: template Cc:

Description

if you have posts that are published shortly one after the other (e.g. through a script or plugin that posts several posts at once) several of them may end up having the same post_date in the wordpress database table. this is due to the fact that mysql datetime seems to only maintain a precision of one second (see also this discussion: http://bugs.mysql.com/bug.php?id=8523).

the problem now is that wordpress functions like get_next_post/get_previous_post (get_adjacent_post resp.) will no longer work correctly if something like this happens as they solely rely on a comparison of the post_date field and they don't treat the case where these timestamps are the same for several posts. the result is that e.g. get_next_post will pick one of the posts having the same timestamp and "jump" over the others, so the user will never see them.

i see two possibilities around this 1.) treat cases with the same post_date by e.g. looking also at the post id (assuming it is always strictly increasing) or probably preferably 2.) make sure that no two posts have the same post_date timestamp by e.g. increasing post_date artificially when publishing the post and if another post already has the same timestamp.

Attachments (2)

8107.patch (1.2 KB) - added by Viper007Bond 5 years ago.
8107.2.patch (1.2 KB) - added by Viper007Bond 5 years ago.
Fix mistype (I had %s instead of %d)

Download all attachments as: .zip

Change History (19)

comment:1 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; post_date get_next_post datetime removed
  • Milestone changed from 2.8 to Future Release

comment:2 Viper007Bond5 years ago

  • Component changed from General to Template
  • Severity changed from normal to minor

Ooof, unless I'm missing something, this is a tricky patch.

The trick is to change the older/newer operator to older/newer OR exact same time AND not the same post ID. That makes the same-time post show up in the next/prev links, but the problem is you then get stuck in a loop, i.e. you go from post A to post B and back to post A. You also can't do a less-than/greater-than on the ID because IDs don't necessarily correspond to the datetime (i.e. you could have older posts with IDs both higher and lower than the current post).

As I said, seems to be pretty hard to get it not stuck in a loop.

comment:3 Denis-de-Bernardy5 years ago

  • Keywords 2nd-opinion added; needs-patch removed

I'd say wontfix/plugin material.

the case would only arise when the publishing of posts is automated. it can, and should, be dealt with by whichever tool is used to publish the post in the first place -- it makes no sense to publish two posts in one go anyway...

comment:4 DD325 years ago

It could also be caused by publishing a group of drafts via QuickEdit too i think.

A 2-part conditional such as this could work however

(IRC) DD32	Needs to have an order with an ID, and a conditional for ( ( postdate == now && ID < currentID ) || postdate < now)

Viper007Bond5 years ago

comment:5 Viper007Bond5 years ago

See patch. :)

Two minds are indeed better than one. Props DD32 for the inspiration.

comment:6 Viper007Bond5 years ago

  • Keywords has-patch needs-testing added; 2nd-opinion removed

Viper007Bond5 years ago

Fix mistype (I had %s instead of %d)

comment:7 Denis-de-Bernardy5 years ago

  • Milestone changed from Future Release to 2.9
  • Priority changed from normal to low

comment:8 ryan4 years ago

  • Milestone changed from 2.9 to Future Release

comment:9 Viper007Bond4 years ago

  • Milestone changed from Future Release to 3.1

Let's finally test this and fix this.

comment:10 nacin3 years ago

  • Milestone changed from Awaiting Triage to Future Release

Marked #12650 as a duplicate. Is this patch still good? I noted in the other ticket that I'm not sure you can rely on the ID, but it's better than skipping the post all together.

comment:11 salcode6 months ago

  • Cc salcode added

comment:12 nacin3 months ago

  • Component changed from Template to Posts, Post Types
  • Focuses template added

comment:13 billerickson7 weeks ago

no, this patch is no longer good due to the introduction of WP_Adjacent_Post in #26937

comment:14 follow-up: billerickson7 weeks ago

  • Keywords dev-feedback added; has-patch removed

In versions 3.8 and less, when using previous/next post links any posts with the same published date and time as the current post are skipped.

In version 3.9, you get stuck in a loop, going back and forth between posts in the same date. This is because the date_query has 'inclusive' => true and we're excluding the current post.

I played around with it a bit and couldn't come up with a single WP Query that queried either posts ordered by date with different published dates OR posts ordered by ID with the same published date. Doing 'orderby' => 'date ID' didn't do it.

I think we might need to do multiple queries. Keep the current one, but before we return the adjacent post, check to see if the date matches the current post. If it does, query for posts with that same date, sorted by post ID (to end the loop we're in). If no results for that, do the original adjacent post query but set 'inclusive' => false.

I'm happy to write the patch, but that's a lot of queries. Is there a more efficient approach?

Last edited 7 weeks ago by billerickson (previous) (diff)

comment:15 Viper007Bond7 weeks ago

You shouldn't need multiple queries, especially for an edge case like this (not worth adding more queries for 0.0001% of cases). I got it working with just one via my patch. We should be able to come up with some solution using WP_Query, even if we have to add functionality to WP_Query.

Last edited 7 weeks ago by Viper007Bond (previous) (diff)

comment:16 in reply to: ↑ 14 SergeyBiryukov7 weeks ago

Replying to billerickson:

I played around with it a bit and couldn't come up with a single WP Query that queried either posts ordered by date with different published dates OR posts ordered by ID with the same published date. Doing 'orderby' => 'date ID' didn't do it.

'order' => 'ASC', 'orderby' => 'date ID' with a subsequent array_reverse() would probably work, see comment:14:ticket:26042 and 26042.3.diff:ticket:26042.

comment:17 billerickson6 weeks ago

What would the array_reverse() be applied to? We're querying for a single post ( 'posts_per_page' => 1 ).

If you have two posts with the same datetime, then the adjacent post will always be the other one. Key query_vars:

'posts_per_page' => 1,
'post__not_in' => array( $this->current_post->ID ),
'date_query' => array(
  array(
      $date_query_key => $this->current_post->post_date,
      'inclusive' => true,
  )
),

Whether you're looking for previous or next, both posts will be included due to the inclusive line. And since we're excluding the current post, the other post will always be returned.

Note: See TracTickets for help on using tickets.