Skip to content

Commit cccf3a2

Browse files
🍒 PM-18636 Hide coach mark card if any login ciphers exist (#4797)
1 parent ea6d561 commit cccf3a2

File tree

2 files changed

+87
-5
lines changed

2 files changed

+87
-5
lines changed

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

+21
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ class FirstTimeActionManagerImpl @Inject constructor(
159159
get() = settingsDiskSource
160160
.getShouldShowAddLoginCoachMarkFlow()
161161
.map { it ?: true }
162+
.mapFalseIfAnyLoginCiphersAvailable()
162163
.combine(
163164
featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
164165
) { shouldShow, featureIsEnabled ->
@@ -172,6 +173,7 @@ class FirstTimeActionManagerImpl @Inject constructor(
172173
get() = settingsDiskSource
173174
.getShouldShowGeneratorCoachMarkFlow()
174175
.map { it ?: true }
176+
.mapFalseIfAnyLoginCiphersAvailable()
175177
.combine(
176178
featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow),
177179
) { shouldShow, featureFlagEnabled ->
@@ -294,4 +296,23 @@ class FirstTimeActionManagerImpl @Inject constructor(
294296
return settingsDiskSource.getShowAutoFillSettingBadge(userId) ?: false &&
295297
!autofillEnabledManager.isAutofillEnabled
296298
}
299+
300+
/**
301+
* If there are any existing "Login" type ciphers then we'll map the current value
302+
* of the receiver Flow to `false`.
303+
*/
304+
@OptIn(ExperimentalCoroutinesApi::class)
305+
private fun Flow<Boolean>.mapFalseIfAnyLoginCiphersAvailable(): Flow<Boolean> =
306+
authDiskSource
307+
.activeUserIdChangesFlow
308+
.filterNotNull()
309+
.flatMapLatest { activeUserId ->
310+
combine(
311+
flow = this,
312+
flow2 = vaultDiskSource.getCiphers(activeUserId),
313+
) { currentValue, ciphers ->
314+
currentValue && ciphers.none { it.login != null }
315+
}
316+
}
317+
.distinctUntilChanged()
297318
}

Diff for: app/src/test/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManagerTest.kt

+66-5
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,10 @@ class FirstTimeActionManagerTest {
303303

304304
@Test
305305
fun `shouldShowAddLoginCoachMarkFlow updates when disk source updates`() = runTest {
306+
fakeAuthDiskSource.userState = MOCK_USER_STATE
306307
// Enable the feature for this test.
307308
mutableOnboardingFeatureFlow.update { true }
308309
firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
309-
// Null will be mapped to false.
310310
assertTrue(awaitItem())
311311
fakeSettingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false)
312312
assertFalse(awaitItem())
@@ -316,15 +316,45 @@ class FirstTimeActionManagerTest {
316316
@Test
317317
fun `shouldShowAddLoginCoachMarkFlow updates when feature flag for onboarding updates`() =
318318
runTest {
319+
fakeAuthDiskSource.userState = MOCK_USER_STATE
319320
firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
320-
// Null will be mapped to false but feature being "off" will override to true.
321321
assertFalse(awaitItem())
322322
mutableOnboardingFeatureFlow.update { true }
323-
// Will use the value from disk source (null ?: false).
324323
assertTrue(awaitItem())
325324
}
326325
}
327326

327+
@Suppress("MaxLineLength")
328+
@Test
329+
fun `if there are any login ciphers available for the active user should not show add login coach marks`() =
330+
runTest {
331+
val mockJsonWithNoLogin = mockk<SyncResponseJson.Cipher> {
332+
every { login } returns null
333+
}
334+
val mockJsonWithLogin = mockk<SyncResponseJson.Cipher> {
335+
every { login } returns mockk()
336+
}
337+
fakeAuthDiskSource.userState = MOCK_USER_STATE
338+
// Enable feature flag so flow emits updates from disk.
339+
mutableOnboardingFeatureFlow.update { true }
340+
mutableCiphersListFlow.update {
341+
listOf(
342+
mockJsonWithNoLogin,
343+
mockJsonWithNoLogin,
344+
)
345+
}
346+
firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test {
347+
assertTrue(awaitItem())
348+
mutableCiphersListFlow.update {
349+
listOf(
350+
mockJsonWithLogin,
351+
mockJsonWithNoLogin,
352+
)
353+
}
354+
assertFalse(awaitItem())
355+
}
356+
}
357+
328358
@Suppress("MaxLineLength")
329359
@Test
330360
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 {
335365

336366
@Test
337367
fun `shouldShowGeneratorCoachMarkFlow updates when disk source updates`() = runTest {
368+
fakeAuthDiskSource.userState = MOCK_USER_STATE
338369
// Enable feature flag so flow emits updates from disk.
339370
mutableOnboardingFeatureFlow.update { true }
340371
firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
341-
// Null will be mapped to false.
342372
assertTrue(awaitItem())
343373
fakeSettingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false)
344374
assertFalse(awaitItem())
@@ -348,15 +378,46 @@ class FirstTimeActionManagerTest {
348378
@Test
349379
fun `shouldShowGeneratorCoachMarkFlow updates when onboarding feature value changes`() =
350380
runTest {
381+
fakeAuthDiskSource.userState = MOCK_USER_STATE
351382
firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
352-
// Null will be mapped to false
353383
assertFalse(awaitItem())
354384
mutableOnboardingFeatureFlow.update { true }
355385
// Take the value from disk.
356386
assertTrue(awaitItem())
357387
}
358388
}
359389

390+
@Suppress("MaxLineLength")
391+
@Test
392+
fun `if there are any login ciphers available for the active user should not show generator coach marks`() =
393+
runTest {
394+
val mockJsonWithNoLogin = mockk<SyncResponseJson.Cipher> {
395+
every { login } returns null
396+
}
397+
val mockJsonWithLogin = mockk<SyncResponseJson.Cipher> {
398+
every { login } returns mockk()
399+
}
400+
fakeAuthDiskSource.userState = MOCK_USER_STATE
401+
// Enable feature flag so flow emits updates from disk.
402+
mutableOnboardingFeatureFlow.update { true }
403+
mutableCiphersListFlow.update {
404+
listOf(
405+
mockJsonWithNoLogin,
406+
mockJsonWithNoLogin,
407+
)
408+
}
409+
firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test {
410+
assertTrue(awaitItem())
411+
mutableCiphersListFlow.update {
412+
listOf(
413+
mockJsonWithLogin,
414+
mockJsonWithNoLogin,
415+
)
416+
}
417+
assertFalse(awaitItem())
418+
}
419+
}
420+
360421
@Suppress("MaxLineLength")
361422
@Test
362423
fun `markCoachMarkTourCompleted for the GENERATOR type sets the value to true in the disk source for should show generator coach mark`() {

0 commit comments

Comments
 (0)