WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 5 months ago

#9978 new enhancement

Sticky Posts are not ordered correctly after selection

Reported by: beaulebens Owned by: ryan
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8
Component: Query Keywords: has-patch, stickies
Focuses: Cc:

Description

In WP_Query, sticky posts are added/moved to the top of the posts array after everything else is selected and ordered according to the query parameters.

Sticky posts are loaded as a list of post_IDs from the database, according to the order they were marked as sticky.

Sticky posts that were not originally part of the returned posts are queried and added into the collection of sticky posts separately.

None of the handling of sticky posts, or the handling of the entire post array after stickies are added, is date ordered by date/title/whatever was requested.

Stickies need to be loaded/extracted into a separate array, ordered (independently) according to the original query, then added to the top of the post array.

Attachments (1)

9978.diff (2.4 KB) - added by wonderboymusic 9 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 beaulebens5 years ago

See Also: #9979 for details on an ordering bug/limitation that will affect the fixing of this bug.

comment:2 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added

comment:3 Denis-de-Bernardy5 years ago

  • Type changed from defect (bug) to enhancement

wonderboymusic9 months ago

comment:4 wonderboymusic9 months ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 3.7

Moving to 3.7 for more discussion

Here is what currently happens:

  1. Posts are queried
  2. If the result set contains stickies, those stickies are arbitrarily moved to the top
  3. If there are stickies that weren't queried, and they don't appear in post_not__in, extra stickies are queried and shoved between the stickies at the top and the rest of the post
  4. No "order" is even considered

What 9978.diff does:

  1. Posts are queried
  2. If the result set contains stickies, those stickies are removed from the results
  3. All stickies that aren't in post_not__in are queried in get_posts() with the args order and orderby passed
  4. All stickies are placed at the top after returning ordered
  5. Profit.

Also of note, no stickies are currently added to $this->posts when fields is passed as an arg - expected behavior?

comment:5 wonderboymusic7 months ago

  • Keywords commit added

I still like this one - gonna let it simmer with the commit tag to see if anyone objects

comment:6 jeremyfelt7 months ago

I think we need to figure out how to test this. I just figured out how to duplicate what I think is the issue, but after applying 9978.diff, it got weirder–one sticky post showed up twice. Still working through it now to get a better grasp.

comment:7 nacin7 months ago

  • Keywords commit removed
  • Milestone changed from 3.7 to Future Release

one sticky post showed up twice

comment:8 jeremyfelt7 months ago

// Loop over posts and remove stickies.
foreach ( $this->posts as $i => $post ) {
	if ( in_array( $post->ID, $sticky_posts ) ) {
		// Remove sticky from current position
		array_splice( $this->posts, $i, 1 );
	}
}

In array_splice() here, $this->posts is rekeyed each time. As the posts are looped through, it is possible that $i stops matching and results in the removal of an incorrect post. We can add a 4th parameter to array_splice() to maintain the value, and then reindex it on our own after. Not sure if that's the best approach or not.

Also, orderby is not filled in fill_query_vars(), so an undefined index notice may be thrown when $q['orderby'] is referenced for the sticky post query if the query hasn't been filtered.

Last edited 7 months ago by jeremyfelt (previous) (diff)

comment:9 wonderboymusic7 months ago

yeah, we can let it die for now - we should do a sticky reboot in 3.9 or 3.10 or something

comment:10 wonderboymusic5 months ago

  • Keywords stickies added
Note: See TracTickets for help on using tickets.