Fixed issue #2020 by checking against past select attribute options (fixes #2020)

app/assets/javascripts/app/controllers/_ui_element/select.coffee

@@ -8,6 +8,9 @@ class App.UiElement.select extends App.UiElement.ApplicationUiElement
     else
       attribute.multiple = ''
 
+    # add deleted historical options if required
+    @addDeletedOptions(attribute, params)
+
     # build options list based on config
     @getConfigOptionList(attribute, params)
 
@@ -31,3 +34,19 @@ class App.UiElement.select extends App.UiElement.ApplicationUiElement
 
     # return item
     $( App.view('generic/select')(attribute: attribute) )
+
+  # 1. If attribute.value is not among the current options, then search within historical options
+  # 2. If attribute.value is not among current and historical options, then add the value itself as an option
+  @addDeletedOptions: (attribute) ->
+    value = attribute.value
+    return if !value
+    return if _.isArray(value)
+    return if !attribute.options
+    return if !_.isObject(attribute.options)
+    return if value of attribute.options
+    return if value in (temp for own prop, temp of attribute.options)
+
+    if attribute.historical_options && value of attribute.historical_options
+      attribute.options[value] = attribute.historical_options[value]
+    else
+      attribute.options[value] = value

app/models/object_manager/attribute.rb

@@ -621,6 +621,12 @@ to send no browser reload event, pass false
       # config changes
       if attribute.to_config
         execute_config_count += 1
+        if attribute.data_option[:options]
+          historical_options = attribute.data_option[:historical_options] || {}
+          historical_options.update(attribute.data_option[:options])
+          historical_options.update(attribute.data_option_new[:options])
+          attribute.data_option_new[:historical_options] = historical_options
+        end
         attribute.data_option = attribute.data_option_new
         attribute.data_option_new = {}
         attribute.to_config = false
@@ -628,6 +634,10 @@ to send no browser reload event, pass false
         next if !attribute.to_create && !attribute.to_migrate && !attribute.to_delete
       end
 
+      if attribute.data_option[:options]
+        attribute.data_option[:historical_options] = attribute.data_option[:options]
+      end
+
       data_type = nil
       if attribute.data_type.match?(/^input|select|tree_select|richtext|textarea|checkbox$/)
         data_type = :string

test/browser/admin_object_manager_test.rb

@@ -347,19 +347,9 @@ class AdminObjectManagerTest < TestCase
         name: 'browser_test7',
       },
     )
