[PM-14843] Allow deletion of items in collections with manage permiss…

…ion (#4299)
bitwarden · Nov 15, 2024 · d125fab · d125fab
1 parent 1321034
commit d125fab
Show file tree

Hide file tree

Showing 6 changed files with 715 additions and 860 deletions.
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt b/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
-                    // does not have "manage" permission for.
-                    val canDelete = vaultData.collectionViewList
-                        .none {
-                            val isItemInCollection = cipherView
-                                ?.collectionIds
-                                ?.contains(it.id) == true
+                    val canDelete = vaultData
+                        .collectionViewList
+                        .hasDeletePermissionInAtLeastOneCollection(cipherView?.collectionIds)
 
-                            isItemInCollection && !it.manage
-                        }
-
-                    // 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)
-                        }
+                    val canAssignToCollections = vaultData
+                        .collectionViewList
+                        .canAssignToCollections(cipherView?.collectionIds)
 
                     // Derive the view state from the current Cipher for Edit mode
                     // or use the current state for Add

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
@@ -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
-                        // does not have "manage" permission for.
-                        val canDelete = collectionsState.data
-                            ?.none {
-                                val itemIsInCollection = cipherViewState.data
-                                    ?.collectionIds
-                                    ?.contains(it.id) == true
-
-                                itemIsInCollection && !it.manage
-                            }
-                            ?: true
-
-                        // 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
+                        val canDelete = collectionsState
+                            .data
+                            .hasDeletePermissionInAtLeastOneCollection(
+                                collectionIds = cipherViewState.data?.collectionIds,
+                            )
+
+                        val canAssignToCollections = collectionsState
+                            .data
+                            .canAssignToCollections(cipherViewState.data?.collectionIds)
 
                         VaultItemStateData(
                             cipher = cipherViewState.data,

diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/util/CollectionViewExtensions.kt b/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
diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt b/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,
+                        ),
                     ),
                 ),
             )