Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Query block respect perPage option when sticky posts exist #41184

Open
Tracked by #41405
roo2 opened this issue May 20, 2022 · 1 comment · Fixed by WordPress/wporg-showcase-2022#145
Open
Tracked by #41405
Labels
[Block] Query Loop Affects the Query Loop Block Needs Decision Needs a decision to be actionable or relevant

Comments

@roo2
Copy link
Contributor

roo2 commented May 20, 2022

Currently if we have sticky posts, they will be prepended to the results of the query block, regardless of what was set in the perPage property.

For example, if we had two posts, one "sticky":

Screen Shot 2022-05-20 at 3 05 38 pm

And the following query in my homepage:

<!-- wp:query {"queryId":35,"query":{"perPage":"1" } } -->

Intuitively, I would expect to see only one post, but I am shown two:

Screen Shot 2022-05-20 at 3 04 36 pm

Why this happens

The Query block is built on top of the WP_Query class [WP_Query docs], which is where this behavior originates WP_Query issue marked as won't fix. This is the default behavior and probably won't be changed on WP_Query, but the core/query block does not have to match the interface of WP_Query exactly (and doesn't, in https://github.com/WordPress/gutenberg/pull/38909/files we change the arguments passed to WP_Query to give more intuitive results for "sticky": "only")

Suggested behavior

There are currently three options for the sticky setting.

Screen Shot 2022-05-20 at 3 37 24 pm

I suggest changing the behavior of the "Include" option (which generates the block markup: "sticky": ""). This is also the default behavior if no "sticky" option is set.

I suggest changing the Include option so that it includes sticky posts first in the list but will respect the perPage option. So a query with <!-- wp:query {"queryId":35,"query":{"perPage":"1", "sticky": "" } } --> would return a single post. Either the latest stickied post, or, if no sticky posts exist, the latest non-stickied post.

Screen Shot 2022-05-20 at 4 26 22 pm

Pseudocode implementation

I replaced the sticky property handling code in build_query_vars_from_query_block with the following:

$sticky = get_option( 'sticky_posts' );
if ( isset( $block->context['query']['sticky'] ) && ! empty( $block->context['query']['sticky'] ) ) {
    /* handle "sticky": "exclude" and "sticky": "only" as before.
     * ...
     */
} else {
    // handle "sticky": "", or case where "sticky" is not set
    $query['ignore_sticky_posts'] = 1;
    $query['post__in'] = $sticky;
}

This change resulted in the behavior I expected, selecting sticky posts first while respecting the "perPage" option. I also chcecked that order still works as before

I was able to test the change by hacking build_query_vars_from_query_block in wp-content/blog.php directly (tested on wpcom ;) ), but it looks like it can be implemented in gutenberg_build_query_vars_from_query_block in the lib/compat folder of gutenberg e.g. https://github.com/WordPress/gutenberg/blob/trunk/lib/compat/wordpress-6.0/blocks.php#L37

@dsas
Copy link
Contributor

dsas commented Feb 4, 2024

Reopening, I can still reproduce the issue on GB 17.6 with WP 6.4.3. I think this was accidentally closed by WordPress/wporg-showcase-2022#145 which worked around the issue in the site showcase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block Needs Decision Needs a decision to be actionable or relevant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants