BIT-502: Save the updated ciphers from the edit screen (#371)

2026-06-08 08:06:32 -05:00 · 2023-12-12 10:26:34 -06:00
parent 65b9005cbe
commit f4db50b700
8 changed files with 843 additions and 98 deletions
--- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/DataStateExtensions.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/DataStateExtensions.kt
@@ -1,6 +1,8 @@
 package com.x8bit.bitwarden.data.platform.repository.util

 import com.x8bit.bitwarden.data.platform.repository.model.DataState
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.transformWhile

 /**
 * Maps the data inside a [DataState] with the given [transform].
@@ -14,3 +16,11 @@ inline fun <T : Any?, R : Any?> DataState<T>.map(
    is DataState.Error -> DataState.Error(error, data?.let(transform))
    is DataState.NoNetwork -> DataState.NoNetwork(data?.let(transform))
 }
+
+/**
+ * Emits all values of a [DataState] [Flow] until it emits a [DataState.Loaded].
+ */
+fun <T : Any?> Flow<DataState<T>>.takeUntilLoaded(): Flow<DataState<T>> = transformWhile {
+    emit(it)
+    it !is DataState.Loaded
+}
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt
@@ -4,19 +4,27 @@ import android.os.Parcelable
 import androidx.annotation.StringRes
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.viewModelScope
+import com.bitwarden.core.CipherView
 import com.x8bit.bitwarden.R
+import com.x8bit.bitwarden.data.platform.repository.model.DataState
+import com.x8bit.bitwarden.data.platform.repository.util.takeUntilLoaded
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
 import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
+import com.x8bit.bitwarden.data.vault.repository.model.UpdateCipherResult
 import com.x8bit.bitwarden.ui.platform.base.BaseViewModel
 import com.x8bit.bitwarden.ui.platform.base.util.Text
 import com.x8bit.bitwarden.ui.platform.base.util.asText
+import com.x8bit.bitwarden.ui.platform.base.util.concat
+import com.x8bit.bitwarden.ui.vault.feature.additem.util.toViewState
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toCipherView
 import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
+import kotlinx.parcelize.IgnoredOnParcel
 import kotlinx.parcelize.Parcelize
 import javax.inject.Inject

@@ -55,6 +63,19 @@ class VaultAddItemViewModel @Inject constructor(

    init {
        stateFlow.onEach { savedStateHandle[KEY_STATE] = it }.launchIn(viewModelScope)
+
+        when (val vaultAddEditType = state.vaultAddEditType) {
+            VaultAddEditType.AddItem -> Unit
+            is VaultAddEditType.EditItem -> {
+                vaultRepository
+                    .getVaultItemStateFlow(vaultAddEditType.vaultItemId)
+                    // We'll stop getting updates as soon as we get some loaded data.
+                    .takeUntilLoaded()
+                    .map { VaultAddItemAction.Internal.VaultDataReceive(it) }
+                    .onEach(::sendAction)
+                    .launchIn(viewModelScope)
+            }
+        }
    }

    override fun handleAction(action: VaultAddItemAction) {
@@ -83,9 +104,21 @@ class VaultAddItemViewModel @Inject constructor(
                handleAddSecureNoteTypeAction(action)
            }

+            is VaultAddItemAction.Internal -> handleInternalActions(action)
+        }
+    }
+
+    private fun handleInternalActions(action: VaultAddItemAction.Internal) {
+        when (action) {
            is VaultAddItemAction.Internal.CreateCipherResultReceive -> {
                handleCreateCipherResultReceive(action)
            }
+
+            is VaultAddItemAction.Internal.UpdateCipherResultReceive -> {
+                handleUpdateCipherResultReceive(action)
+            }
+
+            is VaultAddItemAction.Internal.VaultDataReceive -> handleVaultDataReceive(action)
        }
    }

@@ -115,13 +148,20 @@ class VaultAddItemViewModel @Inject constructor(
        }

        viewModelScope.launch {
-            sendAction(
-                action = VaultAddItemAction.Internal.CreateCipherResultReceive(
-                    createCipherResult = vaultRepository.createCipher(
+            when (val vaultAddEditType = state.vaultAddEditType) {
+                VaultAddEditType.AddItem -> {
+                    val result = vaultRepository.createCipher(cipherView = content.toCipherView())
+                    sendAction(VaultAddItemAction.Internal.CreateCipherResultReceive(result))
+                }
+
+                is VaultAddEditType.EditItem -> {
+                    val result = vaultRepository.updateCipher(
+                        cipherId = vaultAddEditType.vaultItemId,
                        cipherView = content.toCipherView(),
-                    ),
-                ),
-            )
+                    )
+                    sendAction(VaultAddItemAction.Internal.UpdateCipherResultReceive(result))
+                }
+            }
        }
    }

