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 be336b93f..fccb36f6d 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 @@ -158,6 +158,11 @@ class FirstTimeActionManagerImpl @Inject constructor( get() = settingsDiskSource .getHasSeenGeneratorCoachMarkFlow() .map { it ?: false } + .combine( + featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow), + ) { hasSeenTour, featureFlagEnabled -> + hasSeenTour || !featureFlagEnabled + } .distinctUntilChanged() /** diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt index 3dfd702a4..bd976a95e 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreen.kt @@ -329,7 +329,9 @@ private fun ScrollContent( actionText = stringResource(R.string.get_started), onActionClick = passwordHandlers.onGeneratorActionCardClicked, onDismissClick = passwordHandlers.onGeneratorActionCardDismissed, - modifier = Modifier.standardHorizontalMargin(), + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin(), ) Spacer(modifier = Modifier.height(24.dp)) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt index b30ac94df..d1ec7d101 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt @@ -12,12 +12,10 @@ import com.bitwarden.generators.UsernameGeneratorRequest import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager -import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies import com.x8bit.bitwarden.data.platform.manager.util.getActivePoliciesFlow import com.x8bit.bitwarden.data.tools.generator.repository.GeneratorRepository @@ -50,7 +48,6 @@ import com.x8bit.bitwarden.ui.tools.feature.generator.util.toStrictestPolicy import com.x8bit.bitwarden.ui.tools.feature.generator.util.toUsernameGeneratorRequest import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -82,7 +79,6 @@ class GeneratorViewModel @Inject constructor( private val policyManager: PolicyManager, private val reviewPromptManager: ReviewPromptManager, private val firstTimeActionManager: FirstTimeActionManager, - featureFlagManager: FeatureFlagManager, ) : BaseViewModel<GeneratorState, GeneratorEvent, GeneratorAction>( initialState = savedStateHandle[KEY_STATE] ?: run { val generatorMode = GeneratorArgs(savedStateHandle).type @@ -131,13 +127,9 @@ class GeneratorViewModel @Inject constructor( firstTimeActionManager .hasSeenGeneratorCoachMarkFlow - .combine( - featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow), - ) { hasSeenCoachMarkTour, featureEnabled -> - // The result of this will hide the card if the tour has been shown or if the - // onboarding flow feature is off. + .map { hasSeenCoachMarkTour -> GeneratorAction.Internal.HasSeenGeneratorCoachMarkValueChangeReceive( - newValue = hasSeenCoachMarkTour || !featureEnabled, + hasSeenCoachMark = hasSeenCoachMarkTour, ) } .onEach(::sendAction) @@ -786,7 +778,7 @@ class GeneratorViewModel @Inject constructor( action: GeneratorAction.Internal.HasSeenGeneratorCoachMarkValueChangeReceive, ) { mutableStateFlow.update { - it.copy(hasSeenExploreGeneratorCard = action.newValue) + it.copy(hasSeenExploreGeneratorCard = action.hasSeenCoachMark) } } @@ -2614,7 +2606,7 @@ sealed class GeneratorAction { * The value for the hasSeenGeneratorCoachMark has changed. */ data class HasSeenGeneratorCoachMarkValueChangeReceive( - val newValue: Boolean, + val hasSeenCoachMark: Boolean, ) : Internal() } } 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 4fe0c9084..91182b34c 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 @@ -36,8 +36,10 @@ class FirstTimeActionManagerTest { } private val mutableImportLoginsFlow = MutableStateFlow(false) + private val mutableOnboardFeatureFlow = MutableStateFlow(false) private val featureFlagManager = mockk<FeatureFlagManager> { every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlow + every { getFeatureFlagFlow(FlagKey.OnboardingFlow) } returns mutableOnboardFeatureFlow } private val mutableAutofillEnabledFlow = MutableStateFlow(false) @@ -300,14 +302,27 @@ class FirstTimeActionManagerTest { @Test fun `hasSeenGeneratorCoachMarkFlow updates when disk source updates`() = runTest { + // Enable feature flag so flow emits updates from disk. + mutableOnboardFeatureFlow.update { true } firstTimeActionManager.hasSeenGeneratorCoachMarkFlow.test { - // null will be mapped to false + // Null will be mapped to false. assertFalse(awaitItem()) fakeSettingsDiskSource.storeHasSeenGeneratorCoachMark(hasSeen = true) assertTrue(awaitItem()) } } + @Test + fun `hasSeenGeneratorCoachMarkFlow updates when onboarding feature value changes`() = runTest { + firstTimeActionManager.hasSeenGeneratorCoachMarkFlow.test { + // Null will be mapped to false + assertTrue(awaitItem()) + mutableOnboardFeatureFlow.update { true } + // Take the value from disk. + assertFalse(awaitItem()) + } + } + @Test fun `hasSeenGeneratorCoachMarkTour sets the value to true in the disk source`() { assertNull(fakeSettingsDiskSource.getHasSeenGeneratorCoachMark()) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt index 56c8aaeaa..7ba83ed9c 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt @@ -8,13 +8,11 @@ import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.UserState -import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState -import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedCatchAllUsernameResult @@ -121,13 +119,6 @@ class GeneratorViewModelTest : BaseViewModelTest() { every { hasSeenGeneratorCoachMarkFlow } returns mutableHasSeenGeneratorCoachMarkFlow } - private val mutableOnboardFlowFeatureFlagFlow = MutableStateFlow(false) - private val featureFlagManager: FeatureFlagManager = mockk { - every { - getFeatureFlagFlow(FlagKey.OnboardingFlow) - } returns mutableOnboardFlowFeatureFlagFlow - } - @Test fun `initial state should be correct when there is no saved state`() { val viewModel = createViewModel(state = null) @@ -158,7 +149,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { currentEmailAddress = "currentEmail", isUnderPolicy = false, website = "", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) val viewModel = createViewModel( @@ -194,7 +185,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { currentEmailAddress = "currentEmail", isUnderPolicy = false, website = null, - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) val viewModel = createViewModel( @@ -2182,20 +2173,9 @@ class GeneratorViewModelTest : BaseViewModelTest() { assertEquals(10, password.computedMinimumLength) } - @Test - fun `when OnboardFlow feature flag is off, shouldShowExploreGeneratorCard should be false`() { - mutableOnboardFlowFeatureFlagFlow.update { true } - val viewModel = createViewModel() - assertTrue(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) - - mutableOnboardFlowFeatureFlagFlow.update { false } - assertFalse(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) - } - @Suppress("MaxLineLength") @Test fun `when first time action manager has seen generator tour value updates to true shouldShowExploreGeneratorCard should update to false`() { - mutableOnboardFlowFeatureFlagFlow.update { true } val viewModel = createViewModel() assertTrue(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) mutableHasSeenGeneratorCoachMarkFlow.update { true } @@ -2205,7 +2185,6 @@ class GeneratorViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test fun `shouldShowExploreGeneratorCard value should be false if generator screen is in modal mode`() { - mutableOnboardFlowFeatureFlagFlow.update { true } mutableHasSeenGeneratorCoachMarkFlow.update { false } val viewModel = createViewModel( savedStateHandle = createSavedStateHandleWithState( @@ -2222,7 +2201,6 @@ class GeneratorViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test fun `shouldShowExploreGeneratorCard value should be false if generator screen selected type is not password`() { - mutableOnboardFlowFeatureFlagFlow.update { true } mutableHasSeenGeneratorCoachMarkFlow.update { false } val viewModel = createViewModel( savedStateHandle = createSavedStateHandleWithState( @@ -2278,7 +2256,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { avoidAmbiguousChars = avoidAmbiguousChars, ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createPassphraseState( @@ -2297,7 +2275,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { includeNumber = includeNumber, ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createUsernameModeState( @@ -2313,7 +2291,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createForwardedEmailAliasState( @@ -2329,7 +2307,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createAddyIoState( @@ -2344,7 +2322,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createDuckDuckGoState( @@ -2359,7 +2337,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createFastMailState( @@ -2374,7 +2352,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createFirefoxRelayState( @@ -2389,7 +2367,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createForwardEmailState( @@ -2404,7 +2382,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createSimpleLoginState( @@ -2419,7 +2397,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createPlusAddressedEmailState( @@ -2434,7 +2412,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createCatchAllEmailState( @@ -2449,7 +2427,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createRandomWordState( @@ -2466,7 +2444,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", - hasSeenExploreGeneratorCard = true, + hasSeenExploreGeneratorCard = false, ) private fun createSavedStateHandleWithState(state: GeneratorState) = @@ -2484,7 +2462,6 @@ class GeneratorViewModelTest : BaseViewModelTest() { policyManager = policyManager, reviewPromptManager = reviewPromptManager, firstTimeActionManager = firstTimeActionManager, - featureFlagManager = featureFlagManager, ) private fun createViewModel(