From f7150d33cbb467ac510bcd7b9d2d078cd6a4582a Mon Sep 17 00:00:00 2001
From: Ramsey Smith <142836716+ramsey-livefront@users.noreply.github.com>
Date: Wed, 7 Feb 2024 15:52:39 -0700
Subject: [PATCH] BIT-1704: Edit items in collection bug (#977)
---
.../vault/repository/VaultRepositoryImpl.kt | 6 +-
.../addedit/util/CipherViewExtensions.kt | 6 +-
.../vault/repository/VaultRepositoryTest.kt | 2 +-
.../addedit/util/CipherViewExtensionsTest.kt | 171 ++++++++++++++++++
4 files changed, 182 insertions(+), 3 deletions(-)
diff --git 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
index b92784a79..0563915aa 100644
--- 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
@@ -744,7 +744,11 @@ class VaultRepositoryImpl(
}
is UpdateCipherResponseJson.Success -> {
- vaultDiskSource.saveCipher(userId = userId, cipher = response.cipher)
+ vaultDiskSource.saveCipher(
+ userId = userId,
+ cipher = response.cipher
+ .copy(collectionIds = cipherView.collectionIds),
+ )
UpdateCipherResult.Success
}
}
diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt
index 240ce1dca..a2dd91b08 100644
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensions.kt
@@ -114,6 +114,7 @@ fun VaultAddEditState.ViewState.appendFolderAndOwnerData(
),
availableOwners = activeAccount.toAvailableOwners(
collectionViewList = collectionViewList,
+ cipherView = currentContentState.common.originalCipher,
isIndividualVaultDisabled = isIndividualVaultDisabled,
),
),
@@ -165,6 +166,7 @@ private fun UserState.Account.toSelectedOwnerId(cipherView: CipherView?): String
private fun UserState.Account.toAvailableOwners(
collectionViewList: List<CollectionView>,
+ cipherView: CipherView?,
isIndividualVaultDisabled: Boolean,
): List<VaultAddEditState.Owner> =
listOfNotNull(
@@ -188,7 +190,9 @@ private fun UserState.Account.toAvailableOwners(
VaultCollection(
id = collection.id.orEmpty(),
name = collection.name,
- isSelected = false,
+ isSelected = cipherView
+ ?.collectionIds
+ ?.contains(collection.id) == true,
)
},
)
diff --git 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
index 110635804..5d8314b47 100644
--- 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
@@ -2021,7 +2021,7 @@ class VaultRepositoryTest {
coEvery {
vaultDiskSource.saveCipher(
userId = userId,
- cipher = mockCipher,
+ cipher = mockCipher.copy(collectionIds = mockCipherView.collectionIds),
)
} just runs
diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensionsTest.kt
index 5fab1bd0c..7ee17644e 100644
--- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensionsTest.kt
+++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/addedit/util/CipherViewExtensionsTest.kt
@@ -13,10 +13,20 @@ import com.bitwarden.core.PasswordHistoryView
import com.bitwarden.core.SecureNoteType
import com.bitwarden.core.SecureNoteView
import com.x8bit.bitwarden.R
+import com.x8bit.bitwarden.data.auth.repository.model.Organization
+import com.x8bit.bitwarden.data.auth.repository.model.UserState
+import com.x8bit.bitwarden.data.auth.repository.model.VaultUnlockType
+import com.x8bit.bitwarden.data.platform.repository.model.Environment
+import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCipherView
+import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockCollectionView
+import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockFolderView
+import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.manager.resource.ResourceManager
import com.x8bit.bitwarden.ui.vault.feature.addedit.VaultAddEditState
import com.x8bit.bitwarden.ui.vault.feature.addedit.model.UriItem
+import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType
import com.x8bit.bitwarden.ui.vault.model.VaultCardBrand
+import com.x8bit.bitwarden.ui.vault.model.VaultCollection
import com.x8bit.bitwarden.ui.vault.model.VaultLinkedFieldType
import io.mockk.every
import io.mockk.mockk
@@ -33,6 +43,7 @@ class CipherViewExtensionsTest {
private val resourceManager: ResourceManager = mockk {
every { getString(R.string.clone) } returns "Clone"
+ every { getString(R.string.folder_none) } returns "No Folder"
}
@BeforeEach
@@ -248,6 +259,166 @@ class CipherViewExtensionsTest {
result,
)
}
+
+ @Test
+ fun `validateCipherOrReturnErrorState with valid cipher should return provided state`() {
+ val providedState = VaultAddEditState.ViewState.Loading
+
+ val result = createMockCipherView(number = 1)
+ .validateCipherOrReturnErrorState(
+ currentAccount = createAccount(),
+ vaultAddEditType = VaultAddEditType.AddItem,
+ ) { _, _ -> providedState }
+
+ assertEquals(providedState, result)
+ }
+
+ @Test
+ fun `validateCipherOrReturnErrorState with EditItem null cipher type should return Error`() {
+ val providedState = VaultAddEditState.ViewState.Loading
+
+ val result = null
+ .validateCipherOrReturnErrorState(
+ currentAccount = createAccount(),
+ vaultAddEditType = VaultAddEditType.EditItem(vaultItemId = "mockId-1"),
+ ) { _, _ -> providedState }
+
+ assertEquals(
+ VaultAddEditState.ViewState.Error(R.string.generic_error_message.asText()),
+ result,
+ )
+ }
+
+ @Test
+ fun `validateCipherOrReturnErrorState with null account type should return Error`() {
+ val providedState = VaultAddEditState.ViewState.Loading
+
+ val result = createMockCipherView(number = 1)
+ .validateCipherOrReturnErrorState(
+ currentAccount = null,
+ vaultAddEditType = VaultAddEditType.AddItem,
+ ) { _, _ -> providedState }
+
+ assertEquals(
+ VaultAddEditState.ViewState.Error(R.string.generic_error_message.asText()),
+ result,
+ )
+ }
+
+ @Test
+ fun `appendFolderAndOwnerData should append folder and owner data`() {
+ val viewState = createSecureNoteViewState(withFolderAndOwnerData = false)
+ val account = createAccount()
+ val folderView = listOf(createMockFolderView(number = 1))
+ val collectionList = listOf(createMockCollectionView(number = 1))
+
+ val result = viewState.appendFolderAndOwnerData(
+ folderViewList = folderView,
+ collectionViewList = collectionList,
+ activeAccount = account,
+ isIndividualVaultDisabled = false,
+ resourceManager = resourceManager,
+ )
+
+ assertEquals(
+ createSecureNoteViewState(withFolderAndOwnerData = true),
+ result,
+ )
+ }
+
+ private fun createSecureNoteViewState(
+ cipherView: CipherView = createMockCipherView(number = 1),
+ withFolderAndOwnerData: Boolean,
+ ): VaultAddEditState.ViewState.Content =
+ VaultAddEditState.ViewState.Content(
+ common = VaultAddEditState.ViewState.Content.Common(
+ originalCipher = cipherView,
+ name = "cipher",
+ favorite = false,
+ masterPasswordReprompt = true,
+ notes = "Lots of notes",
+ customFieldData = listOf(
+ VaultAddEditState.Custom.BooleanField(
+ itemId = TEST_ID,
+ name = "TestBoolean",
+ value = false,
+ ),
+ VaultAddEditState.Custom.TextField(
+ itemId = TEST_ID,
+ name = "TestText",
+ value = "TestText",
+ ),
+ VaultAddEditState.Custom.HiddenField(
+ itemId = TEST_ID,
+ name = "TestHidden",
+ value = "TestHidden",
+ ),
+ ),
+ availableFolders = emptyList(),
+ availableOwners = emptyList(),
+ )
+ .let {
+ if (withFolderAndOwnerData) {
+ it.copy(
+ selectedFolderId = "mockId-1",
+ selectedOwnerId = "mockOrganizationId-1",
+ availableFolders = listOf(
+ VaultAddEditState.Folder(
+ id = null,
+ name = "No Folder",
+ ),
+ VaultAddEditState.Folder(
+ id = "mockId-1",
+ name = "mockName-1",
+ ),
+ ),
+ availableOwners = listOf(
+ VaultAddEditState.Owner(
+ id = null,
+ name = "activeEmail",
+ collections = emptyList(),
+ ),
+ VaultAddEditState.Owner(
+ id = "mockOrganizationId-1",
+ name = "organizationName",
+ collections = listOf(
+ VaultCollection(
+ id = "mockId-1",
+ name = "mockName-1",
+ isSelected = true,
+ ),
+ ),
+ ),
+ ),
+ )
+ } else {
+ it
+ }
+ },
+ isIndividualVaultDisabled = true,
+ type = VaultAddEditState.ViewState.Content.ItemType.SecureNotes,
+ )
+
+ private fun createAccount(): UserState.Account =
+ UserState.Account(
+ userId = "activeUserId",
+ name = "activeName",
+ email = "activeEmail",
+ avatarColorHex = "#ffecbc49",
+ environment = Environment.Eu,
+ isPremium = true,
+ isLoggedIn = false,
+ isVaultUnlocked = false,
+ needsPasswordReset = false,
+ organizations = listOf(
+ Organization(
+ id = "mockOrganizationId-1",
+ name = "organizationName",
+ ),
+ ),
+ isBiometricsEnabled = true,
+ vaultUnlockType = VaultUnlockType.MASTER_PASSWORD,
+ )
}
private val DEFAULT_BASE_CIPHER_VIEW: CipherView = CipherView(