Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.41k stars 1.99k forks source link

Twenty Twenty themes are showing up as upgrade required #94310

Closed lsl closed 2 weeks ago

lsl commented 1 month ago

Quick summary

When searching for twenty twenty themes, e.g. https://wordpress.com/themes/all?s=2011 or https://wordpress.com/themes/all?s=2013 the upgrade badge is shown despite no upgrade being required.

This isn't happening in all cases but searching directly is relatively reproducible.

Upgrades are not actually required.

Steps to reproduce

https://wordpress.com/themes/all?s=2011

What you expected to happen

Not be showing an upgrade badge

What actually happened

Upgrade badge shown

Screenshot(33)

Impact

Some (< 50%)

Available workarounds?

Yes, easy to implement

If the above answer is "Yes...", outline the workaround.

No response

Platform (Simple and/or Atomic)

No response

Logs or notes

p1725838592447979-slack-C07H21B2W59

lsl commented 1 month ago

Upgrades are not actually required, tagging high priority because it looks bad.

lsl commented 1 month ago

Seems to go away if you view the theme directly and then go back to the search page, could be something to do with what info we load from dotorg and when.

dsas commented 1 month ago

Seems to go away if you view the theme directly and then go back to the search page, could be something to do with what info we load from dotorg and when.

Searching in /themes separately searches both the dotcom and dotorg indexes.

Twenty Eleven is "delisted" on dotcom, meaning that it doesn't show up when browsing or in search results because we don't want to promote the theme, however we do want to support the people already using it and not give them any cause for alarm, so the theme detail page continues to work.

This issue happens because the search for 2011 shows the dotorg search result, and the theme info page shows the result from dotcom.

dsas commented 1 month ago

A potential (but perhaps partial) fix could be to ignored "delisted" when searching, or at least when for searches that match the title field.

dsas commented 1 month ago

Oh, I've misremembered, it should already ignore "delisted" when searching on the name field: fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Syvo%2Sgurzrf%2Srynfgvpfrnepu.cuc%3Se%3Q74q5o7or%23372-og

2011 isn't the name but searching twenty eleven shows the same problem

rcrdortiz commented 1 month ago

A potential fix could be to always match search results against our WPCom theme list and override community versions with our WPCom version so that the Upgrade badge isn't shown.

The issue seems to be that when searching for the term "2011" we get no results from WPCom themes but we do get results from the .org search. I don't think it's feasible to implement the same search engine on WPCom so the next easiest solution is matching against our WPCom themes.

lsl commented 1 month ago

Just filter them from the dotorg list too? Going by the p2 you linked in slack they were delisted because they have various problems or aren't being maintained.

It sounds like you can still install it if we link to the listing directly using the slug, that's a good enough workaround to start with.

zaguiini commented 3 weeks ago

@lsl do you mean hardcoding a list of ignored themes in Calypso? That's a new layer of indirection.

The actual problem is that some TT themes don't exist in our ES index (and thus never gets returned), nor is marked as retired in the list (fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Syvo%2Sjcpbz%2Qgurzrf.cuc%3Se%3Q71s3n5r7%2315-og.)

Our theme search actually wants to include retired themes:

image

But since there's no matched result, it assumes it's a regular 3rd-party theme and requires a Business site to be installed.

My recommendation here is to add these retired themes to the ES index so they are returned when searched for. We could additionally add some documentation so that future retirees are also added.

cc @dsas @rcrdortiz what do you think?

dsas commented 3 weeks ago

@zaguiini, it's kinda confusing but there's a difference between "delisted" and "retired", checkout the docs here: PCYsg-S9k-p2

Twenty Eleven is in our elastic search index, see the URL in 35b96-pb

zaguiini commented 3 weeks ago

Will not show in the theme listing, but it will show in search results

That's true; I can fetch it by searching for "eleven" instead of "2011."

image

I'm guessing that we're only searching within the title. What do you think of including description, too? That way, it'd mimic the Dotorg index behavior. "2011" is in the description.

I tried adding delisted: true to the Dotcom search, but it still doesn't return the theme if I search for 2011 specifically. So, it does seem that the problem happens because we're not adding the description when fetching themes while Dotorg does.

dsas commented 2 weeks ago

It is searching within the description, if you look at build_modern_queries (fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Syvo%2Sgurzrf%2Srynfgvpfrnepu.cuc%3Se%3Qq9sr9pn2%23ohvyq_zbqrea_dhrevrf-og) it is searching over all_content which is a concatenation of various fields including the description.

If I add &delisted=true to the search URL then it doesn't include delisted themes - I don't get a search result for 2011.

If I comment out the delisted filter here fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Syvo%2Sgurzrf%2Srynfgvpfrnepu.cuc%3Se%3Qq9sr9pn2%23531-og then I do get a search result for 2011.

Looking at the API registration (fbhepr%2Skers%2Sjcpbz%2Schoyvp.ncv%2Serfg%2Sjcpbz%2Qwfba%2Qraqcbvagf.cuc%3Se%3Q6nn818o2%231165-og) it doesn't declare delisted as a query_parameter, which explains why it's not having an effect on the search.