-    click(css: '.content.active .tab-pane.active div.js-execute')
+    sleep 1
-    watch_for(
+    object_manager_attribute_migrate
-      css: '.modal',
+
-      value: 'restart',
-    )
-    watch_for_disappear(
-      css:     '.modal',
-      timeout: 7.minutes,
-    )
-    sleep 5
-    watch_for(
-      css: '.content.active',
-    )
     match_not(
       css: '.content.active',
       value: 'Database Update required',
@@ -670,5 +660,118 @@ class AdminObjectManagerTest < TestCase
     unsorted_options = select_element.find_elements(xpath: './*').map(&:text).reject { |x| x == '-' }
     log unsorted_options.inspect
     assert_equal options, unsorted_options
+
+    object_manager_attribute_delete(
+      data: {
+        name: 'select_attributes_sorting_test',
+      },
+    )
+    object_manager_attribute_migrate
   end
+
+  def test_deleted_select_attributes
+    @browser = browser_instance
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    options = Hash[ %w[äöü cat delete dog ß].map { |x| [x, "#{x.capitalize} Display"] } ]
+    options_no_dog = options.except('dog')
+    options_no_dog_no_delete = options_no_dog.except('delete')
+
+    tasks_close_all()
+
+    object_manager_attribute_create(
+      data: {
+        name: 'select_attributes_delete_test',
+        display: 'Select Attributes Delete Test',
+        data_type: 'Select',
+        data_option: {
+          options: options,
+        },
+      },
+    )
+    object_manager_attribute_migrate
+
+    ticket = ticket_create(
+      data: {
+        customer: 'nico',
+        group:    'Users',
+        title:    'select_attributes_delete_test',
+        body:     'select_attributes_delete_test',
+      },
+      custom_data_select: {
+        select_attributes_delete_test: 'Delete Display',
+      },
+      disable_group_check: true,
+    )
+
+    watch_for(
+      css: '.content.active select[name="select_attributes_delete_test"]',
+    )
+
+    # confirm that all options and their display values are there and are in the correct order
+    select_element = @browser.find_elements(css: '.content.active select[name="select_attributes_delete_test"]')[0]
+    unsorted_options = select_element.find_elements(xpath: './*').map { |o| o.attribute('value') }.reject { |x| x == '' }
+    assert_equal options.keys, unsorted_options
+    unsorted_display_options = select_element.find_elements(xpath: './*').map(&:text).reject { |x| x == '-' }
+    assert_equal options.values, unsorted_display_options
+
+    # confirm that the "delete" option is selected and that its display text is indeed "Delete Display"
+    selected_option = select_element.find_elements(css: 'option:checked')[0]
+    assert_equal 'delete', selected_option.attribute('value')
+    assert_equal 'Delete Display', selected_option.text
+
+    object_manager_attribute_update(
+      data: {
+        name: 'select_attributes_delete_test',
+        data_option: {
+          options: options_no_dog_no_delete,
+        },
+      },
+    )
+    object_manager_attribute_migrate
+
+    # open the previously created ticket and verify its attribute selection
+    click(
+      xpath:   '//a/div[contains(text(),"select_attributes_delete_test")]',
+    )
+    # confirm that all options and their display values are there and are in the correct order
+    select_element = @browser.find_elements(css: '.content.active select[name="select_attributes_delete_test"]')[0]
+    unsorted_options = select_element.find_elements(xpath: './*').map { |o| o.attribute('value') }.reject { |x| x == '' }
+    assert_equal options_no_dog.keys, unsorted_options
+    unsorted_display_options = select_element.find_elements(xpath: './*').map(&:text).reject { |x| x == '-' }
+    assert_equal options_no_dog.values, unsorted_display_options
+
+    # confirm that the "delete" option is still selected and that its display text is still indeed "Delete Display"
+    selected_option = select_element.find_elements(css: 'option:checked')[0]
+    assert_equal 'delete', selected_option.attribute('value')
+    assert_equal 'Delete Display', selected_option.text
+
+    # create a new ticket and check that the deleted options no longer appear
+    click(
+      css: 'a[href="#ticket/create"]',
+      mute_log: true,
+    )
+
+    watch_for(
+      css: 'select[name="select_attributes_delete_test"]',
+    )
+
+    select_element = @browser.find_elements(css: 'select[name="select_attributes_delete_test"]')[0]
+    unsorted_options = select_element.find_elements(xpath: './*').map { |o| o.attribute('value') }.reject { |x| x == '' }
+    assert_equal options_no_dog_no_delete.keys, unsorted_options
+    unsorted_display_options = select_element.find_elements(xpath: './*').map(&:text).reject { |x| x == '-' }
+    assert_equal options_no_dog_no_delete.values, unsorted_display_options
+
+    object_manager_attribute_delete(
+      data: {
+        name: 'select_attributes_delete_test',
+      },
+    )
+    object_manager_attribute_migrate
+  end
+
 end

test/browser_test_helper.rb

@@ -3641,6 +3641,7 @@ wait untill text in selector disabppears
       data_type: 'Text',
       data_option: {
         default: 'abc',
+        maxlength: 20,
       },
     },
     error: 'already exists'
@@ -3717,6 +3718,12 @@ wait untill text in selector disabppears
     instance = params[:browser] || @browser
     data     = params[:data]
 
+    # make sure that required params are supplied
+    %i[name display data_type].each do |s|
+      next if data.key? s
+      raise "missing required param #{s} in object_manager_attribute_create()"
+    end
+
     click(
       browser:  instance,
       css:      'a[href="#manage"]',
@@ -3736,94 +3743,8 @@ wait untill text in selector disabppears
       css:      '.content.active .js-new',
       mute_log: true,
     )
