Skip to content

Add site settings for third-party blocks and theme style gating#22785

Open
jkmassel wants to merge 16 commits intotrunkfrom
jkmassel/site-settings-rust
Open

Add site settings for third-party blocks and theme style gating#22785
jkmassel wants to merge 16 commits intotrunkfrom
jkmassel/site-settings-rust

Conversation

@jkmassel
Copy link
Copy Markdown
Contributor

@jkmassel jkmassel commented Apr 9, 2026

Summary

Extracts the site settings changes from #22579 and layers them on top of #22765.

  • New "Use Third-Party Blocks" site setting — toggle in Site Settings gated behind GutenbergKitPluginsFeature and editor assets route support, with DB migration (v70→71), full data model plumbing (SiteSettingsModel, SiteSettingsInterface, WPComSiteSettings), and preference XML
  • Enhanced "Use Theme Styles" gating — contextual warnings when the site lacks editor settings support (disabled) or when the active theme is not a block theme
  • SiteSettingsProvider interface — injectable abstraction over SiteSettingsTable for reading site settings and determining block editor default, replacing static SiteUtils calls
  • EditorSettingsRepository + ThemeRepository — discovers editor settings / editor assets route support via WpApiClient.apiRoot().get() and fetches the active theme to detect block theme support; results cached in per-site AppPrefs
  • Background capability fetchMySiteViewModel.onResume() triggers fetchEditorCapabilitiesForSite so results are available before the user opens Site Settings; failures show a snackbar when there's no cached data, otherwise fall back silently

Test plan

  • Verify DB migration runs cleanly on upgrade (v70→71) (the app will crash if not, so this should be pretty straightforward)
  • Toggle both settings on, force-quit the app, then reopen Site Settings and verify both toggles still reflect the new state — exercises the DB read-back path, which schema-creation alone won't surface

Theme Style Tests:

Before testing, ensure that the following feature flags are enabled:

  • Experimental Features > "Experimental Block Editor"

When the app is not signed into WP.com:

  • Log into vanilla.wpmt.co, view Site Settings – note that theme styles are available but there's a notice about things potentially not looking right. Draft a new post, note that the editor is styled.
  • Log into jetpack.wpmt.co, view Site Settings – note that theme styles are available without a warning. Draft a new post, note that the editor uses a custom font (it might be easier to see the difference if you disable theme styles then look again).

When the app is signed into WP.com:

  • View site settings for wpmobilep2.wordpress.com. Note that theme styles are supported. Start drafting a new post – note that the title is themed.
  • View site settings for jetpack.wpmt.co – note that theme styles are available without a warning. Draft a new post, note that the editor uses a custom font.

Third-Party Blocks Tests:

Before testing, ensure that the following feature flags are enabled:

  • Experimental Features > "Experimental Block Editor"
  • Debug Settings > `gutenberg_kit_plugins`

When the app is not signed into WP.com:

  • Log into vanilla.wpmt.co, view Site Settings – note that third-party blocks are unavailable on this site.
  • Log into jetpack.wpmt.co, view Site Settings – note that third-party blocks are available. Draft a new post. Try out some of the Jetpack blocks to validate that they work.

When the app is signed into WP.com:

  • View site settings for jetpack.wpmt.co – note that third-party blocks are available. Draft a new post. Try out some of the Jetpack blocks to validate that they work.
  • View site settings for wpmobilep2.wordpress.com – note that third-party blocks are available. Draft a new post. Try out some of the Jetpack blocks to validate that they work.
  • View site settings for an atomic site – note that third-party blocks are available. Draft a new post. Try out some of the Jetpack blocks to validate that they work.

@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented Apr 9, 2026

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 9, 2026

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr22785-a6b3b94
Build Number1488
Application IDorg.wordpress.android.prealpha
Commita6b3b94
Installation URL5eclp8av3asgg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 9, 2026

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr22785-a6b3b94
Build Number1488
Application IDcom.jetpack.android.prealpha
Commita6b3b94
Installation URL3r51k632gi528
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Base automatically changed from jkmassel/wpapiclient-wpcom-support to trunk April 15, 2026 18:31
@jkmassel jkmassel force-pushed the jkmassel/site-settings-rust branch 3 times, most recently from 540e9af to d50de44 Compare April 15, 2026 19:58
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 15, 2026

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

