Skip to content

CMM-2304: Detect Jetpack status on the application-password path - #23262

Merged
nbradbury merged 8 commits into
trunkfrom
cmm-2304-third-attempt
Aug 28, 2026
Merged

CMM-2304: Detect Jetpack status on the application-password path#23262
nbradbury merged 8 commits into
trunkfrom
cmm-2304-third-attempt

Conversation

@nbradbury

@nbradbury nbradbury commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Description

TL/DR: Sites added with an application password never get their Jetpack flags detected, so the Jetpack app shows "Install Jetpack" when tapping Stats on a site that already has it. This PR adds detection to the WPAPI fetch path in fluxc.


During the site fetch, the jetpack/ REST namespace is used as a cheap negative filter, then wp/v2/plugins confirms the actual Jetpack plugin (the namespace alone also comes from Boost/Protect/Social), and jetpack/v4/connection (via wordpress-rs) supplies the connection state and WP.com blog id. Anything that can't be determined — e.g. a non-admin application password can't list plugins — is carried forward from the stored row, never flipped. SiteStore also stops discarding the whole update when the discovered blog id collides with an existing WP.com copy of the same site, and refreshes from the stored row so a stale in-memory model can't undo previous writes.

No app-module changes: once the flags are real, the existing stats gate routes correctly. The WordPress app's Jetpack-removal overlay false positive (Bug 2 in the Linear issue) is a separate follow-up PR.

Fixes Bug 1 of https://linear.app/a8c/issue/CMM-2304/

Testing instructions

Jetpack app, site with Jetpack connected:

  1. Add a self-hosted site with Jetpack installed and connected via site address + application password (admin user, not signed into WP.com).
  2. Cold-start the app, then tap Stats from My Site (tap it more than once).
  • Signed out: a WP.com login prompt appears instead of the "Install Jetpack" screen.
  • Signed in with the connected account: Stats opens.

Site without Jetpack:

  1. Add a self-hosted site with no Jetpack plugin via application password.
  2. Cold-start and tap Stats.
  • The Connect Jetpack screen still appears (unchanged behavior).

@dangermattic

dangermattic commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator
3 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ Class JetpackConnectionStatusFetcher is missing tests, but unit-tests-exemption label was set to ignore this.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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
Versionpr23262-a3bc5d0
Build Number1498
Application IDcom.jetpack.android.prealpha
Commita3bc5d0
Installation URL3gloabr109rl0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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
Versionpr23262-a3bc5d0
Build Number1498
Application IDorg.wordpress.android.prealpha
Commita3bc5d0
Installation URL1rrfhv5gll2d0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 8.98876% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.80%. Comparing base (b9bfaf6) to head (a3bc5d0).
⚠️ Report is 1 commits behind head on trunk.

Files with missing lines Patch % Lines
...uxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt 0.00% 50 Missing ⚠️
...st/wpapi/jetpack/JetpackConnectionStatusFetcher.kt 0.00% 22 Missing ⚠️
...ava/org/wordpress/android/fluxc/store/SiteStore.kt 40.00% 4 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #23262      +/-   ##
==========================================
- Coverage   37.82%   37.80%   -0.03%     
==========================================
  Files        2347     2348       +1     
  Lines      128362   128445      +83     
  Branches    17774    17805      +31     
==========================================
+ Hits        48555    48560       +5     
- Misses      75851    75925      +74     
- Partials     3956     3960       +4     

☔ View full report in Codecov by Harness.
📢 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.

@nbradbury
nbradbury marked this pull request as ready for review August 25, 2026 15:16
@nbradbury
nbradbury marked this pull request as draft August 25, 2026 15:16
Resolve the stored row before dispatching a site fetch, carry it into
every WPAPI fetch as previousSite (one carry-forward mechanism with
fresh-over-stale precedence), yield a blog id another row already owns
before writing instead of retrying after the constraint fires, treat
non-null detection results as definitive, and reuse one WpRequestExecutor
in the connection fetcher.
Collapse the three identical JetpackConnectionState constructions into
one and derive isConnected from wpComSiteId, making the null-iff-
disconnected invariant structural instead of documented.
@wpmobilebot

This comment was marked as resolved.

Replace the UrlUtils scheme handling in storedWPAPISite with plain
string checks (UrlUtils touches unmocked Android APIs under plain
JUnit), and let the fetchSite error tests tolerate the intended
stored-row read while still asserting nothing is written.
@nbradbury
nbradbury marked this pull request as ready for review August 25, 2026 18:35
@nbradbury
nbradbury requested a review from adalpari August 25, 2026 18:35
@adalpari

Copy link
Copy Markdown
Contributor

Could you double check this is intended?

  1. Absent namespaces is treated the same as “Jetpack not installed” — can wipe a live connection ⚠️ (highest confidence)
    SiteWPAPIRestClient.kt:163

val hasJetpackNamespace = namespaces?.any { it.startsWith(JETPACK_API_NAMESPACE_PREFIX) } ?: false
if (!hasJetpackNamespace) return JetpackPluginState(isActive = false, version = null)
namespaces == null (Success with empty/malformed body, or an altered REST index) yields a definitive isActive = false. In applyJetpackState (:248) that hits plugin != null && !plugin.isActive, calling setIsJetpackConnected(false) and siteId = 0 — regressing the exact state this PR establishes and re-triggering the “Install Jetpack” screen. The reliable-negative reasoning holds when namespaces is present but lacks jetpack/; it doesn’t hold when the field is absent. Consider distinguishing null (→ carry forward) from present-without-jetpack (→ definitive false).

And I think this one is worth a shot too.

  1. storedWPAPISite only tries both URL schemes when the input is scheme-less
    SiteStore.kt:1472 — if payload.url has a wrong-but-present scheme (http:// when the row was stored https://), only one lookup runs and the stored row is missed, so previousSite is null and the local id / jetpack carry-forward is lost.

@adalpari adalpari left a comment

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 a great fix and it works as expected!!

I just left a couple of comments and another addition:

  • When asked to log into WP.COM account after trying to open stats, I'm being redirected to MySite screen. Would it be possible to be redirected ti stats directly?

An absent namespaces list now carries the stored Jetpack state forward
instead of definitively clearing it, and the stored-row lookup tries
both schemes even when the caller's URL already has one.
@nbradbury

Copy link
Copy Markdown
Contributor Author

@adalpari Both of those are good catches - fixed in 531d316.

@nbradbury
nbradbury enabled auto-merge (squash) August 28, 2026 12:32
@nbradbury
nbradbury merged commit e6f795f into trunk Aug 28, 2026
23 checks passed
@nbradbury
nbradbury deleted the cmm-2304-third-attempt branch August 28, 2026 12:40
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