diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteViewModel.kt index 479884efdfcf..ced74da2f625 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteViewModel.kt @@ -45,7 +45,7 @@ import org.wordpress.android.ui.mysite.cards.applicationpassword.ApplicationPass import org.wordpress.android.ui.mysite.items.listitem.SiteCapabilityChecker import org.wordpress.android.ui.posts.GutenbergKitWarmupHelper import org.wordpress.android.ui.utils.UiString -import org.wordpress.android.repositories.EditorSettingsRepository +import org.wordpress.android.ui.mysite.cards.connectivity.SiteConnectivityBannerViewModelSlice @Suppress("LargeClass", "LongMethod", "LongParameterList") class MySiteViewModel @Inject constructor( @@ -67,7 +67,7 @@ class MySiteViewModel @Inject constructor( private val applicationPasswordViewModelSlice: ApplicationPasswordViewModelSlice, private val gutenbergKitWarmupHelper: GutenbergKitWarmupHelper, private val siteCapabilityChecker: SiteCapabilityChecker, - private val editorSettingsRepository: EditorSettingsRepository, + private val siteConnectivityBannerViewModelSlice: SiteConnectivityBannerViewModelSlice, ) : ScopedViewModel(mainDispatcher) { private val _onSnackbarMessage = MutableLiveData>() private val _onNavigation = MutableLiveData>() @@ -126,15 +126,17 @@ class MySiteViewModel @Inject constructor( applicationPasswordViewModelSlice.uiModel, accountDataViewModelSlice.uiModel, dashboardCardsViewModelSlice.uiModel, - dashboardItemsViewModelSlice.uiModel + dashboardItemsViewModelSlice.uiModel, + siteConnectivityBannerViewModelSlice.uiModel, ) { siteInfoHeaderCard, applicationPAsswordModel, accountData, dashboardCards, - siteItems -> + siteItems, + connectivityBanner -> val nonNullSiteInfoHeaderCard = siteInfoHeaderCard ?: return@merge buildNoSiteState(accountData?.url, accountData?.name) - val headerList = listOfNotNull(nonNullSiteInfoHeaderCard, applicationPAsswordModel) + val headerList = listOfNotNull(nonNullSiteInfoHeaderCard, applicationPAsswordModel, connectivityBanner) return@merge if (!dashboardCards.isNullOrEmpty()) SiteSelected(dashboardData = headerList + dashboardCards) else if (!siteItems.isNullOrEmpty()) @@ -150,6 +152,7 @@ class MySiteViewModel @Inject constructor( dashboardCardsViewModelSlice.initialize(viewModelScope) dashboardItemsViewModelSlice.initialize(viewModelScope) accountDataViewModelSlice.initialize(viewModelScope) + siteConnectivityBannerViewModelSlice.initialize(viewModelScope) } private fun shouldShowDashboard(site: SiteModel): Boolean { @@ -170,12 +173,10 @@ class MySiteViewModel @Inject constructor( siteCapabilityChecker.clearCacheForSite(site.siteId) } buildDashboardOrSiteItems(site) - launch { - fetchEditorCapabilitiesWithSnackbar( - site, - isUserInitiated = isPullToRefresh - ) - } + siteConnectivityBannerViewModelSlice.fetchCapabilities( + site, + isUserInitiated = isPullToRefresh + ) } ?: run { accountDataViewModelSlice.onRefresh() } @@ -187,39 +188,15 @@ class MySiteViewModel @Inject constructor( selectedSiteRepository.updateSiteSettingsIfNecessary() selectedSiteRepository.getSelectedSite()?.let { buildDashboardOrSiteItems(it) - launch { - fetchEditorCapabilitiesWithSnackbar( - it, - isUserInitiated = false - ) - } + siteConnectivityBannerViewModelSlice.fetchCapabilities( + it, + isUserInitiated = false + ) } ?: run { accountDataViewModelSlice.onResume() } } - private suspend fun fetchEditorCapabilitiesWithSnackbar( - site: SiteModel, - isUserInitiated: Boolean - ) { - val ok = editorSettingsRepository - .fetchEditorCapabilitiesForSite(site) - val hasCache = editorSettingsRepository - .hasCachedCapabilities(site) - if (!ok && (isUserInitiated || !hasCache)) { - _onSnackbarMessage.postValue( - Event( - SnackbarMessageHolder( - UiString.UiStringRes( - R.string - .site_settings_fetch_failed - ) - ) - ) - ) - } - } - private fun checkAndShowJetpackFullPluginInstallOnboarding() { selectedSiteRepository.getSelectedSite()?.let { selectedSite -> if (getShowJetpackFullPluginInstallOnboardingUseCase.execute(selectedSite)) { @@ -334,6 +311,7 @@ class MySiteViewModel @Inject constructor( private fun onSitePicked(site: SiteModel) { siteInfoHeaderCardViewModelSlice.buildCard(site) applicationPasswordViewModelSlice.buildCard(site) + siteConnectivityBannerViewModelSlice.clearBanner() dashboardItemsViewModelSlice.clearValue() dashboardCardsViewModelSlice.clearValue() dashboardCardsViewModelSlice.resetShownTracker() diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt new file mode 100644 index 000000000000..4bcde23b63ef --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSlice.kt @@ -0,0 +1,60 @@ +package org.wordpress.android.ui.mysite.cards.connectivity + +import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.repositories.EditorSettingsRepository +import org.wordpress.android.ui.mysite.MySiteCardAndItem +import javax.inject.Inject + +class SiteConnectivityBannerViewModelSlice @Inject constructor( + private val editorSettingsRepository: EditorSettingsRepository, +) { + private lateinit var scope: CoroutineScope + + private val _uiModel = MutableLiveData() + val uiModel: LiveData = _uiModel + + /* Site capabilities rarely change, so once we've successfully fetched them for a site we + skip subsequent non-user-initiated fetches in this slice's lifetime. Failed fetches do + not populate this set, so a transient network failure recovers on the next onResume. + User-initiated calls (PTR, banner retry) always bypass this gate. */ + private val fetchedCapabilitiesForSite = mutableSetOf() + + fun initialize(scope: CoroutineScope) { + this.scope = scope + } + + fun fetchCapabilities(site: SiteModel, isUserInitiated: Boolean) { + scope.launch { + if (site.id in fetchedCapabilitiesForSite && !isUserInitiated) { + return@launch + } + val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site) + if (ok) { + fetchedCapabilitiesForSite.add(site.id) + } + val hasCache = editorSettingsRepository.hasCachedCapabilities(site) + if (ok || hasCache) { + _uiModel.postValue(null) + } else { + _uiModel.postValue(buildBanner(site)) + } + } + } + + fun clearBanner() { + _uiModel.postValue(null) + } + + private fun buildBanner(site: SiteModel): MySiteCardAndItem.Item.SingleActionCard = + MySiteCardAndItem.Item.SingleActionCard( + textResource = R.string.site_connectivity_banner_text, + imageResource = R.drawable.ic_notice_white_24dp, + onActionClick = { fetchCapabilities(site, isUserInitiated = true) }, + showLearnMore = false, + ) +} diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 71931f0c758f..d40fae768691 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -705,7 +705,7 @@ Use Third-Party Blocks (Beta) Load third-party blocks from plugins installed on your site. Your site doesn\'t support loading third-party blocks in the editor. - Failed to fetch site settings – some editor functionality may be limited. + Unable to connect to your site. Some functionality might be limited. Password updated To reconnect the app to your self-hosted site, enter the site\'s new password here. Homepage Settings diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt index c6c0ffc6c628..2ca4e29968bf 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt @@ -14,7 +14,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.junit.MockitoJUnitRunner -import org.mockito.kotlin.any import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.never import org.mockito.kotlin.times @@ -40,7 +39,7 @@ import org.wordpress.android.ui.mysite.cards.applicationpassword.ApplicationPass import org.wordpress.android.ui.mysite.cards.siteinfo.SiteInfoHeaderCardViewModelSlice import org.wordpress.android.ui.mysite.items.DashboardItemsViewModelSlice import org.wordpress.android.ui.mysite.items.listitem.SiteCapabilityChecker -import org.wordpress.android.repositories.EditorSettingsRepository +import org.wordpress.android.ui.mysite.cards.connectivity.SiteConnectivityBannerViewModelSlice import org.wordpress.android.ui.pages.SnackbarMessageHolder import org.wordpress.android.ui.posts.GutenbergKitWarmupHelper import org.wordpress.android.ui.sitecreation.misc.SiteCreationSource @@ -106,7 +105,7 @@ class MySiteViewModelTest : BaseUnitTest() { lateinit var siteCapabilityChecker: SiteCapabilityChecker @Mock - lateinit var editorSettingsRepository: EditorSettingsRepository + lateinit var siteConnectivityBannerViewModelSlice: SiteConnectivityBannerViewModelSlice private lateinit var viewModel: MySiteViewModel private lateinit var uiModels: MutableList @@ -143,7 +142,7 @@ class MySiteViewModelTest : BaseUnitTest() { whenever(dashboardCardsViewModelSlice.uiModel).thenReturn(MutableLiveData()) whenever(dashboardItemsViewModelSlice.uiModel).thenReturn(MutableLiveData()) whenever(applicationPasswordViewModelSlice.uiModel).thenReturn(MutableLiveData()) - whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(any())).thenReturn(true) + whenever(siteConnectivityBannerViewModelSlice.uiModel).thenReturn(MutableLiveData()) viewModel = MySiteViewModel( testDispatcher(), @@ -164,7 +163,7 @@ class MySiteViewModelTest : BaseUnitTest() { applicationPasswordViewModelSlice, gutenbergKitWarmupHelper, siteCapabilityChecker, - editorSettingsRepository, + siteConnectivityBannerViewModelSlice, ) uiModels = mutableListOf() snackbars = mutableListOf() @@ -362,8 +361,6 @@ class MySiteViewModelTest : BaseUnitTest() { verify(dashboardCardsViewModelSlice).clearValue() } - - /* LAND ON THE EDITOR A/B EXPERIMENT */ @Test fun `when performFirstStepAfterSiteCreation called, then home page editor is shown`() = test { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt new file mode 100644 index 000000000000..7373c29c6152 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/cards/connectivity/SiteConnectivityBannerViewModelSliceTest.kt @@ -0,0 +1,159 @@ +package org.wordpress.android.ui.mysite.cards.connectivity + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.junit.MockitoJUnitRunner +import org.mockito.kotlin.eq +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.repositories.EditorSettingsRepository +import org.wordpress.android.ui.mysite.MySiteCardAndItem + +private const val TEST_SITE_LOCAL_ID = 42 + +@ExperimentalCoroutinesApi +@RunWith(MockitoJUnitRunner::class) +class SiteConnectivityBannerViewModelSliceTest : BaseUnitTest() { + @Mock + lateinit var editorSettingsRepository: EditorSettingsRepository + + private lateinit var siteTest: SiteModel + private lateinit var slice: SiteConnectivityBannerViewModelSlice + private val emittedBanners = mutableListOf() + + @Before + fun setUp() { + siteTest = SiteModel().apply { id = TEST_SITE_LOCAL_ID } + slice = SiteConnectivityBannerViewModelSlice(editorSettingsRepository) + slice.initialize(testScope()) + slice.uiModel.observeForever { emittedBanners.add(it) } + } + + @Test + fun `given fetch succeeds, when fetchCapabilities invoked, then banner is null`() = test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + + assertThat(emittedBanners.last()).isNull() + } + + @Test + fun `given fetch fails with no cache, when fetchCapabilities invoked, then banner is shown`() = test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) + whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + + val banner = emittedBanners.last() as MySiteCardAndItem.Item.SingleActionCard + assertThat(banner.textResource).isEqualTo(R.string.site_connectivity_banner_text) + assertThat(banner.showLearnMore).isFalse + } + + @Test + fun `given fetch fails but cache exists, when fetchCapabilities invoked, then banner is null`() = test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) + whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + + assertThat(emittedBanners.last()).isNull() + } + + @Test + fun `given prior successful fetch, when fetchCapabilities invoked again non-user-initiated, then fetch skipped`() = + test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + + verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(siteTest) + } + + @Test + fun `given prior failed fetch, when fetchCapabilities invoked again non-user-initiated, then fetch retries`() = + test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false, true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + + verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) + } + + @Test + fun `given prior successful fetch, when user-initiated fetchCapabilities invoked, then fetch runs again`() = + test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + slice.fetchCapabilities(siteTest, isUserInitiated = true) + advanceUntilIdle() + + verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) + } + + @Test + fun `given banner showing, when retry tapped, then fetch runs and bypasses session dedup`() = test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) + whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + val banner = emittedBanners.last() as MySiteCardAndItem.Item.SingleActionCard + + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + banner.onActionClick() + advanceUntilIdle() + + verify(editorSettingsRepository, times(2)).fetchEditorCapabilitiesForSite(siteTest) + assertThat(emittedBanners.last()).isNull() + } + + @Test + fun `when clearBanner invoked, then banner is null`() = test { + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(false) + whenever(editorSettingsRepository.hasCachedCapabilities(siteTest)).thenReturn(false) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + assertThat(emittedBanners.last()).isNotNull + slice.clearBanner() + advanceUntilIdle() + + assertThat(emittedBanners.last()).isNull() + } + + @Test + fun `given two different sites, when fetched in sequence, then both fetches run`() = test { + val otherSite = SiteModel().apply { id = TEST_SITE_LOCAL_ID + 1 } + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(siteTest)).thenReturn(true) + whenever(editorSettingsRepository.fetchEditorCapabilitiesForSite(otherSite)).thenReturn(true) + + slice.fetchCapabilities(siteTest, isUserInitiated = false) + advanceUntilIdle() + slice.fetchCapabilities(otherSite, isUserInitiated = false) + advanceUntilIdle() + + verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(eq(siteTest)) + verify(editorSettingsRepository, times(1)).fetchEditorCapabilitiesForSite(eq(otherSite)) + } +}