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..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,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)) } } } 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/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/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/stats/StatsConnectJetpackActivity.kt b/WordPress/src/main/java/org/wordpress/android/ui/stats/StatsConnectJetpackActivity.kt index 2d3baa140333..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 @@ -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(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 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/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/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/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/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) 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..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 @@ -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,74 @@ 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? { + val username = payload.username + val password = payload.password + val canFetch = hasJetpack && payload.isApplicationPassword && + !username.isNullOrEmpty() && !password.isNullOrEmpty() + + return if (canFetch) { + jetpackConnectionStatusFetcher.fetch(apiRootUrl, username, password) + } else { + null + } + } + + 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 + } +}