From 86d0736858c8f515134ebce49c5485c525ccc4ee Mon Sep 17 00:00:00 2001 From: Ramsey Smith <142836716+ramsey-livefront@users.noreply.github.com> Date: Wed, 24 Jan 2024 11:24:51 -0700 Subject: [PATCH] BIT-1551: Restrict cloning to items not assigned to a collection (#751) --- .../ui/vault/feature/item/VaultItemScreen.kt | 32 ++++-- .../vault/feature/item/VaultItemViewModel.kt | 28 +++++ .../vault/feature/item/VaultItemScreenTest.kt | 108 ++++++++++++++++++ .../feature/item/VaultItemViewModelTest.kt | 12 ++ 4 files changed, 173 insertions(+), 7 deletions(-) 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 dd243afc41..dacfc583a0 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 @@ -45,10 +45,10 @@ import com.x8bit.bitwarden.ui.platform.components.LoadingDialogState import com.x8bit.bitwarden.ui.platform.components.OverflowMenuItemData import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager import com.x8bit.bitwarden.ui.platform.theme.LocalIntentManager +import com.x8bit.bitwarden.ui.platform.util.persistentListOfNotNull 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. @@ -97,6 +97,11 @@ fun VaultItemScreen( onNavigateToMoveToOrganization(event.itemId) } + is VaultItemEvent.NavigateToCollections -> { + // TODO implement Collections in BIT-1575 + Toast.makeText(context, "Not yet implemented.", Toast.LENGTH_SHORT).show() + } + is VaultItemEvent.ShowToast -> { Toast.makeText(context, event.message(resources), Toast.LENGTH_SHORT).show() } @@ -173,11 +178,7 @@ fun VaultItemScreen( } // TODO make action list dependent on item being in an organization BIT-1446 BitwardenOverflowActionItem( - menuItemDataList = persistentListOf( - OverflowMenuItemData( - text = stringResource(id = R.string.delete), - onClick = { pendingDeleteCipher = true }, - ), + menuItemDataList = persistentListOfNotNull( OverflowMenuItemData( text = stringResource(id = R.string.attachments), onClick = remember(viewModel) { @@ -193,7 +194,8 @@ fun VaultItemScreen( onClick = remember(viewModel) { { viewModel.trySendAction(VaultItemAction.Common.CloneClick) } }, - ), + ) + .takeUnless { state.isCipherInCollection }, OverflowMenuItemData( text = stringResource(id = R.string.move_to_organization), onClick = remember(viewModel) { @@ -203,6 +205,22 @@ fun VaultItemScreen( ) } }, + ) + .takeUnless { state.isCipherInCollection }, + OverflowMenuItemData( + text = stringResource(id = R.string.collections), + onClick = remember(viewModel) { + { + viewModel.trySendAction( + VaultItemAction.Common.CollectionsClick, + ) + } + }, + ) + .takeIf { state.isCipherInCollection }, + OverflowMenuItemData( + text = stringResource(id = R.string.delete), + onClick = { pendingDeleteCipher = true }, ), ), ) 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 6332a500f9..eaadc5a24a 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 @@ -102,6 +102,7 @@ class VaultItemViewModel @Inject constructor( is VaultItemAction.Common.AttachmentsClick -> handleAttachmentsClick() is VaultItemAction.Common.CloneClick -> handleCloneClick() is VaultItemAction.Common.MoveToOrganizationClick -> handleMoveToOrganizationClick() + is VaultItemAction.Common.CollectionsClick -> handleCollectionsClick() is VaultItemAction.Common.ConfirmDeleteClick -> handleConfirmDeleteClick() is VaultItemAction.Common.ConfirmRestoreClick -> handleConfirmRestoreClick() } @@ -221,6 +222,10 @@ class VaultItemViewModel @Inject constructor( sendEvent(VaultItemEvent.NavigateToMoveToOrganization(itemId = state.vaultItemId)) } + private fun handleCollectionsClick() { + sendEvent(VaultItemEvent.NavigateToCollections(itemId = state.vaultItemId)) + } + private fun handleConfirmDeleteClick() { mutableStateFlow.update { it.copy( @@ -659,6 +664,17 @@ data class VaultItemState( val isFabVisible: Boolean get() = viewState is ViewState.Content && !isCipherDeleted + /** + * Whether or not the cipher is in a collection. + */ + val isCipherInCollection: Boolean + get() = (viewState as? ViewState.Content) + ?.common + ?.currentCipher + ?.collectionIds + ?.isNotEmpty() + ?: false + /** * Represents the specific view states for the [VaultItemScreen]. */ @@ -936,6 +952,13 @@ sealed class VaultItemEvent { val itemId: String, ) : VaultItemEvent() + /** + * Navigates to the collections screen. + */ + data class NavigateToCollections( + val itemId: String, + ) : VaultItemEvent() + /** * Places the given [message] in your clipboard. */ @@ -1028,6 +1051,11 @@ sealed class VaultItemAction { * The user has clicked the move to organization button. */ data object MoveToOrganizationClick : Common() + + /** + * The user has clicked the collections button. + */ + data object CollectionsClick : 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 374d43d3c7..825932f56b 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 @@ -838,6 +838,114 @@ class VaultItemScreenTest : BaseComposeTest() { viewModel.trySendAction(VaultItemAction.Common.MoveToOrganizationClick) } } + + @Test + fun `Collections menu click should send CollectionsClick action`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_IDENTITY_VIEW_STATE + .copy( + common = DEFAULT_COMMON + .copy(currentCipher = createMockCipherView(1)), + ), + ) + } + // Confirm dropdown version of item is absent + composeTestRule + .onAllNodesWithText("Collections") + .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("Collections") + .filterToOne(hasAnyAncestor(isPopup())) + .performClick() + + composeTestRule + .onNode(isPopup()) + .assertDoesNotExist() + + verify { + viewModel.trySendAction(VaultItemAction.Common.CollectionsClick) + } + } + + @Test + fun `Menu should display correct items when cipher is in a collection`() { + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_IDENTITY_VIEW_STATE + .copy( + common = DEFAULT_COMMON + .copy(currentCipher = createMockCipherView(1)), + ), + ) + } + composeTestRule + .onNodeWithContentDescription("More") + .performClick() + + composeTestRule + .onAllNodesWithText("Attachments") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Collections") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Delete") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Move to Organization") + .filterToOne(hasAnyAncestor(isPopup())) + .assertDoesNotExist() + + composeTestRule + .onAllNodesWithText("Clone") + .filterToOne(hasAnyAncestor(isPopup())) + .assertDoesNotExist() + } + + @Test + fun `Menu should display correct items when cipher is not in a collection`() { + composeTestRule + .onNodeWithContentDescription("More") + .performClick() + + composeTestRule + .onAllNodesWithText("Attachments") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Clone") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Move to Organization") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Delete") + .filterToOne(hasAnyAncestor(isPopup())) + .assertIsDisplayed() + + composeTestRule + .onAllNodesWithText("Collections") + .filterToOne(hasAnyAncestor(isPopup())) + .assertDoesNotExist() + } //endregion common //region login 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 92a454e565..0d05d4a1e3 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 @@ -503,6 +503,18 @@ class VaultItemViewModelTest : BaseViewModelTest() { ) } } + + @Test + fun `on CollectionsClick should emit NavigateToCollections`() = runTest { + val viewModel = createViewModel(state = DEFAULT_STATE) + viewModel.eventFlow.test { + viewModel.trySendAction(VaultItemAction.Common.CollectionsClick) + assertEquals( + VaultItemEvent.NavigateToCollections(itemId = VAULT_ITEM_ID), + awaitItem(), + ) + } + } } @Nested