From 40fa3071ae6350f4d3775bdc318d842d7142e1df Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 7 Dec 2023 15:52:10 -0600 Subject: [PATCH] Add new navigation for the edit item screen (#350) --- .../vaultunlocked/VaultUnlockedNavigation.kt | 20 +++-- .../additem/VaultAddEditItemNavigation.kt | 79 +++++++++++++++++++ .../feature/additem/VaultAddItemNavigation.kt | 33 -------- .../feature/additem/VaultAddItemScreen.kt | 28 ++++--- .../feature/additem/VaultAddItemViewModel.kt | 32 +++++--- .../ui/vault/model/VaultAddEditType.kt | 25 ++++++ .../feature/additem/VaultAddItemScreenTest.kt | 3 + .../additem/VaultAddItemViewModelTest.kt | 55 ++++++++++--- 8 files changed, 204 insertions(+), 71 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddEditItemNavigation.kt delete mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemNavigation.kt create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/vault/model/VaultAddEditType.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt index ec36d1b385..948af0453a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/vaultunlocked/VaultUnlockedNavigation.kt @@ -10,12 +10,12 @@ import com.x8bit.bitwarden.ui.platform.feature.vaultunlockednavbar.VAULT_UNLOCKE import com.x8bit.bitwarden.ui.platform.feature.vaultunlockednavbar.vaultUnlockedNavBarDestination import com.x8bit.bitwarden.ui.tools.feature.send.navigateToNewSend import com.x8bit.bitwarden.ui.tools.feature.send.newSendDestination -import com.x8bit.bitwarden.ui.vault.feature.additem.navigateToVaultAddItem -import com.x8bit.bitwarden.ui.vault.feature.additem.vaultAddItemDestination -import com.x8bit.bitwarden.ui.vault.feature.edit.navigateToVaultEditItem +import com.x8bit.bitwarden.ui.vault.feature.additem.navigateToVaultAddEditItem +import com.x8bit.bitwarden.ui.vault.feature.additem.vaultAddEditItemDestination import com.x8bit.bitwarden.ui.vault.feature.edit.vaultEditItemDestination import com.x8bit.bitwarden.ui.vault.feature.item.navigateToVaultItem import com.x8bit.bitwarden.ui.vault.feature.item.vaultItemDestination +import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType const val VAULT_UNLOCKED_GRAPH_ROUTE: String = "vault_unlocked_graph" @@ -37,17 +37,23 @@ fun NavGraphBuilder.vaultUnlockedGraph( route = VAULT_UNLOCKED_GRAPH_ROUTE, ) { vaultUnlockedNavBarDestination( - onNavigateToVaultAddItem = { navController.navigateToVaultAddItem() }, + onNavigateToVaultAddItem = { + navController.navigateToVaultAddEditItem(VaultAddEditType.AddItem) + }, onNavigateToVaultItem = { navController.navigateToVaultItem(it) }, - onNavigateToVaultEditItem = { navController.navigateToVaultEditItem(it) }, + onNavigateToVaultEditItem = { + navController.navigateToVaultAddEditItem(VaultAddEditType.EditItem(it)) + }, onNavigateToNewSend = { navController.navigateToNewSend() }, onNavigateToDeleteAccount = { navController.navigateToDeleteAccount() }, ) deleteAccountDestination(onNavigateBack = { navController.popBackStack() }) - vaultAddItemDestination(onNavigateBack = { navController.popBackStack() }) + vaultAddEditItemDestination(onNavigateBack = { navController.popBackStack() }) vaultItemDestination( onNavigateBack = { navController.popBackStack() }, - onNavigateToVaultEditItem = { navController.navigateToVaultEditItem(it) }, + onNavigateToVaultEditItem = { + navController.navigateToVaultAddEditItem(VaultAddEditType.EditItem(it)) + }, ) vaultEditItemDestination(onNavigateBack = { navController.popBackStack() }) newSendDestination(onNavigateBack = { navController.popBackStack() }) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddEditItemNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddEditItemNavigation.kt new file mode 100644 index 0000000000..7a0c19dafa --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddEditItemNavigation.kt @@ -0,0 +1,79 @@ +package com.x8bit.bitwarden.ui.vault.feature.additem + +import androidx.lifecycle.SavedStateHandle +import androidx.navigation.NavController +import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions +import androidx.navigation.NavType +import androidx.navigation.compose.composable +import androidx.navigation.navArgument +import com.x8bit.bitwarden.ui.platform.theme.TransitionProviders +import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType + +private const val ADD_TYPE: String = "add" +private const val EDIT_TYPE: String = "edit" +private const val EDIT_ITEM_ID: String = "vault_edit_id" + +private const val ADD_EDIT_ITEM_PREFIX: String = "vault_add_edit_item" +private const val ADD_EDIT_ITEM_TYPE: String = "vault_add_edit_type" + +private const val ADD_EDIT_ITEM_ROUTE: String = + "$ADD_EDIT_ITEM_PREFIX/{$ADD_EDIT_ITEM_TYPE}?$EDIT_ITEM_ID={$EDIT_ITEM_ID}" + +/** + * Class to retrieve vault add & edit arguments from the [SavedStateHandle]. + */ +class VaultAddEditItemArgs( + val vaultAddEditType: VaultAddEditType, +) { + constructor(savedStateHandle: SavedStateHandle) : this( + vaultAddEditType = when (requireNotNull(savedStateHandle[ADD_EDIT_ITEM_TYPE])) { + ADD_TYPE -> VaultAddEditType.AddItem + EDIT_TYPE -> VaultAddEditType.EditItem(requireNotNull(savedStateHandle[EDIT_ITEM_ID])) + else -> throw IllegalStateException("Unknown VaultAddEditType.") + }, + ) +} + +/** + * Add the vault add & edit item screen to the nav graph. + */ +fun NavGraphBuilder.vaultAddEditItemDestination( + onNavigateBack: () -> Unit, +) { + composable( + route = ADD_EDIT_ITEM_ROUTE, + arguments = listOf( + navArgument(ADD_EDIT_ITEM_TYPE) { type = NavType.StringType }, + ), + enterTransition = TransitionProviders.Enter.slideUp, + exitTransition = TransitionProviders.Exit.slideDown, + popEnterTransition = TransitionProviders.Enter.slideUp, + popExitTransition = TransitionProviders.Exit.slideDown, + ) { + VaultAddItemScreen(onNavigateBack) + } +} + +/** + * Navigate to the vault add & edit item screen. + */ +fun NavController.navigateToVaultAddEditItem( + vaultAddEditType: VaultAddEditType, + navOptions: NavOptions? = null, +) { + navigate( + route = "$ADD_EDIT_ITEM_PREFIX/${vaultAddEditType.toTypeString()}" + + "?$EDIT_ITEM_ID=${vaultAddEditType.toIdOrNull()}", + navOptions = navOptions, + ) +} + +private fun VaultAddEditType.toTypeString(): String = + when (this) { + is VaultAddEditType.AddItem -> ADD_TYPE + is VaultAddEditType.EditItem -> EDIT_TYPE + } + +private fun VaultAddEditType.toIdOrNull(): String? = + (this as? VaultAddEditType.EditItem)?.vaultItemId diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemNavigation.kt deleted file mode 100644 index 22ba260066..0000000000 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemNavigation.kt +++ /dev/null @@ -1,33 +0,0 @@ -package com.x8bit.bitwarden.ui.vault.feature.additem - -import androidx.navigation.NavController -import androidx.navigation.NavGraphBuilder -import androidx.navigation.NavOptions -import androidx.navigation.compose.composable -import com.x8bit.bitwarden.ui.platform.theme.TransitionProviders - -private const val ADD_ITEM_ROUTE = "vault_add_item" - -/** - * Add the vault add item screen to the nav graph. - */ -fun NavGraphBuilder.vaultAddItemDestination( - onNavigateBack: () -> Unit, -) { - composable( - route = ADD_ITEM_ROUTE, - enterTransition = TransitionProviders.Enter.slideUp, - exitTransition = TransitionProviders.Exit.slideDown, - popEnterTransition = TransitionProviders.Enter.slideUp, - popExitTransition = TransitionProviders.Exit.slideDown, - ) { - VaultAddItemScreen(onNavigateBack) - } -} - -/** - * Navigate to the vault add item screen. - */ -fun NavController.navigateToVaultAddItem(navOptions: NavOptions? = null) { - navigate(ADD_ITEM_ROUTE, navOptions) -} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreen.kt index 15192c7ab9..bf5fc86c60 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreen.kt @@ -82,7 +82,7 @@ fun VaultAddItemScreen( .nestedScroll(scrollBehavior.nestedScrollConnection), topBar = { BitwardenTopAppBar( - title = stringResource(id = R.string.add_item), + title = state.screenDisplayName(), navigationIcon = painterResource(id = R.drawable.ic_close), navigationIconContentDescription = stringResource(id = R.string.close), onNavigationIconClick = remember(viewModel) { @@ -114,17 +114,21 @@ fun VaultAddItemScreen( .padding(horizontal = 16.dp), ) } - item { - Spacer(modifier = Modifier.height(8.dp)) - TypeOptionsItem( - selectedType = state.selectedType, - onTypeOptionClicked = remember(viewModel) { - { typeOption: VaultAddItemState.ItemTypeOption -> - viewModel.trySendAction(VaultAddItemAction.TypeOptionSelect(typeOption)) - } - }, - modifier = Modifier.padding(horizontal = 16.dp), - ) + if (state.shouldShowTypeSelector) { + item { + Spacer(modifier = Modifier.height(8.dp)) + TypeOptionsItem( + selectedType = state.selectedType, + onTypeOptionClicked = remember(viewModel) { + { typeOption: VaultAddItemState.ItemTypeOption -> + viewModel.trySendAction( + VaultAddItemAction.TypeOptionSelect(typeOption), + ) + } + }, + modifier = Modifier.padding(horizontal = 16.dp), + ) + } } when (val selectedType = state.selectedType) { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt index eb5a95692e..936125e21d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt @@ -12,6 +12,7 @@ import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.vault.feature.additem.VaultAddItemState.ItemType.Card.displayStringResId import com.x8bit.bitwarden.ui.vault.feature.additem.VaultAddItemState.ItemType.Identity.displayStringResId import com.x8bit.bitwarden.ui.vault.feature.vault.util.toCipherView +import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach @@ -37,7 +38,12 @@ class VaultAddItemViewModel @Inject constructor( private val savedStateHandle: SavedStateHandle, private val vaultRepository: VaultRepository, ) : BaseViewModel( - initialState = savedStateHandle[KEY_STATE] ?: INITIAL_STATE, + initialState = savedStateHandle[KEY_STATE] + ?: VaultAddItemState( + vaultAddEditType = VaultAddEditItemArgs(savedStateHandle).vaultAddEditType, + selectedType = VaultAddItemState.ItemType.Login(), + dialog = null, + ), ) { //region Initialization and Overrides @@ -565,13 +571,6 @@ class VaultAddItemViewModel @Inject constructor( } //endregion Utility Functions - - companion object { - val INITIAL_STATE: VaultAddItemState = VaultAddItemState( - selectedType = VaultAddItemState.ItemType.Login(), - dialog = null, - ) - } } /** @@ -583,15 +582,30 @@ class VaultAddItemViewModel @Inject constructor( */ @Parcelize data class VaultAddItemState( + val vaultAddEditType: VaultAddEditType, val selectedType: ItemType, val dialog: DialogState?, ) : Parcelable { + /** + * Helper to determine the screen display name. + */ + val screenDisplayName: Text + get() = when (vaultAddEditType) { + VaultAddEditType.AddItem -> R.string.add_item.asText() + is VaultAddEditType.EditItem -> R.string.edit_item.asText() + } + + /** + * Helper to determine if the UI should display the type selector. + */ + val shouldShowTypeSelector: Boolean get() = vaultAddEditType == VaultAddEditType.AddItem + /** * Provides a list of available item types for the vault. */ val typeOptions: List - get() = ItemTypeOption.values().toList() + get() = ItemTypeOption.entries.toList() /** * Enum representing the main type options for the vault, such as LOGIN, CARD, etc. diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/model/VaultAddEditType.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/model/VaultAddEditType.kt new file mode 100644 index 0000000000..08331f043f --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/model/VaultAddEditType.kt @@ -0,0 +1,25 @@ +package com.x8bit.bitwarden.ui.vault.model + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +/** + * Represents the difference between create a completely new cipher and editing an existing one. + */ +sealed class VaultAddEditType : Parcelable { + /** + * Indicates that we want to create a completely new vault item. + */ + @Parcelize + data object AddItem : VaultAddEditType() + + /** + * Indicates that we want to edit an existing item. + * + * @param vaultItemId The ID of the vault item to edit. + */ + @Parcelize + data class EditItem( + val vaultItemId: String, + ) : VaultAddEditType() +} diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreenTest.kt index 03e819c028..464444672d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreenTest.kt @@ -24,6 +24,7 @@ import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.util.onAllNodesWithTextAfterScroll import com.x8bit.bitwarden.ui.util.onNodeWithContentDescriptionAfterScroll import com.x8bit.bitwarden.ui.util.onNodeWithTextAfterScroll +import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -960,11 +961,13 @@ class VaultAddItemScreenTest : BaseComposeTest() { companion object { private val DEFAULT_STATE_LOGIN = VaultAddItemState( + vaultAddEditType = VaultAddEditType.AddItem, selectedType = VaultAddItemState.ItemType.Login(), dialog = null, ) private val DEFAULT_STATE_SECURE_NOTES = VaultAddItemState( + vaultAddEditType = VaultAddEditType.AddItem, selectedType = VaultAddItemState.ItemType.SecureNotes(), dialog = null, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModelTest.kt index a23b9966ed..955d5b07e7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModelTest.kt @@ -8,6 +8,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import com.x8bit.bitwarden.ui.platform.base.util.Text import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType import io.mockk.coEvery import io.mockk.mockk import kotlinx.coroutines.test.runTest @@ -19,9 +20,25 @@ import org.junit.jupiter.api.Test class VaultAddItemViewModelTest : BaseViewModelTest() { private val initialState = createVaultAddLoginItemState() - private val initialSavedStateHandle = createSavedStateHandleWithState(initialState) + private val initialSavedStateHandle = createSavedStateHandleWithState( + state = initialState, + vaultAddEditType = VaultAddEditType.AddItem, + ) private val vaultRepository: VaultRepository = mockk() + @Test + fun `initial state should be correct when state is null`() = runTest { + val viewModel = createAddVaultItemViewModel( + savedStateHandle = createSavedStateHandleWithState( + state = null, + vaultAddEditType = VaultAddEditType.AddItem, + ), + ) + viewModel.stateFlow.test { + assertEquals(initialState, awaitItem()) + } + } + @Test fun `initial state should be correct`() = runTest { val viewModel = createAddVaultItemViewModel() @@ -381,7 +398,10 @@ class VaultAddItemViewModelTest : BaseViewModelTest() { @BeforeEach fun setup() { initialState = createVaultAddSecureNotesItemState() - initialSavedStateHandle = createSavedStateHandleWithState(initialState) + initialSavedStateHandle = createSavedStateHandleWithState( + state = initialState, + vaultAddEditType = VaultAddEditType.AddItem, + ) viewModel = VaultAddItemViewModel( savedStateHandle = initialSavedStateHandle, vaultRepository = vaultRepository, @@ -521,7 +541,7 @@ class VaultAddItemViewModelTest : BaseViewModelTest() { username: String = "", password: String = "", uri: String = "", - folder: Text = "No Folder".asText(), + folder: Text = R.string.folder_none.asText(), favorite: Boolean = false, masterPasswordReprompt: Boolean = false, notes: String = "", @@ -529,6 +549,7 @@ class VaultAddItemViewModelTest : BaseViewModelTest() { dialogState: VaultAddItemState.DialogState? = null, ): VaultAddItemState = VaultAddItemState( + vaultAddEditType = VaultAddEditType.AddItem, selectedType = VaultAddItemState.ItemType.Login( name = name, username = username, @@ -553,6 +574,7 @@ class VaultAddItemViewModelTest : BaseViewModelTest() { ownership: String = "placeholder@email.com", ): VaultAddItemState = VaultAddItemState( + vaultAddEditType = VaultAddEditType.AddItem, selectedType = VaultAddItemState.ItemType.SecureNotes( name = name, folderName = folder, @@ -564,14 +586,27 @@ class VaultAddItemViewModelTest : BaseViewModelTest() { dialog = null, ) - private fun createSavedStateHandleWithState(state: VaultAddItemState) = - SavedStateHandle().apply { - set("state", state) - } + private fun createSavedStateHandleWithState( + state: VaultAddItemState?, + vaultAddEditType: VaultAddEditType, + ) = SavedStateHandle().apply { + set("state", state) + set( + "vault_add_edit_type", + when (vaultAddEditType) { + VaultAddEditType.AddItem -> "add" + is VaultAddEditType.EditItem -> "edit" + }, + ) + set("vault_edit_id", (vaultAddEditType as? VaultAddEditType.EditItem)?.vaultItemId) + } - private fun createAddVaultItemViewModel(): VaultAddItemViewModel = + private fun createAddVaultItemViewModel( + savedStateHandle: SavedStateHandle = initialSavedStateHandle, + vaultRepo: VaultRepository = vaultRepository, + ): VaultAddItemViewModel = VaultAddItemViewModel( - savedStateHandle = initialSavedStateHandle, - vaultRepository = vaultRepository, + savedStateHandle = savedStateHandle, + vaultRepository = vaultRepo, ) }