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

Page Limiter: Update hook for setting max values so it applies to cached queries. #322

Closed

Conversation

 ryelle
Copy link
Contributor

 @ryelle ryelle commented May 21, 2024

See some discussion on WordPress/wporg-theme-directory#69 , WordPress/wporg-theme-directory#68 .

To recap, on the new theme directory preview , the count of total themes & pages for a given archive would sometimes use the "49 page limit" set in WPORG_Page_Limiter , regardless of logged-in status. This seems to be due to query caching, the found_posts filter only runs before caching, not again after returning the cached results. The found_posts value is cached along with the results, so it's saved when the filter runs for a logged out user, and then returned for a logged in user.

The opposite was also true, logged-out users might see the full 100+ pages available (though attempting to view page 50+ would correctly return a 404).

This PR switches the filter for updating found_posts to run on posts_results and set the query properties directly. This ensures the filter is run for all queries.

I've also set a custom property on $query for the original found_posts value, so that it can be passed through to the Query Total block. Like so: https://github.com/WordPress/wporg-mu-plugins/compare/try/page-limiter-hook — This is a general issue with the page-limit code + Query Total block, we just haven't run into it until now.

To test

  • View the theme directory
  • Logged in views should show all pages
  • Logged out views should show a max of 49 pages
  • Ensure this works on other sites: News, Patterns, Plugins, etc

 @ryelle
Copy link
Contributor Author

I'm going to commit this now since it's a pretty visible bug on the theme directory, and I want to open that to wider feedback today. But feel free to leave any feedback here and I can iterate on it 🙂

bazza pushed a commit that referenced this pull request May 22, 2024
 @ryelle ryelle deleted the update/page-limiter-hook branch May 22, 2024 18:22
Sign up for free to join this conversation on GitHub . Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

1 participant