Fixes #4027 - undefined method `to_hash` on editing select fields in the admin interface after migration to 5.1.

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

@@ -282,3 +282,37 @@ class App.UiElement.ApplicationUiElement
         record.disabled = 'disabled'
       else
         record.disabled = ''
+
+  @findOption: (options, value) ->
+    return if !_.isArray(options)
+    for option in options
+      return option if option.value is value
+      if option.children
+        result = @findOption(option.children, value)
+        return result if result
+
+  # 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) ->
+    return if !_.isEmpty(attribute.relation) # do not apply for attributes with relation, relations will fill options automatically
+    return if attribute.rejectNonExistentValues
+    value = attribute.value
+    return if !value
+    return if !attribute.options
+
+    values = value
+    if !_.isArray(value)
+      values = [value]
+
+    attribute.historical_options ||= {}
+    if _.isArray(attribute.options)
+      for value in values
+        continue if @findOption(attribute.options, value)
+        attribute.options.push(
+          value: value
+          name: attribute.historical_options[value] || value
+        )
+    else
+      for value in values
+        continue if attribute.options[value]
+        attribute.options[value] = attribute.historical_options[value] || value

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

@@ -37,34 +37,6 @@ class App.UiElement.multiselect 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) ->
-    return if !_.isEmpty(attribute.relation) # do not apply for attributes with relation, relations will fill options automatically
-    return if attribute.rejectNonExistentValues
-    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 _.isArray(attribute.options)
-      # Array of Strings (value)
-      return if value of attribute.options
-
-      # Array of Objects (for ordering purposes)
-      return if attribute.options.filter((elem) -> elem.value == value) isnt null
-    else
-      # regular Object
-      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
-
   @_selectedOptionsIsSelected: (value, record) ->
     if _.isArray(value)
       for valueItem in value

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

@@ -59,7 +59,7 @@ class App.UiElement.select extends App.UiElement.ApplicationUiElement
   @shouldDisplayWarn: (selectedVal, attribute, params) ->
     return if !selectedVal
     return if !params
-    
+
     params[attribute.name + '_is_display_warning'](selectedVal)
 
   @toggleDisplayWarn: (warn_visible, attribute, item) ->
@@ -73,30 +73,3 @@ class App.UiElement.select extends App.UiElement.ApplicationUiElement
     warn_elem.html(attribute.warn)
     item.append(warn_elem)
 
-  # 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) ->
-    return if !_.isEmpty(attribute.relation) # do not apply for attributes with relation, relations will fill options automatically
-    return if attribute.rejectNonExistentValues
-    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 _.isArray(attribute.options)
-      # Array of Strings (value)
-      return if value of attribute.options
-
-      # Array of Objects (for ordering purposes)
-      return if attribute.options.filter((elem) -> elem.value == value) isnt null
-    else
-      # regular Object
-      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/assets/javascripts/app/controllers/_ui_element/tree_select.coffee

@@ -1,5 +1,40 @@
 # coffeelint: disable=camel_case_classes
 class App.UiElement.tree_select extends App.UiElement.ApplicationUiElement
+  @render: (attribute, params) ->
+
+    # set multiple option
+    if attribute.multiple
+      attribute.multiple = 'multiple'
+    else
+      attribute.multiple = ''
+
+    # add deleted historical options if required
+    @addDeletedOptions(attribute, params)
+
+    # build options list based on config
+    @getConfigOptionList(attribute, params)
+
+    # build options list based on relation
+    @getRelationOptionList(attribute, params)
+
+    # add null selection if needed
+    @addNullOption(attribute, params)
+
+    # sort attribute.options
+    @sortOptions(attribute, params)
+
+    # find selected/checked item of list
+    if attribute.options
+      @optionsSelect(attribute.options, attribute.value)
+
+    # disable item of list
+    @disabledOptions(attribute, params)
+
+    # filter attributes
+    @filterOption(attribute, params)
+
+    new App.SearchableSelect(attribute: attribute).element()
+
   @optionsSelect: (children, value) ->
     return if !children
     for child in children
