SEO: page through the Content tab's posts and pages#50411
Open
xavier-lc wants to merge 4 commits into
Open
Conversation
`useEntityRecords` resolves exactly one query per call, which is a dead end for paging: the number of pages isn't known until page 1 comes back, and hooks can't be called in a loop. Drop to the selectors the wrapper already calls — same store, same entity cache, same resolvers — behind a `selectPostType` helper mapped over the post types the tab covers. No behaviour change: still one `per_page=100` request per type, and `hasFinishedResolution` matches the wrapper's `hasResolved` exactly (both count a failed request as done rather than spinning forever). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The tab made one `per_page=100` request per post type and merged the results, so a site with more than 100 posts (or 100 pages) silently dropped the overflow — no pagination, no notice. An owner could conclude those posts had no SEO gaps because they weren't listed at all. Page through each collection instead, using the total page count core-data derives from `X-WP-TotalPages`. Selecting a page that hasn't been fetched starts its resolver, so the loop both reads and drives the requests: page 1 of each type goes out first, then every remaining page once the total is known. Verified on a site with 2502 posts + 150 pages: 2652 rows where trunk showed 200, via 27 REST requests, 79kB transferred, ~6s to load. The host serves ~4 requests concurrently (~1s each), so load time grows linearly with content. Throttling the fan-out to a sliding window of 5 pages per type was tried and measured. It cut achieved concurrency from ~4 to ~2.4, because each slide waits on a store update and a React commit: the same site took ~10s, and a much larger site would take longer, not shorter. The window only lowers peak socket count, so it was dropped. What actually scales is server-side pagination, which core REST can't back today — it offers no collection spanning two post types, and no filtering on the SEO post meta the tab's filters read. Hence the client-side merge, and hence the comment pointing the next reader at the real fix. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
DataViews only matches the search string against fields that opt in with `enableGlobalSearch`, which defaults to false. No field on the Content tab set it, so `.some()` ran over an empty list of searchable fields and every row was filtered out: typing anything produced "No results", even "Page" on a site full of pages. Search the title, the SEO title and the meta description. The latter two rendered a Set/Not set badge and their `getValue` returned those sentinel strings, so enabling the flag alone would have matched the badge text rather than the content — searching "set" would return every row with an SEO title. They now return the underlying text; the badges are drawn by `render` from the `has*` flags, and the set/not-set state keeps its own filter-only field. Verified against the real `filterSortAndPaginate`: "Page" matches the page rows, "greeting" matches an SEO title, "feline" matches a meta description, and "set"/"not_set" match nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes JETPACK-1847
Proposed changes
Page through the Content tab's posts and pages. The tab made a single
per_page=100request per post type and merged the results, so a site with more than 100 posts (or 100 pages) silently dropped the overflow — no pagination, no notice. An owner could conclude those posts had no SEO gaps because they were never listed.useSeoPostsnow walks each collection using the total page count core-data derives fromX-WP-TotalPages.Fix the Content tab's search box, which matched nothing..
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Needs a site with more than 100 posts and more than 100 pages — the bug is invisible below that.
Pageinto the search box.setornot_setmatches nothing (it searches the content, not the badge label).Performance note
Once the total is known, every remaining page is requested at once. A host serves only as many as it has PHP workers (~4 concurrent, ~1s each on a stock Atomic sandbox), so ~2.5k posts load in ~6s and the cost grows linearly; tens of thousands of posts would mean hundreds of concurrent
WP_Query-with-meta requests.Throttling that fan-out to a sliding window of 5 pages per type was tried and measured: achieved concurrency fell from ~4 to ~2.4 (each slide waits on a store update and a React commit), so the same site took ~10s and a larger one would take longer, not shorter. The window only lowers peak socket count, so it was dropped. The change that actually scales is server-side pagination, which core REST can't back today — no collection spans two post types, and none filters on the SEO post meta the tab's filters read. Hence the client-side merge. Worth its own issue under JETPACK-1843, alongside JETPACK-1796 (custom post types), which needs the same endpoint.