PM-14009 complete fix importlogins card show logic

app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt

@@ -298,4 +298,20 @@ interface SettingsDiskSource {
      * Emits updates that track [getShowUnlockSettingBadge] for the given [userId].
      */
     fun getShowUnlockSettingBadgeFlow(userId: String): Flow<Boolean?>
+
+    /**
+     * Gets whether or not the given [userId] has signalled they want to import logins later.
+     */
+    fun getShowImportLoginsSettingBadge(userId: String): Boolean?
+
+    /**
+     * Stores the given value for whether or not the given [userId] has signalled they want to
+     * set import logins later, during first time usage.
+     */
+    fun storeShowImportLoginsSettingBadge(userId: String, showBadge: Boolean?)
+
+    /**
+     * Emits updates that track [getShowImportLoginsSettingBadge] for the given [userId].
+     */
+    fun getShowImportLoginsSettingBadgeFlow(userId: String): Flow<Boolean?>
 }

app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt

@@ -35,6 +35,7 @@ private const val INITIAL_AUTOFILL_DIALOG_SHOWN = "addSitePromptShown"
 private const val HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY = "hasUserLoggedInOrCreatedAccount"
 private const val SHOW_AUTOFILL_SETTING_BADGE = "showAutofillSettingBadge"
 private const val SHOW_UNLOCK_SETTING_BADGE = "showUnlockSettingBadge"
+private const val SHOW_IMPORT_LOGINS_SETTING_BADGE = "showImportLoginsSettingBadge"
 private const val LAST_SCHEME_CHANGE_INSTANT = "lastDatabaseSchemeChangeInstant"
 
 /**
@@ -65,6 +66,9 @@ class SettingsDiskSourceImpl(
     private val mutableShowUnlockSettingBadgeFlowMap =
         mutableMapOf<String, MutableSharedFlow<Boolean?>>()
 
+    private val mutableShowImportLoginsSettingBadgeFlowMap =
+        mutableMapOf<String, MutableSharedFlow<Boolean?>>()
+
     private val mutableIsIconLoadingDisabledFlow = bufferedMutableSharedFlow<Boolean?>()
 
     private val mutableIsCrashLoggingEnabledFlow = bufferedMutableSharedFlow<Boolean?>()
@@ -412,6 +416,24 @@ class SettingsDiskSourceImpl(
         getMutableShowUnlockSettingBadgeFlow(userId = userId)
             .onSubscription { emit(getShowUnlockSettingBadge(userId)) }
 
+    override fun getShowImportLoginsSettingBadge(userId: String): Boolean? {
+        return getBoolean(
+            key = SHOW_IMPORT_LOGINS_SETTING_BADGE.appendIdentifier(userId),
+        )
+    }
+
+    override fun storeShowImportLoginsSettingBadge(userId: String, showBadge: Boolean?) {
+        putBoolean(
+            key = SHOW_IMPORT_LOGINS_SETTING_BADGE.appendIdentifier(userId),
+            showBadge,
+        )
+        getMutableShowImportLoginsSettingBadgeFlow(userId).tryEmit(showBadge)
+    }
+
+    override fun getShowImportLoginsSettingBadgeFlow(userId: String): Flow<Boolean?> =
+        getMutableShowImportLoginsSettingBadgeFlow(userId)
+            .onSubscription { emit(getShowImportLoginsSettingBadge(userId)) }
+
     private fun getMutableLastSyncFlow(
         userId: String,
     ): MutableSharedFlow<Instant?> =
@@ -455,4 +477,11 @@ class SettingsDiskSourceImpl(
         mutableShowUnlockSettingBadgeFlowMap.getOrPut(userId) {
             bufferedMutableSharedFlow(replay = 1)
         }
+
+    private fun getMutableShowImportLoginsSettingBadgeFlow(
+        userId: String,
+    ): MutableSharedFlow<Boolean?> =
+        mutableShowImportLoginsSettingBadgeFlowMap.getOrPut(userId) {
+            bufferedMutableSharedFlow(replay = 1)
+        }
 }

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

@@ -61,4 +61,9 @@ interface FirstTimeActionManager {
      * Update the value of the showImportLogins status for the active user.
      */
     fun storeShowImportLogins(showImportLogins: Boolean)
+
+    /**
+     * Update the value of the showImportLoginsSettingsBadge status for the active user.
+     */
+    fun storeShowImportLoginsSettingsBadge(showBadge: Boolean)
 }

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

