WordPress / wordpress.org

WordPress.org Meta, Git-ified. Synced from git://meta.git.wordpress.org/ This repository is just a mirror of the WordPress Meta subversion repository. Please include a link to a pre-existing ticket on https://meta.trac.wordpress.org/ with every pull request.
https://meta.trac.wordpress.org/
109 stars 140 forks source link

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

Closed ryelle closed 3 months ago

ryelle commented 3 months ago

See some discussion on https://github.com/WordPress/wporg-theme-directory/issues/69, https://github.com/WordPress/wporg-theme-directory/issues/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

ryelle commented 3 months ago

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 🙂