Browse Source

Fixes #4407 - [Core Workflow] Remove option does not work with tree select node that has sub-options.

Co-authored-by: Florian Liebe <fl@zammad.com>
Co-authored-by: Dominik Klein <dk@zammad.com>
Rolf Schmidt 1 year ago
parent
commit
d54f6d3ce2

+ 22 - 12
app/assets/javascripts/app/controllers/_ui_element/_application_tree_select.coffee

@@ -12,23 +12,23 @@ class App.UiElement.ApplicationTreeSelect extends App.UiElement.ApplicationUiEle
     newOptions = []
     nullFound = false
     for option, index in options
-      enabled = false
-      for value in values
-        valueArray  = value.split('::')
-        optionArray = option['value'].split('::')
-        continue if valueArray[valueDepth] isnt optionArray[valueDepth]
-        enabled = true
-        break
-
+      enabled = _.contains(values, option.value)
       if nullExists && !option.value && !nullFound
         nullFound = true
         enabled   = true
 
-      if !enabled
-        continue
+      activeChildren = false
+      if option.value && option.children && option.children.length > 0
+        for value in values
+          if value && value.startsWith(option.value + '::')
+            activeChildren = true
 
-      if option['children'] && option['children'].length
-        option['children'] = @filterTreeOptions(values, valueDepth + 1, option['children'], nullExists)
+      if activeChildren
+        option.inactive = !enabled
+        option.children = @filterTreeOptions(values, valueDepth + 1, option.children, nullExists)
+      else
+        option.children = undefined
+        continue if !enabled
 
       newOptions.push(option)
 
@@ -46,6 +46,16 @@ class App.UiElement.ApplicationTreeSelect extends App.UiElement.ApplicationUiEle
     else
       attribute.multiple = ''
 
+    # make sure only available values are set. For the tree selects
+    # we want also to render values which are not selectable but rendered as disabled
+    # e.g. child nodes where the parent node is disabled. Because of this we need
+    # to make sure to not render these values as selected
+    if attribute.value && attribute.filter
+      if attribute.multiple
+        attribute.value = _.intersection(attribute.value, attribute.filter)
+      else if !_.contains(attribute.filter, attribute.value)
+        attribute.value = ''
+
     # add deleted historical options if required
     @addDeletedOptions(attribute, params)
 

+ 0 - 5
app/assets/javascripts/app/views/generic/searchable_select_option.jst.eco

@@ -8,9 +8,4 @@
     <%- @Icon('arrow-right', 'recipientList-arrow') %>
   </span>
   <% end %>
-  <% if @option.inactive is true: %>
-    <span>
-      <%- @Ti('inactive') %>
-    </span>
-  <% end %>
 </li>

+ 1 - 0
app/assets/stylesheets/zammad.scss

@@ -11412,6 +11412,7 @@ output {
     &.is-inactive {
       opacity: 0.73;
       text-decoration: line-through;
+      pointer-events: none;
     }
   }
 

+ 8 - 2
app/frontend/shared/components/CommonLogo/__tests__/CommonLogo.spec.ts

@@ -18,7 +18,10 @@ describe('CommonLogo.vue', () => {
     const img = wrapper.container.querySelector('img')
 
     expect(img).toHaveAttribute('alt', 'Zammad Custom Logo')
-    expect(img).toHaveAttribute('src', '/api/v1/system_assets/product_logo/1234')
+    expect(img).toHaveAttribute(
+      'src',
+      '/api/v1/system_assets/product_logo/1234',
+    )
   })
 
   it('renders default zammad logo', async () => {
@@ -33,6 +36,9 @@ describe('CommonLogo.vue', () => {
     const img = wrapper.container.querySelector('img')
 
     expect(img).not.toHaveAttribute('alt')
-    expect(img).toHaveAttribute('src', '/api/v1/system_assets/product_logo/logo.svg')
+    expect(img).toHaveAttribute(
+      'src',
+      '/api/v1/system_assets/product_logo/logo.svg',
+    )
   })
 })

+ 46 - 15
app/frontend/shared/components/Form/composables/useSelectOptions.ts

@@ -72,6 +72,19 @@ const useSelectOptions = <
     keyBy(translatedOptions.value, 'value'),
   )
 