@@ -99,11 +99,11 @@ class FirstTimeActionManagerImpl @Inject constructor(
             .filterNotNull()
             .flatMapLatest {
                 combine(
-                    getShowImportLoginsFlowInternal(userId = it),
+                    getShowImportLoginsSettingBadgeFlowInternal(userId = it),
                     featureFlagManager.getFeatureFlagFlow(FlagKey.ImportLoginsFlow),
                 ) { showImportLogins, importLoginsEnabled ->
-                    val shouldShowImportLogins = showImportLogins && importLoginsEnabled
-                    listOf(shouldShowImportLogins)
+                    val shouldShowImportLoginsSettings = showImportLogins && importLoginsEnabled
+                    listOf(shouldShowImportLoginsSettings)
                 }
                     .map { list ->
                         list.count { showImportLogins -> showImportLogins }
@@ -129,12 +129,14 @@ class FirstTimeActionManagerImpl @Inject constructor(
                         getShowImportLoginsFlowInternal(userId = activeUserId),
                         settingsDiskSource.getShowUnlockSettingBadgeFlow(userId = activeUserId),
                         settingsDiskSource.getShowAutoFillSettingBadgeFlow(userId = activeUserId),
+                        getShowImportLoginsSettingBadgeFlowInternal(userId = activeUserId),
                     ),
                 ) {
                     FirstTimeState(
                         showImportLoginsCard = it[0],
                         showSetupUnlockCard = it[1],
                         showSetupAutofillCard = it[2],
+                        showImportLoginsCardInSettings = it[3],
                     )
                 }
             }
@@ -144,24 +146,12 @@ class FirstTimeActionManagerImpl @Inject constructor(
                         showImportLoginsCard = null,
                         showSetupUnlockCard = null,
                         showSetupAutofillCard = null,
+                        showImportLoginsCardInSettings = null,
                     ),
                 )
             }
             .distinctUntilChanged()
 
-    /**
-     * Internal implementation to get a flow of the showImportLogins value which takes
-     * into account if the vault is empty.
-     */
-    private fun getShowImportLoginsFlowInternal(userId: String): Flow<Boolean> {
-        return authDiskSource.getShowImportLoginsFlow(userId)
-            .combine(
-                vaultDiskSource.getCiphers(userId),
-            ) { showImportLogins, ciphers ->
-                showImportLogins ?: true && ciphers.isEmpty()
-            }
-    }
-
     /**
      * Get the current [FirstTimeState] of the active user if available, otherwise return
      * a default configuration.
@@ -176,12 +166,15 @@ class FirstTimeActionManagerImpl @Inject constructor(
                         showImportLoginsCard = authDiskSource.getShowImportLogins(it),
                         showSetupUnlockCard = settingsDiskSource.getShowUnlockSettingBadge(it),
                         showSetupAutofillCard = settingsDiskSource.getShowAutoFillSettingBadge(it),
+                        showImportLoginsCardInSettings = settingsDiskSource
+                            .getShowImportLoginsSettingBadge(it),
                     )
                 }
                 ?: FirstTimeState(
                     showImportLoginsCard = null,
                     showSetupUnlockCard = null,
                     showSetupAutofillCard = null,
+                    showImportLoginsCardInSettings = null,
                 )
 
     override fun storeShowUnlockSettingBadge(showBadge: Boolean) {
@@ -207,4 +200,40 @@ class FirstTimeActionManagerImpl @Inject constructor(
             showImportLogins = showImportLogins,
         )
     }
+
+    override fun storeShowImportLoginsSettingsBadge(showBadge: Boolean) {
+        val activeUserId = authDiskSource.userState?.activeUserId ?: return
+        settingsDiskSource.storeShowImportLoginsSettingBadge(
+            userId = activeUserId,
+            showBadge = showBadge,
+        )
+    }
+
+    /**
+     * Internal implementation to get a flow of the showImportLogins value which takes
+     * into account if the vault is empty.
+     */
+    private fun getShowImportLoginsFlowInternal(userId: String): Flow<Boolean> {
+        return authDiskSource
+            .getShowImportLoginsFlow(userId)
+            .combine(
+                vaultDiskSource.getCiphers(userId),
+            ) { showImportLogins, ciphers ->
+                showImportLogins ?: true && ciphers.isEmpty()
+            }
+    }
+
+    /**
+     * Internal implementation to get a flow of the showImportLogins value which takes
+     * into account if the vault is empty.
+     */
+    private fun getShowImportLoginsSettingBadgeFlowInternal(userId: String): Flow<Boolean> {
+        return settingsDiskSource
+            .getShowImportLoginsSettingBadgeFlow(userId)
+            .combine(
+                vaultDiskSource.getCiphers(userId),
+            ) { showImportLogins, ciphers ->
+                showImportLogins ?: false && ciphers.isEmpty()
+            }
+    }
 }

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

