PM-18681 - Update Showing Coach Mark Tour Logic To Only Consider User's Personal Vault (#4863)

This commit is contained in:
Phil Cappelli
2025-03-13 16:46:14 -04:00
committed by GitHub
parent 90cc9f77c5
commit e2779e4edb
2 changed files with 4 additions and 72 deletions

View File

@@ -9,7 +9,6 @@ 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
@@ -301,8 +300,8 @@ class FirstTimeActionManagerImpl @Inject constructor(
}
/**
* 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`.
* 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> =
@@ -313,14 +312,10 @@ class FirstTimeActionManagerImpl @Inject constructor(
combine(
flow = this,
flow2 = vaultDiskSource.getCiphers(activeUserId),
flow3 = authDiskSource.getPoliciesFlow(activeUserId),
) { receiverCurrentValue, ciphers, policies ->
val hasLoginsWithNoOrganizations = ciphers.any {
) { receiverCurrentValue, ciphers ->
receiverCurrentValue && ciphers.none {
it.login != null && it.organizationId == null
}
val onlyOrgPolicy = policies?.any { it.type == PolicyTypeJson.ONLY_ORG } == true
receiverCurrentValue && (!hasLoginsWithNoOrganizations || onlyOrgPolicy)
}
}
.distinctUntilChanged()

View File

@@ -15,9 +15,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 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
@@ -442,67 +440,6 @@ class FirstTimeActionManagerTest {
}
}
@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`() {