PM-18681 - Update Showing Coach Mark Tour Logic To Account for Org Only Policy (#4854)

Author: phil-livefront

app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt

@@ -9,6 +9,7 @@ import com.x8bit.bitwarden.data.platform.manager.model.CoachMarkTourType
 import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
 import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
 import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
+import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
@@ -158,7 +159,7 @@ class FirstTimeActionManagerImpl @Inject constructor(
     override val shouldShowAddLoginCoachMarkFlow: Flow<Boolean>
         get() = settingsDiskSource
             .getShouldShowAddLoginCoachMarkFlow()
-            .map { it ?: true }
+            .map { it != false }
             .mapFalseIfAnyLoginCiphersAvailable()
             .combine(
                 featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
@@ -172,11 +173,13 @@ class FirstTimeActionManagerImpl @Inject constructor(
     override val shouldShowGeneratorCoachMarkFlow: Flow<Boolean>
         get() = settingsDiskSource
             .getShouldShowGeneratorCoachMarkFlow()
-            .map { it ?: true }
+            .map { it != false }
             .mapFalseIfAnyLoginCiphersAvailable()
             .combine(
                 featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
             ) { shouldShow, featureFlagEnabled ->
+                // If the feature flag is off always return true so observers know
+                // the card has not been shown.
                 shouldShow && featureFlagEnabled
             }
             .distinctUntilChanged()
@@ -298,8 +301,8 @@ class FirstTimeActionManagerImpl @Inject constructor(
     }
 
     /**
-     * If there are any existing "Login" type ciphers then we'll map the current value
-     * of the receiver Flow to `false`.
+     * Maps the receiver Flow to `false` if a personal (non-org) Login cipher exists,
+     * unless an `ONLY_ORG` policy is active, in which case it remains `true`.
      */
     @OptIn(ExperimentalCoroutinesApi::class)
     private fun Flow<Boolean>.mapFalseIfAnyLoginCiphersAvailable(): Flow<Boolean> =
@@ -310,10 +313,14 @@ class FirstTimeActionManagerImpl @Inject constructor(
                 combine(
                     flow = this,
                     flow2 = vaultDiskSource.getCiphers(activeUserId),
-                ) { currentValue, ciphers ->
-                    currentValue && ciphers.none {
+                    flow3 = authDiskSource.getPoliciesFlow(activeUserId),
+                ) { receiverCurrentValue, ciphers, policies ->
+                    val hasLoginsWithNoOrganizations = ciphers.any {
                         it.login != null && it.organizationId == null
                     }
+                    val onlyOrgPolicy = policies?.any { it.type == PolicyTypeJson.ONLY_ORG } == true
+
+                    receiverCurrentValue && (!hasLoginsWithNoOrganizations || onlyOrgPolicy)
                 }
             }
             .distinctUntilChanged()

app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt

@@ -15,7 +15,9 @@ import com.x8bit.bitwarden.data.platform.manager.model.CoachMarkTourType
 import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
 import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
 import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
+import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockPolicy
 import io.mockk.every
 import io.mockk.mockk
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -425,21 +427,82 @@ class FirstTimeActionManagerTest {
     @Test
     fun `if there are login ciphers attached to an organization we should show coach marks`() =
         runTest {
-            val mockJsonWithNoLoginAndWithOrganizationId = mockk<SyncResponseJson.Cipher> {
+            val mockJsonWithLoginAndWithOrganizationId = mockk<SyncResponseJson.Cipher> {
                 every { login } returns mockk()
                 every { organizationId } returns "1234"
             }
             fakeAuthDiskSource.userState = MOCK_USER_STATE
             // Enable feature flag so flow emits updates from disk.
             mutableOnboardingFeatureFlow.update { true }
             mutableCiphersListFlow.update {
-                listOf(mockJsonWithNoLoginAndWithOrganizationId)
+                listOf(mockJsonWithLoginAndWithOrganizationId)
             }
             firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
                 assertTrue(awaitItem())
             }
         }
 
+    @Test
+    fun `if a user has a org only policy with no login items we show coach marks`() =
+        runTest {
+            val userState = MOCK_USER_STATE
+            fakeAuthDiskSource.userState = userState
+            fakeAuthDiskSource.storePolicies(
+                userState.activeUserId,
+                listOf(
+                    createMockPolicy(
+                        isEnabled = true,
+                        number = 2,
+                        organizationId = "1234",
+                        type = PolicyTypeJson.ONLY_ORG,
+                    ),
+                ),
+            )
+            // Enable feature flag so flow emits updates from disk.
+            mutableOnboardingFeatureFlow.update { true }
+
+            firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
+                assertTrue(awaitItem())
+                // Make sure when we change the disk source that we honor that value.
+                fakeSettingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false)
+                assertFalse(awaitItem())
+            }
+        }
+
+    @Test
+    fun `if a user has a org only policy with login items we show coach marks`() =
+        runTest {
+            val userState = MOCK_USER_STATE
+            fakeAuthDiskSource.userState = userState
+            fakeAuthDiskSource.storePolicies(
+                userState.activeUserId,
+                listOf(
+                    createMockPolicy(
+                        isEnabled = true,
+                        number = 2,
+                        organizationId = "1234",
+                        type = PolicyTypeJson.ONLY_ORG,
+                    ),
+                ),
+            )
+            val mockJsonWithLoginAndWithOrganizationId = mockk<SyncResponseJson.Cipher> {
+                every { login } returns mockk()
+                every { organizationId } returns "1234"
+            }
+            mutableCiphersListFlow.update {
+                listOf(mockJsonWithLoginAndWithOrganizationId)
+            }
+            // Enable feature flag so flow emits updates from disk.
+            mutableOnboardingFeatureFlow.update { true }
+
+            firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
+                assertTrue(awaitItem())
+                // Make sure when we change the disk source that we honor that value.
+                fakeSettingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false)
+                assertFalse(awaitItem())
+            }
+        }
+
     @Suppress("MaxLineLength")
     @Test
     fun `markCoachMarkTourCompleted for the GENERATOR type sets the value to true in the disk source for should show generator coach mark`() {