Merge pull request #12582 from woocommerce/custom-fields/fix-issues

[Custom Fields] Some fixes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/editor/CustomFieldsEditorViewModel.kt

@@ -1,5 +1,6 @@
 package com.woocommerce.android.ui.customfields.editor
 
+import android.os.Parcelable
 import androidx.annotation.StringRes
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.asLiveData
@@ -19,6 +20,7 @@ import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
+import kotlinx.parcelize.Parcelize
 import javax.inject.Inject
 
 @HiltViewModel
@@ -27,6 +29,7 @@ class CustomFieldsEditorViewModel @Inject constructor(
     private val repository: CustomFieldsRepository
 ) : ScopedViewModel(savedStateHandle) {
     companion object {
+        const val CUSTOM_FIELD_CREATED_RESULT_KEY = "custom_field_created"
         const val CUSTOM_FIELD_UPDATED_RESULT_KEY = "custom_field_updated"
         const val CUSTOM_FIELD_DELETED_RESULT_KEY = "custom_field_deleted"
     }
@@ -82,25 +85,27 @@ class CustomFieldsEditorViewModel @Inject constructor(
     }
 
     fun onDoneClicked() {
-        val value = requireNotNull(customFieldDraft.value)
-        if (storedValue == null) {
-            // Check for duplicate keys before inserting the new custom field
-            // For more context: pe5sF9-33t-p2#comment-3880
-            launch {
+        launch {
+            val value = requireNotNull(customFieldDraft.value)
+            if (value.id == null) {
+                // Check for duplicate keys before inserting the new custom field
+                // For more context: pe5sF9-33t-p2#comment-3880
                 val existingFields = repository.getDisplayableCustomFields(navArgs.parentItemId)
                 if (existingFields.any { it.key == value.key }) {
                     keyErrorMessage.value = UiString.UiStringRes(R.string.custom_fields_editor_key_error_duplicate)
-                } else {
-                    triggerEvent(
-                        MultiLiveEvent.Event.ExitWithResult(data = value, key = CUSTOM_FIELD_UPDATED_RESULT_KEY)
-                    )
+                    return@launch
                 }
             }
-        } else {
-            // When editing, we don't need to check for duplicate keys
-            triggerEvent(
-                MultiLiveEvent.Event.ExitWithResult(data = value, key = CUSTOM_FIELD_UPDATED_RESULT_KEY)
-            )
+
+            val event = if (storedValue == null) {
+                MultiLiveEvent.Event.ExitWithResult(data = value, key = CUSTOM_FIELD_CREATED_RESULT_KEY)
+            } else {
+                MultiLiveEvent.Event.ExitWithResult(
+                    data = CustomFieldUpdateResult(oldKey = storedValue.key, updatedField = value),
+                    key = CUSTOM_FIELD_UPDATED_RESULT_KEY
+                )
+            }
+            triggerEvent(event)
         }
     }
 
@@ -158,4 +163,10 @@ class CustomFieldsEditorViewModel @Inject constructor(
         @StringRes val labelResource: Int,
         val content: String
     ) : MultiLiveEvent.Event()
+
+    @Parcelize
+    data class CustomFieldUpdateResult(
+        val oldKey: String,
+        val updatedField: CustomFieldUiModel
+    ) : Parcelable
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsFragment.kt

@@ -15,6 +15,7 @@ import com.woocommerce.android.ui.compose.composeView
 import com.woocommerce.android.ui.customfields.CustomFieldContentType
 import com.woocommerce.android.ui.customfields.CustomFieldUiModel
 import com.woocommerce.android.ui.customfields.editor.CustomFieldsEditorViewModel
+import com.woocommerce.android.ui.customfields.editor.CustomFieldsEditorViewModel.CustomFieldUpdateResult
 import com.woocommerce.android.ui.main.AppBarStatus
 import com.woocommerce.android.util.ActivityUtils
 import com.woocommerce.android.util.ChromeCustomTabUtils
@@ -64,12 +65,11 @@ class CustomFieldsFragment : BaseFragment() {
     }
 
     private fun handleResults() {
-        handleResult<CustomFieldUiModel>(CustomFieldsEditorViewModel.CUSTOM_FIELD_UPDATED_RESULT_KEY) { result ->
-            if (result.id == null) {
-                viewModel.onCustomFieldInserted(result)
-            } else {
-                viewModel.onCustomFieldUpdated(result)
-            }
+        handleResult<CustomFieldUiModel>(CustomFieldsEditorViewModel.CUSTOM_FIELD_CREATED_RESULT_KEY) { result ->
+            viewModel.onCustomFieldInserted(result)
+        }
+        handleResult<CustomFieldUpdateResult>(CustomFieldsEditorViewModel.CUSTOM_FIELD_UPDATED_RESULT_KEY) { result ->
+            viewModel.onCustomFieldUpdated(result.oldKey, result.updatedField)
         }
         handleResult<CustomFieldUiModel>(CustomFieldsEditorViewModel.CUSTOM_FIELD_DELETED_RESULT_KEY) { result ->
             viewModel.onCustomFieldDeleted(result)

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsScreen.kt

@@ -41,6 +41,7 @@ import androidx.compose.ui.text.SpanStyle
 import androidx.compose.ui.text.UrlAnnotation
 import androidx.compose.ui.text.buildAnnotatedString
 import androidx.compose.ui.text.style.TextOverflow
+import androidx.compose.ui.text.withStyle
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
 import com.woocommerce.android.R
@@ -182,29 +183,37 @@ private fun CustomFieldItem(
             )
             Spacer(modifier = Modifier.height(4.dp))
 
-            val text = buildAnnotatedString {
-                if (customField.contentType != CustomFieldContentType.TEXT) {
-                    pushUrlAnnotation(UrlAnnotation(customField.value))
-                    pushStyle(SpanStyle(color = MaterialTheme.colors.primary))
-                }
-                append(customField.valueStrippedHtml)
-            }
-            ClickableText(
-                text = text,
-                style = MaterialTheme.typography.body2.copy(
-                    color = MaterialTheme.colors.onSurface
-                ),
-                maxLines = 2,
-                overflow = TextOverflow.Ellipsis,
-                onClick = { offset ->
-                    text.getUrlAnnotations(
-                        start = offset,
-                        end = offset
-                    ).firstOrNull()?.let { _ ->
-                        onValueClicked(customField)
+            if (customField.contentType != CustomFieldContentType.TEXT) {
+                val text = buildAnnotatedString {
+                    withStyle(SpanStyle(color = MaterialTheme.colors.primary)) {
+                        pushUrlAnnotation(UrlAnnotation(customField.value))
+                        append(customField.value)
                     }
                 }
-            )
+                ClickableText(
+                    text = text,
+                    style = MaterialTheme.typography.body2.copy(
+                        color = MaterialTheme.colors.onSurface
+                    ),
+                    maxLines = 2,
+                    overflow = TextOverflow.Ellipsis,
+                    onClick = { offset ->
+                        text.getUrlAnnotations(
+                            start = offset,
+                            end = offset
+                        ).firstOrNull()?.let { _ ->
+                            onValueClicked(customField)
+                        }
+                    }
+                )
+            } else {
+                Text(
+                    text = customField.value,
+                    maxLines = 2,
+                    overflow = TextOverflow.Ellipsis,
+                    style = MaterialTheme.typography.body2
+                )
+            }
         }
 
         Icon(

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsViewModel.kt

@@ -97,9 +97,14 @@ class CustomFieldsViewModel @Inject constructor(
         }
     }
 
-    fun onCustomFieldUpdated(result: CustomFieldUiModel) {
+    fun onCustomFieldUpdated(oldValueKey: String, result: CustomFieldUiModel) {
         pendingChanges.update {
-            it.copy(editedFields = it.editedFields.filterNot { field -> field.id == result.id } + result)
+            if (result.id == null) {
+                // We are updating a field that was just added and hasn't been saved yet
+                it.copy(insertedFields = it.insertedFields.filterNot { field -> field.key == oldValueKey } + result)
+            } else {
+                it.copy(editedFields = it.editedFields.filterNot { field -> field.id == result.id } + result)
+            }
         }
     }
 

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/customfields/editor/CustomFieldsEditorViewModelTest.kt

@@ -171,7 +171,7 @@ class CustomFieldsEditorViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `when done is clicked, then exit with result`() = testBlocking {
+    fun `given editing an existing field, when done is clicked, then exit with result`() = testBlocking {
         setup(editing = true)
 
         val events = viewModel.event.runAndCaptureValues {
@@ -182,12 +182,35 @@ class CustomFieldsEditorViewModelTest : BaseUnitTest() {
 
         assertThat(events).isEqualTo(
             MultiLiveEvent.Event.ExitWithResult(
-                data = CustomFieldUiModel(id = CUSTOM_FIELD_ID, key = "new key", value = "new value"),
+                data = CustomFieldsEditorViewModel.CustomFieldUpdateResult(
+                    CUSTOM_FIELD.key,
+                    CustomFieldUiModel(id = CUSTOM_FIELD_ID, key = "new key", value = "new value")
+                ),
                 key = CustomFieldsEditorViewModel.CUSTOM_FIELD_UPDATED_RESULT_KEY
             )
         )
     }
 
+    @Test
+    fun `given creating a new field, when done is clicked, then exit with result`() = testBlocking {
+        setup(editing = false) {
+            whenever(repository.getDisplayableCustomFields(PARENT_ITEM_ID)).thenReturn(emptyList())
+        }
+
+        val events = viewModel.event.runAndCaptureValues {
+            viewModel.onKeyChanged("key")
+            viewModel.onValueChanged("value")
+            viewModel.onDoneClicked()
+        }.last()
+
+        assertThat(events).isEqualTo(
+            MultiLiveEvent.Event.ExitWithResult(
+                data = CustomFieldUiModel(key = "key", value = "value"),
+                key = CustomFieldsEditorViewModel.CUSTOM_FIELD_CREATED_RESULT_KEY
+            )
+        )
+    }
+
     @Test
     fun `given adding a new field, when key is duplicate, then show error`() = testBlocking {
         setup(editing = false) {

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/customfields/list/CustomFieldsViewModelTest.kt

@@ -217,7 +217,7 @@ class CustomFieldsViewModelTest : BaseUnitTest() {
         setup()
 
         val state = viewModel.state.runAndCaptureValues {
-            viewModel.onCustomFieldUpdated(customField)
+            viewModel.onCustomFieldUpdated(CUSTOM_FIELDS.first().key, customField)
             advanceUntilIdle()
         }.last()
 
@@ -230,8 +230,8 @@ class CustomFieldsViewModelTest : BaseUnitTest() {
         setup()
 
         val state = viewModel.state.runAndCaptureValues {
-            viewModel.onCustomFieldUpdated(customField.copy(value = "new value"))
-            viewModel.onCustomFieldUpdated(customField.copy(value = "new value 2"))
+            viewModel.onCustomFieldUpdated(CUSTOM_FIELDS.first().key, customField.copy(value = "new value"))
+            viewModel.onCustomFieldUpdated(CUSTOM_FIELDS.first().key, customField.copy(value = "new value 2"))
             advanceUntilIdle()
         }.last()
 
@@ -256,6 +256,25 @@ class CustomFieldsViewModelTest : BaseUnitTest() {
         assertThat(state.customFields.last().value).isEqualTo(customField.value)
     }
 
+    @Test
+    fun `when adding a custom field then updating it, then confirm the field is not duplicated`() = testBlocking {
+        val customField = CustomFieldUiModel(
+            key = "new key",
+            value = "new value"
+        )
+        setup()
+
+        val state = viewModel.state.runAndCaptureValues {
+            viewModel.onCustomFieldInserted(customField)
+            viewModel.onCustomFieldUpdated(customField.key, customField.copy(value = "new value 2"))
+            advanceUntilIdle()
+        }.last()
+
+        assertThat(state.customFields).hasSize(CUSTOM_FIELDS.size + 1)
+        assertThat(state.customFields.last().key).isEqualTo(customField.key)
+        assertThat(state.customFields.last().value).isEqualTo("new value 2")
+    }
+
     @Test
     fun `when deleting a custom field, then custom fields are refreshed`() = testBlocking {
         val customField = CustomFieldUiModel(CUSTOM_FIELDS.first())
@@ -325,7 +344,7 @@ class CustomFieldsViewModelTest : BaseUnitTest() {
 
         setup()
 
-        viewModel.onCustomFieldUpdated(updatedField)
+        viewModel.onCustomFieldUpdated(CUSTOM_FIELDS.first().key, updatedField)
         viewModel.onCustomFieldInserted(insertedField)
         viewModel.onSaveClicked()