From 89bec4fa457f3ea84971aa4392ca64eb2af93f35 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 20 Aug 2026 14:55:39 -0400 Subject: [PATCH 1/5] CMM-2304: Detect Jetpack status for application-password sites Sites added with an application password never had their Jetpack fields populated, so the Jetpack app offered to install a plugin that was already there and the WordPress app told sites without Jetpack that they had it. The WPAPI fetch now derives isJetpackInstalled from the site's REST namespaces and reads the connection state and WP.com blog id over wordpress-rs, and the two unsolicited Jetpack-app promos are gated on whether the selected site can actually use that app. --- .../android/ui/ActivityLauncher.java | 6 + .../JetpackFeatureRemovalOverlayUtil.kt | 10 +- .../JetpackRestConnectionViewModel.kt | 11 +- .../android/ui/main/WPMainActivity.java | 3 +- .../JetpackFeatureCardHelper.kt | 10 +- .../items/DashboardItemsViewModelSlice.kt | 2 +- .../JetpackFeatureCardViewModelSlice.kt | 5 +- .../android/ui/mysite/menu/MenuActivity.kt | 6 + .../util/extensions/SiteModelExtensions.kt | 8 + .../JetpackFeatureRemovalOverlayUtilTest.kt | 26 ++- .../DashboardItemsViewModelSliceTest.kt | 2 +- .../JetpackFeatureCardHelperTest.kt | 32 +++- .../JetpackFeatureCardViewModelSliceTest.kt | 31 +-- .../jetpack/JetpackConnectionStatusFetcher.kt | 98 ++++++++++ .../rest/wpapi/site/SiteWPAPIRestClient.kt | 80 +++++++- .../wpapi/site/SiteWPAPIRestClientTest.kt | 178 ++++++++++++++++++ 16 files changed, 474 insertions(+), 34 deletions(-) create mode 100644 libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/jetpack/JetpackConnectionStatusFetcher.kt create mode 100644 libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClientTest.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java index ae2ccf42af72..eb01f806b09d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java @@ -1635,6 +1635,12 @@ public static void loginForJetpackStats(Fragment fragment) { fragment.startActivityForResult(intent, RequestCodes.DO_LOGIN); } + public static void loginForJetpackStats(Activity activity) { + Intent intent = new Intent(activity, LoginActivity.class); + LoginFlow.JETPACK_STATS.putInto(intent); + activity.startActivityForResult(intent, RequestCodes.DO_LOGIN); + } + /* * open the passed url in the device's external browser */ diff --git a/WordPress/src/main/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtil.kt b/WordPress/src/main/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtil.kt index 093980ecf672..18d9b15b4962 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtil.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtil.kt @@ -1,8 +1,10 @@ package org.wordpress.android.ui.jetpackoverlay import org.wordpress.android.analytics.AnalyticsTracker +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil.JetpackFeatureCollectionOverlaySource.APP_OPEN import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper +import org.wordpress.android.util.extensions.canUseJetpackApp import javax.inject.Inject private const val CURRENT_PHASE_KEY = "phase" @@ -18,8 +20,14 @@ class JetpackFeatureRemovalOverlayUtil @Inject constructor( return jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures() } - fun shouldShowFeatureCollectionJetpackOverlayForFirstTime(): Boolean { + /** + * The app-open overlay tells the user their site has the Jetpack plugin and that its features moved + * to the Jetpack app, so it's only shown for a site that can actually use that app. Sites without + * Jetpack don't mark the overlay as seen, so it still appears the first time one that can is selected. + */ + fun shouldShowFeatureCollectionJetpackOverlayForFirstTime(site: SiteModel?): Boolean { return jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures() && + site.canUseJetpackApp() && !jetpackFeatureOverlayShownTracker.getFeatureCollectionOverlayShown() } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt index 302d54e9e3a7..5d25d1ebe9b2 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt @@ -577,14 +577,21 @@ class JetpackRestConnectionViewModel @Inject constructor( * - Self-hosted site using REST API * - Application password has been set * - Site isn't already connected to Jetpack - * - Jetpack is not installed or the installed jetpack version is 14.2 or above + * - Jetpack is not installed, its version is unknown, or the installed version is 14.2 or above + * + * The version is unknown for sites added with an application password: Jetpack is detected there + * from the site's REST namespaces, which don't carry a version. Blocking on that would send them + * to the web-view connection flow, which signs in through wp-login.php and so can't work with an + * application password — this flow is the only one they have. */ fun canInitiateJetpackRestConnection(site: SiteModel): Boolean { return BuildConfig.IS_JETPACK_APP && site.isUsingSelfHostedRestApi && !site.wpApiRestUrl.isNullOrEmpty() && !site.isJetpackConnected - && (!site.isJetpackInstalled || checkMinimalVersion(site.jetpackVersion, JETPACK_LIMIT_VERSION)) + && (!site.isJetpackInstalled + || site.jetpackVersion.isNullOrEmpty() + || checkMinimalVersion(site.jetpackVersion, JETPACK_LIMIT_VERSION)) } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java index 32d5244a6316..7071353b2c32 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java @@ -572,7 +572,8 @@ private void showSignInForResultBasedOnIsJetpackAppBuildConfig(Activity activity } private void displayJetpackFeatureCollectionOverlayIfNeeded() { - if (mJetpackFeatureRemovalOverlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime()) { + if (mJetpackFeatureRemovalOverlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime( + mSelectedSiteRepository.getSelectedSite())) { JetpackFeatureFullScreenOverlayFragment.newInstance( false, JetpackFeatureCollectionOverlaySource.APP_OPEN diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelper.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelper.kt index c00e518ee163..c6b6c1b9152c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelper.kt @@ -2,12 +2,14 @@ package org.wordpress.android.ui.mysite.cards.jetpackfeature import org.wordpress.android.R import org.wordpress.android.analytics.AnalyticsTracker.Stat +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.jetpackoverlay.JETPACK_REMOVAL_TRACKING_NAME import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.ui.utils.UiString import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.DateTimeUtilsWrapper import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper +import org.wordpress.android.util.extensions.canUseJetpackApp import org.wordpress.android.util.config.PhaseThreeBlogPostLinkConfig import java.util.Date import javax.inject.Inject @@ -19,10 +21,14 @@ class JetpackFeatureCardHelper @Inject constructor( private val dateTimeUtilsWrapper: DateTimeUtilsWrapper, private val phaseThreeBlogPostLinkConfig: PhaseThreeBlogPostLinkConfig ) { - fun shouldShowJetpackFeatureCard(): Boolean { + /** + * The card promotes the Jetpack app for the site the user is looking at, so it's only shown for a + * site that can actually use it — see [canUseJetpackApp]. + */ + fun shouldShowJetpackFeatureCard(site: SiteModel?): Boolean { val isWordPressApp = !buildConfigWrapper.isJetpackApp val exceedsShowFrequency = exceedsShowFrequencyAndResetJetpackFeatureCardLastShownTimestampIfNeeded() - return isWordPressApp && !isJetpackCardHiddenByUser() && exceedsShowFrequency + return isWordPressApp && site.canUseJetpackApp() && !isJetpackCardHiddenByUser() && exceedsShowFrequency } private fun isJetpackCardHiddenByUser(): Boolean = appPrefsWrapper.getShouldHideJetpackFeatureCard() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/DashboardItemsViewModelSlice.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/DashboardItemsViewModelSlice.kt index 631f8a4ed289..56fe865b80d9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/DashboardItemsViewModelSlice.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/DashboardItemsViewModelSlice.kt @@ -78,7 +78,7 @@ class DashboardItemsViewModelSlice @Inject constructor( job?.cancel() job = scope.launch(bgDispatcher) { _isRefreshing.postValue(true) - jetpackFeatureCardViewModelSlice.buildJetpackFeatureCard() + jetpackFeatureCardViewModelSlice.buildJetpackFeatureCard(site) siteItemsViewModelSlice.buildSiteItems(site) sotw2023NudgeCardViewModelSlice.buildCard() _isRefreshing.postValue(false) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSlice.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSlice.kt index 920f28554e71..97630acfad7c 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSlice.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSlice.kt @@ -3,6 +3,7 @@ package org.wordpress.android.ui.mysite.items.jetpackfeaturecard import androidx.lifecycle.MutableLiveData import androidx.lifecycle.distinctUntilChanged import org.wordpress.android.analytics.AnalyticsTracker +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil import org.wordpress.android.ui.mysite.MySiteCardAndItem import org.wordpress.android.ui.mysite.SiteNavigationAction @@ -22,8 +23,8 @@ class JetpackFeatureCardViewModelSlice @Inject constructor( private val _uiModel = MutableLiveData() val uiModel = _uiModel.distinctUntilChanged() - suspend fun buildJetpackFeatureCard() { - if (!jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()){ + suspend fun buildJetpackFeatureCard(site: SiteModel?) { + if (!jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)){ _uiModel.postValue(null) return } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt index 4c1e81cc4484..ef185f72438b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt @@ -135,6 +135,12 @@ class MenuActivity : BaseAppCompatActivity() { is SiteNavigationAction.OpenNewStats -> NewStatsActivity.start(this, StatsLaunchedFrom.ROW) + is SiteNavigationAction.ConnectJetpackForStats -> + ActivityLauncher.viewConnectJetpackForStats(this, action.site) + + is SiteNavigationAction.StartWPComLoginForJetpackStats -> + ActivityLauncher.loginForJetpackStats(this) + is SiteNavigationAction.OpenDomains -> ActivityLauncher.viewDomainsDashboardActivity( this, action.site diff --git a/WordPress/src/main/java/org/wordpress/android/util/extensions/SiteModelExtensions.kt b/WordPress/src/main/java/org/wordpress/android/util/extensions/SiteModelExtensions.kt index 67609c27b161..a8df0a079cdd 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/extensions/SiteModelExtensions.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/extensions/SiteModelExtensions.kt @@ -31,3 +31,11 @@ val SiteModel.stateLogInformation: String */ fun SiteModel.activeJetpackConnectionPluginValues(): List? = activeJetpackConnectionPlugins?.split(",") + +/** + * @return true if the Jetpack app has anything to offer this site. The Jetpack-powered features live + * behind WordPress.com, so a self-hosted site without Jetpack gets nothing out of switching apps — + * and telling its owner that their site has the Jetpack plugin is simply wrong. + */ +fun SiteModel?.canUseJetpackApp(): Boolean = + this != null && (isWPCom || isJetpackInstalled || isJetpackConnected) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtilTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtilTest.kt index a04d8a439642..bead1963b983 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtilTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/jetpackoverlay/JetpackFeatureRemovalOverlayUtilTest.kt @@ -9,6 +9,7 @@ import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil.JetpackFeatureCollectionOverlaySource import org.wordpress.android.util.analytics.AnalyticsTrackerWrapper @@ -38,7 +39,7 @@ class JetpackFeatureRemovalOverlayUtilTest { fun `given the Jetpack app, when checking the feature collection overlay, then it is not shown`() { whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(false) - assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime()).isFalse + assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(jetpackSite())).isFalse } @Test @@ -46,7 +47,7 @@ class JetpackFeatureRemovalOverlayUtilTest { whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(true) whenever(shownTracker.getFeatureCollectionOverlayShown()).thenReturn(false) - assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime()).isTrue + assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(jetpackSite())).isTrue } @Test @@ -54,7 +55,21 @@ class JetpackFeatureRemovalOverlayUtilTest { whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(true) whenever(shownTracker.getFeatureCollectionOverlayShown()).thenReturn(true) - assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime()).isFalse + assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(jetpackSite())).isFalse + } + + @Test + fun `given a site without Jetpack, then the feature collection overlay is not shown`() { + whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(true) + + assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(SiteModel())).isFalse + } + + @Test + fun `given no selected site, then the feature collection overlay is not shown`() { + whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(true) + + assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(null)).isFalse } @Test @@ -77,4 +92,9 @@ class JetpackFeatureRemovalOverlayUtilTest { assertThat(overlayUtil.shouldHideJetpackFeatures()).isTrue } + + private fun jetpackSite() = SiteModel().apply { + setIsJetpackInstalled(true) + setIsJetpackConnected(true) + } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/DashboardItemsViewModelSliceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/DashboardItemsViewModelSliceTest.kt index 8b9acd5580fa..3b7b9effe65b 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/DashboardItemsViewModelSliceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/DashboardItemsViewModelSliceTest.kt @@ -94,7 +94,7 @@ class DashboardItemsViewModelSliceTest: BaseUnitTest() { dashboardItemsViewModelSlice.buildItems(mockSite) verify(siteItemsViewModelSlice, atLeastOnce()).buildSiteItems(any()) - verify(jetpackFeatureCardViewModelSlice, atMost(1)).buildJetpackFeatureCard() + verify(jetpackFeatureCardViewModelSlice, atMost(1)).buildJetpackFeatureCard(mockSite) verify(siteItemsViewModelSlice, atMost(1)).buildSiteItems(mockSite) verify(sotw2023NudgeCardViewModelSlice, atMost(1)).buildCard() } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelperTest.kt index 9db6bb377c86..767593589832 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/jetpackfeature/JetpackFeatureCardHelperTest.kt @@ -8,6 +8,7 @@ import org.mockito.Mock import org.mockito.junit.MockitoJUnitRunner import org.mockito.kotlin.any import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.DateTimeUtils @@ -50,7 +51,7 @@ class JetpackFeatureCardHelperTest { fun `when jetpack app, then jetpack feature card should not show`() { setTest(isJetpackApp = true) - val result = helper.shouldShowJetpackFeatureCard() + val result = helper.shouldShowJetpackFeatureCard(jetpackSite()) assertThat(result).isFalse } @@ -59,7 +60,7 @@ class JetpackFeatureCardHelperTest { fun `when hide card has been set, then jetpack feature card should not shown`() { setTest(isCardHiddenByUser = true) - val result = helper.shouldShowJetpackFeatureCard() + val result = helper.shouldShowJetpackFeatureCard(jetpackSite()) assertThat(result).isFalse } @@ -68,7 +69,7 @@ class JetpackFeatureCardHelperTest { fun `given remind later is set, when shown frequency is not exceeded, then jetpack feature card is not shown`() { setTest(lastShownTimestamp = getDateXDaysAgoInMilliseconds(1)) - val result = helper.shouldShowJetpackFeatureCard() + val result = helper.shouldShowJetpackFeatureCard(jetpackSite()) assertThat(result).isFalse } @@ -77,11 +78,29 @@ class JetpackFeatureCardHelperTest { fun `given remind later is set, when shown frequency is exceeded, then jetpack feature card is shown`() { setTest(lastShownTimestamp = getDateXDaysAgoInMilliseconds(9)) - val result = helper.shouldShowJetpackFeatureCard() + val result = helper.shouldShowJetpackFeatureCard(jetpackSite()) assertThat(result).isTrue } + @Test + fun `when the site has no Jetpack, then jetpack feature card should not show`() { + setTest() + + val result = helper.shouldShowJetpackFeatureCard(SiteModel()) + + assertThat(result).isFalse + } + + @Test + fun `when there is no selected site, then jetpack feature card should not show`() { + setTest() + + val result = helper.shouldShowJetpackFeatureCard(null) + + assertThat(result).isFalse + } + private fun setTest( isJetpackApp: Boolean = false, isCardHiddenByUser: Boolean = false, @@ -117,6 +136,11 @@ class JetpackFeatureCardHelperTest { private fun getDateXDaysAgoInMilliseconds(daysAgo: Int) = System.currentTimeMillis().minus(DAY_IN_MILLISECONDS * daysAgo) + private fun jetpackSite() = SiteModel().apply { + setIsJetpackInstalled(true) + setIsJetpackConnected(true) + } + companion object { private const val DAY_IN_MILLISECONDS = 86400000 } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSliceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSliceTest.kt index 715f077fd356..10997c6295ac 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSliceTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/jetpackfeaturecard/JetpackFeatureCardViewModelSliceTest.kt @@ -13,6 +13,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest import org.wordpress.android.analytics.AnalyticsTracker +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.ui.mysite.MySiteCardAndItem import org.wordpress.android.ui.mysite.SiteNavigationAction import org.wordpress.android.ui.mysite.cards.jetpackfeature.JetpackFeatureCardHelper @@ -35,6 +36,8 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { private lateinit var navigationEvents: MutableList + private val site = SiteModel() + @Before fun setUp() { viewModelSlice = JetpackFeatureCardViewModelSlice( @@ -56,10 +59,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should not be shown, ui model is null`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(false) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(false) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) // then assertNull(uiModels.last()) @@ -68,10 +71,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, ui model is not null`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) // then assertNotNull(uiModels.last()) @@ -80,10 +83,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, onJetpackFeatureCardClick is called`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) uiModels.last()?.onClick?.click() advanceUntilIdle() @@ -95,10 +98,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, onJetpackFeatureCardHideMenuItemClick is called`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) uiModels.last()?.onHideMenuItemClick?.click() advanceUntilIdle() @@ -110,10 +113,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, onJetpackFeatureCardLearnMoreClick is called`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) uiModels.last()?.onLearnMoreClick?.click() advanceUntilIdle() @@ -125,10 +128,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, onJetpackFeatureCardRemindMeLaterClick is called`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) uiModels.last()?.onRemindMeLaterItemClick?.click() advanceUntilIdle() @@ -140,10 +143,10 @@ class JetpackFeatureCardViewModelSliceTest: BaseUnitTest() { @Test fun `given jetpack feature card should be shown, onJetpackFeatureCardMoreMenuClick is called`() = test { // given - whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()).thenReturn(true) + whenever(jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)).thenReturn(true) // when - viewModelSlice.buildJetpackFeatureCard() + viewModelSlice.buildJetpackFeatureCard(site) uiModels.last()?.onMoreMenuClick?.click() advanceUntilIdle() diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/jetpack/JetpackConnectionStatusFetcher.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/jetpack/JetpackConnectionStatusFetcher.kt new file mode 100644 index 000000000000..82ac80a78a86 --- /dev/null +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/jetpack/JetpackConnectionStatusFetcher.kt @@ -0,0 +1,98 @@ +package org.wordpress.android.fluxc.network.rest.wpapi.jetpack + +import okhttp3.Interceptor +import org.wordpress.android.fluxc.module.OkHttpClientQualifiers +import org.wordpress.android.fluxc.network.rest.wpapi.rs.WpNetworkAvailabilityProvider +import org.wordpress.android.fluxc.utils.AppLogWrapper +import org.wordpress.android.util.AppLog +import rs.wordpress.api.kotlin.EmptyAppNotifier +import rs.wordpress.api.kotlin.WpRequestExecutor +import uniffi.wp_api.JetpackConnectionClient +import uniffi.wp_api.JetpackConnectionStatus +import uniffi.wp_api.ParsedUrl +import uniffi.wp_api.WpApiClientDelegate +import uniffi.wp_api.WpApiMiddlewarePipeline +import uniffi.wp_api.WpAuthenticationProvider +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton + +/** + * A self-hosted site's Jetpack connection to WordPress.com. [wpComSiteId] is the blog ID WordPress.com + * assigned the site when it connected, and is null whenever [isConnected] is false. + */ +data class JetpackConnectionState( + val isConnected: Boolean, + val wpComSiteId: Long? +) + +/** + * Reads a self-hosted site's Jetpack connection state over wordpress-rs. + * + * Only the Jetpack plugin knows whether a site is connected to WordPress.com and which blog ID it was + * given. Sites added with an application password have no other source for either: the WP.com REST site + * payload that populates those fields for Jetpack sites is never fetched for them. + */ +@Singleton +class JetpackConnectionStatusFetcher @Inject constructor( + @Named(OkHttpClientQualifiers.INTERCEPTORS) private val interceptors: Set<@JvmSuppressWildcards Interceptor>, + private val networkAvailabilityProvider: WpNetworkAvailabilityProvider, + private val appLogWrapper: AppLogWrapper +) { + /** + * Returns the site's Jetpack connection state, or null when it can't be determined — the site is + * unreachable, the endpoint isn't there, or Jetpack is older than 14.2, which is where these + * endpoints were added. Callers should read null as "unchanged", not as "not connected". + */ + @Suppress("TooGenericExceptionCaught") + suspend fun fetch( + apiRootUrl: String, + username: String, + password: String + ): JetpackConnectionState? = try { + buildClient(apiRootUrl, username, password).use { client -> + when (val status = client.status()) { + is JetpackConnectionStatus.NotConnected -> JetpackConnectionState( + isConnected = false, + wpComSiteId = null + ) + + is JetpackConnectionStatus.Site -> JetpackConnectionState( + isConnected = true, + wpComSiteId = status.blogId.toLong() + ) + + is JetpackConnectionStatus.User -> JetpackConnectionState( + isConnected = true, + wpComSiteId = status.blogId.toLong() + ) + } + } + } catch (e: Exception) { + appLogWrapper.d(AppLog.T.API, "$TAG: couldn't read the Jetpack connection status: ${e.message}") + null + } + + private fun buildClient( + apiRootUrl: String, + username: String, + password: String + ) = JetpackConnectionClient( + apiRootUrl = ParsedUrl.parse(apiRootUrl), + delegate = WpApiClientDelegate( + authProvider = WpAuthenticationProvider.staticWithUsernameAndPassword(username, password), + requestExecutor = WpRequestExecutor( + interceptors = interceptors.toList(), + networkAvailabilityProvider = networkAvailabilityProvider + ), + middlewarePipeline = WpApiMiddlewarePipeline(emptyList()), + // Reading the status is a passive check made during a site refresh, so a rejected + // credential shouldn't raise the app-wide "re-authenticate" prompt on its own. + appNotifier = EmptyAppNotifier() + ) + ) + + companion object { + private const val TAG = "JetpackConnectionStatusFetcher" + } +} diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt index c22db09f136f..b47e66bb3c15 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt @@ -13,6 +13,8 @@ import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIDiscoveryUtils import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIGsonRequestBuilder import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse.Error import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse.Success +import org.wordpress.android.fluxc.network.rest.wpapi.jetpack.JetpackConnectionState +import org.wordpress.android.fluxc.network.rest.wpapi.jetpack.JetpackConnectionStatusFetcher import org.wordpress.android.fluxc.store.SiteStore.FetchWPAPISitePayload import org.wordpress.android.fluxc.utils.extensions.getPasswordProcessed import org.wordpress.android.fluxc.utils.extensions.getUserNameProcessed @@ -26,19 +28,27 @@ import javax.inject.Singleton class SiteWPAPIRestClient @Inject constructor( private val wpapiGsonRequestBuilder: WPAPIGsonRequestBuilder, private val discoveryWPAPIRestClient: DiscoveryWPAPIRestClient, + private val jetpackConnectionStatusFetcher: JetpackConnectionStatusFetcher, dispatcher: Dispatcher, @Named(OkHttpClientQualifiers.CUSTOM_SSL) requestQueue: RequestQueue, userAgent: UserAgent ) : BaseWPAPIRestClient(dispatcher, requestQueue, userAgent) { companion object { private const val WOO_API_NAMESPACE_PREFIX = "wc/" + private const val JETPACK_API_NAMESPACE_PREFIX = "jetpack/" private const val FETCH_API_CALL_FIELDS = "name,description,gmt_offset,url,authentication,namespaces" private const val APPLICATION_PASSWORDS_URL_SUFFIX = "authorize-application.php" } + /** + * @param previousSite the site this fetch is refreshing, when there is one. Fields that this + * fetch can't determine on a given run are carried forward from it rather than reset, because + * the model returned here replaces the stored row wholesale. + */ suspend fun fetchWPAPISite( - payload: FetchWPAPISitePayload + payload: FetchWPAPISitePayload, + previousSite: SiteModel? = null ): SiteModel { val cleanedUrl = UrlUtils.addUrlSchemeIfNeeded(payload.url, false).let { urlWithScheme -> DiscoveryUtils.stripKnownPaths(urlWithScheme) @@ -57,6 +67,13 @@ class SiteWPAPIRestClient @Inject constructor( return when (result) { is Success -> { val response = result.data + // Jetpack registers its REST namespace only while the plugin is active, so its + // presence is exactly what isJetpackInstalled documents: installed and activated. + val hasJetpack = response?.namespaces?.any { + it.startsWith(JETPACK_API_NAMESPACE_PREFIX) + } ?: false + val jetpackConnection = fetchJetpackConnectionState(hasJetpack, discoveredWpApiUrl, payload) + SiteModel().apply { name = response?.name description = response?.description @@ -65,6 +82,7 @@ class SiteWPAPIRestClient @Inject constructor( hasWooCommerce = response?.namespaces?.any { it.startsWith(WOO_API_NAMESPACE_PREFIX) } ?: false + applyJetpackState(hasJetpack, jetpackConnection, previousSite) applicationPasswordsAuthorizeUrl = response?.authentication?.applicationPasswords ?.endpoints?.authorization @@ -97,15 +115,71 @@ class SiteWPAPIRestClient @Inject constructor( suspend fun fetchWPAPISite( site: SiteModel ): SiteModel { - return fetchWPAPISite( + val fetchedSite = fetchWPAPISite( payload = FetchWPAPISitePayload( url = site.url, username = site.getUserNameProcessed(), password = site.getPasswordProcessed(), isApplicationPassword = site.hasApplicationPassword(), - ) + ), + previousSite = site ) + + if (!fetchedSite.isError) { + // Carry the local id so SiteStore.updateSite finds the stored row and preserves the + // editor preference, and so SiteSqlUtils matches the row by local id rather than by + // SITE_ID + URL — that match misses, and inserts a duplicate site, as soon as this + // fetch starts carrying a real WP.com blog id. + fetchedSite.id = site.id + } + return fetchedSite + } + + /** + * Jetpack is the only source of a self-hosted site's connection state and of the WP.com blog ID + * it was assigned, and reading it needs credentials, so this is skipped for sites without an + * application password. Returns null when the state is unknown, including for every site with no + * Jetpack namespace — those never make the request. + */ + private suspend fun fetchJetpackConnectionState( + hasJetpack: Boolean, + apiRootUrl: String, + payload: FetchWPAPISitePayload + ): JetpackConnectionState? { + if (!hasJetpack || !payload.isApplicationPassword) return null + + val username = payload.username + val password = payload.password + if (username.isNullOrEmpty() || password.isNullOrEmpty()) return null + + return jetpackConnectionStatusFetcher.fetch(apiRootUrl, username, password) + } + + private fun SiteModel.applyJetpackState( + hasJetpack: Boolean, + connection: JetpackConnectionState?, + previousSite: SiteModel? + ) { + setIsJetpackInstalled(hasJetpack) + when { + !hasJetpack -> { + setIsJetpackConnected(false) + siteId = 0L + } + + connection != null -> { + setIsJetpackConnected(connection.isConnected) + siteId = connection.wpComSiteId ?: 0L + } + + // The connection state couldn't be read on this run, so keep what was already known + // rather than reporting the site as disconnected. + else -> { + setIsJetpackConnected(previousSite?.isJetpackConnected ?: false) + siteId = previousSite?.siteId ?: 0L + } + } } private fun discoverApiEndpoint( diff --git a/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClientTest.kt b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClientTest.kt new file mode 100644 index 000000000000..fe116a408beb --- /dev/null +++ b/libs/fluxc/src/test/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClientTest.kt @@ -0,0 +1,178 @@ +package org.wordpress.android.fluxc.network.rest.wpapi.site + +import com.android.volley.RequestQueue +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever +import org.robolectric.RobolectricTestRunner +import org.wordpress.android.fluxc.Dispatcher +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.network.UserAgent +import org.wordpress.android.fluxc.network.discovery.DiscoveryWPAPIRestClient +import org.wordpress.android.fluxc.network.discovery.RootWPAPIRestResponse +import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIGsonRequestBuilder +import org.wordpress.android.fluxc.network.rest.wpapi.WPAPIResponse.Success +import org.wordpress.android.fluxc.network.rest.wpapi.jetpack.JetpackConnectionState +import org.wordpress.android.fluxc.network.rest.wpapi.jetpack.JetpackConnectionStatusFetcher +import org.wordpress.android.fluxc.store.SiteStore.FetchWPAPISitePayload +import org.wordpress.android.fluxc.test + +// Robolectric: the fetch runs the site URL through UrlUtils.addUrlSchemeIfNeeded, which calls into +// android.webkit.URLUtil, and this module doesn't enable returnDefaultValues. +@RunWith(RobolectricTestRunner::class) +class SiteWPAPIRestClientTest { + private val wpapiGsonRequestBuilder: WPAPIGsonRequestBuilder = mock() + private val discoveryWPAPIRestClient: DiscoveryWPAPIRestClient = mock() + private val jetpackConnectionStatusFetcher: JetpackConnectionStatusFetcher = mock() + private val dispatcher: Dispatcher = mock() + private val requestQueue: RequestQueue = mock() + private val userAgent: UserAgent = mock() + + private lateinit var restClient: SiteWPAPIRestClient + + @Before + fun setUp() { + whenever(discoveryWPAPIRestClient.discoverWPAPIBaseURL(any())).thenReturn(API_ROOT_URL) + restClient = SiteWPAPIRestClient( + wpapiGsonRequestBuilder, + discoveryWPAPIRestClient, + jetpackConnectionStatusFetcher, + dispatcher, + requestQueue, + userAgent + ) + } + + @Test + fun `given the jetpack namespace, when the site is connected, then the wpcom id is stored`() = test { + stubRootResponse(namespaces = listOf("wp/v2", "jetpack/v4")) + stubConnectionState(JetpackConnectionState(isConnected = true, wpComSiteId = WPCOM_SITE_ID)) + + val result = restClient.fetchWPAPISite(applicationPasswordPayload()) + + assertThat(result.isJetpackInstalled).isTrue + assertThat(result.isJetpackConnected).isTrue + assertThat(result.siteId).isEqualTo(WPCOM_SITE_ID) + } + + @Test + fun `given the jetpack namespace, when the site is not connected, then there is no wpcom id`() = test { + stubRootResponse(namespaces = listOf("wp/v2", "jetpack/v4")) + stubConnectionState(JetpackConnectionState(isConnected = false, wpComSiteId = null)) + + val result = restClient.fetchWPAPISite(applicationPasswordPayload()) + + assertThat(result.isJetpackInstalled).isTrue + assertThat(result.isJetpackConnected).isFalse + assertThat(result.siteId).isEqualTo(0L) + } + + @Test + fun `given no jetpack namespace, then the stored jetpack state is cleared without a status request`() = test { + stubRootResponse(namespaces = listOf("wp/v2")) + + val result = restClient.fetchWPAPISite( + payload = applicationPasswordPayload(), + previousSite = connectedSite() + ) + + assertThat(result.isJetpackInstalled).isFalse + assertThat(result.isJetpackConnected).isFalse + assertThat(result.siteId).isEqualTo(0L) + verifyNoInteractions(jetpackConnectionStatusFetcher) + } + + @Test + fun `given an unreadable connection status, then the previously known jetpack state is kept`() = test { + stubRootResponse(namespaces = listOf("wp/v2", "jetpack/v4")) + stubConnectionState(null) + + val result = restClient.fetchWPAPISite( + payload = applicationPasswordPayload(), + previousSite = connectedSite() + ) + + assertThat(result.isJetpackConnected).isTrue + assertThat(result.siteId).isEqualTo(WPCOM_SITE_ID) + } + + @Test + fun `given no application password, then the connection status is not requested`() = test { + stubRootResponse(namespaces = listOf("wp/v2", "jetpack/v4")) + + val result = restClient.fetchWPAPISite( + FetchWPAPISitePayload( + url = SITE_URL, + username = USERNAME, + password = PASSWORD, + isApplicationPassword = false + ) + ) + + assertThat(result.isJetpackInstalled).isTrue + assertThat(result.isJetpackConnected).isFalse + verifyNoInteractions(jetpackConnectionStatusFetcher) + } + + @Test + fun `when refreshing a stored site, then its local id is carried onto the fetched model`() = test { + stubRootResponse(namespaces = listOf("wp/v2")) + + val result = restClient.fetchWPAPISite(connectedSite()) + + assertThat(result.id).isEqualTo(LOCAL_ID) + } + + private suspend fun stubRootResponse(namespaces: List) { + whenever( + wpapiGsonRequestBuilder.syncGetRequest( + restClient = any(), + url = any(), + params = any(), + body = any(), + clazz = eq(RootWPAPIRestResponse::class.java), + enableCaching = any(), + cacheTimeToLive = any(), + nonce = anyOrNull(), + headers = any() + ) + ).thenReturn(Success(RootWPAPIRestResponse(name = "Site", namespaces = namespaces))) + } + + private suspend fun stubConnectionState(state: JetpackConnectionState?) { + whenever(jetpackConnectionStatusFetcher.fetch(any(), any(), any())).thenReturn(state) + } + + private fun applicationPasswordPayload() = FetchWPAPISitePayload( + url = SITE_URL, + username = USERNAME, + password = PASSWORD, + isApplicationPassword = true + ) + + private fun connectedSite() = SiteModel().apply { + id = LOCAL_ID + url = SITE_URL + siteId = WPCOM_SITE_ID + apiRestUsernamePlain = USERNAME + apiRestPasswordPlain = PASSWORD + setIsJetpackInstalled(true) + setIsJetpackConnected(true) + } + + companion object { + private const val SITE_URL = "https://site.com" + private const val API_ROOT_URL = "https://site.com/wp-json" + private const val USERNAME = "username" + private const val PASSWORD = "password" + private const val LOCAL_ID = 7 + private const val WPCOM_SITE_ID = 123456L + } +} From 9f724c23b19c9e9a1c02a395a9e852ca67f1ff79 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 20 Aug 2026 15:06:46 -0400 Subject: [PATCH 2/5] Satisfy detekt's ReturnCount in fetchJetpackConnectionState The guard clauses put the function at three returns against a limit of two, so they collapse into a single condition. --- .../network/rest/wpapi/site/SiteWPAPIRestClient.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt index b47e66bb3c15..79f216615928 100644 --- a/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt +++ b/libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpapi/site/SiteWPAPIRestClient.kt @@ -147,13 +147,16 @@ class SiteWPAPIRestClient @Inject constructor( apiRootUrl: String, payload: FetchWPAPISitePayload ): JetpackConnectionState? { - if (!hasJetpack || !payload.isApplicationPassword) return null - val username = payload.username val password = payload.password - if (username.isNullOrEmpty() || password.isNullOrEmpty()) return null + val canFetch = hasJetpack && payload.isApplicationPassword && + !username.isNullOrEmpty() && !password.isNullOrEmpty() - return jetpackConnectionStatusFetcher.fetch(apiRootUrl, username, password) + return if (canFetch) { + jetpackConnectionStatusFetcher.fetch(apiRootUrl, username, password) + } else { + null + } } private fun SiteModel.applyJetpackState( From 072c6ed184d7b8c6b08990efbf8e78c0956638d0 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 20 Aug 2026 15:38:49 -0400 Subject: [PATCH 3/5] Move the MenuActivity Jetpack stats branches to their own issue CMM-2304 lists MenuActivity's missing navigation branches under "Separate bugs found while tracing" and asks for them to be split out, so revert them here and keep this branch to Jetpack detection. The change is unrelated to detection: those actions were silently dropped by `else -> {}` regardless of a site's Jetpack state. It also only covers two of the three missing branches -- ShowJetpackRemovalStaticPostersView is still unhandled -- which is better resolved together in the follow-up. --- .../java/org/wordpress/android/ui/ActivityLauncher.java | 6 ------ .../org/wordpress/android/ui/mysite/menu/MenuActivity.kt | 6 ------ 2 files changed, 12 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java index eb01f806b09d..ae2ccf42af72 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java @@ -1635,12 +1635,6 @@ public static void loginForJetpackStats(Fragment fragment) { fragment.startActivityForResult(intent, RequestCodes.DO_LOGIN); } - public static void loginForJetpackStats(Activity activity) { - Intent intent = new Intent(activity, LoginActivity.class); - LoginFlow.JETPACK_STATS.putInto(intent); - activity.startActivityForResult(intent, RequestCodes.DO_LOGIN); - } - /* * open the passed url in the device's external browser */ diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt index ef185f72438b..4c1e81cc4484 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/menu/MenuActivity.kt @@ -135,12 +135,6 @@ class MenuActivity : BaseAppCompatActivity() { is SiteNavigationAction.OpenNewStats -> NewStatsActivity.start(this, StatsLaunchedFrom.ROW) - is SiteNavigationAction.ConnectJetpackForStats -> - ActivityLauncher.viewConnectJetpackForStats(this, action.site) - - is SiteNavigationAction.StartWPComLoginForJetpackStats -> - ActivityLauncher.loginForJetpackStats(this) - is SiteNavigationAction.OpenDomains -> ActivityLauncher.viewDomainsDashboardActivity( this, action.site From 40a18fa5889cfff035024f113bf42a8731d4a36e Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 20 Aug 2026 15:58:09 -0400 Subject: [PATCH 4/5] Require the signed-in account to own a site's Jetpack connection Detecting `isJetpackConnected` for application-password sites was not enough to open Stats. The flag says the site is connected to *an* account, but the blog ID belongs to whichever account made the connection, which needn't be the one signed in here. Stats are served by WordPress.com, so the request reached the right site and was refused: `403 - user cannot view stats`. Add `WpComSiteAccessChecker`, which asks whether this account can reach a site over the WordPress.com REST API -- true for sites already accessed that way, and otherwise only when the same blog ID also arrived from `/me/sites`, since the account's site list is the record of what it can reach. `ListItemActionHandler` now requires that before routing to Stats, so an unowned site falls through to the connection flow instead of a screen that errors. `canInitiateJetpackRestConnection` takes the same signal, because its "already connected" check would otherwise reject these sites and drop them into the web-view flow; the REST flow's ConnectUser step is exactly what links the signed-in account. --- .../JetpackRestConnectionViewModel.kt | 13 ++++++-- .../ui/mysite/cards/ListItemActionHandler.kt | 12 +++++-- .../ui/stats/StatsConnectJetpackActivity.kt | 7 ++++- .../android/util/WpComSiteAccessChecker.kt | 31 +++++++++++++++++++ .../JetpackRestConnectionViewModelTest.kt | 19 ++++++++++++ .../mysite/cards/ListItemActionHandlerTest.kt | 31 ++++++++++++++++++- 6 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/util/WpComSiteAccessChecker.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt index 5d25d1ebe9b2..ba09963ae532 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt @@ -576,19 +576,26 @@ class JetpackRestConnectionViewModel @Inject constructor( * - Jetpack app * - Self-hosted site using REST API * - Application password has been set - * - Site isn't already connected to Jetpack + * - Site isn't already connected to Jetpack for the account signed in here * - Jetpack is not installed, its version is unknown, or the installed version is 14.2 or above * * The version is unknown for sites added with an application password: Jetpack is detected there * from the site's REST namespaces, which don't carry a version. Blocking on that would send them * to the web-view connection flow, which signs in through wp-login.php and so can't work with an * application password — this flow is the only one they have. + * + * @param hasWpComAccess whether the signed-in WordPress.com account can already reach the site, + * per [org.wordpress.android.util.WpComSiteAccessChecker]. A site connected to a *different* + * account still needs this flow: its ConnectUser step is what links the account signed in here. + * Defaults to true so callers with no reason to distinguish keep the plain "already connected" + * behaviour. */ - fun canInitiateJetpackRestConnection(site: SiteModel): Boolean { + @JvmOverloads + fun canInitiateJetpackRestConnection(site: SiteModel, hasWpComAccess: Boolean = true): Boolean { return BuildConfig.IS_JETPACK_APP && site.isUsingSelfHostedRestApi && !site.wpApiRestUrl.isNullOrEmpty() - && !site.isJetpackConnected + && (!site.isJetpackConnected || !hasWpComAccess) && (!site.isJetpackInstalled || site.jetpackVersion.isNullOrEmpty() || checkMinimalVersion(site.jetpackVersion, JETPACK_LIMIT_VERSION)) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandler.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandler.kt index 47e393468fcd..ebc26b48060f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandler.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandler.kt @@ -8,12 +8,14 @@ import org.wordpress.android.ui.blaze.blazecampaigns.campaignlisting.CampaignLis import org.wordpress.android.ui.mysite.SiteNavigationAction import org.wordpress.android.ui.mysite.items.listitem.ListItemAction import org.wordpress.android.ui.newstats.NewStatsRouting +import org.wordpress.android.util.WpComSiteAccessChecker import javax.inject.Inject class ListItemActionHandler @Inject constructor( private val accountStore: AccountStore, private val blazeFeatureUtils: BlazeFeatureUtils, - private val newStatsRouting: NewStatsRouting + private val newStatsRouting: NewStatsRouting, + private val wpComSiteAccessChecker: WpComSiteAccessChecker ) { fun handleAction( action: ListItemAction, @@ -51,8 +53,12 @@ class ListItemActionHandler @Inject constructor( // If the user is not logged in and the site is already connected to Jetpack, ask to login. !accountStore.hasAccessToken() && site.isJetpackConnected -> SiteNavigationAction.StartWPComLoginForJetpackStats - // If it's a WordPress.com or Jetpack site, show the Stats screen. - site.isWPCom || site.isJetpackInstalled && site.isJetpackConnected -> { + // If it's a WordPress.com or Jetpack site, show the Stats screen. Stats are served by + // WordPress.com, so a Jetpack site also has to be connected to the account signed in here — + // being connected to some other account reaches the right site and is still refused. + site.isWPCom || + (site.isJetpackInstalled && site.isJetpackConnected && + wpComSiteAccessChecker.hasWpComAccess(site)) -> { if (newStatsRouting.isNewStatsEnabled()) { SiteNavigationAction.OpenNewStats } else { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt index 2d3baa140333..5b230e6094cb 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt @@ -25,6 +25,7 @@ import org.wordpress.android.ui.mysite.SelectedSiteRepository import org.wordpress.android.util.AppLog import org.wordpress.android.util.AppLog.T.API import org.wordpress.android.util.WPUrlUtils +import org.wordpress.android.util.WpComSiteAccessChecker import org.wordpress.android.util.extensions.getSerializableExtraCompat import javax.inject.Inject import android.R as AndroidR @@ -45,6 +46,9 @@ class StatsConnectJetpackActivity : BaseAppCompatActivity() { @Inject lateinit var mSelectedSiteRepository: SelectedSiteRepository + @Inject + lateinit var mWpComSiteAccessChecker: WpComSiteAccessChecker + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) initDagger() @@ -142,7 +146,8 @@ class StatsConnectJetpackActivity : BaseAppCompatActivity() { * so we skip the old WebView-based flow */ private fun startJetpackRestConnectionFlow(site: SiteModel): Boolean { - if (JetpackRestConnectionViewModel.canInitiateJetpackRestConnection(site)) { + val hasWpComAccess = mWpComSiteAccessChecker.hasWpComAccess(site) + if (JetpackRestConnectionViewModel.canInitiateJetpackRestConnection(site, hasWpComAccess)) { JetpackRestConnectionActivity.startJetpackRestConnectionFlow( this, JetpackRestConnectionViewModel.ConnectionSource.STATS diff --git a/WordPress/src/main/java/org/wordpress/android/util/WpComSiteAccessChecker.kt b/WordPress/src/main/java/org/wordpress/android/util/WpComSiteAccessChecker.kt new file mode 100644 index 000000000000..30c89d58697f --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/util/WpComSiteAccessChecker.kt @@ -0,0 +1,31 @@ +package org.wordpress.android.util + +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.store.SiteStore +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Answers whether the WordPress.com account signed in to this app can reach a site over the + * WordPress.com REST API. + * + * A site added with an application password carries the blog ID of whichever WordPress.com account + * its Jetpack connection belongs to, and that needn't be the account signed in here — a site can be + * connected to WordPress.com by someone else entirely. So `isJetpackConnected` says the site is + * connected to *an* account, not that it is connected to *this* one, and the WordPress.com-backed + * features (Stats above all) fail with a 403 for the difference. + */ +@Singleton +class WpComSiteAccessChecker @Inject constructor( + private val siteStore: SiteStore +) { + /** + * @return true if this account can use WordPress.com endpoints for [site]. Sites reached over the + * WordPress.com REST API qualify by definition. Anything else has to prove it: the account's own + * site list is the record of what it can reach, so the site qualifies when the same blog ID also + * arrived from `/me/sites`. + */ + fun hasWpComAccess(site: SiteModel): Boolean = + site.isUsingWpComRestApi || + (site.siteId != 0L && siteStore.sitesAccessedViaWPComRest.any { it.siteId == site.siteId }) +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModelTest.kt index 8bb91e1a9a30..9e674155b4a2 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModelTest.kt @@ -394,6 +394,25 @@ class JetpackRestConnectionViewModelTest : BaseUnitTest() { } } + @Test + fun `canInitiateJetpackRestConnection returns true for site connected to another account`() { + if (BuildConfig.IS_JETPACK_APP) { + val site = mock { + on { isUsingSelfHostedRestApi } doReturn true + on { wpApiRestUrl } doReturn "https://example.com/wp-json" + on { isJetpackConnected } doReturn true + on { isJetpackInstalled } doReturn true + on { jetpackVersion } doReturn VALID_JETPACK_VERSION + } + + // the site is connected, but not to the account signed in here, so its ConnectUser step + // is still needed + assertThat( + JetpackRestConnectionViewModel.canInitiateJetpackRestConnection(site, hasWpComAccess = false) + ).isTrue + } + } + @Test fun `canInitiateJetpackRestConnection returns false for old Jetpack version`() { if (BuildConfig.IS_JETPACK_APP) { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandlerTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandlerTest.kt index 8753966eb02e..a940e178567a 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandlerTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/ListItemActionHandlerTest.kt @@ -16,6 +16,7 @@ import org.wordpress.android.ui.blaze.blazecampaigns.campaignlisting.CampaignLis import org.wordpress.android.ui.mysite.SiteNavigationAction import org.wordpress.android.ui.mysite.items.listitem.ListItemAction import org.wordpress.android.ui.newstats.NewStatsRouting +import org.wordpress.android.util.WpComSiteAccessChecker import kotlin.test.assertEquals @ExperimentalCoroutinesApi @@ -30,6 +31,9 @@ class ListItemActionHandlerTest: BaseUnitTest() { @Mock lateinit var newStatsRouting: NewStatsRouting + @Mock + lateinit var wpComSiteAccessChecker: WpComSiteAccessChecker + private val site = SiteModel() private lateinit var listItemActionHandler: ListItemActionHandler @@ -39,7 +43,8 @@ class ListItemActionHandlerTest: BaseUnitTest() { listItemActionHandler = ListItemActionHandler( accountStore, blazeFeatureUtils, - newStatsRouting + newStatsRouting, + wpComSiteAccessChecker ) } @@ -147,6 +152,30 @@ class ListItemActionHandlerTest: BaseUnitTest() { } + @Test + fun `stats item click emits OpenStats if Jetpack site is connected to the signed-in account`() { + site.setIsJetpackInstalled(true) + site.setIsJetpackConnected(true) + whenever(accountStore.hasAccessToken()).thenReturn(true) + whenever(wpComSiteAccessChecker.hasWpComAccess(site)).thenReturn(true) + + val navigationAction = invokeItemClickAction(action = ListItemAction.STATS) + + assertEquals(SiteNavigationAction.OpenStats(site), navigationAction) + } + + @Test + fun `stats item click emits ConnectJetpackForStats if Jetpack site belongs to another account`() { + site.setIsJetpackInstalled(true) + site.setIsJetpackConnected(true) + whenever(accountStore.hasAccessToken()).thenReturn(true) + whenever(wpComSiteAccessChecker.hasWpComAccess(site)).thenReturn(false) + + val navigationAction = invokeItemClickAction(action = ListItemAction.STATS) + + assertEquals(SiteNavigationAction.ConnectJetpackForStats(site), navigationAction) + } + @Test fun `stats item click emits StartWPComLoginForJetpackStats if site is Jetpack and doesn't have access token`() { site.setIsJetpackConnected(true) From c430c490f3ff4652d56c81c97704451ac0cfed32 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 20 Aug 2026 16:03:38 -0400 Subject: [PATCH 5/5] Skip the install pitch for sites that already have Jetpack StatsConnectJetpackActivity pitches installing Jetpack with no branch on whether the site has it, and only consults canInitiateJetpackRestConnection once its button is tapped. Routing an already-installed site here therefore told the user to install a plugin that was in front of them -- the symptom CMM-2304 is about. Start the connection flow directly when Jetpack is installed, and never build the pitch. That flow's install step recognises Jetpack is present and moves on to connecting the account, which is the step actually missing. Sites without Jetpack are unaffected and still get the install screen. --- .../ui/stats/StatsConnectJetpackActivity.kt | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt index 5b230e6094cb..61ae8b45d6a3 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt @@ -52,6 +52,9 @@ class StatsConnectJetpackActivity : BaseAppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) initDagger() + if (savedInstanceState == null && skipInstallPitchForInstalledSite()) { + return + } with(StatsJetpackConnectionActivityBinding.inflate(layoutInflater)) { setContentView(root) initActionBar() @@ -75,6 +78,23 @@ class StatsConnectJetpackActivity : BaseAppCompatActivity() { } } + /** + * This screen pitches installing Jetpack, which is wrong for a site that already has it. Such a + * site is here because its Jetpack connection belongs to another WordPress.com account (or to no + * account yet), so what it needs is the connection flow, whose install step recognises Jetpack is + * present and moves on. Skip straight to it. + * + * @return true if the flow was started, in which case this screen must not be built. + */ + private fun skipInstallPitchForInstalledSite(): Boolean { + val site = intent.getSerializableExtraCompat(WordPress.SITE) + val skip = site != null && site.isJetpackInstalled && startJetpackRestConnectionFlow(site) + if (skip) { + finish() + } + return skip + } + /** * Continue Jetpack connect flow if coming from login/signup magic link. */