From 072c3a992c09bf2b23b2e86797fbbe08d1b5d412 Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:41:14 -0500 Subject: [PATCH] PM-14414 hides autofill card for all users if autofill service is enabled. (#4297) --- .../manager/FirstTimeActionManagerImpl.kt | 27 ++++++++- .../manager/di/PlatformManagerModule.kt | 3 + .../manager/FirstTimeActionManagerTest.kt | 57 ++++++++++++++++++- 3 files changed, 83 insertions(+), 4 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 index d899a6bc9..406a799b4 100644 --- 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 @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.platform.manager import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.repository.util.activeUserIdChangesFlow +import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState @@ -30,6 +31,7 @@ class FirstTimeActionManagerImpl @Inject constructor( private val settingsDiskSource: SettingsDiskSource, private val vaultDiskSource: VaultDiskSource, private val featureFlagManager: FeatureFlagManager, + private val autofillEnabledManager: AutofillEnabledManager, ) : FirstTimeActionManager { private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) @@ -78,7 +80,7 @@ class FirstTimeActionManagerImpl @Inject constructor( .filterNotNull() .flatMapLatest { // Can be expanded to support multiple autofill settings - settingsDiskSource.getShowAutoFillSettingBadgeFlow(userId = it) + getShowAutofillSettingBadgeFlowInternal(userId = it) .map { showAutofillBadge -> listOfNotNull(showAutofillBadge) } @@ -128,7 +130,7 @@ class FirstTimeActionManagerImpl @Inject constructor( listOf( getShowImportLoginsFlowInternal(userId = activeUserId), settingsDiskSource.getShowUnlockSettingBadgeFlow(userId = activeUserId), - settingsDiskSource.getShowAutoFillSettingBadgeFlow(userId = activeUserId), + getShowAutofillSettingBadgeFlowInternal(userId = activeUserId), getShowImportLoginsSettingBadgeFlowInternal(userId = activeUserId), ), ) { @@ -165,7 +167,7 @@ class FirstTimeActionManagerImpl @Inject constructor( FirstTimeState( showImportLoginsCard = authDiskSource.getShowImportLogins(it), showSetupUnlockCard = settingsDiskSource.getShowUnlockSettingBadge(it), - showSetupAutofillCard = settingsDiskSource.getShowAutoFillSettingBadge(it), + showSetupAutofillCard = getShowAutofillSettingBadgeInternal(it), showImportLoginsCardInSettings = settingsDiskSource .getShowImportLoginsSettingBadge(it), ) @@ -236,4 +238,23 @@ class FirstTimeActionManagerImpl @Inject constructor( showImportLogins ?: false && ciphers.isEmpty() } } + + /** + * Internal implementation to get a flow of the showAutofill value which takes + * into account if autofill is already enabled globally. + */ + private fun getShowAutofillSettingBadgeFlowInternal(userId: String): Flow { + return settingsDiskSource + .getShowAutoFillSettingBadgeFlow(userId) + .combine( + autofillEnabledManager.isAutofillEnabledStateFlow, + ) { showAutofill, autofillEnabled -> + showAutofill ?: false && !autofillEnabled + } + } + + private fun getShowAutofillSettingBadgeInternal(userId: String): Boolean { + return settingsDiskSource.getShowAutoFillSettingBadge(userId) ?: false && + !autofillEnabledManager.isAutofillEnabled + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index b962ccb8f..ea3ffc654 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -6,6 +6,7 @@ import androidx.core.content.getSystemService import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.manager.AddTotpItemFromAuthenticatorManager import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager import com.x8bit.bitwarden.data.platform.datasource.disk.EventDiskSource import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource @@ -293,12 +294,14 @@ object PlatformManagerModule { vaultDiskSource: VaultDiskSource, dispatcherManager: DispatcherManager, featureFlagManager: FeatureFlagManager, + autofillEnabledManager: AutofillEnabledManager, ): FirstTimeActionManager = FirstTimeActionManagerImpl( authDiskSource = authDiskSource, settingsDiskSource = settingsDiskSource, vaultDiskSource = vaultDiskSource, dispatcherManager = dispatcherManager, featureFlagManager = featureFlagManager, + autofillEnabledManager = autofillEnabledManager, ) @Provides 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 index 7e7dd0bb5..41e6d341b 100644 --- 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 @@ -8,6 +8,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson import com.x8bit.bitwarden.data.auth.datasource.network.model.TrustedDeviceUserDecryptionOptionsJson import com.x8bit.bitwarden.data.auth.datasource.network.model.UserDecryptionOptionsJson +import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeSettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState @@ -37,17 +38,24 @@ class FirstTimeActionManagerTest { every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlow } + private val mutableAutofillEnabledFlow = MutableStateFlow(false) + private val autofillEnabledManager = mockk { + every { isAutofillEnabledStateFlow } returns mutableAutofillEnabledFlow + every { isAutofillEnabled } returns false + } + private val firstTimeActionManager = FirstTimeActionManagerImpl( authDiskSource = fakeAuthDiskSource, settingsDiskSource = fakeSettingsDiskSource, vaultDiskSource = vaultDiskSource, featureFlagManager = featureFlagManager, dispatcherManager = FakeDispatcherManager(), + autofillEnabledManager = autofillEnabledManager, ) @Suppress("MaxLineLength") @Test - fun `allAutoFillSettingsBadgeCountFlow should emit the value of flags set to true and update when changed`() = + fun `allAutoFillSettingsBadgeCountFlow should emit the value of flags set to true and update when saved value is changed or autofill enabled state changes`() = runTest { fakeAuthDiskSource.userState = MOCK_USER_STATE firstTimeActionManager.allAutofillSettingsBadgeCountFlow.test { @@ -62,6 +70,13 @@ class FirstTimeActionManagerTest { showBadge = false, ) assertEquals(0, awaitItem()) + fakeSettingsDiskSource.storeShowAutoFillSettingBadge( + userId = USER_ID, + showBadge = true, + ) + assertEquals(1, awaitItem()) + mutableAutofillEnabledFlow.update { true } + assertEquals(0, awaitItem()) } } @@ -240,6 +255,46 @@ class FirstTimeActionManagerTest { firstTimeActionManager.storeShowImportLoginsSettingsBadge(showBadge = false) assertFalse(fakeSettingsDiskSource.getShowImportLoginsSettingBadge(userId = USER_ID)!!) } + + @Test + fun `show autofill badge when autofill is already enabled should be false`() { + fakeAuthDiskSource.userState = MOCK_USER_STATE + every { autofillEnabledManager.isAutofillEnabled } returns true + assertFalse(firstTimeActionManager.currentOrDefaultUserFirstTimeState.showSetupAutofillCard) + } + + @Test + fun `first time state flow should update when autofill is enabled`() = runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + + firstTimeActionManager.firstTimeStateFlow.test { + assertEquals( + FirstTimeState( + showImportLoginsCard = true, + ), + awaitItem(), + ) + fakeSettingsDiskSource.storeShowAutoFillSettingBadge( + userId = MOCK_USER_STATE.activeUserId, + showBadge = true, + ) + assertEquals( + FirstTimeState( + showImportLoginsCard = true, + showSetupAutofillCard = true, + ), + awaitItem(), + ) + mutableAutofillEnabledFlow.update { true } + assertEquals( + FirstTimeState( + showImportLoginsCard = true, + showSetupAutofillCard = false, + ), + awaitItem(), + ) + } + } } private const val USER_ID: String = "userId"