[PM-14843] Allow deletion of items in collections with manage permission (#4299)

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt

@@ -53,6 +53,8 @@ import com.x8bit.bitwarden.ui.vault.feature.addedit.util.toDefaultAddTypeContent
 import com.x8bit.bitwarden.ui.vault.feature.addedit.util.toItemType
 import com.x8bit.bitwarden.ui.vault.feature.addedit.util.toViewState
 import com.x8bit.bitwarden.ui.vault.feature.addedit.util.validateCipherOrReturnErrorState
+import com.x8bit.bitwarden.ui.vault.feature.util.canAssignToCollections
+import com.x8bit.bitwarden.ui.vault.feature.util.hasDeletePermissionInAtLeastOneCollection
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toCipherView
 import com.x8bit.bitwarden.ui.vault.model.TotpData
 import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType
@@ -1579,27 +1581,13 @@ class VaultAddEditViewModel @Inject constructor(
                     vaultAddEditType = vaultAddEditType,
                 ) { currentAccount, cipherView ->
 
-                    // Deletion is not allowed when the item is in a collection that the user
+                    val canDelete = vaultData
-                    // does not have "manage" permission for.
+                        .collectionViewList
-                    val canDelete = vaultData.collectionViewList
+                        .hasDeletePermissionInAtLeastOneCollection(cipherView?.collectionIds)
-                        .none {
-                            val isItemInCollection = cipherView
-                                ?.collectionIds
-                                ?.contains(it.id) == true
 
-                            isItemInCollection && !it.manage
+                    val canAssignToCollections = vaultData
-                        }
+                        .collectionViewList
-
+                        .canAssignToCollections(cipherView?.collectionIds)
-                    // Assigning to a collection is not allowed when the item is in a collection
-                    // that the user does not have "manage" and "edit" permission for.
-                    val canAssignToCollections = vaultData.collectionViewList
-                        .none {
-                            val isItemInCollection = cipherView
-                                ?.collectionIds
-                                ?.contains(it.id) == true
-
-                            isItemInCollection && (!it.manage || it.readOnly)
-                        }
 
                     // Derive the view state from the current Cipher for Edit mode
                     // or use the current state for Add

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt

@@ -28,6 +28,8 @@ import com.x8bit.bitwarden.ui.platform.base.util.concat
 import com.x8bit.bitwarden.ui.vault.feature.item.model.TotpCodeItemData
 import com.x8bit.bitwarden.ui.vault.feature.item.model.VaultItemStateData
 import com.x8bit.bitwarden.ui.vault.feature.item.util.toViewState
+import com.x8bit.bitwarden.ui.vault.feature.util.canAssignToCollections
+import com.x8bit.bitwarden.ui.vault.feature.util.hasDeletePermissionInAtLeastOneCollection
 import com.x8bit.bitwarden.ui.vault.model.VaultCardBrand
 import com.x8bit.bitwarden.ui.vault.model.VaultLinkedFieldType
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -103,29 +105,15 @@ class VaultItemViewModel @Inject constructor(
                     // we map it to the appropriate value below.
                 }
                     .mapNullable {
-                        // Deletion is not allowed when the item is in a collection that the user
+                        val canDelete = collectionsState
-                        // does not have "manage" permission for.
+                            .data
-                        val canDelete = collectionsState.data
+                            .hasDeletePermissionInAtLeastOneCollection(
-                            ?.none {
+                                collectionIds = cipherViewState.data?.collectionIds,
-                                val itemIsInCollection = cipherViewState.data
+                            )
-                                    ?.collectionIds
-                                    ?.contains(it.id) == true
 
-                                itemIsInCollection && !it.manage
+                        val canAssignToCollections = collectionsState
-                            }
+                            .data
-                            ?: true
+                            .canAssignToCollections(cipherViewState.data?.collectionIds)
-
-                        // Assigning to a collection is not allowed when the item is in a collection
-                        // that the user does not have "manage" and "edit" permission for.
-                        val canAssignToCollections = collectionsState.data
-                            ?.none {
-                                val itemIsInCollection = cipherViewState.data
-                                    ?.collectionIds
-                                    ?.contains(it.id) == true
-
-                                itemIsInCollection && !it.manage && it.readOnly
-                            }
-                            ?: true
 
                         VaultItemStateData(
                             cipher = cipherViewState.data,

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/util/CollectionViewExtensions.kt

@@ -88,3 +88,36 @@ fun String.toCollectionDisplayName(list: List<CollectionView>): String {
 
     return collectionName
 }
+
+/**
+ * Checks if the user has delete permission in at least one collection.
+ *
+ * Deletion is allowed when the item is in any collection that the user has "manage" permission for.
+ */
+fun List<CollectionView>?.hasDeletePermissionInAtLeastOneCollection(collectionIds: List<String>?) =
+    this
+        ?.takeUnless { it.isEmpty() }
+        ?.any {
+            collectionIds
+                ?.contains(it.id)
+                ?.let { isInCollection -> !isInCollection || it.manage }
+                ?: true
+        }
+        ?: true
+
+/**
+ * Checks if the user has permission to assign an item to a collection.
+ *
+ * Assigning to a collection is not allowed when the item is in a collection that the user does not
+ * have "manage" and "edit" permission for.
+ */
+fun List<CollectionView>?.canAssignToCollections(currentCollectionIds: List<String>?) =
+    this
+        ?.none {
+            val itemIsInCollection = currentCollectionIds
+                ?.contains(it.id)
+                ?: false
+
+            itemIsInCollection && (!it.manage || it.readOnly)
+        }
+        ?: true

app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt

@@ -1259,6 +1259,11 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
                             manage = true,
                             readOnly = false,
                         ),
+                        createMockCollectionView(
+                            number = 2,
+                            manage = false,
+                            readOnly = true,
+                        ),
                     ),
                 ),
             )

