PM-38280: Fix: Update collection API to V2

app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/CipherManager.kt

@@ -130,7 +130,6 @@ interface CipherManager {
      */
     suspend fun updateCipherCollections(
         cipherId: String,
-        cipherView: CipherView,
         collectionIds: List<String>,
     ): ShareCipherResult
 

app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt

@@ -442,7 +442,6 @@ class CipherManagerImpl(
 
     override suspend fun updateCipherCollections(
         cipherId: String,
-        cipherView: CipherView,
         collectionIds: List<String>,
     ): ShareCipherResult {
         val userId = activeUserId ?: return ShareCipherResult.Error(error = NoActiveUserException())
@@ -451,17 +450,18 @@ class CipherManagerImpl(
                 cipherId = cipherId,
                 body = UpdateCipherCollectionsJsonRequest(collectionIds = collectionIds),
             )
-            .flatMap {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = cipherView.copy(collectionIds = collectionIds),
-                )
-            }
-            .onSuccess { encryptionContext ->
-                vaultDiskSource.saveCipher(
-                    userId = userId,
-                    cipher = encryptionContext.toEncryptedNetworkCipherResponse(),
-                )
+            .onSuccess { response ->
+                response
+                    .cipher
+                    ?.let {
+                        // Save the updated cipher to disk.
+                        vaultDiskSource.saveCipher(userId = userId, cipher = it)
+                    }
+                    ?: run {
+                        // The user no longer has any collection access to the cipher after
+                        // the update, so remove it from disk.
+                        vaultDiskSource.deleteCipher(userId = userId, cipherId = cipherId)
+                    }
             }
             .fold(
                 onSuccess = { ShareCipherResult.Success },

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModel.kt

@@ -309,7 +309,6 @@ class VaultMoveToOrganizationViewModel @Inject constructor(
                     VaultMoveToOrganizationAction.Internal.ShareCipherResultReceive(
                         shareCipherResult = vaultRepository.updateCipherCollections(
                             cipherId = state.vaultItemId,
-                            cipherView = cipherView,
                             collectionIds = collectionIds,
                         ),
                         message = BitwardenString.item_updated.asText(),

app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/CipherViewUtil.kt

@@ -49,6 +49,7 @@ fun createMockCipherView(
     totp: String? = "mockTotp-$number",
     organizationId: String? = "mockOrganizationId-$number",
     folderId: String? = "mockId-$number",
+    collectionIds: List<String> = listOf("mockId-$number"),
     notes: String? = "mockNotes-$number",
     password: String? = "mockPassword-$number",
     clock: Clock = FIXED_CLOCK,
@@ -75,7 +76,7 @@ fun createMockCipherView(
         id = "mockId-$number",
         organizationId = organizationId,
         folderId = folderId,
-        collectionIds = listOf("mockId-$number"),
+        collectionIds = collectionIds,
         key = "mockKey-$number",
         name = "mockName-$number",
         notes = notes,

app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt

@@ -18,6 +18,7 @@ import com.bitwarden.network.model.ShareCipherJsonRequest
 import com.bitwarden.network.model.SyncResponseJson
 import com.bitwarden.network.model.UnarchiveCipherResponseJson
 import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
+import com.bitwarden.network.model.UpdateCipherCollectionsResponseJson
 import com.bitwarden.network.model.UpdateCipherResponseJson
 import com.bitwarden.network.model.createMockAttachment
 import com.bitwarden.network.model.createMockAttachmentResponse
@@ -1511,7 +1512,6 @@ class CipherManagerTest {
 
             val result = cipherManager.updateCipherCollections(
                 cipherId = "cipherId",
-                cipherView = mockk(),
                 collectionIds = emptyList(),
             )
 
@@ -1520,109 +1520,96 @@ class CipherManagerTest {
 
     @Test
     @Suppress("MaxLineLength")
-    fun `updateCipherCollections with cipherService updateCipherCollections success should return ShareCipherResultSuccess`() =
+    fun `updateCipherCollections with cipherService updateCipherCollections failure should return Error`() =
         runTest {
             fakeAuthDiskSource.userState = MOCK_USER_STATE
             val userId = "mockId-1"
-            coEvery {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = createMockCipherView(number = 1),
-                )
-            } returns createMockEncryptionContext(
-                number = 1,
-                cipher = createMockSdkCipher(number = 1, clock = clock),
-            ).asSuccess()
+            val cipherId = "mockCipherId-1"
+            val error = Throwable("Fail")
             coEvery {
                 ciphersService.updateCipherCollections(
-                    cipherId = "mockId-1",
+                    cipherId = cipherId,
                     body = UpdateCipherCollectionsJsonRequest(
                         collectionIds = listOf("mockId-1"),
                     ),
                 )
-            } returns Unit.asSuccess()
-            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+            } returns error.asFailure()
 
             val result = cipherManager.updateCipherCollections(
-                cipherId = "mockId-1",
-                cipherView = createMockCipherView(number = 1).copy(
-                    collectionIds = listOf("mockId-1"),
-                ),
+                cipherId = cipherId,
                 collectionIds = listOf("mockId-1"),
             )
 
-            assertEquals(ShareCipherResult.Success, result)
+            assertEquals(ShareCipherResult.Error(error), result)
+            coVerify(exactly = 0) {
+                vaultDiskSource.deleteCipher(userId = userId, cipherId = any())
+                vaultDiskSource.saveCipher(userId = userId, cipher = any())
+            }
         }
 
     @Test
     @Suppress("MaxLineLength")
-    fun `updateCipherCollections with updateCipherCollections shareCipher failure should return ShareCipherResultError`() =
+    fun `updateCipherCollections with cipherService updateCipherCollections success and null cipher should delete the cipher and return Success`() =
         runTest {
             fakeAuthDiskSource.userState = MOCK_USER_STATE
             val userId = "mockId-1"
-            coEvery {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = createMockCipherView(number = 1),
-                )
-            } returns createMockEncryptionContext(
-                number = 1,
-                cipher = createMockSdkCipher(number = 1, clock = clock),
-            ).asSuccess()
-            val error = Throwable("Fail")
+            val cipherId = "mockCipherId-1"
+            val response = UpdateCipherCollectionsResponseJson(cipher = null)
             coEvery {
                 ciphersService.updateCipherCollections(
-                    cipherId = "mockId-1",
+                    cipherId = cipherId,
                     body = UpdateCipherCollectionsJsonRequest(
                         collectionIds = listOf("mockId-1"),
                     ),
                 )
-            } returns error.asFailure()
-            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+            } returns response.asSuccess()
+            coEvery { vaultDiskSource.deleteCipher(userId = userId, cipherId = cipherId) } just runs
 
             val result = cipherManager.updateCipherCollections(
-                cipherId = "mockId-1",
-                cipherView = createMockCipherView(number = 1).copy(
-                    collectionIds = listOf("mockId-1"),
-                ),
+                cipherId = cipherId,
                 collectionIds = listOf("mockId-1"),
             )
 
-            assertEquals(ShareCipherResult.Error(error = error), result)
+            assertEquals(ShareCipherResult.Success, result)
+            coVerify(exactly = 0) {
+                vaultDiskSource.saveCipher(userId = userId, cipher = any())
+            }
+            coVerify(exactly = 1) {
+                vaultDiskSource.deleteCipher(userId = userId, cipherId = cipherId)
+            }
         }
 
     @Test
     @Suppress("MaxLineLength")
-    fun `updateCipherCollections with updateCipherCollections encryptCipher failure should return ShareCipherResultError`() =
+    fun `updateCipherCollections with cipherService updateCipherCollections success and valid cipher should save the cipher and return Success`() =
         runTest {
             fakeAuthDiskSource.userState = MOCK_USER_STATE
             val userId = "mockId-1"
-            val error = Throwable("Fail")
-            coEvery {
-                vaultSdkSource.encryptCipher(
-                    userId = userId,
-                    cipherView = createMockCipherView(number = 1),
-                )
-            } returns error.asFailure()
+            val cipherId = "mockCipherId-1"
+            val cipher = mockk<SyncResponseJson.Cipher>()
+            val response = UpdateCipherCollectionsResponseJson(cipher = cipher)
             coEvery {
                 ciphersService.updateCipherCollections(
-                    cipherId = "mockId-1",
+                    cipherId = cipherId,
                     body = UpdateCipherCollectionsJsonRequest(
                         collectionIds = listOf("mockId-1"),
                     ),
                 )
-            } returns Unit.asSuccess()
-            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+            } returns response.asSuccess()
+            coEvery { vaultDiskSource.saveCipher(userId = userId, cipher = cipher) } just runs
 
             val result = cipherManager.updateCipherCollections(
-                cipherId = "mockId-1",
-                cipherView = createMockCipherView(number = 1).copy(
-                    collectionIds = listOf("mockId-1"),
-                ),
+                cipherId = cipherId,
                 collectionIds = listOf("mockId-1"),
             )
 
-            assertEquals(ShareCipherResult.Error(error = error), result)
+            assertEquals(ShareCipherResult.Success, result)
+            coVerify(exactly = 0) {
+                vaultDiskSource.deleteCipher(userId = userId, cipherId = any())
+            }
+            coVerify(exactly = 1) {
+                vaultDiskSource.saveCipher(userId = userId, cipher = cipher)
+            }
         }
 
     @Test

app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModelTest.kt

@@ -513,7 +513,6 @@ class VaultMoveToOrganizationViewModelTest : BaseViewModelTest() {
             coEvery {
                 vaultRepository.updateCipherCollections(
                     cipherId = "mockCipherId",
-                    cipherView = createMockCipherView(number = 1),
                     collectionIds = listOf("mockId-1"),
                 )
             } returns ShareCipherResult.Success
@@ -533,7 +532,6 @@ class VaultMoveToOrganizationViewModelTest : BaseViewModelTest() {
             coVerify {
                 vaultRepository.updateCipherCollections(
                     cipherId = "mockCipherId",
-                    cipherView = createMockCipherView(number = 1),
                     collectionIds = listOf("mockId-1"),
                 )
             }

network/src/main/kotlin/com/bitwarden/network/api/CiphersApi.kt

@@ -11,6 +11,7 @@ import com.bitwarden.network.model.NetworkResult
 import com.bitwarden.network.model.ShareCipherJsonRequest
 import com.bitwarden.network.model.SyncResponseJson
 import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
+import com.bitwarden.network.model.UpdateCipherCollectionsResponseJson
 import okhttp3.MultipartBody
 import retrofit2.http.Body
 import retrofit2.http.DELETE
@@ -115,11 +116,11 @@ internal interface CiphersApi {
     /**
      * Updates a cipher's collections.
      */
-    @PUT("ciphers/{cipherId}/collections")
+    @PUT("ciphers/{cipherId}/collections_v2")
     suspend fun updateCipherCollections(
         @Path("cipherId") cipherId: String,
         @Body body: UpdateCipherCollectionsJsonRequest,
-    ): NetworkResult<Unit>
+    ): NetworkResult<UpdateCipherCollectionsResponseJson>
 
     /**
      * Hard deletes a cipher.

network/src/main/kotlin/com/bitwarden/network/model/UpdateCipherCollectionsResponseJson.kt

@@ -0,0 +1,15 @@
+package com.bitwarden.network.model
+
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+
+/**
+ * Represents API response model for updating a cipher's collections.
+ *
+ * @property cipher The updated cipher, or `null` if the user no longer has access.
+ */
+@Serializable
+data class UpdateCipherCollectionsResponseJson(
+    @SerialName("cipher")
+    val cipher: SyncResponseJson.Cipher?,
+)

network/src/main/kotlin/com/bitwarden/network/service/CiphersService.kt

@@ -16,6 +16,7 @@ import com.bitwarden.network.model.ShareCipherJsonRequest
 import com.bitwarden.network.model.SyncResponseJson
 import com.bitwarden.network.model.UnarchiveCipherResponseJson
 import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
+import com.bitwarden.network.model.UpdateCipherCollectionsResponseJson
 import com.bitwarden.network.model.UpdateCipherResponseJson
 import java.io.File
 
@@ -101,7 +102,7 @@ interface CiphersService {
     suspend fun updateCipherCollections(
         cipherId: String,
         body: UpdateCipherCollectionsJsonRequest,
-    ): Result<Unit>
+    ): Result<UpdateCipherCollectionsResponseJson>
 
     /**
      * Attempt to hard delete a cipher.

network/src/main/kotlin/com/bitwarden/network/service/CiphersServiceImpl.kt

@@ -20,6 +20,7 @@ import com.bitwarden.network.model.ShareCipherJsonRequest
 import com.bitwarden.network.model.SyncResponseJson
 import com.bitwarden.network.model.UnarchiveCipherResponseJson
 import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
+import com.bitwarden.network.model.UpdateCipherCollectionsResponseJson
 import com.bitwarden.network.model.UpdateCipherResponseJson
 import com.bitwarden.network.model.toBitwardenError
 import com.bitwarden.network.util.NetworkErrorCode
@@ -235,7 +236,7 @@ internal class CiphersServiceImpl(
     override suspend fun updateCipherCollections(
         cipherId: String,
         body: UpdateCipherCollectionsJsonRequest,
-    ): Result<Unit> =
+    ): Result<UpdateCipherCollectionsResponseJson> =
         ciphersApi
             .updateCipherCollections(
                 cipherId = cipherId,