CMM-2304: Detect Jetpack status for application-password sites

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()
     }
 

WordPress/src/main/java/org/wordpress/android/ui/jetpackrestconnection/JetpackRestConnectionViewModel.kt

@@ -576,15 +576,29 @@ class JetpackRestConnectionViewModel @Inject constructor(
          * - Jetpack app
          * - 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
+         * - 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.isJetpackInstalled || checkMinimalVersion(site.jetpackVersion, JETPACK_LIMIT_VERSION))
+                    && (!site.isJetpackConnected || !hasWpComAccess)
+                    && (!site.isJetpackInstalled
+                    || site.jetpackVersion.isNullOrEmpty()
+                    || checkMinimalVersion(site.jetpackVersion, JETPACK_LIMIT_VERSION))
         }
     }
 }

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

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 {

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()

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)

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<MySiteCardAndItem.Card.JetpackFeatureCard?>()
     val uiModel = _uiModel.distinctUntilChanged()
 
-    suspend fun buildJetpackFeatureCard() {
-        if (!jetpackFeatureCardHelper.shouldShowJetpackFeatureCard()){
+    suspend fun buildJetpackFeatureCard(site: SiteModel?) {
+        if (!jetpackFeatureCardHelper.shouldShowJetpackFeatureCard(site)){
             _uiModel.postValue(null)
             return
         }

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,9 +46,15 @@ class StatsConnectJetpackActivity : BaseAppCompatActivity() {
     @Inject
     lateinit var mSelectedSiteRepository: SelectedSiteRepository
 
+    @Inject
+    lateinit var mWpComSiteAccessChecker: WpComSiteAccessChecker
+
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         initDagger()
+        if (savedInstanceState == null && skipInstallPitchForInstalledSite()) {
+            return
+        }
         with(StatsJetpackConnectionActivityBinding.inflate(layoutInflater)) {
             setContentView(root)
             initActionBar()
@@ -71,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<SiteModel>(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.
      */
@@ -142,7 +166,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

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 })
+}

WordPress/src/main/java/org/wordpress/android/util/extensions/SiteModelExtensions.kt

@@ -31,3 +31,11 @@ val SiteModel.stateLogInformation: String
  */
 fun SiteModel.activeJetpackConnectionPluginValues(): List<String>? =
     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)

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,23 +39,37 @@ 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
     fun `given the WordPress app and the overlay was never shown, then it is shown`() {
         whenever(jetpackFeatureRemovalHelper.shouldRemoveJetpackFeatures()).thenReturn(true)
         whenever(shownTracker.getFeatureCollectionOverlayShown()).thenReturn(false)
 
-        assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime()).isTrue
+        assertThat(overlayUtil.shouldShowFeatureCollectionJetpackOverlayForFirstTime(jetpackSite())).isTrue
     }
 
     @Test
     fun `given the WordPress app and the overlay was already shown, then it is not shown again`() {
         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)
+    }
 }

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<SiteModel> {
+                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) {

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()
     }