From 05dd272e3568421f8e76063221093c315b78b646 Mon Sep 17 00:00:00 2001 From: Dave Severns Date: Mon, 20 Jan 2025 10:34:17 -0500 Subject: [PATCH 1/2] PM-16630 add logins action card PM-16621 add explore generator card to be able to trigger coach marks --- .../datasource/disk/SettingsDiskSource.kt | 30 +++++ .../datasource/disk/SettingsDiskSourceImpl.kt | 40 +++++++ .../manager/FirstTimeActionManager.kt | 17 +++ .../manager/FirstTimeActionManagerImpl.kt | 37 +++++++ .../manager/model/CoachMarkTourType.kt | 12 ++ .../feature/generator/GeneratorScreen.kt | 17 +++ .../feature/generator/GeneratorViewModel.kt | 72 +++++++++++- .../generator/handlers/PasswordHandlers.kt | 13 +++ .../addedit/VaultAddEditItemContent.kt | 19 ++++ .../feature/addedit/VaultAddEditScreen.kt | 1 + .../feature/addedit/VaultAddEditViewModel.kt | 74 +++++++++++++ .../handlers/VaultAddEditLoginTypeHandlers.kt | 12 ++ app/src/main/res/values/strings.xml | 4 + .../datasource/disk/SettingsDiskSourceTest.kt | 61 +++++++++++ .../disk/util/FakeSettingsDiskSource.kt | 33 ++++++ .../manager/FirstTimeActionManagerTest.kt | 68 ++++++++++++ .../feature/generator/GeneratorScreenTest.kt | 86 +++++++++++++++ .../generator/GeneratorViewModelTest.kt | 88 +++++++++++++++ .../feature/addedit/VaultAddEditScreenTest.kt | 103 ++++++++++++++++++ .../addedit/VaultAddEditViewModelTest.kt | 100 +++++++++++++++++ 20 files changed, 886 insertions(+), 1 deletion(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/CoachMarkTourType.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt index 89632a875a0..dad2afc3841 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt @@ -361,4 +361,34 @@ interface SettingsDiskSource { * Stores the given [count] completed create send actions for the device. */ fun storeCreateSendActionCount(count: Int?) + + /** + * Gets the Boolean value of if the Add Login CoachMark tour has been interacted with. + */ + fun getShouldShowAddLoginCoachMark(): Boolean? + + /** + * Stores a value for if the Add Login CoachMark tour has been interacted with + */ + fun storeShouldShowAddLoginCoachMark(shouldShow: Boolean?) + + /** + * Returns an [Flow] to observe updates to the "HasSeenAddLoginCoachMark" value. + */ + fun getShouldShowAddLoginCoachMarkFlow(): Flow + + /** + * Gets the Boolean value of if the Generator CoachMark tour has been interacted with. + */ + fun getShouldShowGeneratorCoachMark(): Boolean? + + /** + * Stores a value for if the Generator CoachMark tour has been interacted with + */ + fun storeShouldShowGeneratorCoachMark(shouldShow: Boolean?) + + /** + * Returns an [Flow] to observe updates to the "HasSeenGeneratorCoachMark" value. + */ + fun getShouldShowGeneratorCoachMarkFlow(): Flow } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt index 09f17081dfd..a8299e6276b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt @@ -39,6 +39,8 @@ private const val IS_VAULT_REGISTERED_FOR_EXPORT = "isVaultRegisteredForExport" private const val ADD_ACTION_COUNT = "addActionCount" private const val COPY_ACTION_COUNT = "copyActionCount" private const val CREATE_ACTION_COUNT = "createActionCount" +private const val SHOULD_SHOW_ADD_LOGIN_COACH_MARK = "shouldShowAddLoginCoachMark" +private const val SHOULD_SHOW_GENERATOR_COACH_MARK = "shouldShowGeneratorCoachMark" /** * Primary implementation of [SettingsDiskSource]. @@ -78,6 +80,10 @@ class SettingsDiskSourceImpl( private val mutableHasUserLoggedInOrCreatedAccountFlow = bufferedMutableSharedFlow() + private val mutableHasSeenAddLoginCoachMarkFlow = bufferedMutableSharedFlow() + + private val mutableHasSeenGeneratorCoachMarkFlow = bufferedMutableSharedFlow() + private val mutableScreenCaptureAllowedFlowMap = mutableMapOf>() @@ -185,6 +191,8 @@ class SettingsDiskSourceImpl( // - screen capture allowed // - show autofill setting badge // - show unlock setting badge + // - has seen add login coach mark + // - has seen generator coach mark } override fun getAccountBiometricIntegrityValidity( @@ -486,6 +494,38 @@ class SettingsDiskSourceImpl( ) } + override fun getShouldShowAddLoginCoachMark(): Boolean? = + getBoolean(key = SHOULD_SHOW_ADD_LOGIN_COACH_MARK) + + override fun storeShouldShowAddLoginCoachMark(shouldShow: Boolean?) { + putBoolean( + key = SHOULD_SHOW_ADD_LOGIN_COACH_MARK, + value = shouldShow, + ) + mutableHasSeenAddLoginCoachMarkFlow.tryEmit(shouldShow) + } + + override fun getShouldShowAddLoginCoachMarkFlow(): Flow = + mutableHasSeenAddLoginCoachMarkFlow.onSubscription { + emit(getBoolean(key = SHOULD_SHOW_ADD_LOGIN_COACH_MARK)) + } + + override fun getShouldShowGeneratorCoachMark(): Boolean? = + getBoolean(key = SHOULD_SHOW_GENERATOR_COACH_MARK) + + override fun storeShouldShowGeneratorCoachMark(shouldShow: Boolean?) { + putBoolean( + key = SHOULD_SHOW_GENERATOR_COACH_MARK, + value = shouldShow, + ) + mutableHasSeenGeneratorCoachMarkFlow.tryEmit(shouldShow) + } + + override fun getShouldShowGeneratorCoachMarkFlow(): Flow = + mutableHasSeenGeneratorCoachMarkFlow.onSubscription { + emit(getShouldShowGeneratorCoachMark()) + } + private fun getMutableLastSyncFlow( userId: String, ): MutableSharedFlow = diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManager.kt index c79119d12bf..6bb60dc0f95 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/FirstTimeActionManager.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.platform.manager +import com.x8bit.bitwarden.data.platform.manager.model.CoachMarkTourType import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow @@ -39,6 +40,17 @@ interface FirstTimeActionManager { */ val firstTimeStateFlow: Flow + /** + * Returns observable flow of if a user on the device has seen the Add Login coach mark tour. + */ + val shouldShowAddLoginCoachMarkFlow: Flow + + /** + * Returns observable flow of if a user on the device has seen the Generator screen + * coach mark tour. + */ + val shouldShowGeneratorCoachMarkFlow: Flow + /** * Get the current [FirstTimeState] of the active user if available, otherwise return * a default configuration. @@ -66,4 +78,9 @@ interface FirstTimeActionManager { * Update the value of the showImportLoginsSettingsBadge status for the active user. */ fun storeShowImportLoginsSettingsBadge(showBadge: Boolean) + + /** + * Can be called to indicate that a user has seen the AddLogin coach mark tour. + */ + fun markCoachMarkTourCompleted(tourCompleted: CoachMarkTourType) } 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 406a799b4a6..3c7fc0b1f0f 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 @@ -5,6 +5,7 @@ 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.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 @@ -154,6 +155,30 @@ class FirstTimeActionManagerImpl @Inject constructor( } .distinctUntilChanged() + override val shouldShowAddLoginCoachMarkFlow: Flow + get() = settingsDiskSource + .getShouldShowAddLoginCoachMarkFlow() + .map { it ?: true } + .combine( + featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow), + ) { shouldShow, featureIsEnabled -> + // If the feature flag is off always return true so observers know + // the card has not been shown. + shouldShow && featureIsEnabled + } + .distinctUntilChanged() + + override val shouldShowGeneratorCoachMarkFlow: Flow + get() = settingsDiskSource + .getShouldShowGeneratorCoachMarkFlow() + .map { it ?: true } + .combine( + featureFlagManager.getFeatureFlagFlow(FlagKey.OnboardingFlow), + ) { shouldShow, featureFlagEnabled -> + shouldShow && featureFlagEnabled + } + .distinctUntilChanged() + /** * Get the current [FirstTimeState] of the active user if available, otherwise return * a default configuration. @@ -211,6 +236,18 @@ class FirstTimeActionManagerImpl @Inject constructor( ) } + override fun markCoachMarkTourCompleted(tourCompleted: CoachMarkTourType) { + when (tourCompleted) { + CoachMarkTourType.ADD_LOGIN -> { + settingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false) + } + + CoachMarkTourType.GENERATOR -> { + settingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false) + } + } + } + /** * Internal implementation to get a flow of the showImportLogins value which takes * into account if the vault is empty. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/CoachMarkTourType.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/CoachMarkTourType.kt new file mode 100644 index 00000000000..97adfc2dc15 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/model/CoachMarkTourType.kt @@ -0,0 +1,12 @@ +package com.x8bit.bitwarden.data.platform.manager.model + +/** + * Enumerated values to represent all the possible coach mark tours that can be + * completed. + * + * @see com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager + */ +enum class CoachMarkTourType { + ADD_LOGIN, + GENERATOR, +} 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 08293c8fd69..bd976a95e4c 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 @@ -44,6 +44,7 @@ import com.x8bit.bitwarden.ui.platform.components.appbar.action.OverflowMenuItem import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton import com.x8bit.bitwarden.ui.platform.components.button.BitwardenStandardIconButton import com.x8bit.bitwarden.ui.platform.components.button.BitwardenTextButton +import com.x8bit.bitwarden.ui.platform.components.card.BitwardenActionCard import com.x8bit.bitwarden.ui.platform.components.card.BitwardenInfoCalloutCard import com.x8bit.bitwarden.ui.platform.components.dropdown.BitwardenMultiSelectButton import com.x8bit.bitwarden.ui.platform.components.field.BitwardenPasswordField @@ -319,6 +320,22 @@ private fun ScrollContent( Spacer(modifier = Modifier.height(12.dp)) } + if (state.shouldShowExploreGeneratorCard) { + BitwardenActionCard( + cardTitle = stringResource(R.string.explore_the_generator), + cardSubtitle = stringResource( + R.string.learn_more_about_generating_secure_login_credentials_with_guided_tour, + ), + actionText = stringResource(R.string.get_started), + onActionClick = passwordHandlers.onGeneratorActionCardClicked, + onDismissClick = passwordHandlers.onGeneratorActionCardDismissed, + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin(), + ) + Spacer(modifier = Modifier.height(24.dp)) + } + GeneratedStringItem( generatedText = state.generatedText, onRegenerateClick = onRegenerateClick, 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 ca1a3ebba2b..e62775490b3 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,9 +12,11 @@ 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.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.CoachMarkTourType 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 @@ -68,7 +70,7 @@ private const val NO_GENERATED_TEXT: String = "-" * * @property savedStateHandle Handles the saved state of this ViewModel. */ -@Suppress("LargeClass") +@Suppress("LargeClass", "LongParameterList") @HiltViewModel class GeneratorViewModel @Inject constructor( private val savedStateHandle: SavedStateHandle, @@ -77,6 +79,7 @@ class GeneratorViewModel @Inject constructor( private val authRepository: AuthRepository, private val policyManager: PolicyManager, private val reviewPromptManager: ReviewPromptManager, + private val firstTimeActionManager: FirstTimeActionManager, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] ?: run { val generatorMode = GeneratorArgs(savedStateHandle).type @@ -105,6 +108,7 @@ class GeneratorViewModel @Inject constructor( .getActivePolicies() .any(), website = (generatorMode as? GeneratorMode.Modal.Username)?.website, + shouldShowCoachMarkTour = false, ) }, ) { @@ -121,6 +125,16 @@ class GeneratorViewModel @Inject constructor( .map { GeneratorAction.Internal.PasswordGeneratorPolicyReceive(it) } .onEach(::sendAction) .launchIn(viewModelScope) + + firstTimeActionManager + .shouldShowGeneratorCoachMarkFlow + .map { hasSeenCoachMarkTour -> + GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive( + shouldShowCoachMarkTour = hasSeenCoachMarkTour, + ) + } + .onEach(::sendAction) + .launchIn(viewModelScope) } override fun handleAction(action: GeneratorAction) { @@ -134,9 +148,26 @@ class GeneratorViewModel @Inject constructor( is GeneratorAction.MainType -> handleMainTypeAction(action) is GeneratorAction.Internal -> handleInternalAction(action) GeneratorAction.LifecycleResume -> handleOnResumed() + GeneratorAction.ExploreGeneratorCardDismissed -> handleExploreCardDismissed() + GeneratorAction.StartExploreGeneratorTour -> handleStartExploreGeneratorTour() } } + private fun handleExploreCardDismissed() { + coachMarkTourCompleted() + } + + private fun handleStartExploreGeneratorTour() { + coachMarkTourCompleted() + // TODO: PM-16622 send show coach mark event. + } + + private fun coachMarkTourCompleted() { + firstTimeActionManager.markCoachMarkTourCompleted( + tourCompleted = CoachMarkTourType.GENERATOR, + ) + } + private fun handleOnResumed() { // when the screen resumes we need to refresh the options for the current option from // disk in the event they were changed while the screen was in the foreground. @@ -233,6 +264,10 @@ class GeneratorViewModel @Inject constructor( is GeneratorAction.Internal.PasswordGeneratorPolicyReceive -> { handlePasswordGeneratorPolicyReceive(action) } + + is GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive -> { + handleHasSeenCoachMarkValueChange(action) + } } } @@ -746,6 +781,14 @@ class GeneratorViewModel @Inject constructor( } } + private fun handleHasSeenCoachMarkValueChange( + action: GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive, + ) { + mutableStateFlow.update { + it.copy(shouldShowCoachMarkTour = action.shouldShowCoachMarkTour) + } + } + private fun handlePasswordGeneratorPolicyReceive( action: GeneratorAction.Internal.PasswordGeneratorPolicyReceive, ) { @@ -1726,6 +1769,7 @@ data class GeneratorState( val isUnderPolicy: Boolean = false, val website: String? = null, var passcodePolicyOverride: PasscodePolicyOverride? = null, + private val shouldShowCoachMarkTour: Boolean, ) : Parcelable { /** @@ -1741,6 +1785,15 @@ data class GeneratorState( is GeneratorMode.Modal.Username -> emptyList() } + /** + * Whether to show the action card which starts the coach mark tour. Should only show + * if has not be interacted with prior and the screen mode is the default. + */ + val shouldShowExploreGeneratorCard: Boolean + get() = shouldShowCoachMarkTour && + generatorMode is GeneratorMode.Default && + selectedType is MainType.Password + /** * Enum representing the main type options for the generator, such as PASSWORD PASSPHRASE, and * USERNAME. @@ -2173,6 +2226,16 @@ sealed class GeneratorAction { val mainTypeOption: GeneratorState.MainTypeOption, ) : GeneratorAction() + /** + * User clicked the close button on the action card for exploring the generator. + */ + data object ExploreGeneratorCardDismissed : GeneratorAction() + + /** + * User has clicked the call to action to start the coach mark tour. + */ + data object StartExploreGeneratorTour : GeneratorAction() + /** * Represents actions related to the [GeneratorState.MainType] in the generator feature. */ @@ -2545,6 +2608,13 @@ sealed class GeneratorAction { data class UpdateGeneratedForwardedServiceUsernameResult( val result: GeneratedForwardedServiceUsernameResult, ) : Internal() + + /** + * The value for the hasSeenGeneratorCoachMark has changed. + */ + data class ShouldShowGeneratorCoachMarkValueChangeReceive( + val shouldShowCoachMarkTour: Boolean, + ) : Internal() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/handlers/PasswordHandlers.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/handlers/PasswordHandlers.kt index e97d1f684e5..9528fbe15fe 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/handlers/PasswordHandlers.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/handlers/PasswordHandlers.kt @@ -20,6 +20,8 @@ data class PasswordHandlers( val onPasswordMinNumbersCounterChange: (Int) -> Unit, val onPasswordMinSpecialCharactersChange: (Int) -> Unit, val onPasswordToggleAvoidAmbiguousCharsChange: (Boolean) -> Unit, + val onGeneratorActionCardClicked: () -> Unit, + val onGeneratorActionCardDismissed: () -> Unit, ) { @Suppress("UndocumentedPublicClass") companion object { @@ -27,6 +29,7 @@ data class PasswordHandlers( * Creates an instance of [PasswordHandlers] by binding actions to the provided * [GeneratorViewModel]. */ + @Suppress("LongMethod") fun create( viewModel: GeneratorViewModel, ): PasswordHandlers = PasswordHandlers( @@ -87,6 +90,16 @@ data class PasswordHandlers( ), ) }, + onGeneratorActionCardDismissed = { + viewModel.trySendAction( + GeneratorAction.ExploreGeneratorCardDismissed, + ) + }, + onGeneratorActionCardClicked = { + viewModel.trySendAction( + GeneratorAction.StartExploreGeneratorTour, + ) + }, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt index a11795b87cc..e0cf75c42d0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditItemContent.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.standardHorizontalMargin +import com.x8bit.bitwarden.ui.platform.components.card.BitwardenActionCard import com.x8bit.bitwarden.ui.platform.components.card.BitwardenInfoCalloutCard import com.x8bit.bitwarden.ui.platform.components.coachmark.CoachMarkScope import com.x8bit.bitwarden.ui.platform.components.dropdown.BitwardenMultiSelectButton @@ -50,6 +51,7 @@ fun CoachMarkScope.VaultAddEditContent( onPreviousCoachMark: () -> Unit, onCoachMarkTourComplete: () -> Unit, onCoachMarkDismissed: () -> Unit, + shouldShowLearnAboutLoginsCard: Boolean, ) { val launcher = permissionsManager.getLauncher( onResult = { isGranted -> @@ -79,6 +81,23 @@ fun CoachMarkScope.VaultAddEditContent( } } + if (shouldShowLearnAboutLoginsCard) { + item { + Spacer(modifier = Modifier.height(height = 12.dp)) + BitwardenActionCard( + cardTitle = stringResource(R.string.learn_about_new_logins), + cardSubtitle = stringResource( + R.string.we_ll_walk_you_through_the_key_features_to_add_a_new_login, + ), + actionText = stringResource(R.string.get_started), + onActionClick = loginItemTypeHandlers.onStartLoginCoachMarkTour, + onDismissClick = loginItemTypeHandlers.onDismissLearnAboutLoginsCard, + modifier = Modifier + .fillMaxWidth() + .standardHorizontalMargin(), + ) + } + } item { Spacer(modifier = Modifier.height(height = 12.dp)) BitwardenListHeaderText( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt index 7e6ebbcefad..14067b8644a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreen.kt @@ -384,6 +384,7 @@ fun VaultAddEditScreen( coachMarkState.coachingComplete(onComplete = scrollBackToTop) }, onCoachMarkDismissed = scrollBackToTop, + shouldShowLearnAboutLoginsCard = state.shouldShowLearnAboutNewLogins, modifier = Modifier .imePadding() .fillMaxSize(), diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt index 322b1dd0818..01cef046703 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt @@ -15,10 +15,12 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CreateCredentialReques import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2RegisterCredentialResult import com.x8bit.bitwarden.data.autofill.fido2.model.UserVerificationRequirement import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials +import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager +import com.x8bit.bitwarden.data.platform.manager.model.CoachMarkTourType import com.x8bit.bitwarden.data.platform.manager.model.OrganizationEvent import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager import com.x8bit.bitwarden.data.platform.manager.util.toAutofillSaveItemOrNull @@ -103,6 +105,7 @@ class VaultAddEditViewModel @Inject constructor( private val clock: Clock, private val organizationEventManager: OrganizationEventManager, private val networkConnectionManager: NetworkConnectionManager, + private val firstTimeActionManager: FirstTimeActionManager, ) : BaseViewModel( // We load the state from the savedStateHandle for testing purposes. initialState = savedStateHandle[KEY_STATE] @@ -170,6 +173,7 @@ class VaultAddEditViewModel @Inject constructor( shouldShowCloseButton = autofillSaveItem == null && fido2AttestationOptions == null, shouldExitOnSave = shouldExitOnSave, supportedItemTypes = getSupportedItemTypeOptions(), + shouldShowCoachMarkTour = false, ) }, ) { @@ -211,6 +215,16 @@ class VaultAddEditViewModel @Inject constructor( } .onEach(::sendAction) .launchIn(viewModelScope) + + firstTimeActionManager + .shouldShowAddLoginCoachMarkFlow + .map { shouldShowTour -> + VaultAddEditAction.Internal.ShouldShowAddLoginCoachMarkValueChangeReceive( + shouldShowCoachMarkTour = shouldShowTour, + ) + } + .onEach(::sendAction) + .launchIn(viewModelScope) } override fun handleAction(action: VaultAddEditAction) { @@ -967,9 +981,32 @@ class VaultAddEditViewModel @Inject constructor( VaultAddEditAction.ItemType.LoginType.ClearFido2CredentialClick -> { handleLoginClearFido2Credential() } + + VaultAddEditAction.ItemType.LoginType.LearnAboutLoginsDismissed -> { + handleLearnAboutLoginsDismissed() + } + + VaultAddEditAction.ItemType.LoginType.StartLearnAboutLogins -> { + handleStartLearnAboutLogins() + } } } + private fun handleStartLearnAboutLogins() { + coachMarkTourCompleted() + sendEvent(VaultAddEditEvent.StartAddLoginItemCoachMarkTour) + } + + private fun handleLearnAboutLoginsDismissed() { + coachMarkTourCompleted() + } + + private fun coachMarkTourCompleted() { + firstTimeActionManager.markCoachMarkTourCompleted( + tourCompleted = CoachMarkTourType.ADD_LOGIN, + ) + } + private fun handleLoginUsernameTextInputChange( action: VaultAddEditAction.ItemType.LoginType.UsernameTextChange, ) { @@ -1445,6 +1482,20 @@ class VaultAddEditViewModel @Inject constructor( is VaultAddEditAction.Internal.ValidateFido2PinResultReceive -> { handleValidateFido2PinResultReceive(action) } + + is VaultAddEditAction.Internal.ShouldShowAddLoginCoachMarkValueChangeReceive -> { + handleHasSeenAddLoginCoachMarkValueChange(action) + } + } + } + + private fun handleHasSeenAddLoginCoachMarkValueChange( + action: VaultAddEditAction.Internal.ShouldShowAddLoginCoachMarkValueChangeReceive, + ) { + mutableStateFlow.update { + it.copy( + shouldShowCoachMarkTour = action.shouldShowCoachMarkTour, + ) } } @@ -1992,6 +2043,7 @@ data class VaultAddEditState( // Internal val shouldExitOnSave: Boolean = false, val totpData: TotpData? = null, + private val shouldShowCoachMarkTour: Boolean, ) : Parcelable { /** @@ -2040,6 +2092,11 @@ data class VaultAddEditState( ?.canAssignToCollections ?: false + val shouldShowLearnAboutNewLogins: Boolean + get() = shouldShowCoachMarkTour && + ((viewState as? ViewState.Content)?.type is ViewState.Content.ItemType.Login) && + isAddItemMode + /** * Enum representing the main type options for the vault, such as LOGIN, CARD, etc. * @@ -2850,6 +2907,16 @@ sealed class VaultAddEditAction { * Represents the action to clear the fido2 credential. */ data object ClearFido2CredentialClick : LoginType() + + /** + * User has clicked the call to action on the learn about logins card. + */ + data object StartLearnAboutLogins : LoginType() + + /** + * User has dismissed the learn about logins card. + */ + data object LearnAboutLoginsDismissed : LoginType() } /** @@ -3138,6 +3205,13 @@ sealed class VaultAddEditAction { data class ValidateFido2PinResultReceive( val result: ValidatePinResult, ) : Internal() + + /** + * The value for the hasSeenAddLoginCoachMark has changed. + */ + data class ShouldShowAddLoginCoachMarkValueChangeReceive( + val shouldShowCoachMarkTour: Boolean, + ) : Internal() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/handlers/VaultAddEditLoginTypeHandlers.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/handlers/VaultAddEditLoginTypeHandlers.kt index 11b45622dd3..20968018116 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/handlers/VaultAddEditLoginTypeHandlers.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/handlers/VaultAddEditLoginTypeHandlers.kt @@ -42,6 +42,8 @@ data class VaultAddEditLoginTypeHandlers( val onAddNewUriClick: () -> Unit, val onPasswordVisibilityChange: (Boolean) -> Unit, val onClearFido2CredentialClick: () -> Unit, + val onStartLoginCoachMarkTour: () -> Unit, + val onDismissLearnAboutLoginsCard: () -> Unit, ) { @Suppress("UndocumentedPublicClass") companion object { @@ -123,6 +125,16 @@ data class VaultAddEditLoginTypeHandlers( VaultAddEditAction.ItemType.LoginType.ClearFido2CredentialClick, ) }, + onStartLoginCoachMarkTour = { + viewModel.trySendAction( + VaultAddEditAction.ItemType.LoginType.StartLearnAboutLogins, + ) + }, + onDismissLearnAboutLoginsCard = { + viewModel.trySendAction( + VaultAddEditAction.ItemType.LoginType.LearnAboutLoginsDismissed, + ) + }, ) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0dda881244c..f4f4cc745e0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1121,4 +1121,8 @@ Do you want to switch to this account? Mutual TLS Single tap passkey creation Single tap passkey sign-on + Learn about new logins + We\'ll walk you through the key features to add a new login. + Explore the generator + Learn more about generating secure login credentials with a guided tour. diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt index 56ffce26b97..282c9abed89 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt @@ -1253,4 +1253,65 @@ class SettingsDiskSourceTest { settingsDiskSource.storeCreateSendActionCount(count = 1) assertEquals(1, fakeSharedPreferences.getInt(createActionCountKey, 0)) } + + @Test + fun `getShouldShowAddLoginCoachMark should pull value from SharedPreferences`() { + val hasSeenAddLoginCoachMarkKey = "bwPreferencesStorage:shouldShowAddLoginCoachMark" + fakeSharedPreferences.edit { putBoolean(hasSeenAddLoginCoachMarkKey, true) } + assertTrue(settingsDiskSource.getShouldShowAddLoginCoachMark() == true) + } + + @Test + fun `storeShouldShowAddLoginCoachMark should update SharedPreferences`() { + val hasSeenAddLoginCoachMarkKey = "bwPreferencesStorage:shouldShowAddLoginCoachMark" + settingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = true) + assertTrue( + fakeSharedPreferences.getBoolean( + key = hasSeenAddLoginCoachMarkKey, + defaultValue = false, + ), + ) + } + + @Test + fun `getHasSeenAddLoginCoachMarkFlow emits changes to stored value`() = runTest { + settingsDiskSource.getShouldShowAddLoginCoachMarkFlow().test { + assertNull(awaitItem()) + settingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false) + assertFalse(awaitItem() ?: true) + settingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = true) + assertTrue(awaitItem() ?: false) + } + } + + @Suppress("MaxLineLength") + @Test + fun `getShouldShowGeneratorCoachMarkGeneratorCoachMark should pull value from SharedPreferences`() { + val hasSeenGeneratorCoachMarkKey = "bwPreferencesStorage:shouldShowGeneratorCoachMark" + fakeSharedPreferences.edit { putBoolean(hasSeenGeneratorCoachMarkKey, true) } + assertTrue(settingsDiskSource.getShouldShowGeneratorCoachMark() == true) + } + + @Test + fun `storeShouldShowGeneratorCoachMarkGeneratorCoachMark should update SharedPreferences`() { + val hasSeenGeneratorCoachMarkKey = "bwPreferencesStorage:shouldShowGeneratorCoachMark" + settingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = true) + assertTrue( + fakeSharedPreferences.getBoolean( + key = hasSeenGeneratorCoachMarkKey, + defaultValue = false, + ), + ) + } + + @Test + fun `getHasSeenGeneratorCoachMarkFlow emits changes to stored value`() = runTest { + settingsDiskSource.getShouldShowGeneratorCoachMarkFlow().test { + assertNull(awaitItem()) + settingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false) + assertFalse(awaitItem() ?: true) + settingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = true) + assertTrue(awaitItem() ?: false) + } + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index 4c2f7d64594..460c821190a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -40,9 +40,13 @@ class FakeSettingsDiskSource : SettingsDiskSource { private val mutableHasUserLoggedInOrCreatedAccount = bufferedMutableSharedFlow() + private val mutableHasSeenAddLoginCoachMarkFlow = bufferedMutableSharedFlow() + private val mutableScreenCaptureAllowedFlowMap = mutableMapOf>() + private val mutableHasSeenGeneratorCoachMarkFlow = bufferedMutableSharedFlow() + private var storedAppLanguage: AppLanguage? = null private var storedAppTheme: AppTheme = AppTheme.DEFAULT private val storedLastSyncTime = mutableMapOf() @@ -70,6 +74,8 @@ class FakeSettingsDiskSource : SettingsDiskSource { private var addCipherActionCount: Int? = null private var generatedActionCount: Int? = null private var createSendActionCount: Int? = null + private var hasSeenAddLoginCoachMark: Boolean? = null + private var hasSeenGeneratorCoachMark: Boolean? = null private val mutableShowAutoFillSettingBadgeFlowMap = mutableMapOf>() @@ -390,6 +396,33 @@ class FakeSettingsDiskSource : SettingsDiskSource { createSendActionCount = count } + override fun getShouldShowAddLoginCoachMark(): Boolean? { + return hasSeenAddLoginCoachMark + } + + override fun storeShouldShowAddLoginCoachMark(shouldShow: Boolean?) { + hasSeenAddLoginCoachMark = shouldShow + mutableHasSeenAddLoginCoachMarkFlow.tryEmit(shouldShow) + } + + override fun getShouldShowAddLoginCoachMarkFlow(): Flow = + mutableHasSeenAddLoginCoachMarkFlow.onSubscription { + emit(getShouldShowAddLoginCoachMark()) + } + + override fun getShouldShowGeneratorCoachMark(): Boolean? = + hasSeenGeneratorCoachMark + + override fun storeShouldShowGeneratorCoachMark(shouldShow: Boolean?) { + hasSeenGeneratorCoachMark = shouldShow + mutableHasSeenGeneratorCoachMarkFlow.tryEmit(shouldShow) + } + + override fun getShouldShowGeneratorCoachMarkFlow(): Flow = + mutableHasSeenGeneratorCoachMarkFlow.onSubscription { + emit(hasSeenGeneratorCoachMark) + } + //region Private helper functions private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow { return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) { 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 f5f9eae4fd2..964f39ad66d 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 @@ -11,6 +11,7 @@ import com.x8bit.bitwarden.data.auth.datasource.network.model.UserDecryptionOpti 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.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 @@ -22,6 +23,7 @@ import kotlinx.coroutines.flow.update import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.time.ZonedDateTime @@ -35,8 +37,10 @@ class FirstTimeActionManagerTest { } private val mutableImportLoginsFlow = MutableStateFlow(false) + private val mutableOnboardingFeatureFlow = MutableStateFlow(false) private val featureFlagManager = mockk { every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlow + every { getFeatureFlagFlow(FlagKey.OnboardingFlow) } returns mutableOnboardingFeatureFlow } private val mutableAutofillEnabledFlow = MutableStateFlow(false) @@ -296,6 +300,70 @@ class FirstTimeActionManagerTest { ) } } + + @Test + fun `shouldShowAddLoginCoachMarkFlow updates when disk source updates`() = runTest { + // Enable the feature for this test. + mutableOnboardingFeatureFlow.update { true } + firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test { + // Null will be mapped to false. + assertTrue(awaitItem()) + fakeSettingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false) + assertFalse(awaitItem()) + } + } + + @Test + fun `shouldShowAddLoginCoachMarkFlow updates when feature flag for onboarding updates`() = + runTest { + firstTimeActionManager.shouldShowAddLoginCoachMarkFlow.test { + // Null will be mapped to false but feature being "off" will override to true. + assertFalse(awaitItem()) + mutableOnboardingFeatureFlow.update { true } + // Will use the value from disk source (null ?: false). + assertTrue(awaitItem()) + } + } + + @Suppress("MaxLineLength") + @Test + fun `markCoachMarkTourCompleted for the ADD_LOGIN type sets the value to true in the disk source for should show add logins coach mark`() { + assertNull(fakeSettingsDiskSource.getShouldShowAddLoginCoachMark()) + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.ADD_LOGIN) + assertTrue(fakeSettingsDiskSource.getShouldShowAddLoginCoachMark() == false) + } + + @Test + fun `shouldShowGeneratorCoachMarkFlow updates when disk source updates`() = runTest { + // Enable feature flag so flow emits updates from disk. + mutableOnboardingFeatureFlow.update { true } + firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test { + // Null will be mapped to false. + assertTrue(awaitItem()) + fakeSettingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false) + assertFalse(awaitItem()) + } + } + + @Test + fun `shouldShowGeneratorCoachMarkFlow updates when onboarding feature value changes`() = + runTest { + firstTimeActionManager.shouldShowGeneratorCoachMarkFlow.test { + // Null will be mapped to false + assertFalse(awaitItem()) + mutableOnboardingFeatureFlow.update { true } + // Take the value from disk. + assertTrue(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`() { + assertNull(fakeSettingsDiskSource.getShouldShowGeneratorCoachMark()) + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.GENERATOR) + assertTrue(fakeSettingsDiskSource.getShouldShowGeneratorCoachMark() == false) + } } private const val USER_ID: String = "userId" diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt index c70d39c797a..99c8c69b0b2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorScreenTest.kt @@ -10,9 +10,11 @@ import androidx.compose.ui.test.assertIsOn import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.filterToOne import androidx.compose.ui.test.hasAnyAncestor +import androidx.compose.ui.test.hasAnySibling import androidx.compose.ui.test.hasClickAction import androidx.compose.ui.test.hasContentDescription import androidx.compose.ui.test.hasProgressBarRangeInfo +import androidx.compose.ui.test.hasText import androidx.compose.ui.test.isDialog import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onChildren @@ -1521,6 +1523,89 @@ class GeneratorScreenTest : BaseComposeTest() { verify { viewModel.trySendAction(GeneratorAction.LifecycleResume) } } + @Suppress("MaxLineLength") + @Test + fun `Explore generator card shows when default mode and shouldShowCoachMarkTour is true`() { + updateState( + state = DEFAULT_STATE.copy(shouldShowCoachMarkTour = true), + ) + composeTestRule + .onNodeWithText("Explore the generator") + .assertIsDisplayed() + } + + @Suppress("MaxLineLength") + @Test + fun `Explore generator card does not show when default mode and shouldShowCoachMarkTour is false`() { + updateState( + state = DEFAULT_STATE.copy(shouldShowCoachMarkTour = false), + ) + composeTestRule + .onNodeWithText("Explore the generator") + .assertDoesNotExist() + } + + @Suppress("MaxLineLength") + @Test + fun `Explore generator card does not show when modal mode and shouldShowCoachMarkTour is true`() { + updateState( + state = DEFAULT_STATE.copy( + shouldShowCoachMarkTour = true, + generatorMode = GeneratorMode.Modal.Password, + ), + ) + composeTestRule + .onNodeWithText("Explore the generator") + .assertDoesNotExist() + } + + @Suppress("MaxLineLength") + @Test + fun `Explore generator card does not when default mode, shouldShowCoachMarkTour is true, and main type is not password`() { + updateState( + state = DEFAULT_STATE.copy( + shouldShowCoachMarkTour = true, + generatorMode = GeneratorMode.Modal.Password, + selectedType = GeneratorState.MainType.Username(), + ), + ) + composeTestRule + .onNodeWithText("Explore the generator") + .assertDoesNotExist() + } + + @Test + fun `Clicking close button on generator card send ExploreGeneratorCardDismissed action`() { + updateState( + state = DEFAULT_STATE.copy(shouldShowCoachMarkTour = true), + ) + composeTestRule + .onNode( + hasContentDescription("Close") + and hasAnySibling(hasText("Explore the generator")), + ) + .performClick() + + verify(exactly = 1) { + viewModel.trySendAction(GeneratorAction.ExploreGeneratorCardDismissed) + } + } + + @Suppress("MaxLineLength") + @Test + fun `Clicking call to action button on generator card send StartExploreGeneratorTour action`() { + updateState( + state = DEFAULT_STATE.copy(shouldShowCoachMarkTour = true), + ) + composeTestRule + .onNodeWithText("Get started") + .performClick() + + verify(exactly = 1) { + viewModel.trySendAction(GeneratorAction.StartExploreGeneratorTour) + } + } + //endregion Random Word Tests private fun updateState(state: GeneratorState) { @@ -1532,4 +1617,5 @@ private val DEFAULT_STATE = GeneratorState( generatedText = "", selectedType = GeneratorState.MainType.Password(), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = false, ) 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 057d1bd61a6..a6364cb67dd 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,9 +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.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.CoachMarkTourType import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow @@ -37,11 +39,14 @@ import io.mockk.mockk import io.mockk.runs import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.test.runTest import kotlinx.serialization.json.JsonNull import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test @@ -109,6 +114,12 @@ class GeneratorViewModelTest : BaseViewModelTest() { every { registerGeneratedResultAction() } just runs } + private val mutableShouldShowGeneratorCoachMarkFlow = MutableStateFlow(true) + private val firstTimeActionManager: FirstTimeActionManager = mockk { + every { markCoachMarkTourCompleted(CoachMarkTourType.GENERATOR) } just runs + every { shouldShowGeneratorCoachMarkFlow } returns mutableShouldShowGeneratorCoachMarkFlow + } + @Test fun `initial state should be correct when there is no saved state`() { val viewModel = createViewModel(state = null) @@ -139,6 +150,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { currentEmailAddress = "currentEmail", isUnderPolicy = false, website = "", + shouldShowCoachMarkTour = true, ) val viewModel = createViewModel( @@ -174,6 +186,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { currentEmailAddress = "currentEmail", isUnderPolicy = false, website = null, + shouldShowCoachMarkTour = true, ) val viewModel = createViewModel( @@ -2160,6 +2173,67 @@ class GeneratorViewModelTest : BaseViewModelTest() { ) assertEquals(10, password.computedMinimumLength) } + + @Suppress("MaxLineLength") + @Test + fun `when first time action manager should show generator tour value updates to false shouldShowExploreGeneratorCard should update to false`() { + val viewModel = createViewModel() + assertTrue(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) + mutableShouldShowGeneratorCoachMarkFlow.update { false } + assertFalse(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) + } + + @Suppress("MaxLineLength") + @Test + fun `shouldShowExploreGeneratorCard value should be false if generator screen is in modal mode`() { + mutableShouldShowGeneratorCoachMarkFlow.update { false } + val viewModel = createViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = createPasswordState().copy( + shouldShowCoachMarkTour = false, + generatorMode = GeneratorMode.Modal.Password, + ), + ), + ) + + assertFalse(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) + } + + @Suppress("MaxLineLength") + @Test + fun `shouldShowExploreGeneratorCard value should be false if generator screen selected type is not password`() { + mutableShouldShowGeneratorCoachMarkFlow.update { false } + val viewModel = createViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = createUsernameModeState().copy( + shouldShowCoachMarkTour = true, + ), + ), + ) + + assertFalse(viewModel.stateFlow.value.shouldShowExploreGeneratorCard) + } + + @Suppress("MaxLineLength") + @Test + fun `ExploreGeneratorCardDismissed action calls first time action manager hasSeenGeneratorCoachMarkTour called`() { + val viewModel = createViewModel() + + viewModel.trySendAction(GeneratorAction.ExploreGeneratorCardDismissed) + verify(exactly = 1) { + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.GENERATOR) + } + } + + @Suppress("MaxLineLength") + @Test + fun `StartExploreGeneratorTour action calls first time action manager hasSeenGeneratorCoachMarkTour called and show coach mark event sent`() { + val viewModel = createViewModel() + viewModel.trySendAction(GeneratorAction.StartExploreGeneratorTour) + verify(exactly = 1) { + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.GENERATOR) + } + } //region Helper Functions @Suppress("LongParameterList") @@ -2187,6 +2261,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { avoidAmbiguousChars = avoidAmbiguousChars, ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createPassphraseState( @@ -2205,6 +2280,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { includeNumber = includeNumber, ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createUsernameModeState( @@ -2220,6 +2296,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createForwardedEmailAliasState( @@ -2235,6 +2312,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createAddyIoState( @@ -2249,6 +2327,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createDuckDuckGoState( @@ -2263,6 +2342,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createFastMailState( @@ -2277,6 +2357,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createFirefoxRelayState( @@ -2291,6 +2372,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createForwardEmailState( @@ -2305,6 +2387,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createSimpleLoginState( @@ -2319,6 +2402,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createPlusAddressedEmailState( @@ -2333,6 +2417,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createCatchAllEmailState( @@ -2347,6 +2432,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createRandomWordState( @@ -2363,6 +2449,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { ), ), currentEmailAddress = "currentEmail", + shouldShowCoachMarkTour = true, ) private fun createSavedStateHandleWithState(state: GeneratorState) = @@ -2379,6 +2466,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { authRepository = authRepository, policyManager = policyManager, reviewPromptManager = reviewPromptManager, + firstTimeActionManager = firstTimeActionManager, ) private fun createViewModel( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt index 00eba61a2bd..892fdc71000 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditScreenTest.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.test.click import androidx.compose.ui.test.filter import androidx.compose.ui.test.filterToOne import androidx.compose.ui.test.hasAnyAncestor +import androidx.compose.ui.test.hasAnySibling import androidx.compose.ui.test.hasContentDescription import androidx.compose.ui.test.hasSetTextAction import androidx.compose.ui.test.hasText @@ -3560,6 +3561,101 @@ class VaultAddEditScreenTest : BaseComposeTest() { .assertDoesNotExist() } + @Suppress("MaxLineLength") + @Test + fun `learn about add logins card should show when state is add mode, login type content, and should show coach mark tour is true`() { + mutableStateFlow.value = DEFAULT_STATE_LOGIN.copy( + vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN), + shouldShowCoachMarkTour = true, + ) + + composeTestRule + .onNodeWithText("Learn about new logins") + .assertIsDisplayed() + } + + @Suppress("MaxLineLength") + @Test + fun `learn about add logins card should not show when state is add mode, login type content, and should show coach mark tour is false`() { + mutableStateFlow.value = DEFAULT_STATE_LOGIN.copy( + vaultAddEditType = VaultAddEditType.AddItem( + vaultItemCipherType = VaultItemCipherType.LOGIN, + ), + shouldShowCoachMarkTour = false, + ) + + composeTestRule + .onNodeWithText("Learn about new logins") + .assertDoesNotExist() + } + + @Suppress("MaxLineLength") + @Test + fun `learn about add logins card should not show when state is edit mode, login type content, and should show coach mark tour is true`() { + mutableStateFlow.value = DEFAULT_STATE_LOGIN.copy( + vaultAddEditType = VaultAddEditType.EditItem(vaultItemId = "1234"), + shouldShowCoachMarkTour = true, + ) + + composeTestRule + .onNodeWithText("Learn about new logins") + .assertDoesNotExist() + } + + @Suppress("MaxLineLength") + @Test + fun `learn about add logins card should not show when state is add mode, card type content, and should show coach mark tour is true`() { + mutableStateFlow.value = DEFAULT_STATE_CARD.copy( + vaultAddEditType = VaultAddEditType.EditItem(vaultItemId = "1234"), + shouldShowCoachMarkTour = false, + ) + + composeTestRule + .onNodeWithText("Learn about new logins") + .assertDoesNotExist() + } + + @Suppress("MaxLineLength") + @Test + fun `when learn about logins card is showing, clicking the close button sends LearnAboutLoginsDismissed action`() { + mutableStateFlow.value = DEFAULT_STATE_LOGIN.copy( + vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN), + shouldShowCoachMarkTour = true, + ) + + composeTestRule + .onNode( + hasContentDescription("Close") + and hasAnySibling(hasText("Learn about new logins")), + ) + .performClick() + + verify(exactly = 1) { + viewModel.trySendAction( + VaultAddEditAction.ItemType.LoginType.LearnAboutLoginsDismissed, + ) + } + } + + @Suppress("MaxLineLength") + @Test + fun `when learn about logins card is showing, clicking the call to action sends StartLearnAboutLogins action`() { + mutableStateFlow.value = DEFAULT_STATE_LOGIN.copy( + vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN), + shouldShowCoachMarkTour = true, + ) + + composeTestRule + .onNodeWithText("Get started") + .performClick() + + verify(exactly = 1) { + viewModel.trySendAction( + VaultAddEditAction.ItemType.LoginType.StartLearnAboutLogins, + ) + } + } + //region Helper functions private fun updateLoginType( @@ -3685,6 +3781,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { dialog = VaultAddEditState.DialogState.Generic(message = "test".asText()), vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN), supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val DEFAULT_STATE_LOGIN = VaultAddEditState( @@ -3696,6 +3793,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { ), dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val DEFAULT_STATE_IDENTITY = VaultAddEditState( @@ -3707,6 +3805,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { ), dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val DEFAULT_STATE_CARD = VaultAddEditState( @@ -3718,6 +3817,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { ), dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) @Suppress("MaxLineLength") @@ -3740,6 +3840,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { dialog = null, vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.SECURE_NOTE), supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val DEFAULT_STATE_SECURE_NOTES = VaultAddEditState( @@ -3751,6 +3852,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { ), dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val DEFAULT_STATE_SSH_KEYS = VaultAddEditState( @@ -3762,6 +3864,7 @@ class VaultAddEditScreenTest : BaseComposeTest() { ), dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries, + shouldShowCoachMarkTour = false, ) private val ALTERED_COLLECTIONS = listOf( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt index 4e6119ec366..e42021a7994 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt @@ -25,11 +25,13 @@ import com.x8bit.bitwarden.data.autofill.fido2.model.createMockFido2CreateCreden import com.x8bit.bitwarden.data.autofill.model.AutofillSaveItem import com.x8bit.bitwarden.data.autofill.model.AutofillSelectionData import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager +import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManagerImpl import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager +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.OrganizationEvent import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance @@ -87,10 +89,13 @@ import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test @@ -163,6 +168,12 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { every { isNetworkConnected } returns true } + private val mutableShouldShowAddLoginCoachMarkFlow = MutableStateFlow(false) + private val firstTimeActionManager = mockk { + every { markCoachMarkTourCompleted(CoachMarkTourType.ADD_LOGIN) } just runs + every { shouldShowAddLoginCoachMarkFlow } returns mutableShouldShowAddLoginCoachMarkFlow + } + @BeforeEach fun setup() { mockkStatic(CipherView::toViewState) @@ -191,6 +202,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { shouldExitOnSave = false, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }, + shouldShowCoachMarkTour = false, ) val viewModel = createAddVaultItemViewModel( savedStateHandle = createSavedStateHandleWithState( @@ -273,6 +285,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { dialog = null, supportedItemTypes = VaultAddEditState.ItemTypeOption.entries .filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }, + shouldShowCoachMarkTour = false, ), viewModel.stateFlow.value, ) @@ -2601,6 +2614,90 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { } } + @Suppress("MaxLineLength") + @Test + fun `when first time action manager should show logins tour value updates to false shouldShowLearnAboutNewLogins should update to false`() { + mutableShouldShowAddLoginCoachMarkFlow.update { true } + val viewModel = createAddVaultItemViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = createVaultAddItemState( + typeContentViewState = createLoginTypeContentViewState(), + ), + vaultAddEditType = VaultAddEditType.AddItem( + vaultItemCipherType = VaultItemCipherType.LOGIN, + ), + ), + ) + assertTrue(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) + mutableShouldShowAddLoginCoachMarkFlow.update { false } + assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) + } + + @Suppress("MaxLineLength") + @Test + fun `when first time action manager value is true, but type content is not login shouldShowLearnAboutNewLogins should be false`() { + mutableShouldShowAddLoginCoachMarkFlow.update { true } + val viewModel = createAddVaultItemViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = createVaultAddItemState( + typeContentViewState = createLoginTypeContentViewState(), + ), + vaultAddEditType = VaultAddEditType.AddItem( + vaultItemCipherType = VaultItemCipherType.LOGIN, + ), + ), + ) + assertTrue(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) + viewModel.trySendAction( + VaultAddEditAction.Common.TypeOptionSelect( + VaultAddEditState.ItemTypeOption.SSH_KEYS, + ), + ) + assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) + } + + @Suppress("MaxLineLength") + @Test + fun `when first time action manager value is false, but edit type is EditItem shouldShowLearnAboutNewLogins should be false`() { + val viewModel = createAddVaultItemViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = createVaultAddItemState( + vaultAddEditType = VaultAddEditType.EditItem(vaultItemId = "1234"), + typeContentViewState = createLoginTypeContentViewState(), + ), + vaultAddEditType = VaultAddEditType.EditItem( + vaultItemId = "1234", + ), + ), + ) + assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins) + } + + @Suppress("MaxLineLength") + @Test + fun `LearnAboutLoginsDismissed action calls first time action manager hasSeenAddLoginCoachMarkTour called`() { + val viewModel = createAddVaultItemViewModel() + + viewModel.trySendAction(VaultAddEditAction.ItemType.LoginType.LearnAboutLoginsDismissed) + verify(exactly = 1) { + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.ADD_LOGIN) + } + } + + @Suppress("MaxLineLength") + @Test + fun `StartLearnAboutLogins action calls first time action manager hasSeenAddLoginCoachMarkTour called and show coach mark event sent`() = + runTest { + val viewModel = createAddVaultItemViewModel() + viewModel.eventFlow.test { + viewModel.trySendAction(VaultAddEditAction.ItemType.LoginType.StartLearnAboutLogins) + assertEquals(VaultAddEditEvent.StartAddLoginItemCoachMarkTour, awaitItem()) + } + verify(exactly = 1) { + firstTimeActionManager.markCoachMarkTourCompleted(CoachMarkTourType.ADD_LOGIN) + } + } + @Nested inner class VaultAddEditIdentityTypeItemActions { private lateinit var viewModel: VaultAddEditViewModel @@ -3126,6 +3223,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { clock = fixedClock, organizationEventManager = organizationEventManager, networkConnectionManager = networkConnectionManager, + firstTimeActionManager = firstTimeActionManager, ) } @@ -4276,6 +4374,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { shouldExitOnSave = shouldExitOnSave, totpData = totpData, supportedItemTypes = supportedItemTypes, + shouldShowCoachMarkTour = false, ) @Suppress("LongParameterList") @@ -4385,6 +4484,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() { clock = clock, organizationEventManager = organizationEventManager, networkConnectionManager = networkConnectionManager, + firstTimeActionManager = firstTimeActionManager, ) private fun createVaultData( From b1c4d302f69b4e73b700cef7557c646cd04aac2c Mon Sep 17 00:00:00 2001 From: Dave Severns Date: Fri, 24 Jan 2025 10:58:01 -0500 Subject: [PATCH 2/2] naming --- .../platform/datasource/disk/SettingsDiskSource.kt | 4 ++-- .../datasource/disk/SettingsDiskSourceImpl.kt | 4 ++-- .../tools/feature/generator/GeneratorViewModel.kt | 10 +++++----- .../vault/feature/addedit/VaultAddEditViewModel.kt | 6 +++--- .../datasource/disk/SettingsDiskSourceTest.kt | 4 ++-- .../datasource/disk/util/FakeSettingsDiskSource.kt | 13 +++++++------ .../feature/generator/GeneratorViewModelTest.kt | 2 +- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt index dad2afc3841..976ed2fee6d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt @@ -373,7 +373,7 @@ interface SettingsDiskSource { fun storeShouldShowAddLoginCoachMark(shouldShow: Boolean?) /** - * Returns an [Flow] to observe updates to the "HasSeenAddLoginCoachMark" value. + * Returns an [Flow] to observe updates to the "ShouldShowAddLoginCoachMark" value. */ fun getShouldShowAddLoginCoachMarkFlow(): Flow @@ -388,7 +388,7 @@ interface SettingsDiskSource { fun storeShouldShowGeneratorCoachMark(shouldShow: Boolean?) /** - * Returns an [Flow] to observe updates to the "HasSeenGeneratorCoachMark" value. + * Returns an [Flow] to observe updates to the "ShouldShowGeneratorCoachMark" value. */ fun getShouldShowGeneratorCoachMarkFlow(): Flow } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt index a8299e6276b..0a15d2864e4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt @@ -191,8 +191,8 @@ class SettingsDiskSourceImpl( // - screen capture allowed // - show autofill setting badge // - show unlock setting badge - // - has seen add login coach mark - // - has seen generator coach mark + // - should show add login coach mark + // - should show generator coach mark } override fun getAccountBiometricIntegrityValidity( 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 e62775490b3..b762063fc41 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 @@ -128,9 +128,9 @@ class GeneratorViewModel @Inject constructor( firstTimeActionManager .shouldShowGeneratorCoachMarkFlow - .map { hasSeenCoachMarkTour -> + .map { shouldShowCoachMarkTour -> GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive( - shouldShowCoachMarkTour = hasSeenCoachMarkTour, + shouldShowCoachMarkTour = shouldShowCoachMarkTour, ) } .onEach(::sendAction) @@ -266,7 +266,7 @@ class GeneratorViewModel @Inject constructor( } is GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive -> { - handleHasSeenCoachMarkValueChange(action) + handleShouldShowCoachMarkValueChange(action) } } } @@ -781,7 +781,7 @@ class GeneratorViewModel @Inject constructor( } } - private fun handleHasSeenCoachMarkValueChange( + private fun handleShouldShowCoachMarkValueChange( action: GeneratorAction.Internal.ShouldShowGeneratorCoachMarkValueChangeReceive, ) { mutableStateFlow.update { @@ -2610,7 +2610,7 @@ sealed class GeneratorAction { ) : Internal() /** - * The value for the hasSeenGeneratorCoachMark has changed. + * The value for the shouldShowGeneratorCoachMark has changed. */ data class ShouldShowGeneratorCoachMarkValueChangeReceive( val shouldShowCoachMarkTour: Boolean, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt index 01cef046703..2f748549f06 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt @@ -1484,12 +1484,12 @@ class VaultAddEditViewModel @Inject constructor( } is VaultAddEditAction.Internal.ShouldShowAddLoginCoachMarkValueChangeReceive -> { - handleHasSeenAddLoginCoachMarkValueChange(action) + handleShouldShowAddLoginCoachMarkValueChange(action) } } } - private fun handleHasSeenAddLoginCoachMarkValueChange( + private fun handleShouldShowAddLoginCoachMarkValueChange( action: VaultAddEditAction.Internal.ShouldShowAddLoginCoachMarkValueChangeReceive, ) { mutableStateFlow.update { @@ -3207,7 +3207,7 @@ sealed class VaultAddEditAction { ) : Internal() /** - * The value for the hasSeenAddLoginCoachMark has changed. + * The value for the shouldShowAddLoginCoachMark has changed. */ data class ShouldShowAddLoginCoachMarkValueChangeReceive( val shouldShowCoachMarkTour: Boolean, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt index 282c9abed89..ae924034371 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt @@ -1274,7 +1274,7 @@ class SettingsDiskSourceTest { } @Test - fun `getHasSeenAddLoginCoachMarkFlow emits changes to stored value`() = runTest { + fun `getShouldShowAddLoginCoachMarkFlow emits changes to stored value`() = runTest { settingsDiskSource.getShouldShowAddLoginCoachMarkFlow().test { assertNull(awaitItem()) settingsDiskSource.storeShouldShowAddLoginCoachMark(shouldShow = false) @@ -1305,7 +1305,7 @@ class SettingsDiskSourceTest { } @Test - fun `getHasSeenGeneratorCoachMarkFlow emits changes to stored value`() = runTest { + fun `getShouldShowGeneratorCoachMarkFlow emits changes to stored value`() = runTest { settingsDiskSource.getShouldShowGeneratorCoachMarkFlow().test { assertNull(awaitItem()) settingsDiskSource.storeShouldShowGeneratorCoachMark(shouldShow = false) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index 460c821190a..16d011aeccf 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -40,12 +40,13 @@ class FakeSettingsDiskSource : SettingsDiskSource { private val mutableHasUserLoggedInOrCreatedAccount = bufferedMutableSharedFlow() - private val mutableHasSeenAddLoginCoachMarkFlow = bufferedMutableSharedFlow() + private val mutableShouldShowAddLoginCoachMarkFlow = bufferedMutableSharedFlow() private val mutableScreenCaptureAllowedFlowMap = mutableMapOf>() - private val mutableHasSeenGeneratorCoachMarkFlow = bufferedMutableSharedFlow() + private val mutableShouldShowGeneratorCoachMarkFlow = + bufferedMutableSharedFlow() private var storedAppLanguage: AppLanguage? = null private var storedAppTheme: AppTheme = AppTheme.DEFAULT @@ -402,11 +403,11 @@ class FakeSettingsDiskSource : SettingsDiskSource { override fun storeShouldShowAddLoginCoachMark(shouldShow: Boolean?) { hasSeenAddLoginCoachMark = shouldShow - mutableHasSeenAddLoginCoachMarkFlow.tryEmit(shouldShow) + mutableShouldShowAddLoginCoachMarkFlow.tryEmit(shouldShow) } override fun getShouldShowAddLoginCoachMarkFlow(): Flow = - mutableHasSeenAddLoginCoachMarkFlow.onSubscription { + mutableShouldShowAddLoginCoachMarkFlow.onSubscription { emit(getShouldShowAddLoginCoachMark()) } @@ -415,11 +416,11 @@ class FakeSettingsDiskSource : SettingsDiskSource { override fun storeShouldShowGeneratorCoachMark(shouldShow: Boolean?) { hasSeenGeneratorCoachMark = shouldShow - mutableHasSeenGeneratorCoachMarkFlow.tryEmit(shouldShow) + mutableShouldShowGeneratorCoachMarkFlow.tryEmit(shouldShow) } override fun getShouldShowGeneratorCoachMarkFlow(): Flow = - mutableHasSeenGeneratorCoachMarkFlow.onSubscription { + mutableShouldShowGeneratorCoachMarkFlow.onSubscription { emit(hasSeenGeneratorCoachMark) } 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 a6364cb67dd..b257e7e7988 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 @@ -2216,7 +2216,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `ExploreGeneratorCardDismissed action calls first time action manager hasSeenGeneratorCoachMarkTour called`() { + fun `ExploreGeneratorCardDismissed action calls first time action manager markCoachMarkTourCompleted with GENERATOR value called`() { val viewModel = createViewModel() viewModel.trySendAction(GeneratorAction.ExploreGeneratorCardDismissed)