[POS] Better error handling for eligibility

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() {
@@ -531,7 +532,7 @@ class AppPrefsTest {
         val siteId = 123
 
         // WHEN
-        AppPrefs.setPOSTabVisibilityForSite(siteId, true)
+        AppPrefs.setPOSTabVisibilityForSite(siteId)
         val result = AppPrefs.isPOSTabVisibleForSite(siteId)
 
         // THEN
@@ -549,4 +550,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
+    }
 }

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) {
@@ -1183,10 +1185,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
         )
     }
 
@@ -1197,6 +1199,28 @@ object AppPrefs {
         )
     }
 
+    fun clearPOSTabVisibilityForSite(siteId: Int) {
+        remove(PrefKeyString("${UndeletablePrefKey.POS_TAB_VISIBILITY}:$siteId"))
+    }
+
+    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
+        )
+    }
+
+    fun clearPOSLaunchableForSite(siteId: Int) {
+        remove(PrefKeyString("${UndeletablePrefKey.POS_LAUNCHABLE}:$siteId"))
+    }
+
     /**
      * Remove all user and site-related preferences.
      */

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<Boolean> {
         if (forceRefresh) {
             ssrFetcher.invalidate()
         }
@@ -25,9 +26,11 @@ class WooPOSIsRemotelyEnabled @Inject constructor(
                 val settingsMap: Map<String, Any> = 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())
     }
 }

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

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

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,
@@ -39,51 +43,115 @@ class WooPosCanBeLaunchedInTab @Inject constructor(
     private suspend fun checkLaunchability(forceRefresh: Boolean = false): WooPosLaunchability {
         val site = selectedSite.getOrNull()
             ?: return WooPosLaunchability.NotLaunchable(
-                WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected
+                reason = WooPosLaunchability.NonLaunchabilityReason.NoSiteSelected
             )
 
-        val wooCoreVersion = if (forceRefresh) {
-            fetchWooCoreVersion()
-        } else {
-            getWooCoreCachedVersion()
-        } ?: return WooPosLaunchability.NotLaunchable(
-            WooPosLaunchability.NonLaunchabilityReason.WooCommercePluginNotFound
-        )
+        val cachedPositive = appPrefs.isPOSLaunchableForSite(site.id)
 
-        if (!isWooCoreSupportsOrderAutoDraftsAndExtraPaymentsProps(wooCoreVersion)) {
-            return WooPosLaunchability.NotLaunchable(
-                WooPosLaunchability.NonLaunchabilityReason.UnsupportedWooCommerceVersion
-            )
+        getNonLaunchabilityReasonFromVersionAndFeatureSwitch(forceRefresh, cachedPositive)?.let {
+            return prepareNotLaunchableStateWithCacheUpdate(site.id, it)
         }
 
-        if (isFeatureSwitchSupported(wooCoreVersion) && !isRemotelyEnabled(forceRefresh)) {
-            return WooPosLaunchability.NotLaunchable(
-                WooPosLaunchability.NonLaunchabilityReason.FeatureSwitchDisabled
-            )
+        getNonLaunchabilityReasonFromSiteSettingsAndCurrency(site, forceRefresh, cachedPositive)?.let {
+            return prepareNotLaunchableStateWithCacheUpdate(site.id, it)
         }
 
-        val siteSettings = if (forceRefresh) {
-            wooCommerceStore.fetchSiteGeneralSettings(site).model
+        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 reasonIfNoPositiveCache(cachedPositive)
+
+        return if (!isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) {
+            WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency
         } else {
-            wooCommerceStore.getSiteSettings(site)
-                ?: wooCommerceStore.fetchSiteGeneralSettings(site).model
+            null
         }
+    }
 
-        if (siteSettings == null) {
-            return WooPosLaunchability.NotLaunchable(
-                WooPosLaunchability.NonLaunchabilityReason.SiteSettingsUnavailable
-            )
+    private fun reasonIfNoPositiveCache(hasCachedPositive: Boolean): WooPosLaunchability.NonLaunchabilityReason? =
+        if (hasCachedPositive) {
+            null
+        } else {
+            WooPosLaunchability.NonLaunchabilityReason.UnknownNoPositiveCache
         }
 
-        return if (isCountryAndCurrencySupported(siteSettings.countryCode, siteSettings.currencyCode)) {
-            WooPosLaunchability.Launchable
+    private suspend fun getWooCoreVersion(forceRefresh: Boolean): String? =
+        if (forceRefresh) fetchWooCoreVersion() else getWooCoreCachedVersion()
+
+    /**
+     * 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.NonLaunchabilityReason.UnsupportedWooCommerceVersion
         } else {
-            WooPosLaunchability.NotLaunchable(
-                WooPosLaunchability.NonLaunchabilityReason.UnsupportedCurrency
-            )
+            null
         }
+
+    /**
+     * 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,
+        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? =
+        if (forceRefresh) {
+            wooCommerceStore.fetchSiteGeneralSettings(site).model
+        } else {
+            wooCommerceStore.getSiteSettings(site) ?: wooCommerceStore.fetchSiteGeneralSettings(site).model
+        }
+
     private fun isCountryAndCurrencySupported(countryCode: String, currency: String) =
         SUPPORTED_COUNTRY_CURRENCY_PAIRS.any {
             it.first.equals(countryCode, true) && it.second.equals(currency, true)
@@ -117,5 +185,6 @@ sealed class WooPosLaunchability {
         FeatureSwitchDisabled,
         UnsupportedCurrency,
         NoSiteSelected,
+        UnknownNoPositiveCache
     }
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/tab/WooPosTabController.kt

@@ -10,6 +10,7 @@ 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
@@ -20,7 +21,8 @@ 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,22 @@ 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)
+                if (tabShouldBeVisible) {
+                    appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId())
+                } else {
+                    appPrefs.clearPOSTabVisibilityForSite(selectedSite.getSelectedSiteId())
+                }
+            }
+
+            result.onFailure { error ->
+                wooPosLog.i("POS Tab Visibility Value cannot be determined")
+            }
+
+            analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(result))
         }
     }