BIT-1575: Update cipher collections functionality (#904)

2024-10-31 15:15:34 +03:00 · 2024-01-31 12:04:28 -07:00 · 2024-01-31 12:04:28 -07:00 · 2d0353d744
commit 2d0353d744
parent bb0c91ee5a
11 changed files with 297 additions and 30 deletions
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/CiphersApi.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/CiphersApi.kt
@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrganizationJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import okhttp3.MultipartBody
 import retrofit2.http.Body
 import retrofit2.http.DELETE
@ -71,6 +72,15 @@ interface CiphersApi {
        @Body body: ShareCipherJsonRequest,
    ): Result<SyncResponseJson.Cipher>

+    /**
+     * Updates a cipher's collections.
+     */
+    @PUT("ciphers/{cipherId}/collections")
+    suspend fun updateCipherCollections(
+        @Path("cipherId") cipherId: String,
+        @Body body: UpdateCipherCollectionsJsonRequest,
+    ): Result<Unit>
+
    /**
     * Hard deletes a cipher.
     */
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/UpdateCipherCollectionsJsonRequest.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/UpdateCipherCollectionsJsonRequest.kt
@ -0,0 +1,15 @@
+package com.x8bit.bitwarden.data.vault.datasource.network.model
+
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+
+/**
+ * Represents an update cipher collections request.
+ *
+ * @property collectionIds A list of collection ids associated with the cipher.
+ */
+@Serializable
+data class UpdateCipherCollectionsJsonRequest(
+    @SerialName("CollectionIds")
+    val collectionIds: List<String>,
+)
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersService.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersService.kt
@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrganizationJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson

 /**
@ -57,6 +58,14 @@ interface CiphersService {
        body: ShareCipherJsonRequest,
    ): Result<SyncResponseJson.Cipher>

+    /**
+     * Attempt to update a cipher's collections.
+     */
+    suspend fun updateCipherCollections(
+        cipherId: String,
+        body: UpdateCipherCollectionsJsonRequest,
+    ): Result<Unit>
+
    /**
     * Attempt to hard delete a cipher.
     */
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceImpl.kt
@ -12,6 +12,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrg
 import com.x8bit.bitwarden.data.vault.datasource.network.model.FileUploadType
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
 import kotlinx.serialization.json.Json
 import okhttp3.MediaType.Companion.toMediaType
@ -118,6 +119,15 @@ class CiphersServiceImpl(
            body = body,
        )

+    override suspend fun updateCipherCollections(
+        cipherId: String,
+        body: UpdateCipherCollectionsJsonRequest,
+    ): Result<Unit> =
+        ciphersApi.updateCipherCollections(
+            cipherId = cipherId,
+            body = body,
+        )
+
    override suspend fun hardDeleteCipher(cipherId: String): Result<Unit> =
        ciphersApi.hardDeleteCipher(cipherId = cipherId)

--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt
@ -257,6 +257,15 @@ interface VaultRepository : VaultLockManager {
        collectionIds: List<String>,
    ): ShareCipherResult

+    /**
+     * Attempt to update a cipher with the given collectionIds.
+     */
+    suspend fun updateCipherCollections(
+        cipherId: String,
+        cipherView: CipherView,
+        collectionIds: List<String>,
+    ): ShareCipherResult
+
    /**
     * Attempt to create a send. The [fileUri] _must_ be present when the given [SendView] has a
     * [SendView.type] of [SendType.FILE].
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
@ -38,6 +38,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.AttachmentJsonReq
 import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrganizationJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateFolderResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendResponseJson
@ -753,7 +754,7 @@ class VaultRepositoryImpl(
        cipherView: CipherView,
        collectionIds: List<String>,
    ): ShareCipherResult {
-        val userId = activeUserId ?: return ShareCipherResult.Error(null)
+        val userId = activeUserId ?: return ShareCipherResult.Error
        return vaultSdkSource
            .encryptCipher(
                userId = userId,
@ -768,12 +769,40 @@ class VaultRepositoryImpl(
                    ),
                )
            }
+            .onSuccess { vaultDiskSource.saveCipher(userId = userId, cipher = it) }
            .fold(
-                onFailure = { ShareCipherResult.Error(errorMessage = null) },
-                onSuccess = {
-                    vaultDiskSource.saveCipher(userId = userId, cipher = it)
-                    ShareCipherResult.Success
-                },
+                onFailure = { ShareCipherResult.Error },
+                onSuccess = { ShareCipherResult.Success },
+            )
+    }
+
+    override suspend fun updateCipherCollections(
+        cipherId: String,
+        cipherView: CipherView,
+        collectionIds: List<String>,
+    ): ShareCipherResult {
+        val userId = activeUserId ?: return ShareCipherResult.Error
+        return ciphersService
+            .updateCipherCollections(
+                cipherId = cipherId,
+                body = UpdateCipherCollectionsJsonRequest(collectionIds = collectionIds),
+            )
+            .flatMap {
+                vaultSdkSource
+                    .encryptCipher(
+                        userId = userId,
+                        cipherView = cipherView.copy(collectionIds = collectionIds),
+                    )
+            }
+            .onSuccess { cipher ->
+                vaultDiskSource.saveCipher(
+                    userId = userId,
+                    cipher = cipher.toEncryptedNetworkCipherResponse(),
+                )
+            }
+            .fold(
+                onSuccess = { ShareCipherResult.Success },
+                onFailure = { ShareCipherResult.Error },
            )
    }

--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/ShareCipherResult.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/ShareCipherResult.kt
@ -10,8 +10,7 @@ sealed class ShareCipherResult {
    data object Success : ShareCipherResult()

    /**
-     * Generic error while sharing cipher. The optional [errorMessage] may be displayed directly in
-     * the UI when present.
+     * Generic error while sharing cipher.
     */
-    data class Error(val errorMessage: String?) : ShareCipherResult()
+    data object Error : ShareCipherResult()
 }
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModel.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModel.kt
@ -60,14 +60,14 @@ class VaultMoveToOrganizationViewModel @Inject constructor(
        ) { cipherViewState, collectionsState, userState ->
            VaultMoveToOrganizationAction.Internal.VaultDataReceive(
                vaultData = combineDataStates(
-                    dataState1 = cipherViewState.map { Unit },
+                    dataState1 = cipherViewState,
                    dataState2 = collectionsState,
-                    dataState3 = DataState.Loaded(userState).map { Unit },
-                ) { _, collectionsData, _ ->
+                    dataState3 = DataState.Loaded(userState),
+                ) { cipherData, collectionsData, userData ->
                    Triple(
-                        first = cipherViewState.data,
+                        first = cipherData,
                        second = collectionsData,
-                        third = userState,
+                        third = userData,
                    )
                },
            )
@ -270,6 +270,12 @@ class VaultMoveToOrganizationViewModel @Inject constructor(
        cipherView: CipherView,
        contentState: VaultMoveToOrganizationState.ViewState.Content,
    ) {
+        val collectionIds = contentState
+            .selectedOrganization
+            .collections
+            .filter { it.isSelected }
+            .map { it.id }
+
        mutableStateFlow.update {
            it.copy(
                dialogState = VaultMoveToOrganizationState.DialogState.Loading(
@ -280,17 +286,21 @@ class VaultMoveToOrganizationViewModel @Inject constructor(
        viewModelScope.launch {
            trySendAction(
                VaultMoveToOrganizationAction.Internal.ShareCipherResultReceive(
-                    vaultRepository.shareCipher(
-                        cipherId = mutableStateFlow.value.vaultItemId,
-                        cipherView = cipherView.copy(
-                            organizationId = contentState.selectedOrganizationId,
-                        ),
-                        collectionIds = contentState
-                            .selectedOrganization
-                            .collections
-                            .filter { it.isSelected }
-                            .map { it.id },
-                    ),
+                    if (state.onlyShowCollections) {
+                        vaultRepository.updateCipherCollections(
+                            cipherId = mutableStateFlow.value.vaultItemId,
+                            cipherView = cipherView,
+                            collectionIds = collectionIds,
+                        )
+                    } else {
+                        vaultRepository.shareCipher(
+                            cipherId = mutableStateFlow.value.vaultItemId,
+                            cipherView = cipherView.copy(
+                                organizationId = contentState.selectedOrganizationId,
+                            ),
+                            collectionIds = collectionIds,
+                        )
+                    },
                ),
            )
        }
--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/CiphersServiceTest.kt
@ -7,6 +7,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.api.CiphersApi
 import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrganizationJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.FileUploadType
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockAttachmentJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockAttachmentJsonResponse
@ -211,6 +212,22 @@ class CiphersServiceTest : BaseServiceTest() {
        )
    }

+    @Test
+    fun `updateCipherCollections should execute the updateCipherCollections API`() = runTest {
+        server.enqueue(MockResponse().setResponseCode(200))
+
+        val result = ciphersService.updateCipherCollections(
+            cipherId = "mockId-1",
+            body = UpdateCipherCollectionsJsonRequest(
+                collectionIds = listOf("mockId-1"),
+            ),
+        )
+        assertEquals(
+            Unit,
+            result.getOrThrow(),
+        )
+    }
+
    @Test
    fun `restoreCipher should execute the restoreCipher API`() = runTest {
        server.enqueue(MockResponse().setResponseCode(200))
--- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt
@ -44,6 +44,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.SendFileResponseJ
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SendTypeJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateFolderResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendResponseJson
@ -2748,7 +2749,7 @@ class VaultRepositoryTest {
            )

            assertEquals(
-                ShareCipherResult.Error(null),
+                ShareCipherResult.Error,
                result,
            )
        }
@ -2818,7 +2819,7 @@ class VaultRepositoryTest {
            )

            assertEquals(
-                ShareCipherResult.Error(errorMessage = null),
+                ShareCipherResult.Error,
                result,
            )
        }
@ -2853,7 +2854,129 @@ class VaultRepositoryTest {
            )

            assertEquals(
-                ShareCipherResult.Error(errorMessage = null),
+                ShareCipherResult.Error,
+                result,
+            )
+        }
+
+    @Test
+    fun `updateCipherCollections with no active user should return ShareCipherResult Error`() =
+        runTest {
+            fakeAuthDiskSource.userState = null
+
+            val result = vaultRepository.updateCipherCollections(
+                cipherId = "cipherId",
+                cipherView = mockk(),
+                collectionIds = emptyList(),
+            )
+
+            assertEquals(
+                ShareCipherResult.Error,
+                result,
+            )
+        }
+
+    @Test
+    @Suppress("MaxLineLength")
+    fun `updateCipherCollections with cipherService updateCipherCollections success should return ShareCipherResultSuccess`() =
+        runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            val userId = "mockId-1"
+            coEvery {
+                vaultSdkSource.encryptCipher(
+                    userId = userId,
+                    cipherView = createMockCipherView(number = 1),
+                )
+            } returns createMockSdkCipher(number = 1).asSuccess()
+            coEvery {
+                ciphersService.updateCipherCollections(
+                    cipherId = "mockId-1",
+                    body = UpdateCipherCollectionsJsonRequest(
+                        collectionIds = listOf("mockId-1"),
+                    ),
+                )
+            } returns Unit.asSuccess()
+            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+
+            val result = vaultRepository.updateCipherCollections(
+                cipherId = "mockId-1",
+                cipherView = createMockCipherView(number = 1)
+                    .copy(collectionIds = listOf("mockId-1")),
+                collectionIds = listOf("mockId-1"),
+            )
+
+            assertEquals(
+                ShareCipherResult.Success,
+                result,
+            )
+        }
+
+    @Test
+    @Suppress("MaxLineLength")
+    fun `updateCipherCollections with updateCipherCollections shareCipher failure should return ShareCipherResultError`() =
+        runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            val userId = "mockId-1"
+            coEvery {
+                vaultSdkSource.encryptCipher(
+                    userId = userId,
+                    cipherView = createMockCipherView(number = 1),
+                )
+            } returns createMockSdkCipher(number = 1).asSuccess()
+            coEvery {
+                ciphersService.updateCipherCollections(
+                    cipherId = "mockId-1",
+                    body = UpdateCipherCollectionsJsonRequest(
+                        collectionIds = listOf("mockId-1"),
+                    ),
+                )
+            } returns Throwable("Fail").asFailure()
+            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+
+            val result = vaultRepository.updateCipherCollections(
+                cipherId = "mockId-1",
+                cipherView = createMockCipherView(number = 1)
+                    .copy(collectionIds = listOf("mockId-1")),
+                collectionIds = listOf("mockId-1"),
+            )
+
+            assertEquals(
+                ShareCipherResult.Error,
+                result,
+            )
+        }
+
+    @Test
+    @Suppress("MaxLineLength")
+    fun `updateCipherCollections with updateCipherCollections encryptCipher failure should return ShareCipherResultError`() =
+        runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            val userId = "mockId-1"
+            coEvery {
+                vaultSdkSource.encryptCipher(
+                    userId = userId,
+                    cipherView = createMockCipherView(number = 1),
+                )
+            } returns Throwable("Fail").asFailure()
+            coEvery {
+                ciphersService.updateCipherCollections(
+                    cipherId = "mockId-1",
+                    body = UpdateCipherCollectionsJsonRequest(
+                        collectionIds = listOf("mockId-1"),
+                    ),
+                )
+            } returns Unit.asSuccess()
+            coEvery { vaultDiskSource.saveCipher(userId, any()) } just runs
+
+            val result = vaultRepository.updateCipherCollections(
+                cipherId = "mockId-1",
+                cipherView = createMockCipherView(number = 1)
+                    .copy(collectionIds = listOf("mockId-1")),
+                collectionIds = listOf("mockId-1"),
+            )
+
+            assertEquals(
+                ShareCipherResult.Error,
                result,
            )
        }
--- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModelTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/movetoorganization/VaultMoveToOrganizationViewModelTest.kt
@ -301,7 +301,7 @@ class VaultMoveToOrganizationViewModelTest : BaseViewModelTest() {
                cipherView = createMockCipherView(number = 1),
                collectionIds = listOf("mockId-1"),
            )
-        } returns ShareCipherResult.Error(errorMessage = null)
+        } returns ShareCipherResult.Error
        viewModel.stateFlow.test {
            assertEquals(
                initialState.copy(
@ -378,6 +378,41 @@ class VaultMoveToOrganizationViewModelTest : BaseViewModelTest() {
        }
    }

+    @Test
+    fun `MoveClick with onlyShowCollections true should invoke updateCipherCollections`() =
+        runTest {
+            val viewModel = createViewModel(
+                savedStateHandle = createSavedStateHandleWithState(
+                    state = createVaultMoveToOrganizationState(
+                        onlyShowCollections = true,
+                    ),
+                ),
+            )
+            mutableCollectionFlow.tryEmit(value = DataState.Loaded(DEFAULT_COLLECTIONS))
+            mutableVaultItemFlow.tryEmit(value = DataState.Loaded(createMockCipherView(number = 1)))
+            coEvery {
+                vaultRepository.updateCipherCollections(
+                    cipherId = "mockCipherId",
+                    cipherView = createMockCipherView(number = 1),
+                    collectionIds = listOf("mockId-1"),
+                )
+            } returns ShareCipherResult.Success
+            viewModel.eventFlow.test {
+                viewModel.actionChannel.trySend(VaultMoveToOrganizationAction.MoveClick)
+                assertEquals(
+                    VaultMoveToOrganizationEvent.NavigateBack,
+                    awaitItem(),
+                )
+            }
+            coVerify {
+                vaultRepository.updateCipherCollections(
+                    cipherId = "mockCipherId",
+                    cipherView = createMockCipherView(number = 1),
+                    collectionIds = listOf("mockId-1"),
+                )
+            }
+        }
+
    private fun createViewModel(
        savedStateHandle: SavedStateHandle = initialSavedStateHandle,
        vaultRepo: VaultRepository = vaultRepository,
@ -403,11 +438,12 @@ class VaultMoveToOrganizationViewModelTest : BaseViewModelTest() {
        viewState: VaultMoveToOrganizationState.ViewState = VaultMoveToOrganizationState.ViewState.Loading,
        vaultItemId: String = "mockCipherId",
        dialogState: VaultMoveToOrganizationState.DialogState? = null,
+        onlyShowCollections: Boolean = false,
    ): VaultMoveToOrganizationState = VaultMoveToOrganizationState(
        vaultItemId = vaultItemId,
        viewState = viewState,
        dialogState = dialogState,
-        onlyShowCollections = false,
+        onlyShowCollections = onlyShowCollections,
    )
 }