+  const disabledOptionValueLookup = computed(() => {
+    return Object.values(optionValueLookup.value).reduce(
+      (disabledValues: SelectValue[], option) => {
+        if (option.disabled) {
+          disabledValues.push(option.value)
+        }
+
+        return disabledValues
+      },
+      [],
+    )
+  })
+
   const sortedOptions = computed(() => {
     const { sorting } = context.value
 
@@ -147,13 +160,25 @@ const useSelectOptions = <
     return targetElements
   }
 
-  const handleValuesForNonExistingOptions = () => {
+  const handleValuesForNonExistingOrDisabledOptions = (
+    rejectNonExistentValues?: boolean,
+  ) => {
     if (!hasValue.value || context.value.pendingValueUpdate) return
 
+    const localRejectNonExistentValues = rejectNonExistentValues ?? true
+
     if (context.value.multiple) {
       const availableValues = currentValue.value.filter(
-        (selectValue: string | number) =>
-          typeof optionValueLookup.value[selectValue] !== 'undefined',
+        (selectValue: string | number) => {
+          const selectValueOption = optionValueLookup.value[selectValue]
+          return (
+            (localRejectNonExistentValues &&
+              typeof selectValueOption !== 'undefined' &&
+              selectValueOption?.disabled !== true) ||
+            (!localRejectNonExistentValues &&
+              selectValueOption?.disabled !== true)
+          )
+        },
       ) as SelectValue[]
 
       if (availableValues.length !== currentValue.value.length) {
@@ -163,14 +188,19 @@ const useSelectOptions = <
       return
     }
 
-    if (typeof optionValueLookup.value[currentValue.value] === 'undefined')
+    const currentValueOption = optionValueLookup.value[currentValue.value]
+    if (
+      (localRejectNonExistentValues &&
+        typeof currentValueOption === 'undefined') ||
+      currentValueOption?.disabled
+    )
       clearValue(false)
   }
 
-  // Setup a mechanism to handle missing options, including:
+  // Setup a mechanism to handle missing and disabled options, including:
   //   - appending historical options for current values
   //   - clearing value in case options are missing
-  const setupMissingOptionHandling = () => {
+  const setupMissingOrDisabledOptionHandling = () => {
     const { historicalOptions } = context.value
 
     // When we are in a "create" form situation and no 'rejectNonExistentValues' flag
@@ -186,7 +216,7 @@ const useSelectOptions = <
     // Remember current optionValueLookup in node context.
     context.value.optionValueLookup = optionValueLookup
 
-    // TODO: Workaround, because currently the "nulloption" exists also for multiselect fields (#4513).
+    // TODO: Workaround for empty string, because currently the "nulloption" exists also for multiselect fields (#4513).
     if (context.value.multiple) {
       watch(
         () =>
@@ -240,14 +270,15 @@ const useSelectOptions = <
       )
     }
 
-    // Reject non-existent values during the initialization phase.
-    //   Note that this behavior is controlled by a dedicated flag.
-    if (context.value.rejectNonExistentValues)
-      handleValuesForNonExistingOptions()
+    // Reject non-existent or disabled option values during the initialization phase (note that
+    //  the non-existent values behavior is controlled by a dedicated flag).
+    handleValuesForNonExistingOrDisabledOptions(
+      context.value.rejectNonExistentValues,
+    )
 
-    // Set up a watcher that clears a missing option value on subsequent mutations of the options prop.
-    //   In this case, the dedicated flag is ignored.
-    watch(() => options.value, handleValuesForNonExistingOptions)
+    // Set up a watcher that clears a missing option value or disabled options on subsequent mutations
+    //  of the options prop (in this case, the dedicated "rejectNonExistentValues" flag is ignored).
+    watch(options, () => handleValuesForNonExistingOrDisabledOptions())
   }
 
   return {
@@ -262,7 +293,7 @@ const useSelectOptions = <
     getSelectedOptionStatus,
     selectOption,
     getDialogFocusTargets,
-    setupMissingOptionHandling,
+    setupMissingOrDisabledOptionHandling,
     appendedOptions,
   }
 }

+ 2 - 2
app/frontend/shared/components/Form/fields/FieldSelect/FieldSelectInput.vue

@@ -30,7 +30,7 @@ const {
   getSelectedOptionIcon,
   getSelectedOptionLabel,
   getSelectedOptionStatus,
-  setupMissingOptionHandling,
+  setupMissingOrDisabledOptionHandling,
 } = useSelectOptions(toRef(props.context, 'options'), contextReactive)
 
 const select = ref<CommonSelectInstance>()
@@ -48,7 +48,7 @@ const openSelectDialog = () => {
 useFormBlock(contextReactive, openSelectDialog)
 
 useSelectPreselect(sortedOptions, contextReactive)
-setupMissingOptionHandling()
+setupMissingOrDisabledOptionHandling()
 </script>
 
 <template>

+ 50 - 1
app/frontend/shared/components/Form/fields/FieldSelect/__tests__/FieldSelect.spec.ts

@@ -1,5 +1,6 @@
 // Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
+import type { SetRequired } from 'type-fest'
 import { cloneDeep, keyBy } from 'lodash-es'
 import { getByText, waitFor } from '@testing-library/vue'
 import { FormKit } from '@formkit/vue'
@@ -7,6 +8,7 @@ import { renderComponent } from '#tests/support/components/index.ts'
 import { i18n } from '#shared/i18n.ts'
 import { getNode } from '@formkit/core'
 import { waitForNextTick } from '#tests/support/utils.ts'
+import type { SelectOption } from '../types.ts'
 
 // Mock IntersectionObserver feature by injecting it into the global namespace.
 //   More info here: https://vitest.dev/guide/mocking.html#globals
@@ -18,7 +20,7 @@ const IntersectionObserverMock = vi.fn(() => ({
 }))
 vi.stubGlobal('IntersectionObserver', IntersectionObserverMock)
 
-const testOptions = [
+const testOptions: SetRequired<SelectOption, 'label'>[] = [
   {
     value: 0,
     label: 'Item A',
@@ -537,6 +539,53 @@ describe('Form - Field - Select - Options', () => {
 
     expect(wrapper.getByRole('listitem')).toHaveTextContent('Item A')
   })
+
+  it('remove values for disabled option on value update (multiple)', async () => {
+    const optionsProp = cloneDeep(testOptions)
+    optionsProp[2].disabled = true
+
+    const wrapper = renderComponent(FormKit, {
+      ...wrapperParameters,
+      props: {
+        id: 'select',
+        ...commonProps,
+        type: 'select',
+        value: [0, 1],
+        options: optionsProp,
+        multiple: true,
+      },
+    })
+
+    expect(wrapper.getAllByRole('listitem')).toHaveLength(2)
+
+    // Change values with one which not exists inside the options (e.g. coming from core workflow).
+    const node = getNode('select')
+    await node?.settled
+    node?.input([1, 2])
+
+    await waitForNextTick(true)
+
+    expect(wrapper.getAllByRole('listitem')).toHaveLength(1)
+  })
+
+  it('remove values for disabled option on initial value (multiple)', async () => {
+    const optionsProp = cloneDeep(testOptions)
+    optionsProp[2].disabled = true
+
+    const wrapper = renderComponent(FormKit, {
+      ...wrapperParameters,
+      props: {
+        id: 'select',
+        ...commonProps,
+        type: 'select',
+        value: [0, 1, 2],
+        options: optionsProp,
+        multiple: true,
+      },
+    })
+
+    expect(wrapper.getAllByRole('listitem')).toHaveLength(2)
+  })
 })
 
 describe('Form - Field - Select - Features', () => {

+ 1 - 1
app/frontend/shared/components/Form/fields/FieldSelect/index.ts

@@ -3,7 +3,7 @@
 import createInput from '#shared/form/core/createInput.ts'
 import addLink from '#shared/form/features/addLink.ts'
 import formUpdaterTrigger from '#shared/form/features/formUpdaterTrigger.ts'
-import removeValuesForNonExistingOptions from '#shared/form/features/removeValuesForNonExistingOptions.ts'
+import removeValuesForNonExistingOptions from '#shared/form/features/removeValuesForNonExistingOrDisabledOptions.ts'
 import FieldSelectInput from './FieldSelectInput.vue'
 
 const fieldDefinition = createInput(

+ 2 - 2
app/frontend/shared/components/Form/fields/FieldTreeSelect/FieldTreeSelectInput.vue

@@ -99,7 +99,7 @@ const {
   getSelectedOptionLabel,
   getSelectedOptionStatus,
   getDialogFocusTargets,
-  setupMissingOptionHandling,
+  setupMissingOrDisabledOptionHandling,
 } = useSelectOptions(flatOptions, toRef(props, 'context'))
 
 const openModal = () => {
@@ -148,7 +148,7 @@ const onInputClick = () => {
 
 useSelectPreselect(flatOptions, contextReactive)
 useFormBlock(contextReactive, onInputClick)
-setupMissingOptionHandling()
+setupMissingOrDisabledOptionHandling()
 </script>
 
 <template>

+ 2 - 2
app/frontend/shared/components/Form/fields/FieldTreeSelect/index.ts

@@ -3,7 +3,7 @@
 import createInput from '#shared/form/core/createInput.ts'
 import addLink from '#shared/form/features/addLink.ts'
 import formUpdaterTrigger from '#shared/form/features/formUpdaterTrigger.ts'
-import removeValuesForNonExistingOptions from '#shared/form/features/removeValuesForNonExistingOptions.ts'
+import removeValuesForNonExistingOrDisabledOptions from '#shared/form/features/removeValuesForNonExistingOrDisabledOptions.ts'
 import FieldTreeSelectInput from './FieldTreeSelectInput.vue'
 
 const fieldDefinition = createInput(
@@ -22,7 +22,7 @@ const fieldDefinition = createInput(
     features: [
       addLink,
       formUpdaterTrigger(),
-      removeValuesForNonExistingOptions,
+      removeValuesForNonExistingOrDisabledOptions,
     ],
   },
   { addArrow: true },

Some files were not shown because too many files changed in this diff