From e56c87a061c8d00377f1ecd7c22aaac24460a40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 12 Aug 2025 11:32:40 +0200 Subject: [PATCH 01/27] Return error if reaching remotely failed --- .../ui/woopos/tab/WooPosTabShouldBeVisible.kt | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt index 0b75cf91b3e..74fef971d80 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt @@ -10,6 +10,8 @@ import kotlinx.coroutines.withContext import org.wordpress.android.fluxc.store.WooCommerceStore import javax.inject.Inject +class CouldNotDetermineValueException : Exception() + class WooPosTabShouldBeVisible @Inject constructor( private val selectedSite: SelectedSite, private val isScreenSizeAllowed: WooPosIsScreenSizeAllowed, @@ -17,33 +19,34 @@ class WooPosTabShouldBeVisible @Inject constructor( private val isRemoteFeatureFlagEnabled: IsRemoteFeatureFlagEnabled, private val wooPosLog: WooPosLogWrapper, ) { - suspend operator fun invoke(): Boolean = withContext(Dispatchers.IO) { - val selectedSite = selectedSite.getOrNull() ?: return@withContext false + suspend operator fun invoke(): Result = withContext(Dispatchers.IO) { + val selectedSite = selectedSite.getOrNull() + ?: return@withContext Result.failure(CouldNotDetermineValueException()) if (!isRemoteFeatureFlagEnabled(WOO_POS)) { - return@withContext false.also { + return@withContext Result.success(true).also { wooPosLog.i("POS Tab Not visible reason: Remote feature flag is disabled") } } if (!isScreenSizeAllowed()) { - return@withContext false.also { + return@withContext Result.success(false).also { wooPosLog.i("POS Tab Not visible reason: Screen size is not allowed") } } - val siteSettings = wooCommerceStore.fetchSiteGeneralSettings(selectedSite).model - ?: return@withContext false.also { - wooPosLog.i("POS Tab Not visible reason: Site settings are not available") - } + val siteSettings = wooCommerceStore + .fetchSiteGeneralSettings(selectedSite) + .model + ?: return@withContext Result.failure(CouldNotDetermineValueException()) - return@withContext isCountrySupported( - countryCode = siteSettings.countryCode - ).also { isSupported -> - if (!isSupported) { - wooPosLog.i("POS Tab Not visible reason: Country ${siteSettings.countryCode} is not supported") + return@withContext Result.success( + isCountrySupported(countryCode = siteSettings.countryCode).also { isSupported -> + if (!isSupported) { + wooPosLog.i("POS Tab Not visible reason: Country ${siteSettings.countryCode} is not supported") + } } - } + ) } private fun isCountrySupported(countryCode: String) = SUPPORTED_COUNTRIES.contains(countryCode.lowercase()) From 1051e3897d44a884911077131889ec16da31a5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 12 Aug 2025 11:42:25 +0200 Subject: [PATCH 02/27] Update function call to the new return value --- .../ui/woopos/tab/WooPosTabController.kt | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt index 0f7f3ba15f9..c365f261ffb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt @@ -15,12 +15,14 @@ import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import kotlinx.coroutines.launch import javax.inject.Inject +import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper class WooPosTabController @Inject constructor( private val appPrefs: AppPrefs, private val selectedSite: SelectedSite, private val shouldPosTabBeVisible: WooPosTabShouldBeVisible, - private val analyticsTracker: WooPosAnalyticsTracker + private val analyticsTracker: WooPosAnalyticsTracker, + private val wooPosLog: WooPosLogWrapper ) : DefaultLifecycleObserver { private lateinit var activity: MainActivity @@ -64,10 +66,17 @@ class WooPosTabController @Inject constructor( private fun updateTabVisibilityFromRemoteAndPersist() { activity.lifecycleScope.launch { - val tabShouldBeVisible = shouldPosTabBeVisible() - setPOSTabVisibility(tabShouldBeVisible) - appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId(), tabShouldBeVisible) - analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(tabShouldBeVisible)) + val result = shouldPosTabBeVisible() + + result.onSuccess { tabShouldBeVisible -> + setPOSTabVisibility(tabShouldBeVisible) + appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId(), tabShouldBeVisible) + analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(tabShouldBeVisible)) + } + + result.onFailure { error -> + wooPosLog.i("POS Tab Visibility Value cannot be determined") + } } } From c138585d0782e57206f7a968fc86e4059ddae44b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 12 Aug 2025 11:47:20 +0200 Subject: [PATCH 03/27] Update tracking with new information --- .../ui/woopos/util/analytics/WooPosAnalyticsEvent.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt index 895d153d9c4..69c072260bb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt @@ -104,15 +104,16 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent { } } - data class TabVisibilityChecked(val isVisible: Boolean) : Event() { + data class TabVisibilityChecked(val isVisible: Result) : Event() { override val name: String = "tab_visibility_checked" init { - addProperties( - mapOf( - "is_visible" to isVisible.toString() - ) + val value: String = isVisible.fold( + onSuccess = { it.toString() }, + onFailure = { "unknown" } ) + + addProperties(mapOf("is_visible" to value)) } } From 32b6ee9e0bac59ba4928c805b892686cb5e684e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 12 Aug 2025 12:08:12 +0200 Subject: [PATCH 04/27] Adapt to new function signature --- .../woocommerce/android/ui/woopos/tab/WooPosTabController.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt index c365f261ffb..67b0185e00e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt @@ -71,12 +71,13 @@ class WooPosTabController @Inject constructor( result.onSuccess { tabShouldBeVisible -> setPOSTabVisibility(tabShouldBeVisible) appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId(), tabShouldBeVisible) - analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(tabShouldBeVisible)) } result.onFailure { error -> wooPosLog.i("POS Tab Visibility Value cannot be determined") } + + analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(result)) } } From 5dfd4b92e1a0243f2c2843d73cc6cc5e843ea44d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 12 Aug 2025 12:11:56 +0200 Subject: [PATCH 05/27] Simplify to cache only positive values --- .../src/main/kotlin/com/woocommerce/android/AppPrefs.kt | 4 ++-- .../woocommerce/android/ui/woopos/tab/WooPosTabController.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt index c0d63c3a2ff..f840f7ef9ef 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt @@ -1183,10 +1183,10 @@ object AppPrefs { return getInt(PrefKeyString(channel.name), 0).takeIf { it != 0 } } - fun setPOSTabVisibilityForSite(siteId: Int, visible: Boolean) { + fun setPOSTabVisibilityForSite(siteId: Int) { setBoolean( key = PrefKeyString("${UndeletablePrefKey.POS_TAB_VISIBILITY}:$siteId"), - value = visible + value = true ) } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt index 67b0185e00e..0f4e3bdb08d 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt @@ -70,7 +70,7 @@ class WooPosTabController @Inject constructor( result.onSuccess { tabShouldBeVisible -> setPOSTabVisibility(tabShouldBeVisible) - appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId(), tabShouldBeVisible) + if (tabShouldBeVisible) appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId()) } result.onFailure { error -> From 366728e59fe8fa43ae93ae17c7a8ad050dd472b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:34:44 +0200 Subject: [PATCH 06/27] Add Exception to its own file --- .../common/util/WooPosCouldNotDetermineValueException.kt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/util/WooPosCouldNotDetermineValueException.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/util/WooPosCouldNotDetermineValueException.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/util/WooPosCouldNotDetermineValueException.kt new file mode 100644 index 00000000000..688e1c4b142 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/util/WooPosCouldNotDetermineValueException.kt @@ -0,0 +1,3 @@ +package com.woocommerce.android.ui.woopos.common.util + +class WooPosCouldNotDetermineValueException : Exception() From 4224b7e356f9bcbf2b5b19546a304a5f139158cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:35:16 +0200 Subject: [PATCH 07/27] Add prefs to cache POS launchable status --- .../kotlin/com/woocommerce/android/AppPrefs.kt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt index f840f7ef9ef..b0497ed22b7 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt @@ -210,6 +210,8 @@ object AppPrefs { APPLICATION_STORE_SNAPSHOT_TRACKED_FOR_SITE, POS_TAB_VISIBILITY, + + POS_LAUNCHABLE } fun init(context: Context) { @@ -1197,6 +1199,20 @@ object AppPrefs { ) } + fun setPOSLaunchableForSite(siteId: Int) { + setBoolean( + key = PrefKeyString("${UndeletablePrefKey.POS_LAUNCHABLE}:$siteId"), + value = true + ) + } + + fun isPOSLaunchableForSite(siteId: Int): Boolean { + return getBoolean( + key = PrefKeyString("${UndeletablePrefKey.POS_LAUNCHABLE}:$siteId"), + default = false + ) + } + /** * Remove all user and site-related preferences. */ From e4cbb017970c382e684d7a543b31d8654b848d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:35:56 +0200 Subject: [PATCH 08/27] Adapt to new refactored exception --- .../android/ui/woopos/tab/WooPosTabShouldBeVisible.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt index 74fef971d80..c266802ed9c 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.tab import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.WooPosIsScreenSizeAllowed +import com.woocommerce.android.ui.woopos.common.util.WooPosCouldNotDetermineValueException import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper import com.woocommerce.android.util.IsRemoteFeatureFlagEnabled import com.woocommerce.android.util.RemoteFeatureFlag.WOO_POS @@ -10,8 +11,6 @@ import kotlinx.coroutines.withContext import org.wordpress.android.fluxc.store.WooCommerceStore import javax.inject.Inject -class CouldNotDetermineValueException : Exception() - class WooPosTabShouldBeVisible @Inject constructor( private val selectedSite: SelectedSite, private val isScreenSizeAllowed: WooPosIsScreenSizeAllowed, @@ -21,7 +20,7 @@ class WooPosTabShouldBeVisible @Inject constructor( ) { suspend operator fun invoke(): Result = withContext(Dispatchers.IO) { val selectedSite = selectedSite.getOrNull() - ?: return@withContext Result.failure(CouldNotDetermineValueException()) + ?: return@withContext Result.failure(WooPosCouldNotDetermineValueException()) if (!isRemoteFeatureFlagEnabled(WOO_POS)) { return@withContext Result.success(true).also { @@ -38,7 +37,7 @@ class WooPosTabShouldBeVisible @Inject constructor( val siteSettings = wooCommerceStore .fetchSiteGeneralSettings(selectedSite) .model - ?: return@withContext Result.failure(CouldNotDetermineValueException()) + ?: return@withContext Result.failure(WooPosCouldNotDetermineValueException()) return@withContext Result.success( isCountrySupported(countryCode = siteSettings.countryCode).also { isSupported -> From 2fb1cdc7e645ddebfe64c328362e06474da1e04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:36:19 +0200 Subject: [PATCH 09/27] Handle new case in events --- .../android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt index 69c072260bb..aa145f93eaf 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt @@ -824,6 +824,7 @@ internal fun WooPosLaunchability.NonLaunchabilityReason.toAnalyticsReason(): Str WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled -> "feature_switch_disabled" WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency -> "store_currency" WooPosLaunchability.NonLaunchabilityReason.SiteSettingsUnavailable, + WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache, WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected -> "other" } } From 77750c7e115dd7141ec20a8981a3eb9112c06fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:37:39 +0200 Subject: [PATCH 10/27] Detekt --- .../woocommerce/android/ui/woopos/tab/WooPosTabController.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt index 0f4e3bdb08d..80363a49d10 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt @@ -10,12 +10,12 @@ import com.woocommerce.android.R import com.woocommerce.android.databinding.ActivityMainBinding import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.main.MainActivity +import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper import com.woocommerce.android.ui.woopos.root.WooPosActivity import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import kotlinx.coroutines.launch import javax.inject.Inject -import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper class WooPosTabController @Inject constructor( private val appPrefs: AppPrefs, From ba1f40c5c8e1de5978f05b87a42a1eb75dc527d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:38:25 +0200 Subject: [PATCH 11/27] Reactor to return a Result instead of a boolean, so we can communicate when we couldn't determine the value --- .../android/ui/woopos/WooPOSIsRemotelyEnabled.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabled.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabled.kt index acc9713918e..757e55c332c 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabled.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabled.kt @@ -3,6 +3,7 @@ package com.woocommerce.android.ui.woopos import com.google.common.reflect.TypeToken import com.google.gson.Gson import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.common.util.WooPosCouldNotDetermineValueException import com.woocommerce.android.util.WCSSRModelCachingFetcher import javax.inject.Inject @@ -12,7 +13,7 @@ class WooPOSIsRemotelyEnabled @Inject constructor( private val gson: Gson ) { - suspend operator fun invoke(forceRefresh: Boolean = false): Boolean { + suspend operator fun invoke(forceRefresh: Boolean = false): Result { if (forceRefresh) { ssrFetcher.invalidate() } @@ -25,9 +26,11 @@ class WooPOSIsRemotelyEnabled @Inject constructor( val settingsMap: Map = gson.fromJson(ssr.settings, type) val enabledFeatures = settingsMap["enabled_features"] as? List<*> - return enabledFeatures?.contains("point_of_sale") == true + return enabledFeatures?.let { + Result.success(it.contains("point_of_sale")) + } ?: Result.failure(WooPosCouldNotDetermineValueException()) } } - return false + return Result.failure(WooPosCouldNotDetermineValueException()) } } From e6ea0f1ca6d80d47915e50d591eddfa7fc5f16f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:39:03 +0200 Subject: [PATCH 12/27] Refactor to cache value and communicate when we couldn't determine the value --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 88 ++++++++++++------- 1 file changed, 58 insertions(+), 30 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index 2b2191e454f..94070494abb 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -1,5 +1,6 @@ package com.woocommerce.android.ui.woopos.tab +import com.woocommerce.android.AppPrefs import com.woocommerce.android.extensions.semverCompareTo import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.WooPOSIsRemotelyEnabled @@ -8,6 +9,8 @@ import com.woocommerce.android.util.FetchActiveWCPluginVersion import com.woocommerce.android.util.GetWooCorePluginCachedVersion import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.settings.Settings import org.wordpress.android.fluxc.store.WooCommerceStore import javax.inject.Inject import javax.inject.Singleton @@ -19,6 +22,7 @@ import javax.inject.Singleton */ @Singleton class WooPosCanBeLaunchedInTab @Inject constructor( + private val appPrefs: AppPrefs, private val selectedSite: SelectedSite, private val getWooCoreCachedVersion: GetWooCorePluginCachedVersion, private val fetchWooCoreVersion: FetchActiveWCPluginVersion, @@ -38,51 +42,74 @@ class WooPosCanBeLaunchedInTab @Inject constructor( @Suppress("ReturnCount") private suspend fun checkLaunchability(forceRefresh: Boolean = false): WooPosLaunchability { val site = selectedSite.getOrNull() - ?: return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected - ) + ?: return WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected) - val wooCoreVersion = if (forceRefresh) { - fetchWooCoreVersion() + val cachedPositive = appPrefs.isPOSLaunchableForSite(site.id) + + val wooCoreVersion = resolveWooCoreVersion(forceRefresh) + ?: return fallbackForMissingWooCore(cachedPositive) + + validateWooCoreVersion(wooCoreVersion)?.let { return it } + evaluateFeatureSwitch(wooCoreVersion, forceRefresh, cachedPositive)?.let { return it } + + val siteSettings = resolveSiteSettings(site, forceRefresh) + ?: return fallbackUnknownOrCache(cachedPositive) + + return if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { + WooPosLaunchability.Launchable } else { - getWooCoreCachedVersion() - } ?: return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.WooCommercePluginNotFound - ) + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency) + } + } + private suspend fun resolveWooCoreVersion(forceRefresh: Boolean): String? = + if (forceRefresh) fetchWooCoreVersion() else getWooCoreCachedVersion() + + private fun fallbackForMissingWooCore(cachedPositive: Boolean): WooPosLaunchability = + if (cachedPositive) { + WooPosLaunchability.Launchable + } else { + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.WooCommercePluginNotFound) + } + + private fun validateWooCoreVersion(wooCoreVersion: String): WooPosLaunchability? = if (!isWooCoreSupportsOrderAutoDraftsAndExtraPaymentsProps(wooCoreVersion)) { - return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.UnsupportedWooCommerceVersion - ) + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedWooCommerceVersion) + } else { + null } - if (isFeatureSwitchSupported(wooCoreVersion) && !isRemotelyEnabled(forceRefresh)) { - return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled - ) + private suspend fun evaluateFeatureSwitch( + wooCoreVersion: String, + forceRefresh: Boolean, + cachedPositive: Boolean + ): WooPosLaunchability? { + if (!isFeatureSwitchSupported(wooCoreVersion)) return null + val remote = isRemotelyEnabled(forceRefresh) + return when { + remote.isSuccess && !remote.getOrThrow() -> + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled) + remote.isFailure && cachedPositive -> + WooPosLaunchability.Launchable + remote.isFailure && !cachedPositive -> + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache) + else -> null } + } - val siteSettings = if (forceRefresh) { + private suspend fun resolveSiteSettings(site: SiteModel, forceRefresh: Boolean): Settings? = + if (forceRefresh) { wooCommerceStore.fetchSiteGeneralSettings(site).model } else { - wooCommerceStore.getSiteSettings(site) - ?: wooCommerceStore.fetchSiteGeneralSettings(site).model + wooCommerceStore.getSiteSettings(site) ?: wooCommerceStore.fetchSiteGeneralSettings(site).model } - if (siteSettings == null) { - return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.SiteSettingsUnavailable - ) - } - - return if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { + private fun fallbackUnknownOrCache(cachedPositive: Boolean): WooPosLaunchability = + if (cachedPositive) { WooPosLaunchability.Launchable } else { - WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency - ) + WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache) } - } private fun isCountryAndCurrencySupported(countryCode: String, currency: String) = SUPPORTED_COUNTRY_CURRENCY_PAIRS.any { @@ -117,5 +144,6 @@ sealed class WooPosLaunchability { FeatureSwitchDisabled, UnsupportedCurrency, NoSiteSelected, + UnknownNoPositiveCache } } From 2e0b4b9beab6937cec810f416182a168618e4cc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 09:39:34 +0200 Subject: [PATCH 13/27] Reuse string for our new case --- .../ui/woopos/eligibility/WooPosEligibilityViewModel.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/eligibility/WooPosEligibilityViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/eligibility/WooPosEligibilityViewModel.kt index efd105f7a2e..e7a525320bc 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/eligibility/WooPosEligibilityViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/eligibility/WooPosEligibilityViewModel.kt @@ -96,7 +96,8 @@ class WooPosEligibilityViewModel @Inject constructor( resourceProvider.getString(R.string.woopos_eligibility_reason_unsupported_currency_generic) } } - WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected -> + WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected, + WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache -> resourceProvider.getString(R.string.woopos_eligibility_reason_check_connection) } } From 5f910a95c6e469087480c8a23473276778ea5189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 10:03:27 +0200 Subject: [PATCH 14/27] Update unit tests --- .../ui/woopos/WooPOSIsRemotelyEnabledTest.kt | 37 +++++++------- .../tab/WooPosCanBeLaunchedInTabTest.kt | 45 ++++++++++++----- .../tab/WooPosTabShouldBeVisibleTest.kt | 48 ++++++++++++------- 3 files changed, 84 insertions(+), 46 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt index 20d39423426..b6e9f1b5ec7 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos import com.google.gson.Gson import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.common.util.WooPosCouldNotDetermineValueException import com.woocommerce.android.util.WCSSRModelCachingFetcher import kotlinx.coroutines.test.runTest import org.junit.Before @@ -30,65 +31,67 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given feature enabled remotely when invoked then returns true`() = runTest { - // GIVEN + fun `given feature enabled remotely when invoked then returns success true`() = runTest { val jsonSettings = """{"enabled_features": ["point_of_sale", "other_feature"]}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) whenever(cacheResult.model).thenReturn(ssrModel) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - // WHEN val result = sut.invoke() - // THEN - assertTrue(result) + assertTrue(result.isSuccess) + assertTrue(result.getOrThrow()) } @Test - fun `given feature not in list when invoked then returns false`() = runTest { + fun `given feature not in list when invoked then returns success false`() = runTest { val jsonSettings = """{"enabled_features": ["something_else"]}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) whenever(cacheResult.model).thenReturn(ssrModel) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val result = sut.invoke() + val r = sut.invoke() - assertFalse(result) + assertTrue(r.isSuccess) + assertFalse(r.getOrThrow()) } @Test - fun `given empty feature list when invoked then returns false`() = runTest { + fun `given empty feature list when invoked then returns success false`() = runTest { val jsonSettings = """{"enabled_features": []}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) whenever(cacheResult.model).thenReturn(ssrModel) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val result = sut.invoke() + val r = sut.invoke() - assertFalse(result) + assertTrue(r.isSuccess) + assertFalse(r.getOrThrow()) } @Test - fun `given null result when invoked then returns false`() = runTest { + fun `given null result when invoked then returns failure unknown`() = runTest { whenever(cacheResult.isError).thenReturn(false) whenever(cacheResult.model).thenReturn(null) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val result = sut.invoke() + val r = sut.invoke() - assertFalse(result) + assertTrue(r.isFailure) + assertTrue(r.exceptionOrNull() is WooPosCouldNotDetermineValueException) } @Test - fun `given error result when invoked then returns false`() = runTest { + fun `given error result when invoked then returns failure unknown`() = runTest { whenever(cacheResult.isError).thenReturn(true) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val result = sut.invoke() + val r = sut.invoke() - assertFalse(result) + assertTrue(r.isFailure) + assertTrue(r.exceptionOrNull() is WooPosCouldNotDetermineValueException) } } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt index 0487b1fde70..02d570b0b1f 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt @@ -1,5 +1,6 @@ package com.woocommerce.android.ui.woopos.tab +import com.woocommerce.android.AppPrefs import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.WooPOSIsRemotelyEnabled import com.woocommerce.android.ui.woopos.tab.WooPosLaunchability.Launchable @@ -11,6 +12,7 @@ import com.woocommerce.android.viewmodel.BaseUnitTest import kotlinx.coroutines.ExperimentalCoroutinesApi import org.junit.Before import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.wordpress.android.fluxc.model.LocalOrRemoteId @@ -24,6 +26,7 @@ import kotlin.test.assertEquals @OptIn(ExperimentalCoroutinesApi::class) class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { + private val appPrefs: AppPrefs = mock() private val selectedSite: SelectedSite = mock() private val wooCommerceStore: WooCommerceStore = mock() private val isRemotelyEnabled: WooPOSIsRemotelyEnabled = mock() @@ -31,19 +34,22 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { private val fetchWooCoreVersion: FetchActiveWCPluginVersion = mock() private lateinit var sut: WooPosCanBeLaunchedInTab + private lateinit var siteModel: SiteModel @Before fun setup() = testBlocking { - val siteModel = SiteModel().also { it.id = 1 } + siteModel = SiteModel().also { it.id = 1 } whenever(selectedSite.getOrNull()).thenReturn(siteModel) whenever(getWooCoreVersion()).thenReturn("10.0.0") whenever(fetchWooCoreVersion()).thenReturn("10.0.0") val siteSettings = buildSiteSettings() whenever(wooCommerceStore.getSiteSettings(siteModel)).thenReturn(siteSettings) whenever(wooCommerceStore.fetchSiteGeneralSettings(siteModel)).thenReturn(WooResult(siteSettings)) - whenever(isRemotelyEnabled.invoke(any())).thenReturn(true) + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.success(true)) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) sut = WooPosCanBeLaunchedInTab( + appPrefs = appPrefs, selectedSite = selectedSite, getWooCoreCachedVersion = getWooCoreVersion, fetchWooCoreVersion = fetchWooCoreVersion, @@ -68,7 +74,7 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { @Test fun `given unsupported WooCommerce version, when invoked, then return NotLaunchable with UnsupportedWooCommerceVersion`() = testBlocking { - whenever(getWooCoreVersion()).thenReturn("9.5.0") // lower than 9.6.0 + whenever(getWooCoreVersion()).thenReturn("9.5.0") val result = sut() assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedWooCommerceVersion), result) } @@ -76,17 +82,19 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { @Test fun `given feature switch supported but remotely disabled, when invoked, then return NotLaunchable with FeatureSwitchDisabled`() = testBlocking { whenever(getWooCoreVersion()).thenReturn("10.0.0") - whenever(isRemotelyEnabled.invoke(any())).thenReturn(false) + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.success(false)) val result = sut() assertEquals(NotLaunchable(NonLaunchabilityReason.FeatureSwitchDisabled), result) } @Test - fun `given null site settings, when invoked, then return NotLaunchable with SiteSettingsUnavailable`() = testBlocking { + fun `given null site settings and no cached positive, when invoked, then return NotLaunchable UnknownNoPositiveCache`() = testBlocking { whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) + val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.SiteSettingsUnavailable), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) } @Test @@ -156,28 +164,39 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { } @Test - fun `given forceRefresh true and fetchWooCoreVersion returns null, when invoked, then return NotLaunchable with WooCommercePluginNotFound`() = testBlocking { + fun `given forceRefresh true and fetchWooCoreVersion returns null with no cached positive, when invoked, then NotLaunchable WooCommercePluginNotFound`() = testBlocking { whenever(fetchWooCoreVersion()).thenReturn(null) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) val result = sut(forceRefresh = true) assertEquals(NotLaunchable(NonLaunchabilityReason.WooCommercePluginNotFound), result) } @Test - fun `given forceRefresh true and fetched site settings is null, when invoked, then return NotLaunchable with SiteSettingsUnavailable`() = testBlocking { + fun `given forceRefresh true and fetchWooCoreVersion returns null with cached positive, when invoked, then Launchable`() = testBlocking { + whenever(fetchWooCoreVersion()).thenReturn(null) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(true) + + val result = sut(forceRefresh = true) + assertEquals(Launchable, result) + } + + @Test + fun `given forceRefresh true and fetched site settings is null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache`() = testBlocking { whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) val result = sut(forceRefresh = true) - assertEquals(NotLaunchable(NonLaunchabilityReason.SiteSettingsUnavailable), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) } @Test - fun `given forceRefresh true and fetched site settings with unsupported currency, when invoked, then return NotLaunchable with UnsupportedCurrency`() = testBlocking { - val fetchedSettings = buildSiteSettings(currencyCode = "eur") - whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) + fun `given forceRefresh true and fetched site settings is null with cached positive, when invoked, then Launchable`() = testBlocking { + whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(true) val result = sut(forceRefresh = true) - assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + assertEquals(Launchable, result) } private fun buildSiteSettings( diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt index 1dedeb01d8a..5a3c28db008 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt @@ -2,6 +2,7 @@ package com.woocommerce.android.ui.woopos.tab import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.woopos.WooPosIsScreenSizeAllowed +import com.woocommerce.android.ui.woopos.common.util.WooPosCouldNotDetermineValueException import com.woocommerce.android.util.IsRemoteFeatureFlagEnabled import com.woocommerce.android.util.RemoteFeatureFlag.WOO_POS import com.woocommerce.android.viewmodel.BaseUnitTest @@ -29,10 +30,11 @@ class WooPosTabShouldBeVisibleTest : BaseUnitTest() { private val isRemoteFeatureFlagEnabled: IsRemoteFeatureFlagEnabled = mock() private lateinit var sut: WooPosTabShouldBeVisible + private lateinit var siteModel: SiteModel @Before fun setup() = testBlocking { - val siteModel = SiteModel().also { it.id = 1 } + siteModel = SiteModel().also { it.id = 1 } whenever(selectedSite.getOrNull()).thenReturn(siteModel) whenever(isScreenSizeAllowed()).thenReturn(true) whenever(isRemoteFeatureFlagEnabled(WOO_POS)).thenReturn(true) @@ -41,41 +43,51 @@ class WooPosTabShouldBeVisibleTest : BaseUnitTest() { sut = WooPosTabShouldBeVisible( selectedSite = selectedSite, - wooCommerceStore = wooCommerceStore, isScreenSizeAllowed = isScreenSizeAllowed, + wooCommerceStore = wooCommerceStore, isRemoteFeatureFlagEnabled = isRemoteFeatureFlagEnabled, wooPosLog = mock() ) } @Test - fun `given feature flag enabled and supported country, when invoked, then return true`() = testBlocking { - assertTrue(sut()) + fun `given feature flag enabled and supported country, when invoked, then return success true`() = testBlocking { + val r = sut() + assertTrue(r.isSuccess) + assertTrue(r.getOrThrow()) } @Test - fun `given feature flag disabled, when invoked, then return false`() = testBlocking { + fun `given feature flag disabled, when invoked, then return success true (tab visible anyway)`() = testBlocking { whenever(isRemoteFeatureFlagEnabled(WOO_POS)).thenReturn(false) - assertFalse(sut()) + val r = sut() + assertTrue(r.isSuccess) + assertTrue(r.getOrThrow()) } @Test - fun `given screen size not allowed, when invoked, then return false`() = testBlocking { + fun `given screen size not allowed, when invoked, then return success false`() = testBlocking { whenever(isScreenSizeAllowed()).thenReturn(false) - assertFalse(sut()) + val r = sut() + assertTrue(r.isSuccess) + assertFalse(r.getOrThrow()) } @Test - fun `given null site, when invoked, then return false`() = testBlocking { + fun `given null site, when invoked, then return failure unknown`() = testBlocking { whenever(selectedSite.getOrNull()).thenReturn(null) - assertFalse(sut()) + val r = sut() + assertTrue(r.isFailure) + assertTrue(r.exceptionOrNull() is WooPosCouldNotDetermineValueException) } @Test - fun `given unsupported country, when invoked, then return false`() = testBlocking { + fun `given unsupported country, when invoked, then return success false`() = testBlocking { val siteSettings = buildSiteSettings(countryCode = "ca") whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(siteSettings)) - assertFalse(sut()) + val r = sut() + assertTrue(r.isSuccess) + assertFalse(r.getOrThrow()) } @Test @@ -89,19 +101,23 @@ class WooPosTabShouldBeVisibleTest : BaseUnitTest() { } @Test - fun `given fetched settings with supported country, when invoked, then return true`() = testBlocking { + fun `given fetched settings with supported country, when invoked, then return success true`() = testBlocking { val fetchedSettings = buildSiteSettings(countryCode = "gb") whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) - assertTrue(sut()) + val r = sut() + assertTrue(r.isSuccess) + assertTrue(r.getOrThrow()) } @Test - fun `given fetched settings with unsupported country, when invoked, then return false`() = testBlocking { + fun `given fetched settings with unsupported country, when invoked, then return success false`() = testBlocking { val fetchedSettings = buildSiteSettings(countryCode = "ca") whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) - assertFalse(sut()) + val r = sut() + assertTrue(r.isSuccess) + assertFalse(r.getOrThrow()) } private fun buildSiteSettings(countryCode: String = "us") = From 70ee69acfff974493f549c64fe6fd72d9733fc8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 10:12:24 +0200 Subject: [PATCH 15/27] Detekt --- .../android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt index b6e9f1b5ec7..8bdd4ad9be7 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt @@ -31,7 +31,7 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given feature enabled remotely when invoked then returns success true`() = runTest { + fun `given feature enabled remotely, when invoked, then returns success true`() = runTest { val jsonSettings = """{"enabled_features": ["point_of_sale", "other_feature"]}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) @@ -45,7 +45,7 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given feature not in list when invoked then returns success false`() = runTest { + fun `given feature not in list, when invoked, then returns success false`() = runTest { val jsonSettings = """{"enabled_features": ["something_else"]}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) @@ -59,7 +59,7 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given empty feature list when invoked then returns success false`() = runTest { + fun `given empty feature list, when invoked, then returns success false`() = runTest { val jsonSettings = """{"enabled_features": []}""" whenever(ssrModel.settings).thenReturn(jsonSettings) whenever(cacheResult.isError).thenReturn(false) @@ -73,7 +73,7 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given null result when invoked then returns failure unknown`() = runTest { + fun `given null result, when invoked, then returns failure unknown`() = runTest { whenever(cacheResult.isError).thenReturn(false) whenever(cacheResult.model).thenReturn(null) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) @@ -85,7 +85,7 @@ class WooPOSIsRemotelyEnabledTest { } @Test - fun `given error result when invoked then returns failure unknown`() = runTest { + fun `given error result, when invoked, then returns failure unknown`() = runTest { whenever(cacheResult.isError).thenReturn(true) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) From 5c5b473eef229f38681590907246a30e8bc8e780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 12:52:46 +0200 Subject: [PATCH 16/27] Store the launchable positive value in cache and add tests --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 7 ++++--- .../woopos/tab/WooPosCanBeLaunchedInTabTest.kt | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index 94070494abb..261161a8e05 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -55,10 +55,11 @@ class WooPosCanBeLaunchedInTab @Inject constructor( val siteSettings = resolveSiteSettings(site, forceRefresh) ?: return fallbackUnknownOrCache(cachedPositive) - return if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { - WooPosLaunchability.Launchable + if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { + appPrefs.setPOSLaunchableForSite(site.id) + return WooPosLaunchability.Launchable } else { - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency) + return WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency) } } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt index 02d570b0b1f..99811d275ca 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt @@ -14,6 +14,8 @@ import org.junit.Before import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.fluxc.model.LocalOrRemoteId import org.wordpress.android.fluxc.model.SiteModel @@ -199,6 +201,22 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { assertEquals(Launchable, result) } + @Test + fun `given supported country and currency from cache, when invoked, then sets POS launchable for site`() = testBlocking { + val result = sut() + assertEquals(Launchable, result) + + verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) + } + + @Test + fun `given forceRefresh true with valid data, when invoked, then sets POS launchable for site`() = testBlocking { + val result = sut(forceRefresh = true) + assertEquals(Launchable, result) + + verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) + } + private fun buildSiteSettings( countryCode: String = "US", currencyCode: String = "USD" From 20139a71511969acad3764840cad80b85b2b11e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 13 Aug 2025 16:06:43 +0200 Subject: [PATCH 17/27] Fix wrong value --- .../android/ui/woopos/tab/WooPosTabShouldBeVisible.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt index c266802ed9c..30538e8e6be 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisible.kt @@ -23,7 +23,7 @@ class WooPosTabShouldBeVisible @Inject constructor( ?: return@withContext Result.failure(WooPosCouldNotDetermineValueException()) if (!isRemoteFeatureFlagEnabled(WOO_POS)) { - return@withContext Result.success(true).also { + return@withContext Result.success(false).also { wooPosLog.i("POS Tab Not visible reason: Remote feature flag is disabled") } } From 46d5b690400b77f0b8d3ddd28a8cc7299cf91b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 14 Aug 2025 14:45:17 +0200 Subject: [PATCH 18/27] Refactor to a more meaningful naming --- .../ui/woopos/WooPOSIsRemotelyEnabledTest.kt | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt index 8bdd4ad9be7..b430028c1e2 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/WooPOSIsRemotelyEnabledTest.kt @@ -52,10 +52,10 @@ class WooPOSIsRemotelyEnabledTest { whenever(cacheResult.model).thenReturn(ssrModel) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val r = sut.invoke() + val result = sut.invoke() - assertTrue(r.isSuccess) - assertFalse(r.getOrThrow()) + assertTrue(result.isSuccess) + assertFalse(result.getOrThrow()) } @Test @@ -66,10 +66,10 @@ class WooPOSIsRemotelyEnabledTest { whenever(cacheResult.model).thenReturn(ssrModel) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val r = sut.invoke() + val result = sut.invoke() - assertTrue(r.isSuccess) - assertFalse(r.getOrThrow()) + assertTrue(result.isSuccess) + assertFalse(result.getOrThrow()) } @Test @@ -78,10 +78,10 @@ class WooPOSIsRemotelyEnabledTest { whenever(cacheResult.model).thenReturn(null) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val r = sut.invoke() + val result = sut.invoke() - assertTrue(r.isFailure) - assertTrue(r.exceptionOrNull() is WooPosCouldNotDetermineValueException) + assertTrue(result.isFailure) + assertTrue(result.exceptionOrNull() is WooPosCouldNotDetermineValueException) } @Test @@ -89,9 +89,9 @@ class WooPOSIsRemotelyEnabledTest { whenever(cacheResult.isError).thenReturn(true) whenever(fetcher.load(siteModel)).thenReturn(cacheResult) - val r = sut.invoke() + val result = sut.invoke() - assertTrue(r.isFailure) - assertTrue(r.exceptionOrNull() is WooPosCouldNotDetermineValueException) + assertTrue(result.isFailure) + assertTrue(result.exceptionOrNull() is WooPosCouldNotDetermineValueException) } } From 798c607502b54fb9db15fe13ba4428e18a5a6a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 14 Aug 2025 15:03:02 +0200 Subject: [PATCH 19/27] Add clear values and apply it in the WooPosTabController --- .../src/main/kotlin/com/woocommerce/android/AppPrefs.kt | 8 ++++++++ .../android/ui/woopos/tab/WooPosTabController.kt | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt index b0497ed22b7..7230f5c253e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/AppPrefs.kt @@ -1199,6 +1199,10 @@ object AppPrefs { ) } + fun clearPOSTabVisibilityForSite(siteId: Int) { + remove(PrefKeyString("${UndeletablePrefKey.POS_TAB_VISIBILITY}:$siteId")) + } + fun setPOSLaunchableForSite(siteId: Int) { setBoolean( key = PrefKeyString("${UndeletablePrefKey.POS_LAUNCHABLE}:$siteId"), @@ -1213,6 +1217,10 @@ object AppPrefs { ) } + fun clearPOSLaunchableForSite(siteId: Int) { + remove(PrefKeyString("${UndeletablePrefKey.POS_LAUNCHABLE}:$siteId")) + } + /** * Remove all user and site-related preferences. */ diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt index 80363a49d10..4d9a7fa6887 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt @@ -70,7 +70,11 @@ class WooPosTabController @Inject constructor( result.onSuccess { tabShouldBeVisible -> setPOSTabVisibility(tabShouldBeVisible) - if (tabShouldBeVisible) appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId()) + if (tabShouldBeVisible) { + appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId()) + } else { + appPrefs.clearPOSTabVisibilityForSite(selectedSite.getSelectedSiteId()) + } } result.onFailure { error -> From 60680c898da34a294203c1e016e05d92d64e8376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 14 Aug 2025 15:11:18 +0200 Subject: [PATCH 20/27] Refactor to make it more explicit and remove default else cases --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index 261161a8e05..d0b590bd2dc 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -87,15 +87,27 @@ class WooPosCanBeLaunchedInTab @Inject constructor( ): WooPosLaunchability? { if (!isFeatureSwitchSupported(wooCoreVersion)) return null val remote = isRemotelyEnabled(forceRefresh) - return when { - remote.isSuccess && !remote.getOrThrow() -> - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled) - remote.isFailure && cachedPositive -> - WooPosLaunchability.Launchable - remote.isFailure && !cachedPositive -> - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache) - else -> null - } + + return remote.fold( + onSuccess = { enabled -> + if (enabled) { + null + } else { + WooPosLaunchability.NotLaunchable( + WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled + ) + } + }, + onFailure = { + if (cachedPositive) { + WooPosLaunchability.Launchable + } else { + WooPosLaunchability.NotLaunchable( + WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache + ) + } + } + ) } private suspend fun resolveSiteSettings(site: SiteModel, forceRefresh: Boolean): Settings? = From 2bed5df5445a0e180ca90e9fa0188928fc3e2b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 14 Aug 2025 15:55:44 +0200 Subject: [PATCH 21/27] Reactor for clarity. Do not stop right away if a request fails. --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 76 +++++++++---------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index d0b590bd2dc..53f1a9fc78b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -46,11 +46,12 @@ class WooPosCanBeLaunchedInTab @Inject constructor( val cachedPositive = appPrefs.isPOSLaunchableForSite(site.id) - val wooCoreVersion = resolveWooCoreVersion(forceRefresh) - ?: return fallbackForMissingWooCore(cachedPositive) - - validateWooCoreVersion(wooCoreVersion)?.let { return it } - evaluateFeatureSwitch(wooCoreVersion, forceRefresh, cachedPositive)?.let { return it } + getWooCoreVersion(forceRefresh)?.let { + getNonLaunchabilityReasonFromWooCoreVersion(it)?.let { return WooPosLaunchability.NotLaunchable(it) } + getNonLaunchabilityReasonFromFeatureSwitch(it, forceRefresh, cachedPositive)?.let { + return WooPosLaunchability.NotLaunchable(it) + } + } val siteSettings = resolveSiteSettings(site, forceRefresh) ?: return fallbackUnknownOrCache(cachedPositive) @@ -59,55 +60,50 @@ class WooPosCanBeLaunchedInTab @Inject constructor( appPrefs.setPOSLaunchableForSite(site.id) return WooPosLaunchability.Launchable } else { + appPrefs.clearPOSLaunchableForSite(site.id) return WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency) } } - private suspend fun resolveWooCoreVersion(forceRefresh: Boolean): String? = + private suspend fun getWooCoreVersion(forceRefresh: Boolean): String? = if (forceRefresh) fetchWooCoreVersion() else getWooCoreCachedVersion() - private fun fallbackForMissingWooCore(cachedPositive: Boolean): WooPosLaunchability = - if (cachedPositive) { - WooPosLaunchability.Launchable - } else { - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.WooCommercePluginNotFound) - } - - private fun validateWooCoreVersion(wooCoreVersion: String): WooPosLaunchability? = + /** + * Checks the WooCommerce core version to see if it prevents POS from being launchable. + * Returns the NonLaunchabilityReason if it does, or null if the version is supported. + */ + private fun getNonLaunchabilityReasonFromWooCoreVersion( + wooCoreVersion: String + ): WooPosLaunchability.NonLaunchabilityReason? = if (!isWooCoreSupportsOrderAutoDraftsAndExtraPaymentsProps(wooCoreVersion)) { - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedWooCommerceVersion) + WooPosLaunchability.NonLaunchabilityReason.UnsupportedWooCommerceVersion } else { null } - private suspend fun evaluateFeatureSwitch( + /** + * Checks the feature switch to see if it prevents POS from being launchable. + * Returns the NonLaunchabilityReason if it does, or null if POS might still be launchable. + */ + private suspend fun getNonLaunchabilityReasonFromFeatureSwitch( wooCoreVersion: String, - forceRefresh: Boolean, - cachedPositive: Boolean - ): WooPosLaunchability? { - if (!isFeatureSwitchSupported(wooCoreVersion)) return null - val remote = isRemotelyEnabled(forceRefresh) - - return remote.fold( - onSuccess = { enabled -> - if (enabled) { - null - } else { - WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled - ) - } - }, - onFailure = { - if (cachedPositive) { - WooPosLaunchability.Launchable - } else { - WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache - ) + forceRemoteRefresh: Boolean, + hasCachedLaunchableState: Boolean + ): WooPosLaunchability.NonLaunchabilityReason? { + val result = + if (!isFeatureSwitchSupported(wooCoreVersion)) { + null + } else { + val enabled = isRemotelyEnabled(forceRemoteRefresh).getOrNull() + when { + enabled == true -> null + enabled == false -> WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled + hasCachedLaunchableState -> null + else -> WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache } } - ) + + return result } private suspend fun resolveSiteSettings(site: SiteModel, forceRefresh: Boolean): Settings? = From b57f42f3191593268da9c77c382f93a882873d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Fri, 15 Aug 2025 11:48:10 +0200 Subject: [PATCH 22/27] Improve logic --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 13 ++++++++++--- .../ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index 53f1a9fc78b..e3786869b4e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -46,11 +46,18 @@ class WooPosCanBeLaunchedInTab @Inject constructor( val cachedPositive = appPrefs.isPOSLaunchableForSite(site.id) - getWooCoreVersion(forceRefresh)?.let { - getNonLaunchabilityReasonFromWooCoreVersion(it)?.let { return WooPosLaunchability.NotLaunchable(it) } - getNonLaunchabilityReasonFromFeatureSwitch(it, forceRefresh, cachedPositive)?.let { + val wooCoreVersion = getWooCoreVersion(forceRefresh) + if (wooCoreVersion != null) { + getNonLaunchabilityReasonFromWooCoreVersion(wooCoreVersion)?.let { return WooPosLaunchability.NotLaunchable(it) } + getNonLaunchabilityReasonFromFeatureSwitch(wooCoreVersion, forceRefresh, cachedPositive)?.let { + return WooPosLaunchability.NotLaunchable(it) + } + } else if (!cachedPositive) { + return WooPosLaunchability.NotLaunchable( + WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache + ) } val siteSettings = resolveSiteSettings(site, forceRefresh) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt index 99811d275ca..55c86c1f0e0 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt @@ -166,12 +166,12 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { } @Test - fun `given forceRefresh true and fetchWooCoreVersion returns null with no cached positive, when invoked, then NotLaunchable WooCommercePluginNotFound`() = testBlocking { + fun `given forceRefresh true and fetchWooCoreVersion returns null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache`() = testBlocking { whenever(fetchWooCoreVersion()).thenReturn(null) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) val result = sut(forceRefresh = true) - assertEquals(NotLaunchable(NonLaunchabilityReason.WooCommercePluginNotFound), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) } @Test From c750ee6e4a39e5414c4253da1764ca14c7eeb29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Fri, 15 Aug 2025 14:55:44 +0200 Subject: [PATCH 23/27] Refactor to clear cache when necessary and better readability --- .../ui/woopos/tab/WooPosCanBeLaunchedInTab.kt | 77 ++++++++++++------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt index e3786869b4e..538ca372e37 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTab.kt @@ -42,36 +42,68 @@ class WooPosCanBeLaunchedInTab @Inject constructor( @Suppress("ReturnCount") private suspend fun checkLaunchability(forceRefresh: Boolean = false): WooPosLaunchability { val site = selectedSite.getOrNull() - ?: return WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected) + ?: return WooPosLaunchability.NotLaunchable( + reason = WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected + ) val cachedPositive = appPrefs.isPOSLaunchableForSite(site.id) - val wooCoreVersion = getWooCoreVersion(forceRefresh) - if (wooCoreVersion != null) { - getNonLaunchabilityReasonFromWooCoreVersion(wooCoreVersion)?.let { - return WooPosLaunchability.NotLaunchable(it) - } - getNonLaunchabilityReasonFromFeatureSwitch(wooCoreVersion, forceRefresh, cachedPositive)?.let { - return WooPosLaunchability.NotLaunchable(it) - } - } else if (!cachedPositive) { - return WooPosLaunchability.NotLaunchable( - WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache - ) + getNonLaunchabilityReasonFromVersionAndFeatureSwitch(forceRefresh, cachedPositive)?.let { + return prepareNotLaunchableStateWithCacheUpdate(site.id, it) + } + + getNonLaunchabilityReasonFromSiteSettingsAndCurrency(site, forceRefresh, cachedPositive)?.let { + return prepareNotLaunchableStateWithCacheUpdate(site.id, it) } + appPrefs.setPOSLaunchableForSite(site.id) + return WooPosLaunchability.Launchable + } + + private fun prepareNotLaunchableStateWithCacheUpdate( + siteId: Int, + reason: WooPosLaunchability.NonLaunchabilityReason + ): WooPosLaunchability.NotLaunchable { + if (reason != WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache) { + appPrefs.clearPOSLaunchableForSite(siteId) + } + + return WooPosLaunchability.NotLaunchable(reason) + } + + private suspend fun getNonLaunchabilityReasonFromVersionAndFeatureSwitch( + forceRefresh: Boolean, + cachedPositive: Boolean + ): WooPosLaunchability.NonLaunchabilityReason? { + val wooCoreVersion = getWooCoreVersion(forceRefresh) + ?: return reasonIfNoPositiveCache(cachedPositive) + + return getNonLaunchabilityReasonFromWooCoreVersion(wooCoreVersion) + ?: getNonLaunchabilityReasonFromFeatureSwitch(wooCoreVersion, forceRefresh, cachedPositive) + } + + private suspend fun getNonLaunchabilityReasonFromSiteSettingsAndCurrency( + site: SiteModel, + forceRefresh: Boolean, + cachedPositive: Boolean + ): WooPosLaunchability.NonLaunchabilityReason? { val siteSettings = resolveSiteSettings(site, forceRefresh) - ?: return fallbackUnknownOrCache(cachedPositive) + ?: return reasonIfNoPositiveCache(cachedPositive) - if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { - appPrefs.setPOSLaunchableForSite(site.id) - return WooPosLaunchability.Launchable + return if (!isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) { + WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency } else { - appPrefs.clearPOSLaunchableForSite(site.id) - return WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency) + null } } + private fun reasonIfNoPositiveCache(hasCachedPositive: Boolean): WooPosLaunchability.NonLaunchabilityReason? = + if (hasCachedPositive) { + null + } else { + WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache + } + private suspend fun getWooCoreVersion(forceRefresh: Boolean): String? = if (forceRefresh) fetchWooCoreVersion() else getWooCoreCachedVersion() @@ -120,13 +152,6 @@ class WooPosCanBeLaunchedInTab @Inject constructor( wooCommerceStore.getSiteSettings(site) ?: wooCommerceStore.fetchSiteGeneralSettings(site).model } - private fun fallbackUnknownOrCache(cachedPositive: Boolean): WooPosLaunchability = - if (cachedPositive) { - WooPosLaunchability.Launchable - } else { - WooPosLaunchability.NotLaunchable(WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache) - } - private fun isCountryAndCurrencySupported(countryCode: String, currency: String) = SUPPORTED_COUNTRY_CURRENCY_PAIRS.any { it.first.equals(countryCode, true) && it.second.equals(currency, true) From 1ac9b92b30fa0b47e756b4759cdbc7663678a5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Fri, 15 Aug 2025 15:04:47 +0200 Subject: [PATCH 24/27] Update tests --- .../tab/WooPosCanBeLaunchedInTabTest.kt | 164 +++++++++++------- 1 file changed, 100 insertions(+), 64 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt index 55c86c1f0e0..3b7c4d3e21e 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosCanBeLaunchedInTabTest.kt @@ -42,11 +42,15 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { fun setup() = testBlocking { siteModel = SiteModel().also { it.id = 1 } whenever(selectedSite.getOrNull()).thenReturn(siteModel) + + // Default: WC 10.0.0, settings available and supported, remote enabled, no cached positive whenever(getWooCoreVersion()).thenReturn("10.0.0") whenever(fetchWooCoreVersion()).thenReturn("10.0.0") + val siteSettings = buildSiteSettings() whenever(wooCommerceStore.getSiteSettings(siteModel)).thenReturn(siteSettings) whenever(wooCommerceStore.fetchSiteGeneralSettings(siteModel)).thenReturn(WooResult(siteSettings)) + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.success(true)) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) @@ -61,161 +65,193 @@ class WooPosCanBeLaunchedInTabTest : BaseUnitTest() { ) } + // --- Happy paths --- + @Test - fun `given valid conditions, when invoked, then return Launchable`() = testBlocking { + fun `given valid conditions, when invoked, then return Launchable and set cache`() = testBlocking { val result = sut() assertEquals(Launchable, result) + verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) } @Test - fun `given no site selected, when invoked, then return NotLaunchable with NoSiteSelected`() = testBlocking { - whenever(selectedSite.getOrNull()).thenReturn(null) + fun `given uk country and pounds, when invoked, then return Launchable`() = testBlocking { + val siteSettings = buildSiteSettings(countryCode = "GB", currencyCode = "GBP") + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.NoSiteSelected), result) + assertEquals(Launchable, result) } @Test - fun `given unsupported WooCommerce version, when invoked, then return NotLaunchable with UnsupportedWooCommerceVersion`() = testBlocking { - whenever(getWooCoreVersion()).thenReturn("9.5.0") + fun `given us country and dollars, when invoked, then return Launchable`() = testBlocking { + val siteSettings = buildSiteSettings(countryCode = "US", currencyCode = "USD") + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedWooCommerceVersion), result) + assertEquals(Launchable, result) } @Test - fun `given feature switch supported but remotely disabled, when invoked, then return NotLaunchable with FeatureSwitchDisabled`() = testBlocking { - whenever(getWooCoreVersion()).thenReturn("10.0.0") - whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.success(false)) + fun `given site settings missing but fetched successfully, when invoked, then return Launchable`() = testBlocking { + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) + val fetchedSettings = buildSiteSettings(currencyCode = "usd") + whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.FeatureSwitchDisabled), result) + assertEquals(Launchable, result) } - @Test - fun `given null site settings and no cached positive, when invoked, then return NotLaunchable UnknownNoPositiveCache`() = testBlocking { - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) - whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) - whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) + // --- No site --- + @Test + fun `given no site selected, when invoked, then NotLaunchable NoSiteSelected and no prefs touched`() = testBlocking { + whenever(selectedSite.getOrNull()).thenReturn(null) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.NoSiteSelected), result) + verify(appPrefs, times(0)).setPOSLaunchableForSite(any()) + verify(appPrefs, times(0)).clearPOSLaunchableForSite(any()) } + // --- Version checks --- + @Test - fun `given unsupported currency, when invoked, then return NotLaunchable with UnsupportedCurrency`() = testBlocking { - val siteSettings = buildSiteSettings(currencyCode = "eur") - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + fun `given unsupported WooCommerce version, when invoked, then NotLaunchable UnsupportedWooCommerceVersion and clears cache`() = testBlocking { + whenever(getWooCoreVersion()).thenReturn("9.5.0") val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedWooCommerceVersion), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) } + // Feature-switch unsupported (version >= 9_6_0 but switch threshold not met) @Test - fun `given uk country and pounds, when invoked, then return Launchable`() = testBlocking { - val siteSettings = buildSiteSettings(countryCode = "GB", currencyCode = "GBP") - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + fun `given WC 9_6_0 (switch unsupported) and supported settings, when invoked, then Launchable`() = testBlocking { + whenever(getWooCoreVersion()).thenReturn("9.6.0") val result = sut() assertEquals(Launchable, result) } @Test - fun `given us country and dollars, when invoked, then return Launchable`() = testBlocking { - val siteSettings = buildSiteSettings(countryCode = "US", currencyCode = "USD") - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + fun `given WC 9_6_0 (switch unsupported) and unsupported currency, when invoked, then NotLaunchable UnsupportedCurrency and clears cache`() = testBlocking { + whenever(getWooCoreVersion()).thenReturn("9.6.0") + val bad = buildSiteSettings(currencyCode = "EUR") + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(bad) val result = sut() - assertEquals(Launchable, result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) } + // --- Feature switch checks --- + @Test - fun `given uk country and usd, when invoked, then return Not Launchable`() = testBlocking { - val siteSettings = buildSiteSettings(countryCode = "GB", currencyCode = "USD") - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + fun `given feature switch supported but remotely disabled, when invoked, then NotLaunchable FeatureSwitchDisabled and clears cache`() = testBlocking { + whenever(getWooCoreVersion()).thenReturn("10.0.0") + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.success(false)) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + assertEquals(NotLaunchable(NonLaunchabilityReason.FeatureSwitchDisabled), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) } @Test - fun `given us country and pounds, when invoked, then return Not Launchable`() = testBlocking { - val siteSettings = buildSiteSettings(countryCode = "US", currencyCode = "GBP") - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + fun `given remote check failure and cached positive, when invoked, then Launchable`() = testBlocking { + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.failure(RuntimeException("boom"))) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(true) val result = sut() - assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + assertEquals(Launchable, result) } @Test - fun `given site settings missing but fetched successfully, when invoked, then return Launchable`() = testBlocking { - whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) - val fetchedSettings = buildSiteSettings(currencyCode = "usd") - whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) + fun `given remote check failure and no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache and does not clear`() = testBlocking { + whenever(isRemotelyEnabled.invoke(any())).thenReturn(Result.failure(RuntimeException("boom"))) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) + val result = sut() + assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) + verify(appPrefs, times(0)).clearPOSLaunchableForSite(any()) + } + + // --- Site settings fallback --- + @Test + fun `given null site settings and no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache and does not clear`() = testBlocking { + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) + whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) + whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) val result = sut() - assertEquals(Launchable, result) + assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) + verify(appPrefs, times(0)).clearPOSLaunchableForSite(any()) } @Test - fun `given site settings missing and fetched with unsupported currency, when invoked, then return NotLaunchable with UnsupportedCurrency`() = testBlocking { + fun `given site settings missing and fetched with unsupported currency, when invoked, then NotLaunchable UnsupportedCurrency and clears cache`() = testBlocking { whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(null) val fetchedSettings = buildSiteSettings(currencyCode = "eur") whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(fetchedSettings)) + val result = sut() + assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) + } + + // --- Currency matrix --- + @Test + fun `given uk country and usd, when invoked, then NotLaunchable UnsupportedCurrency and clears cache`() = testBlocking { + val siteSettings = buildSiteSettings(countryCode = "GB", currencyCode = "USD") + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) val result = sut() assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) } @Test - fun `given forceRefresh true with valid data, when invoked, then return Launchable`() = testBlocking { + fun `given us country and pounds, when invoked, then NotLaunchable UnsupportedCurrency and clears cache`() = testBlocking { + val siteSettings = buildSiteSettings(countryCode = "US", currencyCode = "GBP") + whenever(wooCommerceStore.getSiteSettings(any())).thenReturn(siteSettings) + val result = sut() + assertEquals(NotLaunchable(NonLaunchabilityReason.UnsupportedCurrency), result) + verify(appPrefs, times(1)).clearPOSLaunchableForSite(eq(siteModel.id)) + } + + // --- Force refresh paths --- + + @Test + fun `given forceRefresh true with valid data, when invoked, then Launchable and set cache`() = testBlocking { val result = sut(forceRefresh = true) assertEquals(Launchable, result) + verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) } @Test - fun `given forceRefresh true and fetchWooCoreVersion returns null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache`() = testBlocking { + fun `given forceRefresh true and fetchWooCoreVersion returns null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache and does not clear`() = testBlocking { whenever(fetchWooCoreVersion()).thenReturn(null) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) - val result = sut(forceRefresh = true) assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) + verify(appPrefs, times(0)).clearPOSLaunchableForSite(any()) } @Test fun `given forceRefresh true and fetchWooCoreVersion returns null with cached positive, when invoked, then Launchable`() = testBlocking { whenever(fetchWooCoreVersion()).thenReturn(null) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(true) - val result = sut(forceRefresh = true) assertEquals(Launchable, result) } @Test - fun `given forceRefresh true and fetched site settings is null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache`() = testBlocking { + fun `given forceRefresh true and fetched site settings is null with no cached positive, when invoked, then NotLaunchable UnknownNoPositiveCache and does not clear`() = testBlocking { whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(false) - val result = sut(forceRefresh = true) assertEquals(NotLaunchable(NonLaunchabilityReason.UnknownNoPositiveCache), result) + verify(appPrefs, times(0)).clearPOSLaunchableForSite(any()) } @Test fun `given forceRefresh true and fetched site settings is null with cached positive, when invoked, then Launchable`() = testBlocking { whenever(wooCommerceStore.fetchSiteGeneralSettings(any())).thenReturn(WooResult(null)) whenever(appPrefs.isPOSLaunchableForSite(eq(siteModel.id))).thenReturn(true) - val result = sut(forceRefresh = true) assertEquals(Launchable, result) } - @Test - fun `given supported country and currency from cache, when invoked, then sets POS launchable for site`() = testBlocking { - val result = sut() - assertEquals(Launchable, result) - - verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) - } - - @Test - fun `given forceRefresh true with valid data, when invoked, then sets POS launchable for site`() = testBlocking { - val result = sut(forceRefresh = true) - assertEquals(Launchable, result) - - verify(appPrefs, times(1)).setPOSLaunchableForSite(eq(siteModel.id)) - } + // --- private fun buildSiteSettings( countryCode: String = "US", From c609905e623b543dd5494491b3ef7270cab328c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Mon, 18 Aug 2025 15:01:14 +0200 Subject: [PATCH 25/27] Fix test --- .../android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt index 5a3c28db008..607242c0802 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabShouldBeVisibleTest.kt @@ -58,11 +58,11 @@ class WooPosTabShouldBeVisibleTest : BaseUnitTest() { } @Test - fun `given feature flag disabled, when invoked, then return success true (tab visible anyway)`() = testBlocking { + fun `given feature flag disabled, when invoked, then return success false`() = testBlocking { whenever(isRemoteFeatureFlagEnabled(WOO_POS)).thenReturn(false) val r = sut() assertTrue(r.isSuccess) - assertTrue(r.getOrThrow()) + assertFalse(r.getOrThrow()) } @Test From 635cf9ae9f21d2298b804b584df1210751d370bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Mon, 18 Aug 2025 15:50:48 +0200 Subject: [PATCH 26/27] Fix and add more tests --- .../com/woocommerce/android/AppPrefsTest.kt | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt b/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt index 8c5e5c22b21..62d65a4df8a 100644 --- a/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt +++ b/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt @@ -531,7 +531,7 @@ class AppPrefsTest { val siteId = 123 // WHEN - AppPrefs.setPOSTabVisibilityForSite(siteId, true) + AppPrefs.setPOSTabVisibilityForSite(siteId) val result = AppPrefs.isPOSTabVisibleForSite(siteId) // THEN @@ -549,4 +549,47 @@ class AppPrefsTest { // THEN assertThat(result).isFalse } + + @Test + fun givenPOSTabVisibilitySetWhenClearedThenGetterReturnsFalse() { + // GIVEN + val siteId = 789 + AppPrefs.setPOSTabVisibilityForSite(siteId) + assertThat(AppPrefs.isPOSTabVisibleForSite(siteId)).isTrue + + // WHEN + AppPrefs.clearPOSTabVisibilityForSite(siteId) + + // THEN + assertThat(AppPrefs.isPOSTabVisibleForSite(siteId)).isFalse + } + + @Test + fun givenPOSLaunchableNotSetWhenIsPOSLaunchableForSiteThenReturnFalseByDefault() { + val siteId = 111 + + assertThat(AppPrefs.isPOSLaunchableForSite(siteId)).isFalse + } + + @Test + fun givenSetPOSLaunchableForSiteWhenIsPOSLaunchableForSiteThenReturnTrueOnlyForThatSite() { + val siteA = 222 + val siteB = 333 + + AppPrefs.setPOSLaunchableForSite(siteA) + + assertThat(AppPrefs.isPOSLaunchableForSite(siteA)).isTrue + assertThat(AppPrefs.isPOSLaunchableForSite(siteB)).isFalse + } + + @Test + fun givenSetPOSLaunchableForSiteWhenClearPOSLaunchableForSiteThenReturnFalse() { + val siteId = 444 + AppPrefs.setPOSLaunchableForSite(siteId) + assertThat(AppPrefs.isPOSLaunchableForSite(siteId)).isTrue + + AppPrefs.clearPOSLaunchableForSite(siteId) + + assertThat(AppPrefs.isPOSLaunchableForSite(siteId)).isFalse + } } From 42a29753306fb5ea01ec765cd9e8a03648d2e181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 19 Aug 2025 11:02:02 +0200 Subject: [PATCH 27/27] Supress unit test naming rule until we fix it. --- .../androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt b/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt index 62d65a4df8a..68cadd28851 100644 --- a/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt +++ b/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/AppPrefsTest.kt @@ -10,6 +10,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test +@Suppress("UnitTestNamingRule") class AppPrefsTest { @Before fun setup() {