-    modal_ready(browser: instance)
-    element = instance.find_elements(css: '.modal input[name=name]')[0]
-    element.clear
-    element.send_keys(data[:name])
-    element = instance.find_elements(css: '.modal input[name=display]')[0]
-    element.clear
-    element.send_keys(data[:display])
-    select(
-      browser:  instance,
-      css:      '.modal select[name="data_type"]',
-      value:    data[:data_type],
-      mute_log: true,
-    )
-    if data[:data_option]
-      if data[:data_option][:options]
-        if data[:data_type] == 'Boolean'
-          # rubocop:disable Lint/BooleanSymbol
-          element = instance.find_elements(css: '.modal .js-valueTrue').first
-          element.clear
-          element.send_keys(data[:data_option][:options][:true])
-          element = instance.find_elements(css: '.modal .js-valueFalse').first
-          element.clear
-          element.send_keys(data[:data_option][:options][:false])
-          # rubocop:enable Lint/BooleanSymbol
-        elsif data[:data_type] == 'Tree Select'
-          add_tree_options(
-            instance: instance,
-            options:  data[:data_option][:options],
-          )
-        else
-          data[:data_option][:options].each do |key, value|
-            element = instance.find_elements(css: '.modal .js-Table .js-key').last
-            element.clear
-            element.send_keys(key)
-            element = instance.find_elements(css: '.modal .js-Table .js-value').last
-            element.clear
-            element.send_keys(value)
-            element = instance.find_elements(css: '.modal .js-Table .js-add')[0]
-            element.click
-          end
-        end
-      end
 
-      %i[default min max diff].each do |key|
+    object_manager_attribute_perform('create', params)
-        next if !data[:data_option].key?(key)
-        element = instance.find_elements(css: ".modal [name=\"data_option::#{key}\"]").first
-        element.clear
-        element.send_keys(data[:data_option][key])
-      end
-
-      %i[future past].each do |key|
-        next if !data[:data_option].key?(key)
-        select(
-          browser:  instance,
-          css:      ".modal select[name=\"data_option::#{key}\"]",
-          value:    data[:data_option][key],
-          mute_log: true,
-        )
-      end
-
-    end
-    instance.find_elements(css: '.modal button.js-submit')[0].click
-    if params[:error]
-      sleep 4
-      watch_for(
-        css: '.modal',
-        value: params[:error],
-      )
-      click(
-        browser: instance,
-        css:  '.modal .js-close',
-      )
-      modal_disappear(browser: instance)
-      return
-    end
-
-    11.times do
-      element = instance.find_elements(css: 'body')[0]
-      text = element.text
-      if text.match?(/#{Regexp.quote(data[:name])}/)
-        assert(true, 'object manager attribute created')
-        sleep 1
-        return true
-      end
-      sleep 1
-    end
-    screenshot(browser: instance, comment: 'object_manager_attribute_create_failed')
-    raise 'object manager attribute creation failed'
   end
 
 =begin
@@ -3870,92 +3791,8 @@ wait untill text in selector disabppears
       css:     '.content.active .js-new',
     )
     instance.execute_script("$(\".content.active td:contains('#{data[:name]}')\").first().click()")
-    modal_ready(browser: instance)
-    element = instance.find_elements(css: '.modal input[name=display]')[0]
-    element.clear
-    element.send_keys(data[:display])
-    select(
-      browser:  instance,
-      css:      '.modal select[name="data_type"]',
-      value:    data[:data_type],
-      mute_log: true,
-    )
 
