Make WordPress Core

Opened 8 years ago

Closed 2 years ago

#36907 closed enhancement (fixed)

Improved sticky post query

Reported by: spacedmonkey's profile spacedmonkey Owned by: spacedmonkey's profile spacedmonkey
Milestone: 6.0 Priority: normal
Severity: normal Version: 3.5
Component: Query Keywords:
Focuses: performance Cc:

Description

Currently with the WP_Query class calls a get_posts to get a array of posts to include on the home / blog page. However this get_posts will not obey parameters passed to the main WP_Query class.

Attachments (4)

36907.patch (970 bytes) - added by spacedmonkey 8 years ago.
#36907.patch (979 bytes) - added by rehanali 2 years ago.
Added patch
36907.diff (955 bytes) - added by SergeyBiryukov 2 years ago.
36907.2.diff (611 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (24)

@spacedmonkey
8 years ago

#1 @spacedmonkey
8 years ago

  • Keywords has-patch needs-unit-tests added

#2 @spacedmonkey
2 years ago

  • Milestone set to 6.0
  • Owner set to spacedmonkey
  • Status changed from new to assigned

@rehanali
2 years ago

Added patch

#3 @peterwilsoncc
2 years ago

36907.patch looks good to me.

Not to self so I don't look it up again, get_posts() has no_found_rows set already so it's not needed here.

This ticket was mentioned in PR #2445 on WordPress/wordpress-develop by spacedmonkey.


2 years ago
#4

  • Keywords has-unit-tests added; needs-unit-tests removed

#5 @peterwilsoncc
2 years ago

  • Keywords commit added

The pull request looks good for commit, marking as such.

#6 @spacedmonkey
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 52982:

Query: Improved sticky post query

Ensure that the parameters update_post_meta_cache, update_post_term_cache, cache_results, suppress_filters and
lazy_load_term_meta that are passed to WP_Query, are also passed down to the secondary query used to get sticky
posts.

Props Spacedmonkey, peterwilsoncc, rehanali, uday17035.
Fixes #36907.

spacedmonkey commented on PR #2445:


2 years ago
#7

Merged.

@SergeyBiryukov
2 years ago

#8 @SergeyBiryukov
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It's not quite clear why [52982] removed the 'nopaging' => true argument from the get_posts() call in question, as neither the ticket nor the commit message mention the removal.

With that change, only 5 sticky posts are displayed now. This is a backward compatibility break, see #55444.

36907.diff restores 'nopaging' => true and reorder the parameters for consistency with similar calls elsewhere.

#9 @SergeyBiryukov
2 years ago

In 52985:

Tests: Use a more descriptive name for the sticky posts test that verifies the parameters from the main query.

Reorder the parameters of the get_posts() call for consistency with similar calls elsewhere.

Follow-up to [52982].

See #36907.

#10 @SergeyBiryukov
2 years ago

36907.2.diff is a refresh after [52985].

This could also use a unit test.

#11 @SergeyBiryukov
2 years ago

#55444 was marked as a duplicate.

#12 @spacedmonkey
2 years ago

Good catch @SergeyBiryukov . No paging / posts per page was removed at the last minute. It should not have been. I have created a follow on PR at #2454. Can you take a look?

peterwilsoncc commented on PR #2454:


2 years ago
#15

LGTM.

Are you able to put the following test in to Tests_Query_Stickies be sure it's not broken next time?

{{{php
/

  • Ensure all sticky posts are included, no matter how many. *
  • @ticket 36907 */

public function test_query_should_return_all_stickies() {

$old_option = get_option( 'sticky_posts' );
foreach ( self::$posts as $post_id ) {

stick_post( $post_id );

}

$q = new WP_Query( array( 'post_type' => 'post' ) );
22 sticky posts, 1 hello world.
$this->assertCount( 23, $q->posts );

update_option( 'sticky_posts', $old_option );

}
}}}

I am not sure if the options table is included in the test suite's DB transaction. If it is then the last line can be dropped as the table should bre reset as part of tear down.

#16 @peterwilsoncc
2 years ago

  • Keywords commit removed

Removing commit keyword while the follow up commit is finalised.

spacedmonkey commented on PR #2454:


2 years ago
#17

@peterwilsoncc Added a unit test.

#18 @spacedmonkey
2 years ago

In 52990:

Query: Ensure that sticky post query returns all sticky posts.

Ensure that the posts_per_page parameter submit to sticky post query matches the number of sticky posts requested.

Follow-up to [52982]

Props Spacedmonkey, peterwilsoncc.
See #36907.

#19 @peterwilsoncc
2 years ago

  • Keywords has-patch has-unit-tests removed

Just tidying up keywords to keep reports clean.

@spacedmonkey I presume you're keeping this open to monitor, are you able to close once you're happy.

#20 @spacedmonkey
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.