Repository navigation
Themes: share one WordPress.com themes data layer between the Marketplace tab and wpcomsh - #53511
Merged
Merged
Conversation
…omsh wpcomsh and the Marketplace tab each fetched, cached and mapped the WordPress.com theme catalog, so there were two implementations of one job and two sources of truth about which themes exist. Move the three platform-neutral classes into jetpack-mu-wpcom, which wpcomsh bundles, leaving the merger and the service in wpcomsh because merging with WordPress.org results is Atomic's business. They are classmap-autoloaded from the package, so wpcomsh needs no requires for them. The tab still has its own fetch and mapping; that goes next. No behaviour change.
The tab fetched, cached and mapped the catalog itself, in parallel with the shared reader wpcomsh uses for core's own tabs. It now uses that reader, so there is one fetch, one cache and one mapping. The shared mapper fills in more than the tab's own did: download_link, which is what makes install work, whether a theme is installed or active, its tags and its tier. It also appends the demo site's preview args, so the tab's own copy of those is gone. The whole catalog arrives in one request rather than a page at a time, so paging is a slice here. Only the page core asked for is mapped, because mapping reads each theme from disk to say whether it is installed.
wpcomsh_remove_dot_org_themes_based_on_plan() emptied the whole themes API result when a site's plan excludes WordPress.org themes. That was the same thing as removing them only while every result came from WordPress.org, which stopped being true once the Marketplace tab started answering with its own. It now drops the themes WPCom_Themes_Mapper did not mark as ours, which is what the name says and what the core tabs already end up with: they are emptied at this priority and merged back at the next. A theme_information result is one theme rather than a list, so it is left alone instead of having an empty themes array bolted onto it.
has_valid_theme_tier() answered two questions at once: whether a site's plan includes a tier, and whether core's Add Themes tabs should show it. The second is that surface's choice -- partner themes are not unentitled, they need their own subscription -- so a caller wanting the first got the second with it, which is wrong for the Marketplace tab and will be wrong again for the install guard in DOTCOM-18722. WPCom_Themes_Tiers now answers entitlement alone, and the tabs compose their own rule on top of it, where it can be read. The tier-to-capability convention and the premium default for a theme reporting no tier live there too. Behaviour is unchanged. A class rather than functions because a package feature file is only required for WordPress.com users, while this runs on themes_api_result.
A non-200 or unparseable response made handle_request() return null, the caller turned that into an empty array, and the cache kept it for a day. A moment's trouble reaching WordPress.com therefore emptied the Marketplace tab, and core's own tabs, until the entry expired. A failed read is now null all the way to the cache, which does not store it, and reading the body with isset() instead of a null property read stops the warning that failure used to log. Preview URLs are built with add_query_arg so a demo_uri carrying a query string does not end up with two question marks.
…answer Every paid plan includes the partner tier, while each partner theme still needs its own subscription that no plan feature reports. Without the note, an install guard built on site_can_use() alone would let a partner theme through.
The total counts every page, so replacing it with the themes kept from this one made the Marketplace tab report 100 rather than the whole catalog. A result made only of our themes is now left alone, and a WordPress.org one still has its total cleared, which is what the filters that run next expect.
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?
|
github-actions
Bot
added
[mu wpcom Feature] Wpcom Themes
[Package] Jetpack mu wpcom
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! Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
github-actions
Bot
added
the
[Status] Needs Author Reply
Code Coverage SummaryCoverage changed in 4 files.
4 files are newly checked for coverage.
|
… plan The tests asserted that the suite's site lacks community themes, which holds on the local test site and not on the WP Cloud one, where the same assertions failed. Splitting the plan gate from the filtering lets the filtering be tested on its own, which is the part with the logic in it and holds on any site.
The free short-circuit exists because there is no free-themes capability to ask about, which a comment carries better than a test asserting one line of a three-line method. Also folds the premium default in with the tier it defaults from.
Its own Add Themes tabs read through the same cache, so an Atomic site owner notices this one even though the fix is in the package.
Moving shared code into the package and reading tiers from it change nothing a site owner sees, and wpcomsh's two fixes already give that project an entry.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The changed entitlement and plan gates lack direct regression coverage, and two changelog entries overstate failed-read behavior.
6 open findings
What changed in this PR
Centralizes WordPress.com theme catalog handling in jetpack-mu-wpcom for reuse by wpcomsh and the Marketplace tab.
Changes:
- Shares theme fetching, caching, mapping, and tier entitlement logic.
- Preserves WordPress.com themes when filtering plan-restricted results.
- Adds pagination, failure-retry, tier, and filtering coverage.
| File | Description |
|---|---|
projects/plugins/wpcomsh/wpcom-themes/themes.php |
Uses shared classes and refines plan filtering. |
projects/plugins/wpcomsh/wpcom-themes/includes/class-wpcom-themes-service.php |
Delegates tier entitlement checks. |
projects/plugins/wpcomsh/tests/WpcomThemesPlanFilterTest.php |
Tests source-based theme filtering. |
projects/plugins/wpcomsh/changelog/dotcom-18877-tier-policy |
Records shared tier policy. |
projects/plugins/wpcomsh/changelog/dotcom-18877-shared-themes-data-layer |
Records data-layer consolidation. |
projects/plugins/wpcomsh/changelog/dotcom-18877-honest-dot-org-removal |
Records corrected plan filtering. |
projects/plugins/wpcomsh/changelog/dotcom-18877-dont-cache-failures |
Records failed-read cache behavior. |
projects/packages/jetpack-mu-wpcom/tests/php/features/wpcom-themes/Wpcom_Themes_Tiers_Test.php |
Tests tier extraction and feature naming. |
projects/packages/jetpack-mu-wpcom/tests/php/features/wpcom-themes/Wpcom_Themes_Tab_Test.php |
Updates Marketplace integration coverage. |
projects/packages/jetpack-mu-wpcom/src/features/wpcom-themes/wpcom-themes-tab.php |
Uses the shared catalog and mapper. |
projects/packages/jetpack-mu-wpcom/src/features/wpcom-themes/class-wpcom-themes-tiers.php |
Introduces shared tier entitlement logic. |
projects/packages/jetpack-mu-wpcom/src/features/wpcom-themes/class-wpcom-themes-mapper.php |
Moves and improves theme mapping. |
projects/packages/jetpack-mu-wpcom/src/features/wpcom-themes/class-wpcom-themes-cache.php |
Avoids caching failed reads. |
projects/packages/jetpack-mu-wpcom/src/features/wpcom-themes/class-wpcom-themes-api.php |
Preserves failures as uncached null results. |
projects/packages/jetpack-mu-wpcom/changelog/dotcom-18877-tier-policy |
Records the shared tier reader. |
projects/packages/jetpack-mu-wpcom/changelog/dotcom-18877-shared-themes-data-layer |
Records shared catalog infrastructure. |
projects/packages/jetpack-mu-wpcom/changelog/dotcom-18877-dont-cache-failures |
Records failure retry behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A failed read still answers that one request with an empty list; only the next one retries, which the changelog entries claimed away. The entitlement docblock said an unknown host offers nothing, which is not true of the free tier.
add_query_arg's own name says why it beats concatenation, and the require block does not need an account of where the other classes come from. Also drops a before-this-PR aside in the tab tests, and leaves the no-caching rationale in the cache that owns it rather than in both files.
dsas
marked this pull request as ready for review
October 13, 2026 13:22
github-actions
Bot
removed
the
[Status] Needs Author Reply
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
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 DOTCOM-18877
wpcomsh and the Marketplace tab each fetched, cached and mapped the WordPress.com theme catalog — one job, two implementations, two answers about which themes exist. The shared parts move to
jetpack-mu-wpcomrather than wpcomsh, because the tab is expected to reach WordPress.com Simple and wpcomsh is Atomic only.WPCom_Themes_Api,WPCom_Themes_Cache,WPCom_Themes_Mapper, and a newWPCom_Themes_TiersWPCom_Themes_Merger,WPCom_Themes_Service, merging into core's tabs, symlink install, removing WordPress.org themes by planThey are classmap-autoloaded, so wpcomsh needs no
require_once. The tier reader is a class rather than functions because a package feature file is only required for WordPress.com users, while this runs onthemes_api_result.Proposed changes
download_link, which is what the symlink installer matches on, whether a theme is installed or active, its tags, its tier, and the demo preview args. The whole catalog arrives in one request, so paging is a slice, and only the requested page is mapped because mapping reads each theme from disk.theme_informationresult is a single theme, so it is left alone.has_valid_theme_tier()mixed entitlement with a display choice: core's tabs hide partner themes because each needs its own subscription, not because the plan excludes them, so a caller wanting the first got the second with it.WPCom_Themes_Tiersanswers entitlement alone and the tabs compose their rule on top. The tier-to-capability convention and the premium default for a theme reporting no tier live there too.DAY_IN_SECONDS, so brief trouble reaching WordPress.com emptied the Marketplace tab, and core's own tabs, until the entry expired. Failure now stays null all the way to the cache, which does not store it, and reading the body withisset()stops the warning each failure used to log.Behaviour is otherwise unchanged, and the catalog TTL stays at a day, which is what every other cache in this area uses.
Tests: 1360 in the package, 308 in wpcomsh. New coverage for the tier reader, for the plan-based removal including the total that a page-count regression broke, and for a failed read not being cached. Both bugs above were found by clicking rather than by tests, so both have regression tests.
Related product discussion/links
DOTCOM-18635 added the Marketplace tab, with its own copy of this data layer; this PR is the follow-up that shares one.
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Needs an Atomic site. The Marketplace tab also needs the
wpcom-themes-marketplace-tabflag on, in Tools → Feature Flags, which is visible to a proxied Automattician withmanage_options.jetpack build --deps plugins/wpcomsh, thenjetpack rsync wpcomsh <your-site>@sftp.wp.com:htdocs/wp-content/mu-plugins/wpcomsh.?theme=<slug>loaded directly should still work.jetpack test php packages/jetpack-mu-wpcomandjetpack docker phpunit wpcomsh.