-    # if attribute is created, do not be able to select other types anymore
+    object_manager_attribute_perform('update', params)
-    if instance.find_elements(css: '.modal select[name="data_type"] option').count > 1
-      assert(false, 'able to change the data_type of existing attribute which should not be allowed')
-    end
-
-    if data[:data_option]
-      if data[:data_option][:options]
-        if data[:data_type] == 'Boolean'
-          # rubocop:disable Lint/BooleanSymbol
-          element = instance.find_elements(css: '.modal .js-valueTrue').first
-          element.clear
-          element.send_keys(data[:data_option][:options][:true])
-          element = instance.find_elements(css: '.modal .js-valueFalse').first
-          element.clear
-          element.send_keys(data[:data_option][:options][:false])
-          # rubocop:enable Lint/BooleanSymbol
-        else
-          data[:data_option][:options].each do |key, value|
-            element = instance.find_elements(css: '.modal .js-Table .js-key').last
-            element.clear
-            element.send_keys(key)
-            element = instance.find_elements(css: '.modal .js-Table .js-value').last
-            element.clear
-            element.send_keys(value)
-            element = instance.find_elements(css: '.modal .js-Table .js-add')[0]
-            element.click
-          end
-        end
-      end
-
-      %i[default min max diff].each do |key|
-        next if !data[:data_option].key?(key)
-        element = instance.find_elements(css: ".modal [name=\"data_option::#{key}\"]").first
-        element.clear
-        element.send_keys(data[:data_option][key])
-      end
-
-      %i[future past].each do |key|
-        next if !data[:data_option].key?(key)
-        select(
-          browser:  instance,
-          css:      ".modal select[name=\"data_option::#{key}\"]",
-          value:    data[:data_option][key],
-          mute_log: true,
-        )
-      end
-
-    end
-    instance.find_elements(css: '.modal button.js-submit')[0].click
-    if params[:error]
-      sleep 4
-      watch_for(
-        css: '.modal',
-        value: params[:error],
-      )
-      click(
-        browser: instance,
-        css:  '.modal .js-close',
-      )
-      modal_disappear(browser: instance)
-      return
-    end
-
-    11.times do
-      element = instance.find_elements(css: 'body')[0]
-      text = element.text
-      if text.match?(/#{Regexp.quote(data[:name])}/)
-        assert(true, 'object manager attribute updated')
-        sleep 1
-        return true
-      end
-      sleep 1
-    end
-    screenshot(browser: instance, comment: 'object_manager_attribute_update_failed')
-    raise 'object manager attribute update failed'
   end
 
 =begin
@@ -4030,6 +3867,61 @@ wait untill text in selector disabppears
 
   end
 
+=begin
+
+    Execute any pending migrations in the object attribute manager
+
+    object_manager_attribute_migrate(
+      browser: browser2,
+    )
+
+=end
+
+  def object_manager_attribute_migrate(params = {})
+    switch_window_focus(params)
+    log('object_manager_attribute_migrate', params)
+
+    instance = params[:browser] || @browser
+
+    watch_for(
+      browser: instance,
+      css: '.content.active',
+      value: 'Database Update required',
+      mute_log: true,
+    )
+    click(
+      browser: instance,
+      css: '.content.active .tab-pane.active div.js-execute',
+      mute_log: true,
+    )
+    modal_ready(
+      browser: instance,
+    )
+    title_text = instance.find_elements(css: '.modal .modal-title').first.text
+    if title_text == 'Zammad is restarting...'
+      # in the complex case, wait for server to restart
+      modal_disappear(
+        browser: instance,
+        timeout: 7.minutes,
+      )
+    elsif title_text == 'Config has changed'
+      # in the simple case, just click the submit button
+      click(
+        browser: instance,
+        css: '.modal .js-submit',
+        mute_log: true,
+      )
+    else
+      raise "Unknown title text \"#{title_text}\" found when trying to update database"
+    end
+    sleep 5
+    watch_for(
+      browser: instance,
+      css: '.content.active',
+      mute_log: true,
+    )
+  end
+
 =begin
 
   tags_verify(
@@ -4239,4 +4131,149 @@ wait untill text in selector disabppears
     end
     raise "HTTP error #{res.code} while POSTing to #{browser_url}/api/v1/settings/" if res.code != '200'
   end
+
+=begin
+
+  Helper method for both object_manager_attribute_create and object_manager_attribute_update
+
+=end
+
+  def object_manager_attribute_perform(action = 'create', params = {})
+    instance = params[:browser] || @browser
+    data     = params[:data]
+
+    modal_ready(browser: instance)
+
+    if action == 'create'
+      set(
+        browser:  instance,
+        css:      '.modal input[name=name]',
+        value:    data[:name],
+        mute_log: true,
+      )
+    end
+
+    if data[:display]
+      set(
+        browser:  instance,
+        css:      '.modal input[name=display]',
+        value:    data[:display],
+        mute_log: true,
+      )
+    end
+
+    if data[:data_type]
+      select(
+        browser:  instance,
+        css:      '.modal select[name="data_type"]',
+        value:    data[:data_type],
+        mute_log: true,
+      )
+    end
+
+    if data[:data_option]
+      if data[:data_option][:options]
+        if data[:data_type] == 'Boolean'
+          # rubocop:disable Lint/BooleanSymbol
+          element = instance.find_elements(css: '.modal .js-valueTrue').first
+          element.clear
+          element.send_keys(data[:data_option][:options][:true])
+          element = instance.find_elements(css: '.modal .js-valueFalse').first
+          element.clear
+          element.send_keys(data[:data_option][:options][:false])
+          # rubocop:enable Lint/BooleanSymbol
+        elsif data[:data_type] == 'Tree Select'
+          add_tree_options(
+            instance: instance,
+            options:  data[:data_option][:options],
+          )
+        else
+          # first clear all existing entries
+          loop do
+            target = {
+              browser:  instance,
+              css: '.modal .js-Table .js-remove',
+              mute_log: true,
+            }
+            break if !instance.find_elements(css: target[:css])[0]
+            click(target)
+          end
+          sleep 1
+
+          # then populate the table with the new values
+          data[:data_option][:options].each do |key, value|
+            element = instance.find_elements(css: '.modal .js-Table .js-key').last
+            element.clear
+            element.send_keys(key)
+            element = instance.find_elements(css: '.modal .js-Table .js-value').last
+            element.clear
+            element.send_keys(value)
+            element = instance.find_elements(css: '.modal .js-Table .js-add')[0]
+            element.click
+          end
+        end
+      end
+
+      %i[default min max diff].each do |key|
+        next if !data[:data_option].key?(key)
+        element = instance.find_elements(css: ".modal [name=\"data_option::#{key}\"]").first
+        element.clear
+        element.send_keys(data[:data_option][key])
+      end
+
+      %i[future past].each do |key|
+        next if !data[:data_option].key?(key)
+        select(
+          browser:  instance,
+          css:      ".modal select[name=\"data_option::#{key}\"]",
+          value:    data[:data_option][key],
+          mute_log: true,
+        )
+      end
+
+      %i[maxlength].each do |key|
+        next if !data[:data_option].key?(key)
+        set(
+          browser:  instance,
+          css:      ".modal input[name=\"data_option::#{key}\"]",
+          value:    data[:data_option][key],
+          mute_log: true,
+        )
+      end
+    end
+
+    if params[:do_not_submit]
+      assert(true, "attribute #{action}d without submit")
+      return true
+    end
+
+    instance.find_elements(css: '.modal button.js-submit')[0].click
+
+    if params[:error]
+      sleep 4
+      watch_for(
+        css: '.modal',
+        value: params[:error],
+      )
+      click(
+        browser: instance,
+        css:  '.modal .js-close',
+      )
+      modal_disappear(browser: instance)
+      return
+    end
+
+    11.times do
+      element = instance.find_elements(css: 'body')[0]
+      text = element.text
+      if text.match?(/#{Regexp.quote(data[:name])}/)
+        assert(true, 'object manager attribute updated')
+        sleep 1
+        return true
+      end
+      sleep 1
+    end
+    screenshot(browser: instance, comment: "object_manager_attribute_#{action}_failed")
+    raise "object_manager_attribute_#{action}_failed"
+  end
 end