From fb3f2053c5d2d67feb9dcc62ebec0bad07d395ee Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 6 Aug 2024 14:43:53 -0400 Subject: [PATCH] Feedback updates and added feature flag --- .../auth/repository/AuthRepositoryImpl.kt | 3 ++- .../platform/feature/rootnav/RootNavScreen.kt | 2 +- .../auth/repository/AuthRepositoryTest.kt | 25 ++++++++++++++++--- .../feature/rootnav/RootNavScreenTest.kt | 14 ++++++++--- .../feature/rootnav/RootNavViewModelTest.kt | 19 +++++++------- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index f7fef5aae92..ffcf34f7d68 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -80,6 +80,7 @@ import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager +import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository @@ -391,7 +392,7 @@ class AuthRepositoryImpl( !settingsRepository.hasUserLoggedInOrCreatedAccount && featureFlagManager.getFeatureFlag( key = FlagKey.OnboardingCarousel, - forceRefresh = false, + forceRefresh = true, ) override suspend fun deleteAccountWithMasterPassword( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt index be6afc168cd..312d3dfec01 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreen.kt @@ -135,7 +135,7 @@ fun RootNavScreen( LaunchedEffect(state) { when (val currentState = state) { RootNavState.Auth -> navController.navigateToAuthGraph(rootNavOptions) - RootNavState.AuthWithWelcome -> navController.navigateToWelcome(rootNavOptions) + RootNavState.AuthWithWelcome -> navController.navigateToWelcome(rootNavOptions) RootNavState.ResetPassword -> navController.navigateToResetPasswordGraph(rootNavOptions) RootNavState.SetPassword -> navController.navigateToSetPassword(rootNavOptions) RootNavState.Splash -> navController.navigateToSplash(rootNavOptions) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index daee8cb3560..a97b040ca52 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -84,9 +84,11 @@ import com.x8bit.bitwarden.data.auth.repository.util.toUserState import com.x8bit.bitwarden.data.auth.util.YubiKeyResult import com.x8bit.bitwarden.data.auth.util.toSdkParams import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager +import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager +import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.manager.model.NotificationLogoutData import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.Environment @@ -217,6 +219,7 @@ class AuthRepositoryTest { getActivePoliciesFlow(type = PolicyTypeJson.MASTER_PASSWORD) } returns mutableActivePolicyFlow } + private val featureFlagManager: FeatureFlagManager = mockk() private val repository = AuthRepositoryImpl( accountsService = accountsService, @@ -236,6 +239,7 @@ class AuthRepositoryTest { dispatcherManager = dispatcherManager, pushManager = pushManager, policyManager = policyManager, + featureFlagManager = featureFlagManager, ) @BeforeEach @@ -4481,13 +4485,26 @@ class AuthRepositoryTest { } } + @Suppress("MaxLineLength") @Test - fun `hasUserLoggedInOrCreatedAccount should return value from settings repository`() { + fun `getShowWelcomeCarousel should return value from settings repository and feature flag manager`() = runTest { + every { settingsRepository.hasUserLoggedInOrCreatedAccount } returns false + coEvery { + featureFlagManager.getFeatureFlag(FlagKey.OnboardingCarousel, true) + } returns true + assertTrue(repository.getShowWelcomeCarousel()) + every { settingsRepository.hasUserLoggedInOrCreatedAccount } returns true - assertTrue(repository.hasUserLoggedInOrCreatedAccount) + coEvery { + featureFlagManager.getFeatureFlag(FlagKey.OnboardingCarousel, true) + } returns true + assertFalse(repository.getShowWelcomeCarousel()) - every { settingsRepository.hasUserLoggedInOrCreatedAccount } returns false - assertFalse(repository.hasUserLoggedInOrCreatedAccount) + every { settingsRepository.hasUserLoggedInOrCreatedAccount } returns true + coEvery { + featureFlagManager.getFeatureFlag(FlagKey.OnboardingCarousel, true) + } returns false + assertFalse(repository.getShowWelcomeCarousel()) } @Test diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreenTest.kt index 32ef968432e..dc5c89ab95f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavScreenTest.kt @@ -2,7 +2,6 @@ package com.x8bit.bitwarden.ui.platform.feature.rootnav import androidx.navigation.navOptions import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData -import com.x8bit.bitwarden.ui.auth.feature.landing.LANDING_ROUTE import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest import com.x8bit.bitwarden.ui.platform.base.FakeNavHostController import io.mockk.every @@ -65,9 +64,7 @@ class RootNavScreenTest : BaseComposeTest() { assertFalse(isSplashScreenRemoved) // Make sure navigating to Auth works as expected: - rootNavStateFlow.value = RootNavState.Auth( - startDestination = LANDING_ROUTE, - ) + rootNavStateFlow.value = RootNavState.Auth composeTestRule.runOnIdle { fakeNavHostController.assertLastNavigation( route = "auth_graph", @@ -76,6 +73,15 @@ class RootNavScreenTest : BaseComposeTest() { } assertTrue(isSplashScreenRemoved) + // Make sure navigating to Auth with the welcome route works as expected: + rootNavStateFlow.value = RootNavState.AuthWithWelcome + composeTestRule.runOnIdle { + fakeNavHostController.assertLastNavigation( + route = "welcome", + navOptions = expectedNavOptions, + ) + } + // Make sure navigating to vault locked works as expected: rootNavStateFlow.value = RootNavState.VaultLocked composeTestRule.runOnIdle { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt index d02e1082594..393e6ca4a0b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/rootnav/RootNavViewModelTest.kt @@ -11,9 +11,8 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManagerImpl import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance import com.x8bit.bitwarden.data.platform.repository.model.Environment -import com.x8bit.bitwarden.ui.auth.feature.landing.LANDING_ROUTE -import com.x8bit.bitwarden.ui.auth.feature.welcome.WELCOME_ROUTE import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.flow.MutableStateFlow @@ -24,7 +23,7 @@ class RootNavViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(null) private val authRepository = mockk { every { userStateFlow } returns mutableUserStateFlow - every { hasUserLoggedInOrCreatedAccount } returns true + coEvery { getShowWelcomeCarousel() } returns false } private val specialCircumstanceManager = SpecialCircumstanceManagerImpl() @@ -33,19 +32,19 @@ class RootNavViewModelTest : BaseViewModelTest() { mutableUserStateFlow.tryEmit(null) val viewModel = createViewModel() assertEquals( - RootNavState.Auth(startDestination = LANDING_ROUTE), + RootNavState.Auth, viewModel.stateFlow.value, ) } @Suppress("MaxLineLength") @Test - fun `when there are no accounts and the user has not logged on before the nav state should be Auth with start destination welcome`() { - every { authRepository.hasUserLoggedInOrCreatedAccount } returns false + fun `when there are no accounts and the user has not logged on before the nav state should be Auth with the welcome route`() { + coEvery { authRepository.getShowWelcomeCarousel() } returns true mutableUserStateFlow.tryEmit(null) val viewModel = createViewModel() assertEquals( - RootNavState.Auth(startDestination = WELCOME_ROUTE), + RootNavState.AuthWithWelcome, viewModel.stateFlow.value, ) } @@ -76,7 +75,7 @@ class RootNavViewModelTest : BaseViewModelTest() { ) val viewModel = createViewModel() assertEquals( - RootNavState.Auth(startDestination = LANDING_ROUTE), + RootNavState.Auth, viewModel.stateFlow.value, ) } @@ -242,7 +241,7 @@ class RootNavViewModelTest : BaseViewModelTest() { ) val viewModel = createViewModel() assertEquals( - RootNavState.Auth(startDestination = LANDING_ROUTE), + RootNavState.Auth, viewModel.stateFlow.value, ) } @@ -275,7 +274,7 @@ class RootNavViewModelTest : BaseViewModelTest() { ) val viewModel = createViewModel() assertEquals( - RootNavState.Auth(startDestination = LANDING_ROUTE), + RootNavState.Auth, viewModel.stateFlow.value, ) }