Skip to content

Commit

Permalink
PM-14414 hides autofill card for all users if autofill service is ena…
Browse files Browse the repository at this point in the history
…bled. (#4297)
  • Loading branch information
dseverns-livefront authored Nov 13, 2024
1 parent 1e0e483 commit 072c3a9
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
),
) {
Expand Down Expand Up @@ -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),
)
Expand Down Expand Up @@ -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<Boolean> {
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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -37,17 +38,24 @@ class FirstTimeActionManagerTest {
every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlow
}

private val mutableAutofillEnabledFlow = MutableStateFlow(false)
private val autofillEnabledManager = mockk<AutofillEnabledManager> {
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 {
Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 072c3a9

Please sign in to comment.