ryelle
commented
3 months ago @dd32 or @pkevan if you have some time, could you check this out?
Closed ryelle closed 3 months ago
ryelle
commented
3 months ago @dd32 or @pkevan if you have some time, could you check this out?
dd32
commented
3 months ago The cause however is that Jetpack Search is including draft posts in the search results, as the search isn't filtering to post_status=publish for some reason. It should be though, as the WP_Query does include a post_status specification.. AFAIK...
However, previously Jetpack Search wasn't involved in the Theme directory, as we have several SQL modifications to the search which Jetpack Search isn't respecting, since it's unaware of it.
I can't find any custom JP search code for the theme directory, and it seems like non-English theme directory searches are completely broken..
Ahhhh Jetpack Search is only applying for direct /search/...../ requests, using the search form on the page (Which uses in-page JS) goes via the API which does not use Jetpack Search right now
The Jetpack Search Settings page is currently broken, so I've turned JP search off by updating the option directly.
If we want to use Jetpack Search here we need to migrate those search-related query modifications over and probably tweak the fields it's querying.
StevenDufresne
commented
3 months ago Will we be able to fix this? I see the problem still persists.
ryelle
commented
3 months ago I don't think we need to move to Jetpack Search right now (though I say that mostly because it seems like it would be more complicated than our "simple refresh" scope).
That said, I'm also still seeing Jetpack Search results on /search/…/, so it is still enabled. Maybe it should be disabled in code? (for my testing, I disabled it on my sandbox with add_filter( 'jetpack_search_should_handle_query', '__return_false', 100 );).
Also, I think something is caching these queries still (not just for search, but the combination is making the bug more obvious). I've disabled "Advanced Post Caching", but I'm still seeing unexpected results.
Was the previous query cached?
The other maybe-cache issue is that sometimes results for queries are being limited (or not) to 49 pages when they shouldn't be. Logged in users should always see the full result set, while logged out users are limited to 49 pages. But instead, it seems to be semi-random, maybe related to who ran the query first. But it's always the same— if I load https://wordpress.org/themes/browse/commercial/ and see 49 pages, it will be that way logged in and logged out.
dd32
commented
3 months ago That said, I'm also still seeing Jetpack Search results on /search/…/, so it is still enabled.
Yup :( It looks like my attempts to disable it failed..
I still can't get https://wordpress.org/themes/wp-admin/admin.php?page=jetpack-search to work, it throws a Javascript error about must_upgrade undefined. I've confirmed it doesn't have a Jetpack Search subscription..
Oh.. mu-plugins/jetpack-search.php.. That's forced jetpack search to be active for every site.
Disabled. Jetpack Search is no longer active.
Also, I think something is caching these queries still (not just for search, but the combination is making the bug more obvious). I've disabled "Advanced Post Caching", but I'm still seeing unexpected results.
Ah, yes, there is additional caching in play. https://github.com/WordPress/wordpress.org/blob/trunk/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php#L553-L557
That does NOT cache WP_Query as used for page template loads, but caches the API data, and at least with the existing theme, that API cached data is used for the Javascript that immediately repaints the page.
Stiofan
commented
3 months ago As the reporter of this bug, just to make you aware, the more you test this bug, the more it will look "fixed." Testing it over and over and over actually seems to fix it, and that result shows for everyone after that but then breaks again later. So there is 100% some caching element going on also.
ryelle
commented
3 months ago Jetpack Search is no longer active.
Great, it's working properly for me now. I've also tested with the original query "school" from the meta ticket, and that seems to be correct now as well. Thanks for tracking that down!
I've also figured out the caching issue for the theme counts + 49 page limit, I've documented and proposed a fix here: https://github.com/WordPress/wordpress.org/pull/322
Possibly related to https://meta.trac.wordpress.org/ticket/7277.
On the new theme, searching for "twenty" returns only 3 items on the first page, but claims to have 445 items, and 38 pages. Page 2 has 9 items, and so on. All pages should have 12 items until the end.
I think the search is using ElasticSearch, which is returning 12 items, but 9 of those are draft themes.
It looks like the same thing is happening on production, but the theme doesn't have normal pagination so it's not a visible error.