BIT-890 Adding a check for name when we save an item (#355)

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreen.kt

@@ -25,6 +25,9 @@ import androidx.hilt.navigation.compose.hiltViewModel
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import com.x8bit.bitwarden.R
 import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect
+import com.x8bit.bitwarden.ui.platform.base.util.asText
+import com.x8bit.bitwarden.ui.platform.components.BasicDialogState
+import com.x8bit.bitwarden.ui.platform.components.BitwardenBasicDialog
 import com.x8bit.bitwarden.ui.platform.components.BitwardenListHeaderText
 import com.x8bit.bitwarden.ui.platform.components.BitwardenLoadingDialog
 import com.x8bit.bitwarden.ui.platform.components.BitwardenMultiSelectButton
@@ -72,6 +75,16 @@ fun VaultAddItemScreen(
             )
         }
 
+        is VaultAddItemState.DialogState.Error -> BitwardenBasicDialog(
+            visibilityState = BasicDialogState.Shown(
+                title = R.string.an_error_has_occurred.asText(),
+                message = dialogState.message,
+            ),
+            onDismissRequest = remember(viewModel) {
+                { viewModel.trySendAction(VaultAddItemAction.DismissDialog) }
+            },
+        )
+
         null -> Unit
     }
 
@@ -139,11 +152,11 @@ fun VaultAddItemScreen(
                     )
                 }
 
-                VaultAddItemState.ItemType.Card -> {
+                is VaultAddItemState.ItemType.Card -> {
                     // TODO(BIT-507): Create UI for card-type item creation
                 }
 
-                VaultAddItemState.ItemType.Identity -> {
+                is VaultAddItemState.ItemType.Identity -> {
                     // TODO(BIT-667): Create UI for identity-type item creation
                 }
 

app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModel.kt

@@ -9,8 +9,6 @@ import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult
 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.vault.feature.additem.VaultAddItemState.ItemType.Card.displayStringResId
-import com.x8bit.bitwarden.ui.vault.feature.additem.VaultAddItemState.ItemType.Identity.displayStringResId
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toCipherView
 import com.x8bit.bitwarden.ui.vault.model.VaultAddEditType
 import dagger.hilt.android.lifecycle.HiltViewModel
@@ -62,6 +60,10 @@ class VaultAddItemViewModel @Inject constructor(
                 handleCloseClick()
             }
 
+            is VaultAddItemAction.DismissDialog -> {
+                handleDismissDialog()
+            }
+
             is VaultAddItemAction.TypeOptionSelect -> {
                 handleTypeOptionSelect(action)
             }
@@ -85,6 +87,18 @@ class VaultAddItemViewModel @Inject constructor(
     //region Top Level Handlers
 
     private fun handleSaveClick() {
+        if (state.selectedType.name.isBlank()) {
+            mutableStateFlow.update {
+                it.copy(
+                    dialog = VaultAddItemState.DialogState.Error(
+                        R.string.validation_field_required
+                            .asText(R.string.name.asText()),
+                    ),
+                )
+            }
+            return
+        }
+
         mutableStateFlow.update {
             it.copy(
                 dialog = VaultAddItemState.DialogState.Loading(
@@ -94,27 +108,13 @@ class VaultAddItemViewModel @Inject constructor(
         }
 
         viewModelScope.launch {
-            when (state.selectedType) {
-                is VaultAddItemState.ItemType.Login,
-                is VaultAddItemState.ItemType.SecureNotes,
-                -> {
-                    sendAction(
-                        action = VaultAddItemAction.Internal.CreateCipherResultReceive(
-                            createCipherResult = vaultRepository.createCipher(
-                                cipherView = stateFlow.value.selectedType.toCipherView(),
-                            ),
-                        ),
-                    )
-                }
-
-                VaultAddItemState.ItemType.Card -> {
-                    // TODO Add Saving of Card Type (BIT-668)
-                }
-
-                VaultAddItemState.ItemType.Identity -> {
-                    // TODO Add Saving of Identity type (BIT-508)
-                }
-            }
+            sendAction(
+                action = VaultAddItemAction.Internal.CreateCipherResultReceive(
+                    createCipherResult = vaultRepository.createCipher(
+                        cipherView = stateFlow.value.selectedType.toCipherView(),
+                    ),
+                ),
+            )
         }
     }
 
@@ -124,6 +124,12 @@ class VaultAddItemViewModel @Inject constructor(
         )
     }
 
+    private fun handleDismissDialog() {
+        mutableStateFlow.update {
+            it.copy(dialog = null)
+        }
+    }
+
     //endregion Top Level Handlers
 
     //region Type Option Handlers
@@ -156,7 +162,7 @@ class VaultAddItemViewModel @Inject constructor(
     private fun handleSwitchToAddCardItem() {
         mutableStateFlow.update { currentState ->
             currentState.copy(
-                selectedType = VaultAddItemState.ItemType.Card,
+                selectedType = VaultAddItemState.ItemType.Card(),
             )
         }
     }
@@ -164,7 +170,7 @@ class VaultAddItemViewModel @Inject constructor(
     private fun handleSwitchToAddIdentityItem() {
         mutableStateFlow.update { currentState ->
             currentState.copy(
-                selectedType = VaultAddItemState.ItemType.Identity,
+                selectedType = VaultAddItemState.ItemType.Identity(),
             )
         }
     }
@@ -633,10 +639,15 @@ data class VaultAddItemState(
          */
         abstract val displayStringResId: Int
 
+        /**
+         * Represents the name for the item type. This is an abstract property
+         * that must be overridden to save the item
+         */
+        abstract val name: String
+
         /**
          * Represents the login item information.
          *
-         * @property name The name associated with the login item.
          * @property username The username required for the login item.
          * @property password The password required for the login item.
          * @property uri The URI associated with the login item.
@@ -650,7 +661,7 @@ data class VaultAddItemState(
          */
         @Parcelize
         data class Login(
-            val name: String = "",
+            override val name: String = "",
             val username: String = "",
             val password: String = "",
             val uri: String = "",
@@ -679,22 +690,24 @@ data class VaultAddItemState(
 
         /**
          * Represents the `Card` item type.
-         *
-         * @property displayStringResId Resource ID for the display string of the card type.
          */
         @Parcelize
-        data object Card : ItemType() {
+        data class Card(
+            // TODO create the Card Item (BIT-509)
+            override val name: String = "",
+        ) : ItemType() {
             override val displayStringResId: Int
                 get() = ItemTypeOption.CARD.labelRes
         }
 
         /**
          * Represents the `Identity` item type.
-         *
-         * @property displayStringResId Resource ID for the display string of the identity type.
          */
         @Parcelize
-        data object Identity : ItemType() {
+        data class Identity(
+            // TODO create the Identity Item (BIT-667)
+            override val name: String = "",
+        ) : ItemType() {
             override val displayStringResId: Int
                 get() = ItemTypeOption.IDENTITY.labelRes
         }
@@ -702,7 +715,6 @@ data class VaultAddItemState(
         /**
          * Represents the `SecureNotes` item type.
          *
-         * @property name The name associated with the SecureNotes item.
          * @property folder The folder used for the SecureNotes item
          * @property favorite Indicates whether this SecureNotes item is marked as a favorite.
          * @property masterPasswordReprompt Indicates if a master password reprompt is required.
@@ -713,7 +725,7 @@ data class VaultAddItemState(
          */
         @Parcelize
         data class SecureNotes(
-            val name: String = "",
+            override val name: String = "",
             val folderName: Text = DEFAULT_FOLDER,
             val favorite: Boolean = false,
             val masterPasswordReprompt: Boolean = false,
@@ -747,6 +759,12 @@ data class VaultAddItemState(
          */
         @Parcelize
         data class Loading(val label: Text) : DialogState()
+
+        /**
+         * Displays an error dialog to the user.
+         */
+        @Parcelize
+        data class Error(val message: Text) : DialogState()
     }
 }
 
@@ -782,6 +800,11 @@ sealed class VaultAddItemAction {
      */
     data object CloseClick : VaultAddItemAction()
 
+    /**
+     * The user has clicked to dismiss the dialog.
+     */
+    data object DismissDialog : VaultAddItemAction()
+
     /**
      * Represents the action when a type option is selected.
      *

app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemScreenTest.kt

@@ -2,13 +2,16 @@ package com.x8bit.bitwarden.ui.vault.feature.additem
 
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.test.assertIsDisplayed
+import androidx.compose.ui.test.assertIsNotDisplayed
 import androidx.compose.ui.test.assertIsOff
 import androidx.compose.ui.test.assertIsOn
 import androidx.compose.ui.test.assertTextContains
 import androidx.compose.ui.test.click
 import androidx.compose.ui.test.filterToOne
+import androidx.compose.ui.test.hasAnyAncestor
 import androidx.compose.ui.test.hasContentDescription
 import androidx.compose.ui.test.hasSetTextAction
+import androidx.compose.ui.test.isDialog
 import androidx.compose.ui.test.onAllNodesWithText
 import androidx.compose.ui.test.onFirst
 import androidx.compose.ui.test.onLast
@@ -77,6 +80,47 @@ class VaultAddItemScreenTest : BaseComposeTest() {
         }
     }
 
+    @Test
+    fun `clicking dismiss dialog button should send DismissDialog action`() {
+        mutableStateFlow.value = DEFAULT_STATE_LOGIN_DIALOG
+
+        composeTestRule.setContent {
+            VaultAddItemScreen(viewModel = viewModel, onNavigateBack = {})
+        }
+
+        composeTestRule
+            .onAllNodesWithText("Ok")
+            .filterToOne(hasAnyAncestor(isDialog()))
+            .performClick()
+
+        verify {
+            viewModel.trySendAction(
+                VaultAddItemAction.DismissDialog,
+            )
+        }
+    }
+
+    @Test
+    fun `dialog should display when state is updated to do so`() {
+        mutableStateFlow.value = DEFAULT_STATE_LOGIN
+
+        composeTestRule.setContent {
+            VaultAddItemScreen(viewModel = viewModel, onNavigateBack = {})
+        }
+
+        composeTestRule
+            .onAllNodesWithText("Ok")
+            .filterToOne(hasAnyAncestor(isDialog()))
+            .assertIsNotDisplayed()
+
+        mutableStateFlow.value = DEFAULT_STATE_LOGIN_DIALOG
+
+        composeTestRule
+            .onAllNodesWithText("Ok")
+            .filterToOne(hasAnyAncestor(isDialog()))
+            .assertIsDisplayed()
+    }
+
     @Test
     fun `clicking a Type Option should send TypeOptionSelect action`() {
         composeTestRule.setContent {
@@ -112,7 +156,7 @@ class VaultAddItemScreenTest : BaseComposeTest() {
             .onNodeWithContentDescriptionAfterScroll(label = "Type, Login")
             .assertIsDisplayed()
 
-        mutableStateFlow.update { it.copy(selectedType = VaultAddItemState.ItemType.Card) }
+        mutableStateFlow.update { it.copy(selectedType = VaultAddItemState.ItemType.Card()) }
 
         composeTestRule
             .onNodeWithContentDescriptionAfterScroll(label = "Type, Card")
@@ -960,6 +1004,12 @@ class VaultAddItemScreenTest : BaseComposeTest() {
     //endregion Helper functions
 
     companion object {
+        private val DEFAULT_STATE_LOGIN_DIALOG = VaultAddItemState(
+            selectedType = VaultAddItemState.ItemType.Login(),
+            dialog = VaultAddItemState.DialogState.Error("test".asText()),
+            vaultAddEditType = VaultAddEditType.AddItem,
+        )
+
         private val DEFAULT_STATE_LOGIN = VaultAddItemState(
             vaultAddEditType = VaultAddEditType.AddItem,
             selectedType = VaultAddItemState.ItemType.Login(),

app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/additem/VaultAddItemViewModelTest.kt

@@ -59,26 +59,47 @@ class VaultAddItemViewModelTest : BaseViewModelTest() {
     @Test
     fun `SaveClick should show dialog, and remove it once an item is saved`() = runTest {
         val stateWithDialog = createVaultAddLoginItemState(
+            name = "tester",
             dialogState = VaultAddItemState.DialogState.Loading(
                 R.string.saving.asText(),
             ),
         )
 
-        val viewModel = createAddVaultItemViewModel()
+        val stateWithName = createVaultAddLoginItemState(
+            name = "tester",
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithName,
+                vaultAddEditType = VaultAddEditType.AddItem,
+            ),
+        )
+
         coEvery {
             vaultRepository.createCipher(any())
         } returns CreateCipherResult.Success
         viewModel.stateFlow.test {
             viewModel.actionChannel.trySend(VaultAddItemAction.SaveClick)
-            assertEquals(initialState, awaitItem())
+            assertEquals(stateWithName, awaitItem())
             assertEquals(stateWithDialog, awaitItem())
-            assertEquals(initialState, awaitItem())
+            assertEquals(stateWithName, awaitItem())
         }
     }
 
     @Test
     fun `SaveClick should update value to loading`() = runTest {
-        val viewModel = createAddVaultItemViewModel()
+        val stateWithName = createVaultAddLoginItemState(
+            name = "tester",
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithName,
+                vaultAddEditType = VaultAddEditType.AddItem,
+            ),
+        )
+
         coEvery {
             vaultRepository.createCipher(any())
         } returns CreateCipherResult.Success
@@ -90,7 +111,17 @@ class VaultAddItemViewModelTest : BaseViewModelTest() {
 
     @Test
     fun `SaveClick createCipher error should emit ShowToast`() = runTest {
-        val viewModel = createAddVaultItemViewModel()
+        val stateWithName = createVaultAddLoginItemState(
+            name = "tester",
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithName,
+                vaultAddEditType = VaultAddEditType.AddItem,
+            ),
+        )
+
         coEvery {
             vaultRepository.createCipher(any())
         } returns CreateCipherResult.Error
@@ -100,6 +131,56 @@ class VaultAddItemViewModelTest : BaseViewModelTest() {
         }
     }
 
+    @Test
+    fun `Saving item with an empty name field will cause a dialog to show up`() = runTest {
+        val stateWithNoName = createVaultAddSecureNotesItemState(name = "")
+
+        val stateWithNoNameAndDialog = createVaultAddSecureNotesItemState(
+            name = "",
+            dialogState = VaultAddItemState.DialogState.Error(
+                R.string.validation_field_required
+                    .asText(R.string.name.asText()),
+            ),
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = stateWithNoName,
+                vaultAddEditType = VaultAddEditType.AddItem,
+            ),
+        )
+        coEvery { vaultRepository.createCipher(any()) } returns CreateCipherResult.Success
+        viewModel.stateFlow.test {
+            viewModel.actionChannel.trySend(VaultAddItemAction.SaveClick)
+            assertEquals(stateWithNoName, awaitItem())
+            assertEquals(stateWithNoNameAndDialog, awaitItem())
+        }
+    }
+
+    @Test
+    fun `HandleDialogDismiss will remove the current dialog`() = runTest {
+        val errorState = createVaultAddLoginItemState(
+            dialogState = VaultAddItemState.DialogState.Error(
+                R.string.validation_field_required
+                    .asText(R.string.name.asText()),
+            ),
+        )
+
+        val viewModel = createAddVaultItemViewModel(
+            createSavedStateHandleWithState(
+                state = errorState,
+                vaultAddEditType = VaultAddEditType.AddItem,
+            ),
+        )
+
+        coEvery { vaultRepository.createCipher(any()) } returns CreateCipherResult.Success
+        viewModel.stateFlow.test {
+            viewModel.actionChannel.trySend(VaultAddItemAction.DismissDialog)
+            assertEquals(errorState, awaitItem())
+            assertEquals(null, awaitItem().dialog)
+        }
+    }
+
     @Test
     fun `TypeOptionSelect LOGIN should switch to LoginItem`() = runTest {
         val viewModel = createAddVaultItemViewModel()
@@ -572,6 +653,7 @@ class VaultAddItemViewModelTest : BaseViewModelTest() {
         masterPasswordReprompt: Boolean = false,
         notes: String = "",
         ownership: String = "placeholder@email.com",
+        dialogState: VaultAddItemState.DialogState? = null,
     ): VaultAddItemState =
         VaultAddItemState(
             vaultAddEditType = VaultAddEditType.AddItem,
@@ -583,7 +665,7 @@ class VaultAddItemViewModelTest : BaseViewModelTest() {
                 notes = notes,
                 ownership = ownership,
             ),
-            dialog = null,
+            dialog = dialogState,
         )
 
     private fun createSavedStateHandleWithState(