@@ -37,34 +72,3 @@ class App.UiElement.tree_select extends App.UiElement.ApplicationUiElement
   @filterOptionArray: (attribute) ->
     attribute.options = @filterTreeOptions(attribute.filter, 0, attribute.options, attribute.null)
 
-  @render: (attribute, params) ->
-
-    # set multiple option
-    if attribute.multiple
-      attribute.multiple = 'multiple'
-    else
-      attribute.multiple = ''
-
-    # build options list based on config
-    @getConfigOptionList(attribute, params)
-
-    # build options list based on relation
-    @getRelationOptionList(attribute, params)
-
-    # add null selection if needed
-    @addNullOption(attribute, params)
-
-    # sort attribute.options
-    @sortOptions(attribute, params)
-
-    # find selected/checked item of list
-    if attribute.options
-      @optionsSelect(attribute.options, attribute.value)
-
-    # disable item of list
-    @disabledOptions(attribute, params)
-
-    # filter attributes
-    @filterOption(attribute, params)
-
-    new App.SearchableSelect(attribute: attribute).element()

app/assets/javascripts/app/models/overview.coffee

@@ -31,7 +31,7 @@ class App.Overview extends App.Model
       name:    'order::direction'
       display: __('Order by Direction')
       tag:     'select'
-      default: 'down'
+      default: 'DESC'
       null:    false
       translate: true
       options:
@@ -57,7 +57,7 @@ class App.Overview extends App.Model
       name:    'group_direction'
       display: __('Group by Direction')
       tag:     'select'
-      default: 'down'
+      default: 'DESC'
       null:    false
       translate: true
       options:

app/models/object_manager/attribute.rb

@@ -535,6 +535,31 @@ returns
     ObjectManager::Attribute.where('to_create = ? OR to_migrate = ? OR to_delete = ? OR to_config = ?', true, true, true, true)
   end
 
+  def self.attribute_historic_options(attribute)
+    historical_options = attribute.data_option[:historical_options] || {}
+    if attribute.data_option[:options].present?
+      historical_options = historical_options.merge(data_options_hash(attribute.data_option[:options]))
+    end
+    if attribute.data_option_new[:options].present?
+      historical_options = historical_options.merge(data_options_hash(attribute.data_option_new[:options]))
+    end
+    historical_options
+  end
+
+  def self.data_options_hash(options, result = {})
+    return options if options.is_a?(Hash)
+    return {} if !options.is_a?(Array)
+
+    options.each do |option|
+      result[ option[:value] ] = option[:name]
+      if option[:children].present?
+        data_options_hash(option[:children], result)
+      end
+    end
+
+    result
+  end
+
 =begin
 
 start migration of pending attribute migrations
@@ -573,11 +598,8 @@ to send no browser reload event, pass false
       # config changes
       if attribute.to_config
         execute_config_count += 1
-        if attribute.data_type == 'select' && 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
+        if attribute.data_type =~ %r{^(multi|tree_)?select$} && attribute.data_option[:options]
+          attribute.data_option_new[:historical_options] = attribute_historic_options(attribute)
         end
         attribute.data_option = attribute.data_option_new
         attribute.data_option_new = {}
@@ -586,8 +608,8 @@ to send no browser reload event, pass false
         next if !attribute.to_create && !attribute.to_migrate && !attribute.to_delete
       end
 
-      if attribute.data_type == 'select' && attribute.data_option[:options]
-        attribute.data_option[:historical_options] = attribute.data_option[:options]
+      if %r{^(multi|tree_)?select$}.match?(attribute.data_type)
+        attribute.data_option[:historical_options] = attribute_historic_options(attribute)
       end
 
       data_type = nil

public/assets/tests/qunit/form.js

