From c5a3340ac52a30ad3697b71e53e1694a82ea41b6 Mon Sep 17 00:00:00 2001 From: Ramsey Smith <142836716+ramsey-livefront@users.noreply.github.com> Date: Fri, 12 Jan 2024 11:05:50 -0700 Subject: [PATCH] BIT-1304: Options menu UI for view item (#580) --- .../vault/feature/item/VaultItemNavigation.kt | 2 +- .../ui/vault/feature/item/VaultItemScreen.kt | 58 +++++++++++- .../vault/feature/item/VaultItemViewModel.kt | 71 ++++++++++++++- .../vault/feature/item/VaultItemScreenTest.kt | 89 ++++++++++++++++++- .../feature/item/VaultItemViewModelTest.kt | 59 +++++++++++- 5 files changed, 270 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemNavigation.kt index 2f0538aa91..fd18da2d88 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemNavigation.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemNavigation.kt @@ -38,7 +38,7 @@ fun NavGraphBuilder.vaultItemDestination( ) { VaultItemScreen( onNavigateBack = onNavigateBack, - onNavigateToVaultEditItem = onNavigateToVaultEditItem, + onNavigateToVaultAddEditItem = onNavigateToVaultEditItem, ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt index 90f999aace..fc4f16cc98 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt @@ -39,9 +39,11 @@ import com.x8bit.bitwarden.ui.platform.components.BitwardenOverflowActionItem import com.x8bit.bitwarden.ui.platform.components.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState +import com.x8bit.bitwarden.ui.platform.components.OverflowMenuItemData import com.x8bit.bitwarden.ui.vault.feature.item.handlers.VaultCardItemTypeHandlers import com.x8bit.bitwarden.ui.vault.feature.item.handlers.VaultCommonItemTypeHandlers import com.x8bit.bitwarden.ui.vault.feature.item.handlers.VaultLoginItemTypeHandlers +import kotlinx.collections.immutable.persistentListOf /** * Displays the vault item screen. @@ -53,7 +55,7 @@ fun VaultItemScreen( viewModel: VaultItemViewModel = hiltViewModel(), intentHandler: IntentHandler = IntentHandler(context = LocalContext.current), onNavigateBack: () -> Unit, - onNavigateToVaultEditItem: (vaultItemId: String) -> Unit, + onNavigateToVaultAddEditItem: (vaultItemId: String) -> Unit, ) { val state by viewModel.stateFlow.collectAsStateWithLifecycle() val context = LocalContext.current @@ -62,14 +64,28 @@ fun VaultItemScreen( when (event) { VaultItemEvent.NavigateBack -> onNavigateBack() - is VaultItemEvent.NavigateToEdit -> onNavigateToVaultEditItem(event.itemId) + is VaultItemEvent.NavigateToAddEdit -> { + // TODO Implement cloning in BIT-526 + onNavigateToVaultAddEditItem(event.itemId) + } is VaultItemEvent.NavigateToPasswordHistory -> { + // TODO Implement password history in BIT-617 Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show() } is VaultItemEvent.NavigateToUri -> intentHandler.launchUri(event.uri.toUri()) + is VaultItemEvent.NavigateToAttachments -> { + // TODO implement attachments in BIT-522 + Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show() + } + + is VaultItemEvent.NavigateToMoveToOrganization -> { + // TODO Implement move to organization in BIT-844 + Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show() + } + is VaultItemEvent.ShowToast -> { Toast.makeText(context, event.message(resources), Toast.LENGTH_SHORT).show() } @@ -101,7 +117,43 @@ fun VaultItemScreen( { viewModel.trySendAction(VaultItemAction.Common.CloseClick) } }, actions = { - BitwardenOverflowActionItem() + // TODO make action list dependent on item being in an organization BIT-1446 + BitwardenOverflowActionItem( + menuItemDataList = persistentListOf( + OverflowMenuItemData( + text = stringResource(id = R.string.delete), + onClick = remember(viewModel) { + { viewModel.trySendAction(VaultItemAction.Common.DeleteClick) } + }, + ), + OverflowMenuItemData( + text = stringResource(id = R.string.attachments), + onClick = remember(viewModel) { + { + viewModel.trySendAction( + VaultItemAction.Common.AttachmentsClick, + ) + } + }, + ), + OverflowMenuItemData( + text = stringResource(id = R.string.clone), + onClick = remember(viewModel) { + { viewModel.trySendAction(VaultItemAction.Common.CloneClick) } + }, + ), + OverflowMenuItemData( + text = stringResource(id = R.string.move_to_organization), + onClick = remember(viewModel) { + { + viewModel.trySendAction( + VaultItemAction.Common.MoveToOrganizationClick, + ) + } + }, + ), + ), + ) }, ) }, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt index 68630a3ce3..7ca64d3132 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt @@ -97,6 +97,11 @@ class VaultItemViewModel @Inject constructor( is VaultItemAction.Common.HiddenFieldVisibilityClicked -> { handleHiddenFieldVisibilityClicked(action) } + + is VaultItemAction.Common.AttachmentsClick -> handleAttachmentsClick() + is VaultItemAction.Common.CloneClick -> handleCloneClick() + is VaultItemAction.Common.DeleteClick -> handleDeleteClick() + is VaultItemAction.Common.MoveToOrganizationClick -> handleMoveToOrganizationClick() } } @@ -116,7 +121,12 @@ class VaultItemViewModel @Inject constructor( } return@onContent } - sendEvent(VaultItemEvent.NavigateToEdit(state.vaultItemId)) + sendEvent( + VaultItemEvent.NavigateToAddEdit( + itemId = state.vaultItemId, + isClone = false, + ), + ) } } @@ -192,6 +202,28 @@ class VaultItemViewModel @Inject constructor( } } + private fun handleAttachmentsClick() { + sendEvent(VaultItemEvent.NavigateToAttachments(itemId = state.vaultItemId)) + } + + private fun handleCloneClick() { + sendEvent( + VaultItemEvent.NavigateToAddEdit( + itemId = state.vaultItemId, + isClone = true, + ), + ) + } + + private fun handleDeleteClick() { + // TODO Implement delete in BIT-1408 + sendEvent(VaultItemEvent.ShowToast("Not yet implemented.".asText())) + } + + private fun handleMoveToOrganizationClick() { + sendEvent(VaultItemEvent.NavigateToMoveToOrganization(itemId = state.vaultItemId)) + } + //endregion Common Handlers //region Login Type Handlers @@ -759,8 +791,9 @@ sealed class VaultItemEvent { /** * Navigates to the edit screen. */ - data class NavigateToEdit( + data class NavigateToAddEdit( val itemId: String, + val isClone: Boolean, ) : VaultItemEvent() /** @@ -777,6 +810,20 @@ sealed class VaultItemEvent { val uri: String, ) : VaultItemEvent() + /** + * Navigates to the attachments screen. + */ + data class NavigateToAttachments( + val itemId: String, + ) : VaultItemEvent() + + /** + * Navigates to the move to organization screen. + */ + data class NavigateToMoveToOrganization( + val itemId: String, + ) : VaultItemEvent() + /** * Places the given [message] in your clipboard. */ @@ -844,6 +891,26 @@ sealed class VaultItemAction { val field: VaultItemState.ViewState.Content.Common.Custom.HiddenField, val isVisible: Boolean, ) : Common() + + /** + * The user has clicked the delete button. + */ + data object DeleteClick : Common() + + /** + * The user has clicked the attachments button. + */ + data object AttachmentsClick : Common() + + /** + * The user has clicked the clone button. + */ + data object CloneClick : Common() + + /** + * The user has clicked the move to organization button. + */ + data object MoveToOrganizationClick : Common() } /** diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt index 3c86388ce1..e7940bb141 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt @@ -1,15 +1,18 @@ package com.x8bit.bitwarden.ui.vault.feature.item import androidx.compose.ui.test.assert +import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsEnabled import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertTextContains import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.filter import androidx.compose.ui.test.filterToOne import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasContentDescription import androidx.compose.ui.test.isDialog +import androidx.compose.ui.test.isPopup import androidx.compose.ui.test.onAllNodesWithContentDescription import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onChildren @@ -64,16 +67,17 @@ class VaultItemScreenTest : BaseComposeTest() { VaultItemScreen( viewModel = viewModel, onNavigateBack = { onNavigateBackCalled = true }, - onNavigateToVaultEditItem = { onNavigateToVaultEditItemId = it }, + onNavigateToVaultAddEditItem = { onNavigateToVaultEditItemId = it }, intentHandler = intentHandler, ) } } + //region common @Test fun `NavigateToEdit event should invoke onNavigateToVaultEditItem`() { val id = "id1234" - mutableEventFlow.tryEmit(VaultItemEvent.NavigateToEdit(id)) + mutableEventFlow.tryEmit(VaultItemEvent.NavigateToAddEdit(itemId = id, isClone = false)) assertEquals(id, onNavigateToVaultEditItemId) } @@ -544,6 +548,84 @@ class VaultItemScreenTest : BaseComposeTest() { .assertDoesNotExist() } + @Test + fun `Delete option menu click should send DeleteClick action`() { + // Confirm dropdown version of item is absent + composeTestRule + .onAllNodesWithText("Delete") + .filter(hasAnyAncestor(isPopup())) + .assertCountEquals(0) + // Open the overflow menu + composeTestRule.onNodeWithContentDescription("More").performClick() + // Click on the delete item in the dropdown + composeTestRule + .onAllNodesWithText("Delete") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + verify { + viewModel.trySendAction(VaultItemAction.Common.DeleteClick) + } + } + + @Test + fun `Attachments option menu click should send AttachmentsClick action`() { + // Confirm dropdown version of item is absent + composeTestRule + .onAllNodesWithText("Attachments") + .filter(hasAnyAncestor(isPopup())) + .assertCountEquals(0) + // Open the overflow menu + composeTestRule.onNodeWithContentDescription("More").performClick() + // Click on the attachments hint item in the dropdown + composeTestRule + .onAllNodesWithText("Attachments") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + verify { + viewModel.trySendAction(VaultItemAction.Common.AttachmentsClick) + } + } + + @Test + fun `Clone option menu click should send CloneClick action`() { + // Confirm dropdown version of item is absent + composeTestRule + .onAllNodesWithText("Clone") + .filter(hasAnyAncestor(isPopup())) + .assertCountEquals(0) + // Open the overflow menu + composeTestRule.onNodeWithContentDescription("More").performClick() + // Click on the clone item in the dropdown + composeTestRule + .onAllNodesWithText("Clone") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + verify { + viewModel.trySendAction(VaultItemAction.Common.CloneClick) + } + } + + @Test + fun `Move to organization option menu click should send MoveToOrganizationClick action`() { + // Confirm dropdown version of item is absent + composeTestRule + .onAllNodesWithText("Move to Organization") + .filter(hasAnyAncestor(isPopup())) + .assertCountEquals(0) + // Open the overflow menu + composeTestRule.onNodeWithContentDescription("More").performClick() + // Click on the move to organization hint item in the dropdown + composeTestRule + .onAllNodesWithText("Move to Organization") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + verify { + viewModel.trySendAction(VaultItemAction.Common.MoveToOrganizationClick) + } + } + //endregion common + + //region login @Test fun `in login state, linked custom fields should be displayed according to state`() { val linkedFieldUserName = @@ -947,7 +1029,9 @@ class VaultItemScreenTest : BaseComposeTest() { composeTestRule.assertScrollableNodeDoesNotExist("Password history: ") composeTestRule.assertScrollableNodeDoesNotExist("1") } + //endregion login + //region identity @Test fun `in identity state, identityName should be displayed according to state`() { val identityName = "the identity name" @@ -1064,6 +1148,7 @@ class VaultItemScreenTest : BaseComposeTest() { composeTestRule.assertScrollableNodeDoesNotExist(identityName) } + //endregion identity @Test fun `in card state, cardholderName should be displayed according to state`() { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt index ace9717bef..adea84af08 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt @@ -149,7 +149,13 @@ class VaultItemViewModelTest : BaseViewModelTest() { viewModel.eventFlow.test { viewModel.trySendAction(VaultItemAction.Common.EditClick) - assertEquals(VaultItemEvent.NavigateToEdit(VAULT_ITEM_ID), awaitItem()) + assertEquals( + VaultItemEvent.NavigateToAddEdit( + itemId = VAULT_ITEM_ID, + isClone = false, + ), + awaitItem(), + ) } } @@ -327,6 +333,57 @@ class VaultItemViewModelTest : BaseViewModelTest() { mockCipherView.toViewState(isPremiumUser = true) } } + + @Test + fun `on DeleteClick should emit ShowToast`() = runTest { + val viewModel = createViewModel(state = DEFAULT_STATE) + viewModel.eventFlow.test { + viewModel.trySendAction(VaultItemAction.Common.DeleteClick) + assertEquals( + VaultItemEvent.ShowToast("Not yet implemented.".asText()), + awaitItem(), + ) + } + } + + @Test + fun `on AttachmentsClick should emit NavigateToAttachments`() = runTest { + val viewModel = createViewModel(state = DEFAULT_STATE) + viewModel.eventFlow.test { + viewModel.trySendAction(VaultItemAction.Common.AttachmentsClick) + assertEquals( + VaultItemEvent.NavigateToAttachments(itemId = VAULT_ITEM_ID), + awaitItem(), + ) + } + } + + @Test + fun `on CloneClick should emit NavigateToAddEdit with isClone set to true`() = runTest { + val viewModel = createViewModel(state = DEFAULT_STATE) + viewModel.eventFlow.test { + viewModel.trySendAction(VaultItemAction.Common.CloneClick) + assertEquals( + VaultItemEvent.NavigateToAddEdit( + itemId = VAULT_ITEM_ID, + isClone = true, + ), + awaitItem(), + ) + } + } + + @Test + fun `on MoveToOrganizationClick should emit NavigateToMoveToOrganization`() = runTest { + val viewModel = createViewModel(state = DEFAULT_STATE) + viewModel.eventFlow.test { + viewModel.trySendAction(VaultItemAction.Common.MoveToOrganizationClick) + assertEquals( + VaultItemEvent.NavigateToMoveToOrganization(itemId = VAULT_ITEM_ID), + awaitItem(), + ) + } + } } @Nested