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

This commit is contained in:
Rolf Schmidt 2022-04-04 13:15:08 +02:00
parent 2c6710cd3f
commit 68a034e159
8 changed files with 262 additions and 102 deletions

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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()

View file

@ -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:

View file

@ -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

View file

@ -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')
});

View file

@ -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