From c704cd2eca40e7eefc3e7db754312c33a90cec03 Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Mon, 21 Oct 2024 16:51:49 -0400 Subject: [PATCH] PM-13627 show action card on vault settings if applicable (#4101) --- .../settings/vault/VaultSettingsScreen.kt | 35 ++++++++ .../settings/vault/VaultSettingsViewModel.kt | 83 ++++++++++++++++++- .../settings/vault/VaultSettingsScreenTest.kt | 56 +++++++++++++ .../vault/VaultSettingsViewModelTest.kt | 70 ++++++++++++++++ 4 files changed, 240 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreen.kt index caaba598aa..7ef8505ac2 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreen.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.vault import android.widget.Toast +import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth @@ -18,12 +19,17 @@ import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp import androidx.core.net.toUri import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect +import com.x8bit.bitwarden.ui.platform.base.util.standardHorizontalMargin import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar +import com.x8bit.bitwarden.ui.platform.components.badge.NotificationBadge +import com.x8bit.bitwarden.ui.platform.components.card.BitwardenActionCard +import com.x8bit.bitwarden.ui.platform.components.card.actionCardExitAnimation import com.x8bit.bitwarden.ui.platform.components.row.BitwardenExternalLinkRow import com.x8bit.bitwarden.ui.platform.components.row.BitwardenTextRow import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold @@ -90,6 +96,35 @@ fun VaultSettingsScreen( .fillMaxSize() .verticalScroll(rememberScrollState()), ) { + AnimatedVisibility( + visible = state.shouldShowImportCard, + label = "ImportLoginsActionCard", + exit = actionCardExitAnimation(), + ) { + BitwardenActionCard( + cardTitle = stringResource(id = R.string.import_saved_logins), + actionText = stringResource(R.string.get_started), + cardSubtitle = stringResource(R.string.use_a_computer_to_import_logins), + onActionClick = remember(viewModel) { + { + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardCtaClick) + } + }, + onDismissClick = remember(viewModel) { + { + viewModel.trySendAction( + VaultSettingsAction.ImportLoginsCardDismissClick, + ) + } + }, + leadingContent = { + NotificationBadge(notificationCount = 1) + }, + modifier = Modifier + .standardHorizontalMargin() + .padding(top = 12.dp, bottom = 16.dp), + ) + } BitwardenTextRow( text = stringResource(R.string.folders), onClick = remember(viewModel) { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModel.kt index e921fc1361..1db7865eda 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModel.kt @@ -1,7 +1,9 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.vault import androidx.lifecycle.viewModelScope +import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager +import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.model.FlagKey import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.util.toBaseWebVaultImportUrl @@ -16,12 +18,16 @@ import javax.inject.Inject /** * View model for the vault screen. */ +@Suppress("TooManyFunctions") @HiltViewModel class VaultSettingsViewModel @Inject constructor( environmentRepository: EnvironmentRepository, - val featureFlagManager: FeatureFlagManager, + featureFlagManager: FeatureFlagManager, + private val authRepository: AuthRepository, + private val firstTimeActionManager: FirstTimeActionManager, ) : BaseViewModel( initialState = run { + val firstTimeState = firstTimeActionManager.currentOrDefaultUserFirstTimeState VaultSettingsState( importUrl = environmentRepository .environment @@ -29,6 +35,7 @@ class VaultSettingsViewModel @Inject constructor( .toBaseWebVaultImportUrl, isNewImportLoginsFlowEnabled = featureFlagManager .getFeatureFlag(FlagKey.ImportLoginsFlow), + showImportActionCard = firstTimeState.showImportLoginsCard, ) }, ) { @@ -38,6 +45,16 @@ class VaultSettingsViewModel @Inject constructor( .map { VaultSettingsAction.Internal.ImportLoginsFeatureFlagChanged(it) } .onEach(::sendAction) .launchIn(viewModelScope) + + firstTimeActionManager + .firstTimeStateFlow + .map { + VaultSettingsAction.Internal.UserFirstTimeStateChanged( + showImportLoginsCard = it.showImportLoginsCard, + ) + } + .onEach(::sendAction) + .launchIn(viewModelScope) } override fun handleAction(action: VaultSettingsAction): Unit = when (action) { @@ -45,8 +62,37 @@ class VaultSettingsViewModel @Inject constructor( VaultSettingsAction.ExportVaultClick -> handleExportVaultClicked() VaultSettingsAction.FoldersButtonClick -> handleFoldersButtonClicked() VaultSettingsAction.ImportItemsClick -> handleImportItemsClicked() - is VaultSettingsAction.Internal.ImportLoginsFeatureFlagChanged -> { - handleImportLoginsFeatureFlagChanged(action) + VaultSettingsAction.ImportLoginsCardCtaClick -> handleImportLoginsCardClicked() + VaultSettingsAction.ImportLoginsCardDismissClick -> handleImportLoginsCardDismissClicked() + is VaultSettingsAction.Internal -> handleInternalAction(action) + } + + private fun handleInternalAction(action: VaultSettingsAction.Internal) { + when (action) { + is VaultSettingsAction.Internal.ImportLoginsFeatureFlagChanged -> { + handleImportLoginsFeatureFlagChanged(action) + } + + is VaultSettingsAction.Internal.UserFirstTimeStateChanged -> { + handleUserFirstTimeStateChanged(action) + } + } + } + + private fun handleImportLoginsCardDismissClicked() { + dismissImportLoginsCard() + } + + private fun handleImportLoginsCardClicked() { + dismissImportLoginsCard() + sendEvent(VaultSettingsEvent.NavigateToImportVault(state.importUrl)) + } + + private fun handleUserFirstTimeStateChanged( + action: VaultSettingsAction.Internal.UserFirstTimeStateChanged, + ) { + mutableStateFlow.update { + it.copy(showImportActionCard = action.showImportLoginsCard) } } @@ -75,6 +121,11 @@ class VaultSettingsViewModel @Inject constructor( VaultSettingsEvent.NavigateToImportVault(state.importUrl), ) } + + private fun dismissImportLoginsCard() { + if (!state.shouldShowImportCard) return + authRepository.setShowImportLogins(showImportLogins = false) + } } /** @@ -83,7 +134,14 @@ class VaultSettingsViewModel @Inject constructor( data class VaultSettingsState( val importUrl: String, val isNewImportLoginsFlowEnabled: Boolean, -) + private val showImportActionCard: Boolean, +) { + /** + * Should only show the import action card if the import logins feature flag is enabled. + */ + val shouldShowImportCard: Boolean + get() = showImportActionCard && isNewImportLoginsFlowEnabled +} /** * Models events for the vault screen. @@ -141,6 +199,16 @@ sealed class VaultSettingsAction { */ data object ImportItemsClick : VaultSettingsAction() + /** + * Indicates that the user clicked the CTA on the action card. + */ + data object ImportLoginsCardCtaClick : VaultSettingsAction() + + /** + * Indicates that the user dismissed the action card. + */ + data object ImportLoginsCardDismissClick : VaultSettingsAction() + /** * Internal actions not performed by user interation */ @@ -152,5 +220,12 @@ sealed class VaultSettingsAction { data class ImportLoginsFeatureFlagChanged( val isEnabled: Boolean, ) : Internal() + + /** + * Indicates user first time state has changed. + */ + data class UserFirstTimeStateChanged( + val showImportLoginsCard: Boolean, + ) : Internal() } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreenTest.kt index 3857b499fa..d550c3a970 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsScreenTest.kt @@ -7,6 +7,7 @@ import androidx.compose.ui.test.isDialog import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollTo import androidx.core.net.toUri import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest @@ -34,6 +35,7 @@ class VaultSettingsScreenTest : BaseComposeTest() { VaultSettingsState( importUrl = "testUrl/#/tools/import", isNewImportLoginsFlowEnabled = false, + showImportActionCard = false, ), ) private val intentManager: IntentManager = mockk(relaxed = true) { @@ -157,4 +159,58 @@ class VaultSettingsScreenTest : BaseComposeTest() { assertTrue(onNavigateToImportLoginsCalled) verify(exactly = 0) { intentManager.launchUri(testUrl.toUri()) } } + + @Test + fun `when new show action card is true the import logins card should show`() { + mutableStateFlow.update { + it.copy( + showImportActionCard = true, + isNewImportLoginsFlowEnabled = true, + ) + } + composeTestRule + .onNodeWithText("Import saved logins") + .performScrollTo() + .assertIsDisplayed() + mutableStateFlow.update { + it.copy(showImportActionCard = false) + } + composeTestRule + .onNodeWithText("Import saved logins") + .assertDoesNotExist() + } + + @Test + fun `when action card is visible clicking the close icon should send correct action`() { + mutableStateFlow.update { + it.copy( + showImportActionCard = true, + isNewImportLoginsFlowEnabled = true, + ) + } + composeTestRule + .onNodeWithContentDescription("Close") + .performScrollTo() + .performClick() + verify { + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardDismissClick) + } + } + + @Test + fun `when action card is visible get started button should send correct action`() { + mutableStateFlow.update { + it.copy( + showImportActionCard = true, + isNewImportLoginsFlowEnabled = true, + ) + } + composeTestRule + .onNodeWithText("Get started") + .performScrollTo() + .performClick() + verify { + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardCtaClick) + } + } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModelTest.kt index 90ee0d7736..b53fac4256 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/vault/VaultSettingsViewModelTest.kt @@ -1,12 +1,18 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.vault import app.cash.turbine.test +import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager +import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager +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.util.FakeEnvironmentRepository import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import io.mockk.every +import io.mockk.just 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 @@ -22,6 +28,14 @@ class VaultSettingsViewModelTest : BaseViewModelTest() { every { getFeatureFlagFlow(FlagKey.ImportLoginsFlow) } returns mutableImportLoginsFlagFlow every { getFeatureFlag(FlagKey.ImportLoginsFlow) } returns false } + private val authRepository = mockk { + every { setShowImportLogins(any()) } just runs + } + private val mutableFirstTimeStateFlow = MutableStateFlow(DEFAULT_FIRST_TIME_STATE) + private val firstTimeActionManager = mockk { + every { currentOrDefaultUserFirstTimeState } returns DEFAULT_FIRST_TIME_STATE + every { firstTimeStateFlow } returns mutableFirstTimeStateFlow + } @Test fun `BackClick should emit NavigateBack`() = runTest { @@ -67,8 +81,64 @@ class VaultSettingsViewModelTest : BaseViewModelTest() { assertTrue(viewModel.stateFlow.value.isNewImportLoginsFlowEnabled) } + @Test + fun `shouldShowImportCard should update when first time state changes`() = runTest { + mutableImportLoginsFlagFlow.update { true } + val viewModel = createViewModel() + assertTrue(viewModel.stateFlow.value.shouldShowImportCard) + mutableFirstTimeStateFlow.update { + it.copy(showImportLoginsCard = false) + } + assertFalse(viewModel.stateFlow.value.shouldShowImportCard) + } + + @Test + fun `shouldShowImportCard should be false when feature flag not enabled`() = runTest { + val viewModel = createViewModel() + mutableImportLoginsFlagFlow.update { false } + assertFalse(viewModel.stateFlow.value.shouldShowImportCard) + } + + @Suppress("MaxLineLength") + @Test + fun `ImportLoginsCardCtaClick action should set repository value to false and send navigation event`() = + runTest { + val viewModel = createViewModel() + val expected = "https://vault.bitwarden.com/#/tools/import" + mutableImportLoginsFlagFlow.update { true } + viewModel.eventFlow.test { + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardCtaClick) + assertEquals( + VaultSettingsEvent.NavigateToImportVault(url = expected), + awaitItem(), + ) + } + verify(exactly = 1) { authRepository.setShowImportLogins(false) } + } + + @Test + fun `ImportLoginsCardDismissClick action should set repository value to false `() = runTest { + val viewModel = createViewModel() + mutableImportLoginsFlagFlow.update { true } + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardDismissClick) + verify(exactly = 1) { authRepository.setShowImportLogins(false) } + } + + @Suppress("MaxLineLength") + @Test + fun `ImportLoginsCardDismissClick action should not set repository value to false if already false`() = + runTest { + val viewModel = createViewModel() + viewModel.trySendAction(VaultSettingsAction.ImportLoginsCardDismissClick) + verify(exactly = 0) { authRepository.setShowImportLogins(false) } + } + private fun createViewModel(): VaultSettingsViewModel = VaultSettingsViewModel( environmentRepository = environmentRepository, featureFlagManager = featureFlagManager, + authRepository = authRepository, + firstTimeActionManager = firstTimeActionManager, ) } + +private val DEFAULT_FIRST_TIME_STATE = FirstTimeState(showImportLoginsCard = true)