Просмотр исходного кода

Feature: Mobile - Field is dirty when the initial core workflow request changes already some value.

Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Rolf Schmidt <rs@zammad.com>
Dominik Klein 2 лет назад
Родитель
Сommit
27dfb4d66d

+ 0 - 1
app/frontend/apps/mobile/pages/user/__tests__/view-user-detail.spec.ts

@@ -8,7 +8,6 @@ import {
 import { visitView } from '@tests/support/components/visitView'
 import { mockGraphQLApi } from '@tests/support/mock-graphql-api'
 import { setupView } from '@tests/support/mock-user'
-import { mockPermissions } from '@tests/support/mock-permissions'
 import { waitUntil, waitUntilApisResolved } from '@tests/support/utils'
 
 describe('visiting user page', () => {

+ 48 - 17
app/frontend/shared/components/Form/Form.vue

@@ -164,6 +164,7 @@ const formElement = ref<HTMLElement>()
 const changeFields = toRef(props, 'changeFields')
 
 const updaterChangedFields = new Set<string>()
+const changeInitialValue = new Map<string, FormFieldValue>()
 
 const autofocusFirstInput = () => {
   nextTick(() => {
@@ -184,16 +185,25 @@ const setFormNode = (node: FormKitNode) => {
 
   node.settled.then(() => {
     showInitialLoadingAnimation.value = false
-    formKitInitialNodesSettled.value = true
 
-    // Reset directly after the initial request.
-    updaterChangedFields.clear()
+    nextTick(() => {
+      changeInitialValue.forEach((value, fieldName) => {
+        getNode(fieldName)?.input(value, false)
+      })
+
+      changeInitialValue.clear()
+
+      formKitInitialNodesSettled.value = true
+
+      // Reset directly after the initial request.
+      updaterChangedFields.clear()
 
-    const formName = node.context?.id || node.name
-    testFlags.set(`${formName}.settled`)
-    emit('settled')
+      const formName = node.context?.id || node.name
+      testFlags.set(`${formName}.settled`)
+      emit('settled')
 
-    if (props.autofocus) autofocusFirstInput()
+      if (props.autofocus) autofocusFirstInput()
+    })
   })
 
   node.on('autofocus', autofocusFirstInput)
@@ -615,23 +625,44 @@ const updateSchemaDataField = (
 const updateChangedFields = (
   changedFields: Record<string, Partial<FormSchemaField>>,
 ) => {
+  const handleUpdatedInitialFieldValue = (
+    fieldName: string,
+    value: FormFieldValue,
+    directly: boolean,
+    field: Partial<FormSchemaField>,
+  ) => {
+    if (value === undefined) return
+
+    if (directly) {
+      field.value = value
+    } else if (!formKitInitialNodesSettled.value) {
+      changeInitialValue.set(fieldName, value)
+    }
+  }
+
   Object.keys(changedFields).forEach(async (fieldName) => {
     if (!schemaData.fields[fieldName]) return
 
-    const { value, ...changedFieldProps } = changedFields[fieldName]
+    const { initialValue, value, ...changedFieldProps } =
+      changedFields[fieldName]
 
     const field: SetRequired<Partial<FormSchemaField>, 'name'> = {
       ...changedFieldProps,
       name: fieldName,
     }
 
-    if (
-      value !== undefined &&
-      (!formKitInitialNodesSettled.value ||
-        (!schemaData.fields[fieldName].show && changedFieldProps.show))
-    ) {
-      field.value = value
-    }
+    const showField =
+      !schemaData.fields[fieldName].show && changedFieldProps.show
+
+    // This happens for the initial updater, when the form is not settled yet or the field was not rendered yet.
+    // In this ase we need to remember the changes and do it afterwards after the form is settled the first time.
+    // Sometimes the value from the server is the "real" initial value, for this the `initialValue` can be used.
+    handleUpdatedInitialFieldValue(
+      fieldName,
+      value ?? initialValue,
+      showField || initialValue !== undefined,
+      field,
+    )
 
     // When a field will be visible with the update call, we need to wait before on a settled form, before we
     // continue (so that we have all values present inside the form).
@@ -651,9 +682,9 @@ const updateChangedFields = (
     if (!formKitInitialNodesSettled.value) return
 
     if (
-      !('value' in field) &&
+      !showField &&
       value !== undefined &&
-      value !== values.value[fieldName]
+      !isEqual(value, values.value[fieldName])
     ) {
       updaterChangedFields.add(fieldName)
       getNode(fieldName)?.input(value, false)

+ 56 - 0
app/frontend/shared/components/Form/__tests__/Form.formUpdater.spec.ts

@@ -101,6 +101,20 @@ const checkFieldRequired = (
   }
 }
 
+const checkFieldDirty = (
+  wrapper: ExtendedRenderResult,
+  label: string,
+  dirty: boolean,
+) => {
+  const outer = getOuterFieldElement(wrapper, label)
+
+  if (dirty) {
+    expect(outer).toHaveAttribute('data-dirty')
+  } else {
+    expect(outer).not.toHaveAttribute('data-dirty')
+  }
+}
+
 const checkFieldDisabled = (
   wrapper: ExtendedRenderResult,
   label: string,
@@ -1203,4 +1217,46 @@ describe('Form.vue - Form Updater - special situtations', () => {
 
     checkFieldRequired(wrapper, 'Example', true)
   })
+
+  test('test dirty flag set for initialValues changes from form update', async () => {
+    const { wrapper } = await renderForm(
+      {
+        formUpdater: {
+          example: {
+            value: 'changed',
+          },
+        },
+      },
+      {
+        props: {
+          initialValues: {
+            example: 'test',
+          },
+        },
+      },
+    )
+
+    checkFieldDirty(wrapper, 'Example', true)
+  })
+
+  test('test dirty flag unset for initialValues changes from form update', async () => {
+    const { wrapper } = await renderForm(
+      {
+        formUpdater: {
+          example: {
+            value: 'test',
+          },
+        },
+      },
+      {
+        props: {
+          initialValues: {
+            example: 'test',
+          },
+        },
+      },
+    )
+
+    checkFieldDirty(wrapper, 'Example', false)
+  })
 })

+ 1 - 0
app/frontend/shared/components/Form/types.ts

@@ -63,6 +63,7 @@ export interface FormSchemaField {
   name: string
   internal?: boolean
   value?: FormFieldValue
+  initialValue?: FormFieldValue
   label?: string
   labelSrOnly?: boolean
   labelPlaceholder?: string

+ 14 - 0
app/models/form_updater/concerns/checks_core_workflow.rb

@@ -27,6 +27,10 @@ module FormUpdater::Concerns::ChecksCoreWorkflow
       params['id'] = object.id
     end
 
+    # Currently we need to convert the relation field values (integer) to strings for the
+    # current core workflow implementation.
+    convert_relation_field_value(params)
+
     {
       'event'                  => 'core_workflow',
       'request_id'             => meta[:request_id],
@@ -36,4 +40,14 @@ module FormUpdater::Concerns::ChecksCoreWorkflow
       'last_changed_attribute' => meta.dig(:changed_field, :name),
     }
   end
+
+  def convert_relation_field_value(params)
+    return if relation_fields.blank?
+
+    relation_fields.each_key do |field_name|
+      next if params[field_name].blank?
+
+      params[field_name] = params[field_name].to_s
+    end
+  end
 end

+ 8 - 0
app/models/form_updater/updater.rb

@@ -65,9 +65,17 @@ class FormUpdater::Updater
       result_initialize_field(name)
 
       result[relation_field[:name]][:options] = relation_resolver.options
+
+      ensure_field_value_int(result[relation_field[:name]])
     end
   end
 
+  def ensure_field_value_int(field)
+    return if field[:value].blank?
+
+    field[:value] = field[:value].to_i
+  end
+
   RELATION_CLASS_PREFIX = 'FormUpdater::Relation::'.freeze
 
   def get_relation_resolver(relation_field)

+ 11 - 11
app/models/form_updater/updater/user/edit.rb

@@ -21,17 +21,17 @@ class FormUpdater::Updater::User::Edit < FormUpdater::Updater
 
   def organization_ids
     {
-      value:   object.organization_ids,
-      options: ::Organization.where(id: object.organization_ids).each_with_object([]) do |organization, options|
-                 options << {
-                   # TODO: needs to be aligned during the autocomplete query implementation
-                   value:        organization.id,
-                   label:        organization.name,
-                   organization: {
-                     active: organization.active,
-                   }
-                 }
-               end,
+      initialValue: object.organization_ids,
+      options:      ::Organization.where(id: object.organization_ids).each_with_object([]) do |organization, options|
+                      options << {
+                        # TODO: needs to be aligned during the autocomplete query implementation
+                        value:        organization.id,
+                        label:        organization.name,
+                        organization: {
+                          active: organization.active,
+                        }
+                      }
+                    end,
     }
   end
 end

+ 17 - 0
spec/models/form_updater/updater/ticket/create_spec.rb

@@ -61,6 +61,23 @@ RSpec.describe(FormUpdater::Updater::Ticket::Create) do
         'priority_id' => include(expected_result['priority_id']),
       )
     end
+
+    it 'returns group_id as integer' do
+      expect(resolved_result.resolve).to include(
+        'group_id' => include(value: Group.last.id)
+      )
+    end
+
+    context 'when group_id is given in data' do
+      let(:data) { { 'group_id' => Group.last.id } }
+
+      it 'returns no new value for group' do
+        expect(resolved_result.resolve).to not_include(
+          'group_id' => include(value: Group.last.id)
+        )
+      end
+    end
+
   end
 
   include_examples 'FormUpdater::ChecksCoreWorkflow', object_name: 'Ticket'

+ 10 - 10
spec/models/form_updater/updater/user/edit_spec.rb

@@ -35,16 +35,16 @@ RSpec.describe(FormUpdater::Updater::User::Edit) do
 
       expect(resolved_result.resolve).to include(
         'organization_ids' => include({
-                                        value:   secondary_organizations.map(&:id),
-                                        options: secondary_organizations.each_with_object([]) do |organization, options|
-                                                   options << {
-                                                     value:        organization.id,
-                                                     label:        organization.name,
-                                                     organization: {
-                                                       active: organization.active,
-                                                     }
-                                                   }
-                                                 end
+                                        initialValue: secondary_organizations.map(&:id),
+                                        options:      secondary_organizations.each_with_object([]) do |organization, options|
+                                                        options << {
+                                                          value:        organization.id,
+                                                          label:        organization.name,
+                                                          organization: {
+                                                            active: organization.active,
+                                                          }
+                                                        }
+                                                      end
                                       }),
       )
     end