BIT-1551: Restrict cloning to items not assigned to a collection (#751)

This commit is contained in:
Ramsey Smith 2024-01-24 11:24:51 -07:00 committed by Álison Fernandes
parent 862d9b5c94
commit 4f0fa96dc0
4 changed files with 173 additions and 7 deletions

View file

@ -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 },
),
),
)

View file

@ -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()
}
/**

View file

@ -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

View file

@ -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