app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/util/CollectionViewExtensionsTest.kt

@@ -3,6 +3,8 @@ package com.x8bit.bitwarden.ui.vault.feature.util
 import com.bitwarden.vault.CollectionView
 import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView
 import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
 
 class CollectionViewExtensionsTest {
@@ -66,4 +68,102 @@ class CollectionViewExtensionsTest {
             collectionName.toCollectionDisplayName(collectionList),
         )
     }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return true if the user has manage permission in at least one collection`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = true),
+            createMockCollectionView(number = 2, manage = false),
+        )
+
+        val collectionIds = listOf("mockId-1", "mockId-2")
+
+        assertTrue(collectionList.hasDeletePermissionInAtLeastOneCollection(collectionIds))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return false if the user does not have manage permission in at least one collection`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = false),
+            createMockCollectionView(number = 2, manage = false),
+        )
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertFalse(collectionList.hasDeletePermissionInAtLeastOneCollection(collectionIds))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return true if the collectionView list is null`() {
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertTrue(null.hasDeletePermissionInAtLeastOneCollection(collectionIds))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return true if the collectionIds list is null`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = true),
+            createMockCollectionView(number = 2, manage = false),
+        )
+        assertTrue(collectionList.hasDeletePermissionInAtLeastOneCollection(null))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return true if the collectionIds list is empty`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = true),
+            createMockCollectionView(number = 2, manage = false),
+        )
+        assertTrue(collectionList.hasDeletePermissionInAtLeastOneCollection(emptyList()))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `hasDeletePermissionInAtLeastOneCollection should return true if the collectionView list is empty`() {
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertTrue(
+            emptyList<CollectionView>().hasDeletePermissionInAtLeastOneCollection(
+                collectionIds,
+            ),
+        )
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `canAssociateToCollections should return true if the user has edit and manage permission`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = true, readOnly = false),
+        )
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertTrue(collectionList.canAssignToCollections(collectionIds))
+    }
+
+    @Suppress("MaxLineLength")
+    @Test
+    fun `canAssociateToCollections should return false if the user does not have manage or edit permission in at least one collection`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = false, readOnly = true),
+            createMockCollectionView(number = 2, manage = false, readOnly = true),
+        )
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertFalse(collectionList.canAssignToCollections(collectionIds))
+    }
+
+    @Test
+    fun `canAssociateToCollections should return true if the collectionView list is null`() {
+        val collectionIds = listOf("mockId-1", "mockId-2")
+        assertTrue(null.canAssignToCollections(collectionIds))
+    }
+
+    @Test
+    fun `canAssociateToCollections should return true if the collectionIds list is null`() {
+        val collectionList: List<CollectionView> = listOf(
+            createMockCollectionView(number = 1, manage = true, readOnly = false),
+            createMockCollectionView(number = 2, manage = false),
+        )
+        assertTrue(collectionList.canAssignToCollections(null))
+    }
 }