WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 11 months ago

#42261 new enhancement

Add Support for No Limit Queries to 'posts_per_rss' .

Reported by: ohryan Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.8.2
Component: Feeds Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

Setting posts_per_rss = -1 will result in a query with LIMIT 0, -1.

Attachments (3)

42261.patch (1.9 KB) - added by mauteri 18 months ago.
Issue was that setting posts_per_page from posts_per_rss should be before checking nopaging and that post_per_page is equal to -1. included unit test in patch.
42261.1.diff (1.9 KB) - added by mauteri 14 months ago.
Fix unit test
42261.2.diff (4.2 KB) - added by mauteri 11 months ago.
Update to fix comment feed to support -1 plus unit test

Download all attachments as: .zip

Change History (10)

#1 @stevenkword
2 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

Thank you for the report @ohryan.

As far as I'm aware, there isn't any document that states this query variable supports this value, but that doesn't mean it shouldn't. I simply wouldn't classify this as a bug and more like an enhancement.

I am in favor of supporting the functionality since the variable is passed on to posts_per_page in WP_Query. The problem seems to be more at the "LIMIT" filters which could be updated. This enhancement would require consideration for comment feeds as well.

Tests should be written against RSS and Atom and should work for all feeds endpoints (home, custom, comments, custom post types, etc.).

#2 @stevenkword
2 years ago

  • Summary changed from Setting posts_per_rss = -1 fails. to Add Support for No Limit Queries to 'posts_per_rss' .

@mauteri
18 months ago

Issue was that setting posts_per_page from posts_per_rss should be before checking nopaging and that post_per_page is equal to -1. included unit test in patch.

#3 @mauteri
18 months ago

  • Keywords needs-patch needs-unit-tests removed

#4 @jorbin
15 months ago

The patch in the test should be updated to not use an anonymous function, and should be reworked in order to not need to be nested.

As mentioned by @stevenkword, comment feeds should also be tested.

#5 @jorbin
15 months ago

  • Keywords needs-patch added

@mauteri
14 months ago

Fix unit test

#6 @mauteri
14 months ago

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

@jorbin @stevenkword I updated the diff and made the filter not a call to an anonymous function. As far as non-post feeds go, this is not related. The bug itself was a bug in get_posts and setting posts_per_page from posts_per_rss, which needed to be checked before nopaging and that post_per_page is equal to -1.

@mauteri
11 months ago

Update to fix comment feed to support -1 plus unit test

#7 @mauteri
11 months ago

@jorbin @stevenkword updated to code. I was wrong, there was also a spot where this would fail for comments feed. I updated diff to fix this issue and added a unit test for it as well.

Note: See TracTickets for help on using tickets.