First sitemap has more URLs than expected with sticky posts

[RavanH]

When a site has $x sticky posts, and more posts than $max_urls (2000 by default) then the first posts sitemap will have $max_urls + $x URLs.

Not a big issue unless there are a huge number of sticky posts and the site is running on a relatively low resource server... Still, an unexpected sitemap size and it is easily fixed by setting ignore_sticky_posts :)

<?php
function wpsm_posts_query_args( $args ) {
        // Ignore stickyness.
        $args['ignore_sticky_posts'] = true;

        return $args;
}
add_filter( 'wp_sitemaps_posts_query_args', 'wpsm_posts_query_args' );

[pbiron]

Thanx for the ticket.

I'll have to think about this a little more, but on first thought it seems to make sense to add 'ignore_sticky_posts' => true to the default args passed to wp_sitemaps_posts_query_args.

@swissspidy do you have thoughts on this?

[swissspidy]

Yeah that sounds reasonable

[pbiron]

55633.diff​ adds 'ignore_sticky_posts' => true to the default args passed to wp_sitemaps_posts_query_args.

@RavanH can you please test this patch (I've tested on a small "play" site but would be good to test on a larger, more realistic site).

[RavanH]

Perfect! I can confirm that the patch works on a large site. :)

[pbiron]

Thanx @RavanH !!!

@swissspidy I milestoned this for 6.1, since we're already in 6.0-beta phase. However, I then noticed that is was marked as a bug...so technically, it could still go into 6.0. I'll let you, as a commiter, make that call.

[audrasjb]

Hello there,
The patch looks good to me, but I'm not sure we really need to add a comment since it is the default behavior of the ignore_sticky_posts argument :)

[pbiron]

Replying to audrasjb:

Hello there,
The patch looks good to me, but I'm not sure we really need to add a comment since it is the default behavior of the ignore_sticky_posts argument :)

That's true, but the naming of that parameter can be very confusing to those who don't know it well...so I always find it useful to include such a comment :-)

[SergeyBiryukov]

Would it be possible to include any unit tests for this?

[pbiron]

Good idea. I'll work some up this weekend and amend the patch.

[pbiron]

@SergeyBiryukov 55633.2.diff​ is the same as 55633.diff​ but adds a unit test.

[audrasjb]

Replying to pbiron:

Replying to audrasjb:

Hello there,
The patch looks good to me, but I'm not sure we really need to add a comment since it is the default behavior of the ignore_sticky_posts argument :)

That's true, but the naming of that parameter can be very confusing to those who don't know it well...so I always find it useful to include such a comment :-)

Alright, 55633.2.diff looks good to me then.

Trac ticket: https://core.trac.wordpress.org/ticket/55633

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See ​GitHub Pull Requests for Code Review in the Core Handbook for more details.

[audrasjb]

In 53548:

Sitemaps: Remove duplicate sticky Posts from Sitemap Post Query.

This changeset sets the ignore_sticky_posts parameter to true in the default arguments passed to wp_sitemaps_posts_query_args.

Props RavanH, pbiron, swissspidy, audrasjb, SergeyBiryukov.
Fixes #55633.

Committed in https://core.trac.wordpress.org/changeset/53548

[audrasjb]

In 53551:

Docs: Add missing @since mention in get_posts_query_args().

Follow-up to [53548].
See #55633.

[SergeyBiryukov]

In 53556:

Tests: Improve the test for sticky posts not being moved to the front in sitemaps.

• Give the test method a most descriptive name.
• Use a single assertion for the URL list instead of a foreach loop to provide more context in case of failure.
• Add a failure message to each assertion, as there are multiple assertions used in the test.

Follow-up to [53548].

See #55633.

[RavanH]

I've implemented this (for now, awaiting 6.1) in the plugin ​https://wordpress.org/plugins/xml-sitemaps-manager/