PM-18636 Hide coach mark card if any login ciphers exist (#4787)

Co-authored-by: Patrick Honkonen <[email protected]> Co-authored-by: Philip Cappelli <[email protected]>
bitwarden · Feb 26, 2025 · 2893c38 · 2893c38
1 parent d04ac5e
commit 2893c38
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 5 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerImpl.kt
@@ -159,6 +159,7 @@ class FirstTimeActionManagerImpl @Inject constructor(
         get() = settingsDiskSource
             .getShouldShowAddLoginCoachMarkFlow()
             .map { it ?: true }
+            .mapFalseIfAnyLoginCiphersAvailable()
             .combine(
                 featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
             ) { shouldShow, featureIsEnabled ->
@@ -172,6 +173,7 @@ class FirstTimeActionManagerImpl @Inject constructor(
         get() = settingsDiskSource
             .getShouldShowGeneratorCoachMarkFlow()
             .map { it ?: true }
+            .mapFalseIfAnyLoginCiphersAvailable()
             .combine(
                 featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
             ) { shouldShow, featureFlagEnabled ->
@@ -294,4 +296,23 @@ class FirstTimeActionManagerImpl @Inject constructor(
         return settingsDiskSource.getShowAutoFillSettingBadge(userId) ?: false &&
             !autofillEnabledManager.isAutofillEnabled
     }
+
+    /**
+     * If there are any existing "Login" type ciphers then we'll map the current value
+     * of the receiver Flow to `false`.
+     */
+    @OptIn(ExperimentalCoroutinesApi::class)
+    private fun Flow<Boolean>.mapFalseIfAnyLoginCiphersAvailable(): Flow<Boolean> =
+        authDiskSource
+            .activeUserIdChangesFlow
+            .filterNotNull()
+            .flatMapLatest { activeUserId ->
+                combine(
+                    flow = this,
+                    flow2 = vaultDiskSource.getCiphers(activeUserId),
+                ) { currentValue, ciphers ->
+                    currentValue && ciphers.none { it.login != null }
+                }
+            }
+            .distinctUntilChanged()
 }
diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt
@@ -303,10 +303,10 @@ class FirstTimeActionManagerTest {
 
     @Test
     fun `shouldShowAddLoginCoachMarkFlow updates when disk source updates`() = runTest {
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
         // Enable the feature for this test.
         mutableOnboardingFeatureFlow.update { true }
         firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
-            // Null will be mapped to false.
             assertTrue(awaitItem())
             fakeSettingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false)
             assertFalse(awaitItem())
@@ -316,15 +316,45 @@ class FirstTimeActionManagerTest {
     @Test
     fun `shouldShowAddLoginCoachMarkFlow updates when feature flag for onboarding updates`() =
         runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
             firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
-                // Null will be mapped to false but feature being "off" will override to true.
                 assertFalse(awaitItem())
                 mutableOnboardingFeatureFlow.update { true }
-                // Will use the value from disk source (null ?: false).
                 assertTrue(awaitItem())
             }
         }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `if there are any login ciphers available for the active user should not show add login coach marks`() =
+        runTest {
+            val mockJsonWithNoLogin = mockk<SyncResponseJson.Cipher> {
+                every { login } returns null
+            }
+            val mockJsonWithLogin = mockk<SyncResponseJson.Cipher> {
+                every { login } returns mockk()
+            }
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            // Enable feature flag so flow emits updates from disk.
+            mutableOnboardingFeatureFlow.update { true }
+            mutableCiphersListFlow.update {
+                listOf(
+                    mockJsonWithNoLogin,
+                    mockJsonWithNoLogin,
+                )
+            }
+            firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
+                assertTrue(awaitItem())
+                mutableCiphersListFlow.update {
+                    listOf(
+                        mockJsonWithLogin,
+                        mockJsonWithNoLogin,
+                    )
+                }
+                assertFalse(awaitItem())
+            }
+        }
+
     @Suppress("MaxLineLength")
     @Test
     fun `markCoachMarkTourCompleted for the ADD_LOGIN type sets the value to true in the disk source for should show add logins coach mark`() {
@@ -335,10 +365,10 @@ class FirstTimeActionManagerTest {
 
     @Test
     fun `shouldShowGeneratorCoachMarkFlow updates when disk source updates`() = runTest {
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
         // Enable feature flag so flow emits updates from disk.
         mutableOnboardingFeatureFlow.update { true }
         firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
-            // Null will be mapped to false.
             assertTrue(awaitItem())
             fakeSettingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false)
             assertFalse(awaitItem())
@@ -348,15 +378,46 @@ class FirstTimeActionManagerTest {
     @Test
     fun `shouldShowGeneratorCoachMarkFlow updates when onboarding feature value changes`() =
         runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
             firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
-                // Null will be mapped to false
                 assertFalse(awaitItem())
                 mutableOnboardingFeatureFlow.update { true }
                 // Take the value from disk.
                 assertTrue(awaitItem())
             }
         }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `if there are any login ciphers available for the active user should not show generator coach marks`() =
+        runTest {
+            val mockJsonWithNoLogin = mockk<SyncResponseJson.Cipher> {
+                every { login } returns null
+            }
+            val mockJsonWithLogin = mockk<SyncResponseJson.Cipher> {
+                every { login } returns mockk()
+            }
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            // Enable feature flag so flow emits updates from disk.
+            mutableOnboardingFeatureFlow.update { true }
+            mutableCiphersListFlow.update {
+                listOf(
+                    mockJsonWithNoLogin,
+                    mockJsonWithNoLogin,
+                )
+            }
+            firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
+                assertTrue(awaitItem())
+                mutableCiphersListFlow.update {
+                    listOf(
+                        mockJsonWithLogin,
+                        mockJsonWithNoLogin,
+                    )
+                }
+                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`() {