@jkmassel jkmassel force-pushed the jkmassel/site-settings-rust branch from 937ccc1 to 1ed0f7a Compare April 15, 2026 21:43
@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Apr 15, 2026

Project dependencies changes

list
! Upgraded Dependencies
rs.wordpress.api:android:alpha-2026-04-20, (changed from trunk-0d94794142482d1b7f9395c0afef57ac991c452e)
rs.wordpress.api:kotlin:alpha-2026-04-20, (changed from trunk-0d94794142482d1b7f9395c0afef57ac991c452e)
tree
 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:trunk-0d94794142482d1b7f9395c0afef57ac991c452e
-|         +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         +--- com.squareup.okhttp3:okhttp-tls:5.3.2
-|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         |    +--- com.squareup.okio:okio:3.16.4 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 -> 2.3.21 (*)
-|         +--- net.java.dev.jna:jna:5.18.1
-|         +--- rs.wordpress.api:kotlin:trunk-0d94794142482d1b7f9395c0afef57ac991c452e
-|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         |    +--- com.squareup.okhttp3:okhttp-tls:5.3.2 (*)
-|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
-|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
+|    \--- rs.wordpress.api:android:alpha-2026-04-20
+|         +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         +--- com.squareup.okhttp3:okhttp-tls:5.3.2
+|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         |    +--- com.squareup.okio:okio:3.16.4 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 -> 2.3.21 (*)
+|         +--- net.java.dev.jna:jna:5.18.1
+|         +--- rs.wordpress.api:kotlin:alpha-2026-04-20
+|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         |    +--- com.squareup.okhttp3:okhttp-tls:5.3.2 (*)
+|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.21 (*)
-\--- rs.wordpress.api:android:trunk-0d94794142482d1b7f9395c0afef57ac991c452e (*)
+\--- rs.wordpress.api:android:alpha-2026-04-20 (*)

@jkmassel jkmassel force-pushed the jkmassel/site-settings-rust branch 9 times, most recently from 5ff6521 to 50fca51 Compare April 16, 2026 16:41
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 51.19048% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.17%. Comparing base (61bd1ed) to head (a6b3b94).
⚠️ Report is 14 commits behind head on trunk.

Files with missing lines Patch % Lines
.../java/org/wordpress/android/ui/prefs/AppPrefs.java 0.00% 27 Missing ⚠️
...press/android/datasets/SiteSettingsProviderImpl.kt 30.76% 9 Missing ⚠️
...s/android/repositories/EditorSettingsRepository.kt 87.69% 8 Missing ⚠️
...roid/ui/newstats/datasource/StatsDataSourceImpl.kt 0.00% 8 Missing ⚠️
.../org/wordpress/android/ui/prefs/AppPrefsWrapper.kt 0.00% 7 Missing ⚠️
...fluxc/network/rest/wpapi/rs/WpApiClientProvider.kt 0.00% 7 Missing ⚠️
...org/wordpress/android/ui/mysite/MySiteViewModel.kt 71.42% 5 Missing and 1 partial ⚠️
...rg/wordpress/android/models/SiteSettingsModel.java 0.00% 3 Missing ⚠️
.../wordpress/android/repositories/ThemeRepository.kt 83.33% 2 Missing ⚠️
...dpress/android/ui/prefs/SiteSettingsInterface.java 0.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22785      +/-   ##
==========================================
+ Coverage   37.15%   37.17%   +0.01%     
==========================================
  Files        2314     2317       +3     
  Lines      124297   124462     +165     
  Branches    16882    16894      +12     
==========================================
+ Hits        46188    46274      +86     
- Misses      74366    74444      +78     
- Partials     3743     3744       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jkmassel jkmassel force-pushed the jkmassel/site-settings-rust branch from 6fad2fb to 694d6d1 Compare April 21, 2026 21:18
@jkmassel jkmassel self-assigned this Apr 22, 2026
jkmassel and others added 8 commits April 22, 2026 09:06
Add a new "Use Third-Party Blocks" toggle in Site Settings, gated
behind GutenbergKit and editor assets support. Enhance the existing
"Use Theme Styles" toggle with contextual warnings when the site
lacks editor settings support or uses a non-block theme.