@@ -1639,15 +1639,39 @@ QUnit.test("Fixes #4024 - Tree select value cannot be set to \"-\" (empty) with
     el:        el,
     model:     {
       configure_attributes: [
-        { name: '4042_select', display: '4042_select', tag: 'select_search', null: true, nulloption: true, multiple: true, options: { 'a': 'a', 'b': 'b' } },
-        { name: '4042_multiselect', display: '4042_multiselect', tag: 'multiselect_search', null: true, nulloption: true, multiple: true, options: { 'a': 'a', 'b': 'b' } },
-        { name: '4042_tree_select', display: '4042_tree_select', tag: 'tree_select_search', null: true, nulloption: true, multiple: true, options: [{ 'value': 'a', 'name': 'a'}, { 'value': 'b', 'name': 'b'}] },
+        { name: '4024_select', display: '4024_select', tag: 'select_search', null: true, nulloption: true, multiple: true, options: { 'a': 'a', 'b': 'b' } },
+        { name: '4024_multiselect', display: '4024_multiselect', tag: 'multiselect_search', null: true, nulloption: true, multiple: true, options: { 'a': 'a', 'b': 'b' } },
+        { name: '4024_tree_select', display: '4024_tree_select', tag: 'tree_select_search', null: true, nulloption: true, multiple: true, options: [{ 'value': 'a', 'name': 'a'}, { 'value': 'b', 'name': 'b'}] },
       ],
     },
     autofocus: true
   });
 
-  assert.equal(el.find('select[name="4042_select"] option[value=""]').text(), '-', '4042_select has nulloption')
-  assert.equal(el.find('select[name="4042_multiselect"] option[value=""]').text(), '-', '4042_multiselect has nulloption')
-  assert.equal(el.find('select[name="4042_tree_select"] option[value=""]').text(), '-', '4042_tree_select has nulloption')
+  assert.equal(el.find('select[name="4024_select"] option[value=""]').text(), '-', '4024_select has nulloption')
+  assert.equal(el.find('select[name="4024_multiselect"] option[value=""]').text(), '-', '4024_multiselect has nulloption')
+  assert.equal(el.find('select[name="4024_tree_select"] option[value=""]').text(), '-', '4024_tree_select has nulloption')
+});
+
+QUnit.test("Fixes #4027 - undefined method `to_hash` on editing select fields in the admin interface after migration to 5.1.", assert => {
+  $('#qunit').append('<hr><h1>Fixes #4027 - undefined method `to_hash` on editing select fields in the admin interface after migration to 5.1.</h1><form id="form23"></form>')
+  var el = $('#form23')
+  new App.ControllerForm({
+    el:        el,
+    model:     {
+      configure_attributes: [
+        { name: '4027_selcet_hash', display: '4027_selcet_hash', tag: 'select', null: true, nulloption: true, options: { 'a': 'a', 'b': 'b' }, value: 'c', historical_options: { c: 'C' } },
+        { name: '4027_selcet_array', display: '4027_selcet_array', tag: 'select', null: true, nulloption: true, options: [{ value: 'a', name: 'a' }, { value: 'b', name: 'b' } ], value: 'c', historical_options: { c: 'C' } },
+        { name: '4027_multiselect_hash', display: '4027_multiselect_hash', tag: 'multiselect', null: true, nulloption: true, options: { 'a': 'a', 'b': 'b' }, value: ['c'], historical_options: { c: 'C' } },
+        { name: '4027_multiselect_array', display: '4027_multiselect_array', tag: 'multiselect', null: true, nulloption: true, options: [{ value: 'a', name: 'a' }, { value: 'b', name: 'b' } ], value: ['c', 'd'], historical_options: { c: 'C', d: 'D' } },
+        { name: '4027_tree_select_array', display: '4027_tree_select_array', tag: 'tree_select', null: true, nulloption: true, options: [{ value: 'a', name: 'a' }, { value: 'b', name: 'b' } ], value: 'b::c', historical_options: { 'b::c': 'C' } },
+      ],
+    },
+    autofocus: true
+  });
+
+  assert.equal(el.find('select[name="4027_selcet_hash"] option[selected]').text(), 'C', '4027_select has historic text')
+  assert.equal(el.find('select[name="4027_selcet_array"] option[selected]').text(), 'C', '4027_selcet_array has historic text')
+  assert.equal(el.find('select[name="4027_multiselect_hash"] option[selected]').text(), 'C', '4027_multiselect_hash has historic text')
+  assert.equal(el.find('select[name="4027_multiselect_array"] option[selected]').text(), 'CD', '4027_multiselect_array has historic text')
+  assert.equal(el.find('div[data-attribute-name="4027_tree_select_array"] .js-input').val(), 'C', '4027_tree_select_array has historic text')
 });

spec/models/object_manager/attribute_spec.rb

@@ -171,6 +171,97 @@ RSpec.describe ObjectManager::Attribute, type: :model do
         expect(described_class.attribute_to_references_hash_objects).to match_array [Trigger, Overview, Job, Sla, Report::Profile ]
       end
     end
+
+    describe '.data_options_hash' do
+      context 'when hash' do
+        let(:check) do
+          {
+            'a' => 'A',
+            'b' => 'B',
+            'c' => 'c',
+          }
+        end
+
+        it 'does return the options as hash' do
+          expect(described_class.data_options_hash(check)).to eq({
+                                                                   'a' => 'A',
+                                                                   'b' => 'B',
+                                                                   'c' => 'c',
+                                                                 })
+        end
+      end
+
+      context 'when array' do
+        let(:check) do
+          [
+            {
+              value: 'a',
+              name:  'A',
+            },
+            {
+              value: 'b',
+              name:  'B',
+            },
+            {
+              value: 'c',
+              name:  'c',
+            },
+          ]
+        end
+
+        it 'does return the options as hash' do
+          expect(described_class.data_options_hash(check)).to eq({
+                                                                   'a' => 'A',
+                                                                   'b' => 'B',
+                                                                   'c' => 'c',
+                                                                 })
+        end
+      end
+
+      context 'when tree array' do
+        let(:check) do
+          [
+            {
+              value: 'a',
+              name:  'A',
+            },
+            {
+              value: 'b',
+              name:  'B',
+            },
+            {
+              value:    'c',
+              name:     'c',
+              children: [
+                {
+                  value: 'c::a',
+                  name:  'c sub a',
+                },
+                {
+                  value: 'c::b',
+                  name:  'c sub b',
+                },
+                {
+                  value: 'c::c',
+                  name:  'c sub c',
+                },
+              ],
+            },
+          ]
+        end
+
+        it 'does return the options as hash' do
+          expect(described_class.data_options_hash(check)).to eq({
+                                                                   'a'    => 'A',
+                                                                   'b'    => 'B',
+                                                                   'c'    => 'c',
+                                                                   'c::a' => 'c sub a',
+                                                                   'c::b' => 'c sub b',
+                                                                   'c::c' => 'c sub c',
+                                                                 })
+        end
+      end
+    end
   end
 
   describe '#data_option_validations' do
@@ -343,4 +434,44 @@ RSpec.describe ObjectManager::Attribute, type: :model do
       include_examples 'tests the exception on missing past', 'datetime'
     end
   end
+
+  describe 'undefined method `to_hash` on editing select fields in the admin interface after migration to 5.1 #4027', db_strategy: :reset do
+    let(:select_field) { create(:object_manager_attribute_select) }
+
+    before do
+      described_class.migration_execute
+    end
+
+    it 'does save the attribute with sorted options' do
+      add = select_field.attributes.deep_symbolize_keys
+      add[:data_option_new] = add[:data_option]
+      add[:data_option_new][:options] = [
+        {
+          name:  'a',
+          value: 'a',
+        },
+        {
+          name:  'b',
+          value: 'b',
+        },
+        {
+          name:  'c',
+          value: 'c',
+        },
+      ]
+
+      described_class.add(add)
+      described_class.migration_execute
+
+      expect_result = {
+        'key_1' => 'value_1',
+        'key_2' => 'value_2',
+        'key_3' => 'value_3',
+        'a'     => 'a',
+        'b'     => 'b',
+        'c'     => 'c'
+      }
+      expect(select_field.reload.data_option[:historical_options]).to eq(expect_result)
+    end
+  end
 end