@@ -533,6 +573,80 @@ class VaultAddItemViewModel @Inject constructor(
        }
    }

+    private fun handleUpdateCipherResultReceive(
+        action: VaultAddItemAction.Internal.UpdateCipherResultReceive,
+    ) {
+        mutableStateFlow.update { it.copy(dialog = null) }
+        when (action.updateCipherResult) {
+            is UpdateCipherResult.Error -> {
+                // TODO Display error dialog BIT-501
+                sendEvent(VaultAddItemEvent.ShowToast(message = "Save Item Failure"))
+            }
+
+            is UpdateCipherResult.Success -> {
+                sendEvent(VaultAddItemEvent.NavigateBack)
+            }
+        }
+    }
+
+    private fun handleVaultDataReceive(action: VaultAddItemAction.Internal.VaultDataReceive) {
+        when (val vaultDataState = action.vaultDataState) {
+            is DataState.Error -> {
+                mutableStateFlow.update {
+                    it.copy(
+                        viewState = VaultAddItemState.ViewState.Error(
+                            message = R.string.generic_error_message.asText(),
+                        ),
+                    )
+                }
+            }
+
+            is DataState.Loaded -> {
+                mutableStateFlow.update {
+                    it.copy(
+                        viewState = vaultDataState
+                            .data
+                            ?.toViewState()
+                            ?: VaultAddItemState.ViewState.Error(
+                                message = R.string.generic_error_message.asText(),
+                            ),
+                    )
+                }
+            }
+
+            DataState.Loading -> {
+                mutableStateFlow.update {
+                    it.copy(viewState = VaultAddItemState.ViewState.Loading)
+                }
+            }
+
+            is DataState.NoNetwork -> {
+                mutableStateFlow.update {
+                    it.copy(
+                        viewState = VaultAddItemState.ViewState.Error(
+                            message = R.string.internet_connection_required_title
+                                .asText()
+                                .concat(R.string.internet_connection_required_message.asText()),
+                        ),
+                    )
+                }
+            }
+
+            is DataState.Pending -> {
+                mutableStateFlow.update {
+                    it.copy(
+                        viewState = vaultDataState
+                            .data
+                            ?.toViewState()
+                            ?: VaultAddItemState.ViewState.Error(
+                                message = R.string.generic_error_message.asText(),
+                            ),
+                    )
+                }
+            }
+        }
+    }
+
    //endregion Internal Type Handlers

    //region Utility Functions