@@ -5,6 +5,7 @@ package com.x8bit.bitwarden.data.platform.manager.model
  */
 data class FirstTimeState(
     val showImportLoginsCard: Boolean,
+    val showImportLoginsCardInSettings: Boolean,
     val showSetupUnlockCard: Boolean,
     val showSetupAutofillCard: Boolean,
 ) {
@@ -16,9 +17,11 @@ data class FirstTimeState(
         showImportLoginsCard: Boolean? = null,
         showSetupUnlockCard: Boolean? = null,
         showSetupAutofillCard: Boolean? = null,
+        showImportLoginsCardInSettings: Boolean? = null,
     ) : this(
         showImportLoginsCard = showImportLoginsCard ?: true,
         showSetupUnlockCard = showSetupUnlockCard ?: false,
         showSetupAutofillCard = showSetupAutofillCard ?: false,
+        showImportLoginsCardInSettings = showImportLoginsCardInSettings ?: false,
     )
 }

app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModel.kt

@@ -33,7 +33,7 @@ class VaultSettingsViewModel @Inject constructor(
                 .toBaseWebVaultImportUrl,
             isNewImportLoginsFlowEnabled = featureFlagManager
                 .getFeatureFlag(FlagKey.ImportLoginsFlow),
-            showImportActionCard = firstTimeState.showImportLoginsCard,
+            showImportActionCard = firstTimeState.showImportLoginsCardInSettings,
         )
     },
 ) {
@@ -48,7 +48,7 @@ class VaultSettingsViewModel @Inject constructor(
             .firstTimeStateFlow
             .map {
                 VaultSettingsAction.Internal.UserFirstTimeStateChanged(
-                    showImportLoginsCard = it.showImportLoginsCard,
+                    showImportLoginsCard = it.showImportLoginsCardInSettings,
                 )
             }
             .onEach(::sendAction)
@@ -79,7 +79,7 @@ class VaultSettingsViewModel @Inject constructor(
 
     private fun handleImportLoginsCardDismissClicked() {
         if (!state.shouldShowImportCard) return
-        firstTimeActionManager.storeShowImportLogins(showImportLogins = false)
+        firstTimeActionManager.storeShowImportLoginsSettingsBadge(showBadge = false)
     }
 
     private fun handleImportLoginsCardClicked() {

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/importlogins/ImportLoginsViewModel.kt

@@ -97,6 +97,7 @@ class ImportLoginsViewModel @Inject constructor(
             is SyncVaultDataResult.Success -> {
                 if (result.itemsAvailable) {
                     firstTimeActionManager.storeShowImportLogins(showImportLogins = false)
+                    firstTimeActionManager.storeShowImportLoginsSettingsBadge(showBadge = false)
                     mutableStateFlow.update {
                         it.copy(
                             showBottomSheet = true,
@@ -160,6 +161,8 @@ class ImportLoginsViewModel @Inject constructor(
 
     private fun handleConfirmImportLater() {
         dismissDialog()
+        firstTimeActionManager.storeShowImportLogins(showImportLogins = false)
+        firstTimeActionManager.storeShowImportLoginsSettingsBadge(showBadge = true)
         sendEvent(ImportLoginsEvent.NavigateBack)
     }
 

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt

@@ -186,6 +186,7 @@ class VaultViewModel @Inject constructor(
     }
 
     private fun handleDismissImportActionCard() {
+        firstTimeActionManager.storeShowImportLoginsSettingsBadge(true)
         if (!state.showImportActionCard) return
         firstTimeActionManager.storeShowImportLogins(false)
     }

app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt

@@ -1062,10 +1062,10 @@ class SettingsDiskSourceTest {
     @Test
     fun `storeShowAutoFillSettingBadge should update the flow value`() = runTest {
         val mockUserId = "mockUserId"
-        settingsDiskSource.storeShowAutoFillSettingBadge(mockUserId, true)
         settingsDiskSource.getShowAutoFillSettingBadgeFlow(userId = mockUserId).test {
             // The initial values of the Flow are in sync
-            assertTrue(awaitItem() ?: false)
+            assertFalse(awaitItem() ?: false)
+            settingsDiskSource.storeShowAutoFillSettingBadge(mockUserId, true)
             assertTrue(awaitItem() ?: false)
 
             // update the value to false
@@ -1103,10 +1103,10 @@ class SettingsDiskSourceTest {
     @Test
     fun `storeShowUnlockSettingsBadge should update the flow value`() = runTest {
         val mockUserId = "mockUserId"
-        settingsDiskSource.storeShowUnlockSettingBadge(mockUserId, true)
         settingsDiskSource.getShowUnlockSettingBadgeFlow(userId = mockUserId).test {
             // The initial values of the Flow are in sync
-            assertTrue(awaitItem() ?: false)
+            assertFalse(awaitItem() ?: false)
+            settingsDiskSource.storeShowUnlockSettingBadge(mockUserId, true)
             assertTrue(awaitItem() ?: false)
 
             // update the value to false
@@ -1165,4 +1165,47 @@ class SettingsDiskSourceTest {
             actual,
         )
     }
+
+    @Test
+    fun `getShowImportLoginsSettingBadge should pull from shared preferences`() {
+        val mockUserId = "mockUserId"
+        val showImportLoginsSettingBadgeKey =
+            "bwPreferencesStorage:showImportLoginsSettingBadge_$mockUserId"
+        fakeSharedPreferences.edit {
+            putBoolean(showImportLoginsSettingBadgeKey, true)
+        }
+        assertTrue(
+            settingsDiskSource.getShowImportLoginsSettingBadge(userId = mockUserId)!!,
+        )
+    }
+
+    @Test
+    fun `storeShowImportLoginsSettingBadge should update SharedPreferences`() {
+        val mockUserId = "mockUserId"
+        val showImportLoginsSettingBadgeKey =
+            "bwPreferencesStorage:showImportLoginsSettingBadge_$mockUserId"
+        settingsDiskSource.storeShowImportLoginsSettingBadge(mockUserId, true)
+        assertTrue(
+            fakeSharedPreferences.getBoolean(showImportLoginsSettingBadgeKey, false),
+        )
+    }
+
+    @Test
+    fun `storeShowImportLoginsSettingBadge should update the flow value`() = runTest {
+        val mockUserId = "mockUserId"
+        settingsDiskSource.getShowImportLoginsSettingBadgeFlow(mockUserId).test {
+            // The initial values of the Flow are in sync
+            assertFalse(awaitItem() ?: false)
+            settingsDiskSource.storeShowImportLoginsSettingBadge(
+                userId = mockUserId,
+                showBadge = true,
+            )
+            assertTrue(awaitItem() ?: false)
+            // update the value to false
+            settingsDiskSource.storeShowImportLoginsSettingBadge(
+                userId = mockUserId, false,
+            )
+            assertFalse(awaitItem() ?: true)
+        }
+    }
 }

app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt

@@ -63,6 +63,7 @@ class FakeSettingsDiskSource : SettingsDiskSource {
     private val userSignIns = mutableMapOf<String, Boolean>()
     private val userShowAutoFillBadge = mutableMapOf<String, Boolean?>()
     private val userShowUnlockBadge = mutableMapOf<String, Boolean?>()
+    private val userShowImportLoginsBadge = mutableMapOf<String, Boolean?>()
     private var storedLastDatabaseSchemeChangeInstant: Instant? = null
 
     private val mutableShowAutoFillSettingBadgeFlowMap =
@@ -71,6 +72,9 @@ class FakeSettingsDiskSource : SettingsDiskSource {
     private val mutableShowUnlockSettingBadgeFlowMap =
         mutableMapOf<String, MutableSharedFlow<Boolean?>>()
 
+    private val mutableShowImportLoginsSettingBadgeFlowMap =
+        mutableMapOf<String, MutableSharedFlow<Boolean?>>()
+
     override var appLanguage: AppLanguage? = null
 
     override var appTheme: AppTheme
@@ -323,6 +327,19 @@ class FakeSettingsDiskSource : SettingsDiskSource {
             emit(getShowUnlockSettingBadge(userId = userId))
         }
 
+    override fun getShowImportLoginsSettingBadge(userId: String): Boolean? =
+        userShowImportLoginsBadge[userId]
+
+    override fun storeShowImportLoginsSettingBadge(userId: String, showBadge: Boolean?) {
+        userShowImportLoginsBadge[userId] = showBadge
+        getMutableShowImportLoginsSettingBadgeFlow(userId).tryEmit(showBadge)
+    }
+
+    override fun getShowImportLoginsSettingBadgeFlow(userId: String): Flow<Boolean?> =
+        getMutableShowImportLoginsSettingBadgeFlow(userId = userId).onSubscription {
+            emit(getShowImportLoginsSettingBadge(userId = userId))
+        }
+
     //region Private helper functions
     private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow<Boolean?> {
         return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) {
@@ -369,5 +386,11 @@ class FakeSettingsDiskSource : SettingsDiskSource {
             bufferedMutableSharedFlow(replay = 1)
         }
 
+    private fun getMutableShowImportLoginsSettingBadgeFlow(
+        userId: String,
+    ): MutableSharedFlow<Boolean?> = mutableShowImportLoginsSettingBadgeFlowMap.getOrPut(userId) {
+        bufferedMutableSharedFlow(replay = 1)
+    }
+
     //endregion Private helper functions
 }