From 8cd50840318fa97ba065b444d6fc1278987502d7 Mon Sep 17 00:00:00 2001 From: Pantelis Stampoulis Date: Thu, 25 Jan 2024 18:10:24 +0200 Subject: [PATCH 1/3] Add Site monitoring FF to SiteListItemBuilder --- .../items/listitem/SiteListItemBuilder.kt | 7 +++--- .../config/SiteMonitoringFeatureConfig.kt | 9 +++++-- .../items/listitem/SiteListItemBuilderTest.kt | 24 ++++++++++++++++--- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt index d847635c9626..196e1f8aee2e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt @@ -29,6 +29,7 @@ import org.wordpress.android.ui.utils.UiString.UiStringText import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.DateTimeUtils import org.wordpress.android.util.SiteUtilsWrapper +import org.wordpress.android.util.config.SiteMonitoringFeatureConfig import java.util.GregorianCalendar import java.util.TimeZone import javax.inject.Inject @@ -39,7 +40,8 @@ class SiteListItemBuilder @Inject constructor( private val siteUtilsWrapper: SiteUtilsWrapper, private val buildConfigWrapper: BuildConfigWrapper, private val themeBrowserUtils: ThemeBrowserUtils, - private val jetpackFeatureRemovalPhaseHelper: JetpackFeatureRemovalPhaseHelper + private val jetpackFeatureRemovalPhaseHelper: JetpackFeatureRemovalPhaseHelper, + private val siteMonitoringFeatureConfig: SiteMonitoringFeatureConfig ) { fun buildActivityLogItemIfAvailable(site: SiteModel, onClick: (ListItemAction) -> Unit): ListItem? { val isWpComOrJetpack = siteUtilsWrapper.isAccessedViaWPComRest( @@ -246,8 +248,7 @@ class SiteListItemBuilder @Inject constructor( } fun buildSiteMonitoringItemIfAvailable(site: SiteModel, onClick: (ListItemAction) -> Unit): MySiteCardAndItem? { - // todo: Add the feature flag wrapper once it is available - return if (buildConfigWrapper.isJetpackApp && site.isWPComAtomic && site.isAdmin) { + return if (siteMonitoringFeatureConfig.isEnabled() && site.isWPComAtomic && site.isAdmin) { ListItem( R.drawable.gb_ic_tool, UiStringRes(R.string.site_monitoring), diff --git a/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt b/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt index 0a7789dda5e2..6a2cc90c1fdb 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt @@ -2,14 +2,19 @@ package org.wordpress.android.util.config import org.wordpress.android.BuildConfig import org.wordpress.android.annotation.Feature +import javax.inject.Inject private const val SITE_MONITORING_FEATURE_REMOTE_FIELD = "site_monitoring" @Feature(SITE_MONITORING_FEATURE_REMOTE_FIELD, false) -class SiteMonitoringFeatureConfig( +class SiteMonitoringFeatureConfig @Inject constructor( appConfig: AppConfig ) : FeatureConfig( appConfig, BuildConfig.ENABLE_SITE_MONITORING, SITE_MONITORING_FEATURE_REMOTE_FIELD -) +) { + override fun isEnabled(): Boolean { + return super.isEnabled() && BuildConfig.IS_JETPACK_APP + } +} diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt index 7d579919727b..a8f6e54b341c 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt @@ -29,6 +29,7 @@ import org.wordpress.android.ui.plugins.PluginUtilsWrapper import org.wordpress.android.ui.themes.ThemeBrowserUtils import org.wordpress.android.util.BuildConfigWrapper import org.wordpress.android.util.SiteUtilsWrapper +import org.wordpress.android.util.config.SiteMonitoringFeatureConfig @RunWith(MockitoJUnitRunner::class) class SiteListItemBuilderTest { @@ -53,6 +54,9 @@ class SiteListItemBuilderTest { @Mock lateinit var jetpackFeatureRemovalPhaseHelper: JetpackFeatureRemovalPhaseHelper + @Mock + lateinit var siteMonitoringFeatureConfig: SiteMonitoringFeatureConfig + private lateinit var siteListItemBuilder: SiteListItemBuilder @Before @@ -63,7 +67,8 @@ class SiteListItemBuilderTest { siteUtilsWrapper, buildConfigWrapper, themeBrowserUtils, - jetpackFeatureRemovalPhaseHelper + jetpackFeatureRemovalPhaseHelper, + siteMonitoringFeatureConfig ) } @@ -417,14 +422,25 @@ class SiteListItemBuilderTest { /* SITE MONITORING */ @Test - fun `give jetpack app, when is atomic and admin, then site monitoring item is built`() { - setupSiteMonitoringItems() + fun `give jetpack app, when is atomic and admin and FF is enabled, then site monitoring item is built`() { + setupSiteMonitoringItems( + isSiteMonitoringFeatureFlagEnabled = true + ) val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) assertThat(item).isEqualTo(SITE_MONITORING_ITEM) } + @Test + fun `give jetpack app, when is atomic and admin and FF is disabled, then site monitoring item is not built`() { + setupSiteMonitoringItems() + + val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) + + assertThat(item).isNull() + } + @Test fun `give jetpack app, when is not atomic, then site monitoring item is not built`() { setupSiteMonitoringItems( @@ -460,10 +476,12 @@ class SiteListItemBuilderTest { private fun setupSiteMonitoringItems( isJetpackApp: Boolean = true, + isSiteMonitoringFeatureFlagEnabled: Boolean = false, isAtomic: Boolean = true, isAdmin: Boolean = true ) { whenever(buildConfigWrapper.isJetpackApp).thenReturn(isJetpackApp) + whenever(siteMonitoringFeatureConfig.isEnabled()).thenReturn(isSiteMonitoringFeatureFlagEnabled) whenever(siteModel.isAdmin).thenReturn(isAdmin) whenever(siteModel.isWPComAtomic).thenReturn(isAtomic) } From c1306526a06c2d8b9736bd2eba81ccc8c96dd8e3 Mon Sep 17 00:00:00 2001 From: Pantelis Stampoulis Date: Fri, 26 Jan 2024 13:15:33 +0200 Subject: [PATCH 2/3] Fix tests in SiteListItemBuilderTest --- .../items/listitem/SiteListItemBuilderTest.kt | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt index a8f6e54b341c..f0fe7a9a46a4 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt @@ -422,9 +422,11 @@ class SiteListItemBuilderTest { /* SITE MONITORING */ @Test - fun `give jetpack app, when is atomic and admin and FF is enabled, then site monitoring item is built`() { + fun `give site monitoring FF is true, when is atomic and admin, then site monitoring item is built`() { setupSiteMonitoringItems( - isSiteMonitoringFeatureFlagEnabled = true + isSiteMonitoringFeatureFlagEnabled = true, + isAtomic = true, + isAdmin = true ) val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) @@ -433,18 +435,11 @@ class SiteListItemBuilderTest { } @Test - fun `give jetpack app, when is atomic and admin and FF is disabled, then site monitoring item is not built`() { - setupSiteMonitoringItems() - - val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) - - assertThat(item).isNull() - } - - @Test - fun `give jetpack app, when is not atomic, then site monitoring item is not built`() { + fun `give site monitoring FF is true, when is atomic and NOT admin, then site monitoring item is not built`() { setupSiteMonitoringItems( - isAtomic = false + isSiteMonitoringFeatureFlagEnabled = true, + isAtomic = true, + isAdmin = false ) val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) @@ -453,9 +448,10 @@ class SiteListItemBuilderTest { } @Test - fun `give jetpack app, when is not admin, then site monitoring item is not built`() { + fun `give site monitoring FF is true, when is admin and NOT atomic, then site monitoring item is not built`() { setupSiteMonitoringItems( - isAdmin = false + isSiteMonitoringFeatureFlagEnabled = true, + isAdmin = true ) val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) @@ -464,9 +460,10 @@ class SiteListItemBuilderTest { } @Test - fun `give not jetpack app, when site monitoring item requested, then site monitoring item is not built`() { + fun `give site monitoring FF is false, when is admin and atomic, then site monitoring item is not built`() { setupSiteMonitoringItems( - isJetpackApp = false + isAtomic = true, + isAdmin = true ) val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) @@ -475,12 +472,10 @@ class SiteListItemBuilderTest { } private fun setupSiteMonitoringItems( - isJetpackApp: Boolean = true, isSiteMonitoringFeatureFlagEnabled: Boolean = false, - isAtomic: Boolean = true, - isAdmin: Boolean = true + isAtomic: Boolean = false, + isAdmin: Boolean = false ) { - whenever(buildConfigWrapper.isJetpackApp).thenReturn(isJetpackApp) whenever(siteMonitoringFeatureConfig.isEnabled()).thenReturn(isSiteMonitoringFeatureFlagEnabled) whenever(siteModel.isAdmin).thenReturn(isAdmin) whenever(siteModel.isWPComAtomic).thenReturn(isAtomic) From 343c35291309180fbc9e73aab30410d558f90733 Mon Sep 17 00:00:00 2001 From: Pantelis Stampoulis Date: Fri, 26 Jan 2024 15:47:09 +0200 Subject: [PATCH 3/3] Remove jetpack app check from SiteMonitoringFeatureConfig --- .../items/listitem/SiteListItemBuilder.kt | 7 ++++- .../config/SiteMonitoringFeatureConfig.kt | 7 ++--- .../items/listitem/SiteListItemBuilderTest.kt | 28 ++++++++++++++++--- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt index 196e1f8aee2e..7bc0263630ad 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilder.kt @@ -247,8 +247,13 @@ class SiteListItemBuilder @Inject constructor( } else null } + @Suppress("ComplexCondition") fun buildSiteMonitoringItemIfAvailable(site: SiteModel, onClick: (ListItemAction) -> Unit): MySiteCardAndItem? { - return if (siteMonitoringFeatureConfig.isEnabled() && site.isWPComAtomic && site.isAdmin) { + return if (buildConfigWrapper.isJetpackApp + && site.isWPComAtomic + && site.isAdmin + && siteMonitoringFeatureConfig.isEnabled() + ) { ListItem( R.drawable.gb_ic_tool, UiStringRes(R.string.site_monitoring), diff --git a/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt b/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt index 6a2cc90c1fdb..3102f90ad70a 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/config/SiteMonitoringFeatureConfig.kt @@ -13,8 +13,5 @@ class SiteMonitoringFeatureConfig @Inject constructor( appConfig, BuildConfig.ENABLE_SITE_MONITORING, SITE_MONITORING_FEATURE_REMOTE_FIELD -) { - override fun isEnabled(): Boolean { - return super.isEnabled() && BuildConfig.IS_JETPACK_APP - } -} +) + diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt index f0fe7a9a46a4..d1bfd2e7eac7 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/mysite/items/listitem/SiteListItemBuilderTest.kt @@ -422,8 +422,9 @@ class SiteListItemBuilderTest { /* SITE MONITORING */ @Test - fun `give site monitoring FF is true, when is atomic and admin, then site monitoring item is built`() { + fun `give jetpack app, when FF is true and site is atomic and admin, then site monitoring item is built`() { setupSiteMonitoringItems( + isJetpackApp = true, isSiteMonitoringFeatureFlagEnabled = true, isAtomic = true, isAdmin = true @@ -435,8 +436,9 @@ class SiteListItemBuilderTest { } @Test - fun `give site monitoring FF is true, when is atomic and NOT admin, then site monitoring item is not built`() { + fun `give jetpack app, when FF is true and site is atomic and NOT admin, then site monitoring item is not built`() { setupSiteMonitoringItems( + isJetpackApp = true, isSiteMonitoringFeatureFlagEnabled = true, isAtomic = true, isAdmin = false @@ -448,8 +450,9 @@ class SiteListItemBuilderTest { } @Test - fun `give site monitoring FF is true, when is admin and NOT atomic, then site monitoring item is not built`() { + fun `give jetpack app, when FF is true and site is admin and NOT atomic, then site monitoring item is not built`() { setupSiteMonitoringItems( + isJetpackApp = true, isSiteMonitoringFeatureFlagEnabled = true, isAdmin = true ) @@ -460,8 +463,23 @@ class SiteListItemBuilderTest { } @Test - fun `give site monitoring FF is false, when is admin and atomic, then site monitoring item is not built`() { + fun `give jetpack app, when FF is false and site is admin and atomic, then site monitoring item is not built`() { setupSiteMonitoringItems( + isJetpackApp = true, + isAtomic = true, + isAdmin = true + ) + + val item = siteListItemBuilder.buildSiteMonitoringItemIfAvailable(siteModel, SITE_ITEM_ACTION) + + assertThat(item).isNull() + } + + @Test + fun `give not jetpack app, when FF is true and site is atomic and admin, then site monitoring item is not built`() { + setupSiteMonitoringItems( + isJetpackApp = false, + isSiteMonitoringFeatureFlagEnabled = true, isAtomic = true, isAdmin = true ) @@ -472,10 +490,12 @@ class SiteListItemBuilderTest { } private fun setupSiteMonitoringItems( + isJetpackApp: Boolean = false, isSiteMonitoringFeatureFlagEnabled: Boolean = false, isAtomic: Boolean = false, isAdmin: Boolean = false ) { + whenever(buildConfigWrapper.isJetpackApp).thenReturn(isJetpackApp) whenever(siteMonitoringFeatureConfig.isEnabled()).thenReturn(isSiteMonitoringFeatureFlagEnabled) whenever(siteModel.isAdmin).thenReturn(isAdmin) whenever(siteModel.isWPComAtomic).thenReturn(isAtomic)