@@ -638,6 +752,14 @@ data class VaultAddItemState(
         */
        @Parcelize
        sealed class Content : ViewState() {
+            /**
+             * The original cipher from the vault that the user is editing.
+             *
+             * This is only present when editing a pre-existing cipher.
+             */
+            @IgnoredOnParcel
+            abstract val originalCipher: CipherView?
+
            /**
             * Represents the resource ID for the display string. This is an abstract property
             * that must be overridden by each subclass to provide the appropriate string resource
@@ -680,6 +802,8 @@ data class VaultAddItemState(
             */
            @Parcelize
            data class Login(
+                @IgnoredOnParcel
+                override val originalCipher: CipherView? = null,
                override val name: String = "",
                val username: String = "",
                val password: String = "",
@@ -706,6 +830,8 @@ data class VaultAddItemState(
             */
            @Parcelize
            data class Card(
+                @IgnoredOnParcel
+                override val originalCipher: CipherView? = null,
                override val name: String = "",
                override val masterPasswordReprompt: Boolean = false,
                override val ownership: String = DEFAULT_OWNERSHIP,
@@ -719,6 +845,8 @@ data class VaultAddItemState(
             */
            @Parcelize
            data class Identity(
+                @IgnoredOnParcel
+                override val originalCipher: CipherView? = null,
                override val name: String = "",
                override val masterPasswordReprompt: Boolean = false,
                override val ownership: String = DEFAULT_OWNERSHIP,
@@ -737,6 +865,8 @@ data class VaultAddItemState(
             */
            @Parcelize
            data class SecureNotes(
+                @IgnoredOnParcel
+                override val originalCipher: CipherView? = null,
                override val name: String = "",
                val folderName: Text = DEFAULT_FOLDER,
                val favorite: Boolean = false,
@@ -1006,6 +1136,12 @@ sealed class VaultAddItemAction {
     * Models actions that the [VaultAddItemViewModel] itself might send.
     */
    sealed class Internal : VaultAddItemAction() {
+        /**
+         * Indicates that the vault item data has been received.
+         */
+        data class VaultDataReceive(
+            val vaultDataState: DataState<CipherView?>,
+        ) : Internal()

        /**
         * Indicates a result for creating a cipher has been received.
@@ -1013,5 +1149,12 @@ sealed class VaultAddItemAction {
        data class CreateCipherResultReceive(
            val createCipherResult: CreateCipherResult,
        ) : Internal()
+
+        /**
+         * Indicates a result for updating a cipher has been received.
+         */
+        data class UpdateCipherResultReceive(
+            val updateCipherResult: UpdateCipherResult,
+        ) : Internal()
    }
 }
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/util/CipherViewExtensions.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/util/CipherViewExtensions.kt
@@ -0,0 +1,60 @@
+package com.x8bit.bitwarden.ui.vault.feature.additem.util
+
+import com.bitwarden.core.CipherRepromptType
+import com.bitwarden.core.CipherType
+import com.bitwarden.core.CipherView
+import com.x8bit.bitwarden.R
+import com.x8bit.bitwarden.ui.platform.base.util.asText
+import com.x8bit.bitwarden.ui.vault.feature.additem.VaultAddItemState
+
+/**
+ * Transforms [CipherView] into [VaultAddItemState.ViewState].
+ */
+fun CipherView.toViewState(): VaultAddItemState.ViewState =
+    when (type) {
+        CipherType.LOGIN -> {
+            val loginView = requireNotNull(this.login)
+            VaultAddItemState.ViewState.Content.Login(
+                originalCipher = this,
+                name = this.name,
+                username = loginView.username.orEmpty(),
+                password = loginView.password.orEmpty(),
+                uri = loginView.uris?.firstOrNull()?.uri.orEmpty(),
+                favorite = this.favorite,
+                masterPasswordReprompt = this.reprompt == CipherRepromptType.PASSWORD,
+                notes = this.notes.orEmpty(),
+                // TODO: Update these properties to pull folder from data layer (BIT-501)
+                folderName = this.folderId?.asText() ?: R.string.folder_none.asText(),
+                availableFolders = emptyList(),
+                // TODO: Update this property to pull owner from data layer (BIT-501)
+                ownership = "",
+                // TODO: Update this property to pull available owners from data layer (BIT-501)
+                availableOwners = emptyList(),
+            )
+        }
+
+        CipherType.SECURE_NOTE -> {
+            VaultAddItemState.ViewState.Content.SecureNotes(
+                originalCipher = this,
+                name = this.name,
+                favorite = this.favorite,
+                masterPasswordReprompt = this.reprompt == CipherRepromptType.PASSWORD,
+                notes = this.notes.orEmpty(),
+                // TODO: Update these properties to pull folder from data layer (BIT-501)
+                folderName = this.folderId?.asText() ?: R.string.folder_none.asText(),
+                availableFolders = emptyList(),
+                // TODO: Update this property to pull owner from data layer (BIT-501)
+                ownership = "",
+                // TODO: Update this property to pull available owners from data layer (BIT-501)
+                availableOwners = emptyList(),
+            )
+        }
+
+        CipherType.CARD -> VaultAddItemState.ViewState.Error(
+            message = "Not yet implemented.".asText(),
+        )
+
+        CipherType.IDENTITY -> VaultAddItemState.ViewState.Error(
+            message = "Not yet implemented.".asText(),
+        )
+    }
--- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/vault/util/VaultDataExtensions.kt
@@ -99,53 +99,53 @@ fun VaultAddItemState.ViewState.Content.toCipherView(): CipherView =
 */
 private fun VaultAddItemState.ViewState.Content.Login.toLoginCipherView(): CipherView =
    CipherView(
-        id = null,
-        // TODO use real organization id BIT-780
-        organizationId = null,
-        // TODO use real folder id BIT-528
-        folderId = null,
-        collectionIds = emptyList(),
-        key = null,
-        name = name,
-        notes = notes,
+        // Pulled from original cipher when editing, otherwise uses defaults
+        id = this.originalCipher?.id,
+        collectionIds = this.originalCipher?.collectionIds.orEmpty(),
+        key = this.originalCipher?.key,
+        edit = this.originalCipher?.edit ?: true,
+        viewPassword = this.originalCipher?.viewPassword ?: true,
+        localData = this.originalCipher?.localData,
+        attachments = this.originalCipher?.attachments,
+        organizationUseTotp = this.originalCipher?.organizationUseTotp ?: false,
+        passwordHistory = this.originalCipher?.passwordHistory,
+        creationDate = this.originalCipher?.creationDate ?: Instant.now(),
+        deletedDate = this.originalCipher?.deletedDate,
+        revisionDate = this.originalCipher?.revisionDate ?: Instant.now(),
+
+        // Type specific section
        type = CipherType.LOGIN,
        login = LoginView(
-            username = username,
-            password = password,
-            passwordRevisionDate = null,
+            username = this.username,
+            password = this.password,
+            passwordRevisionDate = this.originalCipher?.login?.passwordRevisionDate,
            uris = listOf(
+                // TODO Implement URI list (BIT-1094)
                LoginUriView(
-                    uri = uri,
-                    // TODO implement uri settings in BIT-1094
+                    uri = this.uri,
+                    // TODO Implement URI settings in (BIT-1094)
                    match = UriMatchType.DOMAIN,
                ),
            ),
            // TODO implement totp in BIT-1066
-            totp = null,
-            autofillOnPageLoad = false,
+            totp = this.originalCipher?.login?.totp,
+            autofillOnPageLoad = this.originalCipher?.login?.autofillOnPageLoad,
        ),
        identity = null,
        card = null,
        secureNote = null,
-        favorite = favorite,
-        reprompt = if (masterPasswordReprompt) {
-            CipherRepromptType.PASSWORD
-        } else {
-            CipherRepromptType.NONE
-        },
-        organizationUseTotp = false,
-        edit = true,
-        viewPassword = true,
-        localData = null,
-        attachments = null,
-        // TODO implement custom fields BIT-529
+
+        // Fields we always grab from the UI
+        name = this.name,
+        notes = this.notes,
+        favorite = this.favorite,
+        // TODO Use real folder ID (BIT-528)
+        folderId = this.originalCipher?.folderId,
+        // TODO Use real organization ID (BIT-780)
+        organizationId = this.originalCipher?.organizationId,
+        reprompt = this.toCipherRepromptType(),
+        // TODO Implement custom fields (BIT-529)
        fields = null,
-        passwordHistory = null,
-        creationDate = Instant.now(),
-        deletedDate = null,
-        // This is a throw away value.
-        // The SDK will eventually remove revisionDate via encryption.
-        revisionDate = Instant.now(),
    )

 /**
@@ -153,39 +153,38 @@ private fun VaultAddItemState.ViewState.Content.Login.toLoginCipherView(): Ciphe
 */
 private fun VaultAddItemState.ViewState.Content.SecureNotes.toSecureNotesCipherView(): CipherView =
    CipherView(
-        id = null,
-        // TODO use real organization id BIT-780
-        organizationId = null,
-        // TODO use real folder id BIT-528
-        folderId = null,
-        collectionIds = emptyList(),
-        key = null,
-        name = name,
-        notes = notes,
+        // Pulled from original cipher when editing, otherwise uses defaults
+        id = this.originalCipher?.id,
+        collectionIds = this.originalCipher?.collectionIds.orEmpty(),
+        key = this.originalCipher?.key,
+        edit = this.originalCipher?.edit ?: true,
+        viewPassword = this.originalCipher?.viewPassword ?: true,
+        localData = this.originalCipher?.localData,
+        attachments = this.originalCipher?.attachments,
+        organizationUseTotp = this.originalCipher?.organizationUseTotp ?: false,
+        passwordHistory = this.originalCipher?.passwordHistory,
+        creationDate = this.originalCipher?.creationDate ?: Instant.now(),
+        deletedDate = this.originalCipher?.deletedDate,
+        revisionDate = this.originalCipher?.revisionDate ?: Instant.now(),
+
+        // Type specific section
        type = CipherType.SECURE_NOTE,
-        secureNote = SecureNoteView(SecureNoteType.GENERIC),
+        secureNote = SecureNoteView(type = SecureNoteType.GENERIC),
        login = null,
        identity = null,
        card = null,
-        favorite = favorite,
-        reprompt = if (masterPasswordReprompt) {
-            CipherRepromptType.PASSWORD
-        } else {
-            CipherRepromptType.NONE
-        },
-        organizationUseTotp = false,
-        edit = true,
-        viewPassword = true,
-        localData = null,
-        attachments = null,
-        // TODO implement custom fields BIT-529
+
+        // Fields we always grab from the UI
+        name = this.name,
+        notes = this.notes,
+        favorite = this.favorite,
+        // TODO Use real folder ID (BIT-528)
+        folderId = this.originalCipher?.folderId,
+        // TODO Use real organization ID (BIT-780)
+        organizationId = this.originalCipher?.organizationId,
+        reprompt = this.toCipherRepromptType(),
+        // TODO Implement custom fields (BIT-529)
        fields = null,
-        passwordHistory = null,
-        creationDate = Instant.now(),
-        deletedDate = null,
-        // This is a throw away value.
-        // The SDK will eventually remove revisionDate via encryption.
-        revisionDate = Instant.now(),
    )

 /**
@@ -199,3 +198,10 @@ private fun VaultAddItemState.ViewState.Content.Identity.toIdentityCipherView():
 */
 private fun VaultAddItemState.ViewState.Content.Card.toCardCipherView(): CipherView =
    TODO("create Card CipherView BIT-668")
+
+private fun VaultAddItemState.ViewState.Content.toCipherRepromptType(): CipherRepromptType =
+    if (this.masterPasswordReprompt) {
+        CipherRepromptType.PASSWORD
+    } else {
+        CipherRepromptType.NONE
+    }