Includes SiteSettingsProvider interface for injectable access to
site settings from the local DB, replacing static SiteUtils calls
for block editor default detection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Gate the "Use Third-Party Blocks" toggle behind the remote
gutenberg_kit_plugins feature flag in addition to the existing
GutenbergKit and editor assets checks. Also simplify the summary
string by removing "and styles" per reviewer feedback.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion

Add ThemeRepository to fetch the active theme via WP API and determine
if it is a block theme. Add EditorSettingsRepository to discover
editor settings and editor assets route support via manifest/API root
queries. Wire SiteSettingsFragment to use EditorSettingsRepository for
gating theme styles and third-party blocks toggles.

Also adds manifest route fetching methods to WpApiClientProvider for
discovering available REST routes on WP.com and self-hosted sites.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove fetchWpComManifestRoutes and fetchSiteManifestRoutes from
WpApiClientProvider. EditorSettingsRepository now uses the standard
WpApiClient.request { it.apiRoot().get() } for all site types,
which already handles WP.com vs self-hosted URL resolution.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Call `EditorSettingsRepository.fetchEditorCapabilitiesForSite` when
the user lands on My Site so the route-discovery and theme-detection
results are available by the time they open Site Settings.

On failure, a snackbar is shown and cached prefs retain their previous
values so settings degrade to disabled rather than crashing.
12 tests covering pref delegation, route discovery persistence,
theme detection, API errors, transport-level error isolation
(one fetch failing doesn't block the other), and the both-fail
case. Also fix MySiteViewModelTest for new constructor param.
Points at Automattic/wordpress-rs#1285 which fixes route discovery
on WP.com simple sites so editor-assets and editor-settings routes
appear in the API root response.

Also adapts StatsDataSourceImpl to the new StatsUtmKeys API (String
replaced with List<StatsUtmKey>).
Switch from hasRoute (exact path match) to hasRouteForEndpoint
(namespace + endpoint) so that WP.com sites — whose route keys
include a sites/{site_id} prefix — are correctly detected.

Requires wordpress-rs PR #1285 which adds hasRouteForEndpoint
to WpApiDetails and the ApiUrlResolver parameter.
@jkmassel jkmassel force-pushed the jkmassel/site-settings-rust branch from 694d6d1 to 305ccc3 Compare April 22, 2026 15:06
@jkmassel jkmassel marked this pull request as ready for review April 22, 2026 15:24
@jkmassel jkmassel requested a review from a team as a code owner April 22, 2026 15:24
@jkmassel jkmassel requested review from nbradbury and removed request for a team April 22, 2026 15:24
@nbradbury
Copy link
Copy Markdown
Contributor

It looks like SiteSettingsProvider is unused except for tests. Can it be removed?

@nbradbury
Copy link
Copy Markdown
Contributor

  • Log into vanilla.wpmt.co, view Site Settings – note that theme styles are available but there's a notice about things potentially not looking right.

I logged into vanilla.wpmt.co but I don't see any notice of things not looking right.

Screenshot_20260427_094722

@jkmassel
Copy link
Copy Markdown
Contributor Author

jkmassel commented Apr 28, 2026

It looks like SiteSettingsProvider is unused except for tests. Can it be removed?

It's what the preloader will use to get its data.

@jkmassel
Copy link
Copy Markdown
Contributor Author

jkmassel commented Apr 28, 2026

  • Log into vanilla.wpmt.co, view Site Settings – note that theme styles are available but there's a notice about things potentially not looking right.

I logged into vanilla.wpmt.co but I don't see any notice of things not looking right.

Sorry about this! The theme changed at some point between my writing the review steps and the the review. I switched it back so it should repro as expected now.

@nbradbury
Copy link
Copy Markdown
Contributor

@jkmassel Claude found a few issues that are worth investigating:

  • MySiteViewModel.onResume() triggers network on every resume. Every return to the My Site tab fires two requests (apiRoot + themes?status=active) even if the user never opens Site Settings. Combined with the launch in refresh(), this can fire twice in close succession (e.g., resume after
    pull-to-refresh). Consider gating to "first time per session" or a TTL — these capability checks rarely change.
  • Cold-launch snackbar. fetchEditorCapabilitiesWithSnackbar shows "Failed to fetch site settings…" whenever there's no cache and the fetch fails, even from non-user-initiated onResume(). First launch with flaky network → user sees a non-actionable error before doing anything. Recommend
    suppressing snackbar entirely when isUserInitiated=false.
  • Duplicate KDoc at EditorSettingsRepository.kt:67-81 — two consecutive /** … */ blocks above fetchEditorCapabilitiesForSite.

@nbradbury
Copy link
Copy Markdown
Contributor

nbradbury commented Apr 29, 2026

This is quite wordy and doesn't really explain the setting to the user. How about, "Your site isn't using a block theme. For the editor to appear correctly, it's recommended to either use a block theme or leave this setting disabled."

Screenshot_20260429_064536

override fun isBlockEditorDefault(site: SiteModel): Boolean {
val editor = site.mobileEditor
if (editor.isNullOrEmpty()) return true
val isWpComSimple = site.isWPCom && !site.isWPComAtomic
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use site.isWPComSimpleSite() here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done in 19e614f

* Returns `true` when both checks complete without
* transport-level failures.
*/
suspend fun fetchEditorCapabilitiesForSite(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be simplified to:

suspend fun fetchEditorCapabilitiesForSite(
        site: SiteModel
    ): Boolean = withContext(ioDispatcher) {
        val results = awaitAll(
            async { fetchRouteSupport(site) },
            async { fetchThemeBlockStyleSupport(site) }
        )
        results.all { it }
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice – done in 334c823

themeRepository.fetchCurrentTheme(site)
if (theme != null) {
AppLog.d(
T.EDITOR,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nitpicky, but why is this split into five string fragments?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was originally four (the main one, plus a line per variable), then I added the non-variable line. 4bf1903 restores my original intent.

Two consecutive KDoc blocks had been left above the function when its
return type changed from Unit to Boolean. Only the second was attached;
fold the return contract into the existing KDoc as @return.
A failed remote fetch in onFetchError was calling getActivity().finish(),
kicking the user out of the screen. Cached settings are already loaded
earlier via mSiteSettings.init(false), so the screen remains usable —
just show the toast and stay put.
site.isWPCom && !site.isWPComAtomic is exactly what
SiteModel#isWPComSimpleSite() returns. Use the named predicate so the
Simple-site definition stays in one place.
@jkmassel
Copy link
Copy Markdown
Contributor Author

MySiteViewModel.onResume() triggers network on every resume. Every return to the My Site tab fires two requests (apiRoot + themes?status=active) even if the user never opens Site Settings. Combined with the launch in refresh(), this can fire twice in close succession (e.g., resume after
pull-to-refresh). Consider gating to "first time per session" or a TTL — these capability checks rarely change.

This seems like a good idea – WDYT about my filing a separate PR for it?

First launch with flaky network → user sees a non-actionable error before doing anything

I think this is a followup PR as well – probably a banner along the top of the "My Site" screen that's more generic like "Unable to connect to your site. Some functionality might be limited" with a "Retry" button. The issue here is that the user can launch the editor from this screen so we need to pre-fetch the site settings. But if that fails, things might not work right. Once they're in the editor, it's too late.

LMK if that plan seems ok?

Duplicate KDoc

Fixed in 60e34fe

Replace supervisorScope + var bookkeeping with awaitAll over two
async blocks. Both helpers shield non-cancellation exceptions
themselves, so coroutineScope semantics (sibling-cancel-on-failure)
are equivalent here — no behavior change.
The five-fragment split was harder to read than the underlying
sentence. Group the static prefix and put each labelled variable on
its own line.
The previous string was wordy and partially redundant with the summary
it gets concatenated with. Trim to three short sentences. Avoid "look
off" — it would clash with the toggle's off state.
@jkmassel
Copy link
Copy Markdown
Contributor Author

This is quite wordy and doesn't really explain the setting to the user. How about, "We're applying your theme styling in the editor. If the editor isn't looking right, disabling this setting should fix it, or you could switch to a block-based theme."

Updated in a6b3b94 to be much shorter – I started with your suggestion and ended up trimming it down a tiny bit more :)

@jkmassel jkmassel requested a review from nbradbury April 30, 2026 20:26
@nbradbury
Copy link
Copy Markdown
Contributor

This seems like a good idea – WDYT about my filing a separate PR for it?

I think that would be fine.

I think this is a followup PR as well – probably a banner along the top of the "My Site" screen that's more generic like "Unable to connect to your site. Some functionality might be limited" with a "Retry" button.

That also sounds fine.

Updated in a6b3b94 to be much shorter

Definitely better!

Copy link
Copy Markdown
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants