diff --git a/app/assets/javascripts/app/controllers/object_manager.coffee b/app/assets/javascripts/app/controllers/object_manager.coffee index 2a6f6cd71..e63bf8830 100644 --- a/app/assets/javascripts/app/controllers/object_manager.coffee +++ b/app/assets/javascripts/app/controllers/object_manager.coffee @@ -1,9 +1,8 @@ # coffeelint: disable=duplicate_key treeParams = (e, params) -> tree = [] - lastLevel = 0 lastLevels = [] - valueLevels = [] + previousValueLevels = [] $(e.target).closest('.modal').find('.js-treeTable .js-key').each( -> $element = $(@) @@ -19,20 +18,19 @@ treeParams = (e, params) -> lastLevels[level-1].children.push item else console.log('ERROR', item) - if level is 0 - valueLevels = [] - else if lastLevel is level - valueLevels.pop() - else if lastLevel > level - down = lastLevel - level - for count in [1..down] - valueLevels.pop() - if lastLevel <= level - valueLevels.push name + + valueLevels = [] + # add previous level values + if level > 0 + for previousLevel in [0..level - 1] + valueLevels.push(previousValueLevels[previousLevel]) + + # add current level value + valueLevels.push(name) item.value = valueLevels.join('::') lastLevels[level] = item - lastLevel = level + previousValueLevels = valueLevels ) if tree[0] diff --git a/db/migrate/20180226085743_issue_1660_fix_tree_select_configurations.rb b/db/migrate/20180226085743_issue_1660_fix_tree_select_configurations.rb new file mode 100644 index 000000000..8ba55b99a --- /dev/null +++ b/db/migrate/20180226085743_issue_1660_fix_tree_select_configurations.rb @@ -0,0 +1,44 @@ +class Issue1660FixTreeSelectConfigurations < ActiveRecord::Migration[5.1] + def change + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + return if attributes.blank? + attributes.each do |attribute| + + next if attribute.data_option.blank? + next if attribute.data_option[:options].blank? + + fixed_options = fix(attribute.data_option[:options]) + + attribute.data_option[:options] = fixed_options + + attribute.save! + end + end + + private + + def attributes + @attributes ||= ObjectManager::Attribute.where(data_type: 'tree_select') + end + + def fix(options, namespace = nil) + + options.tap do |ref| + + ref.each do |option| + + option_namespace = Array(namespace.dup) + option_namespace.push(option['name']) + + option['value'] = option_namespace.join('::') + + next if option['children'].blank? + + option['children'] = fix(option['children'], option_namespace) + end + end + end +end diff --git a/script/build/test_slice_tests.sh b/script/build/test_slice_tests.sh index e23cd0df7..245e54777 100755 --- a/script/build/test_slice_tests.sh +++ b/script/build/test_slice_tests.sh @@ -16,6 +16,7 @@ if [ "$LEVEL" == '1' ]; then rm test/browser/abb_one_group_test.rb rm test/browser/admin_channel_email_test.rb rm test/browser/admin_object_manager_test.rb + rm test/browser/admin_object_manager_tree_select_test.rb rm test/browser/admin_overview_test.rb rm test/browser/admin_role_test.rb # test/browser/agent_navigation_and_title_test.rb @@ -79,6 +80,7 @@ elif [ "$LEVEL" == '2' ]; then # test/browser/abb_one_group_test.rb rm test/browser/admin_channel_email_test.rb rm test/browser/admin_object_manager_test.rb + rm test/browser/admin_object_manager_tree_select_test.rb rm test/browser/admin_overview_test.rb #rm test/browser/admin_role_test.rb rm test/browser/agent_navigation_and_title_test.rb @@ -142,6 +144,7 @@ elif [ "$LEVEL" == '3' ]; then # test/browser/abb_one_group_test.rb rm test/browser/admin_channel_email_test.rb rm test/browser/admin_object_manager_test.rb + rm test/browser/admin_object_manager_tree_select_test.rb rm test/browser/admin_overview_test.rb rm test/browser/admin_role_test.rb rm test/browser/agent_navigation_and_title_test.rb @@ -205,6 +208,7 @@ elif [ "$LEVEL" == '4' ]; then # test/browser/abb_one_group_test.rb rm test/browser/admin_channel_email_test.rb rm test/browser/admin_object_manager_test.rb + rm test/browser/admin_object_manager_tree_select_test.rb rm test/browser/admin_overview_test.rb rm test/browser/admin_role_test.rb rm test/browser/agent_navigation_and_title_test.rb @@ -267,6 +271,7 @@ elif [ "$LEVEL" == '5' ]; then # test/browser/abb_one_group_test.rb # test/browser/admin_channel_email_test.rb # test/browser/admin_object_manager_test.rb + # test/browser/admin_object_manager_tree_select_test.rb # test/browser/admin_overview_test.rb rm test/browser/admin_role_test.rb rm test/browser/agent_navigation_and_title_test.rb @@ -332,6 +337,7 @@ elif [ "$LEVEL" == '6' ]; then rm test/browser/abb_one_group_test.rb rm test/browser/admin_channel_email_test.rb rm test/browser/admin_object_manager_test.rb + rm test/browser/admin_object_manager_tree_select_test.rb rm test/browser/admin_overview_test.rb rm test/browser/admin_role_test.rb rm test/browser/agent_navigation_and_title_test.rb diff --git a/spec/db/migrate/issue_1660_fix_tree_select_configurations_spec.rb b/spec/db/migrate/issue_1660_fix_tree_select_configurations_spec.rb new file mode 100644 index 000000000..21df685ae --- /dev/null +++ b/spec/db/migrate/issue_1660_fix_tree_select_configurations_spec.rb @@ -0,0 +1,224 @@ +require 'rails_helper' + +RSpec.describe Issue1660FixTreeSelectConfigurations, type: :db_migration do + + it 'corrects broken data_option options' do + + # as provided in issue #1775 + expected = [ + { + 'name' => 'Blaak', + 'value' => 'Blaak', + 'children' => [ + { + 'name' => 'BL.-1', + 'value' => 'Blaak::BL.-1', + 'children' => [ + { + 'name' => 'BL.-1.3', + 'value' => 'Blaak::BL.-1::BL.-1.3', + }, + { + 'name' => 'BL.-1.19', + 'value' => 'Blaak::BL.-1::BL.-1.19', + } + ] + }, + { + 'name' => 'BL.0', + 'value' => 'Blaak::BL.0', + 'children' => [ + { + 'name' => 'BL.00.10a', + 'value' => 'Blaak::BL.0::BL.00.10a', + }, + { + 'name' => 'BL.00.7', + 'value' => 'Blaak::BL.0::BL.00.7', + }, + { + 'name' => 'BL.01.18', + 'value' => 'Blaak::BL.0::BL.01.18', + }, + { + 'name' => 'BL.00.12', + 'value' => 'Blaak::BL.0::BL.00.12', + }, + { + 'name' => 'BL.e0.3', + 'value' => 'Blaak::BL.0::BL.e0.3', + }, + ] + }, + { + 'name' => 'BL.1', + 'value' => 'Blaak::BL.1', + 'children' => [ + { + 'name' => 'BL.01.i', + 'value' => 'Blaak::BL.1::BL.01.i', + }, + ] + }, + { + 'name' => 'BL.2', + 'value' => 'Blaak::BL.2', + 'children' => [ + { + 'name' => 'BL.02.2', + 'value' => 'Blaak::BL.2::BL.02.2', + }, + { + 'name' => 'BL.02.4', + 'value' => 'Blaak::BL.2::BL.02.4', + }, + { + 'name' => 'BL.02.5', + 'value' => 'Blaak::BL.2::BL.02.5', + }, + { + 'name' => 'BL.02.6', + 'value' => 'Blaak::BL.2::BL.02.6', + }, + { + 'name' => 'BL.02.7', + 'value' => 'Blaak::BL.2::BL.02.7', + }, + ] + }, + ], + } + ] + + broken = [ + { + 'name' => 'Blaak', + 'value' => 'Blaak', + 'children' => [ + { + 'name' => 'BL.-1', + 'value' => 'Blaak::BL.-1', + 'children' => [ + { + 'name' => 'BL.-1.3', + 'value' => 'Blaak::BL.2::BL.-1.3', + }, + { + 'name' => 'BL.-1.19', + 'value' => 'Blaak::BL.2::BL.-1.19', + } + ] + }, + { + 'name' => 'BL.0', + 'value' => 'Blaak::BL.0', + 'children' => [ + { + 'name' => 'BL.00.10a', + 'value' => 'Blaak::BL.2::BL.00.10a', + }, + { + 'name' => 'BL.00.7', + 'value' => 'Blaak::BL.2::BL.00.7', + }, + { + 'name' => 'BL.01.18', + 'value' => 'Blaak::BL.2::BL.01.18', + }, + { + 'name' => 'BL.00.12', + 'value' => 'Blaak::BL.2::BL.00.12', + }, + { + 'name' => 'BL.e0.3', + 'value' => 'Blaak::BL.2::BL.e0.3', + }, + ] + }, + { + 'name' => 'BL.1', + 'value' => 'Blaak::BL.1', + 'children' => [ + { + 'name' => 'BL.01.i', + 'value' => 'Blaak::BL.2::BL.01.i', + }, + ] + }, + { + 'name' => 'BL.2', + 'value' => 'Blaak::BL.2', + 'children' => [ + { + 'name' => 'BL.02.2', + 'value' => 'Blaak::BL.2::BL.02.2', + }, + { + 'name' => 'BL.02.4', + 'value' => 'Blaak::BL.2::BL.02.4', + }, + { + 'name' => 'BL.02.5', + 'value' => 'Blaak::BL.2::BL.02.5', + }, + { + 'name' => 'BL.02.6', + 'value' => 'Blaak::BL.2::BL.02.6', + }, + { + 'name' => 'BL.02.7', + 'value' => 'Blaak::BL.2::BL.02.7', + }, + ] + }, + ], + } + ] + + attribute = create(:object_manager_attribute_tree_select, data_option: { options: broken }) + + expect do + migrate + end.to change { + attribute.reload.data_option[:options] + } + + expect(attribute.data_option[:options]).to eq(expected) + end + + it 'performs no action for new systems', system_init_done: false do + migrate do |instance| + expect(instance).not_to receive(:attributes) + end + end + + it 'skips blank data_option' do + attribute = create(:object_manager_attribute_tree_select, data_option: {}) + + expect do + migrate + end.not_to change { + attribute.reload.data_option + } + end + + it 'skips blank data_option options' do + attribute = create(:object_manager_attribute_tree_select, data_option: { options: [] }) + + expect do + migrate + end.not_to change { + attribute.reload.data_option[:options] + } + end + + it 'does not change correct data_option options' do + attribute = create(:object_manager_attribute_tree_select) + + expect do + migrate + end.not_to change { + attribute.reload.data_option[:options] + } + end +end diff --git a/test/browser/admin_object_manager_tree_select_test.rb b/test/browser/admin_object_manager_tree_select_test.rb new file mode 100644 index 000000000..976be1775 --- /dev/null +++ b/test/browser/admin_object_manager_tree_select_test.rb @@ -0,0 +1,221 @@ +require 'browser_test_helper' + +class AdminObjectManagerTreeSelectTest < TestCase + + def test_basic_a + + @browser = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + tasks_close_all() + + object_manager_attribute_create( + data: { + name: 'browser_test_tree_select1', + display: 'Browser Test TreeSelect1', + data_type: 'Tree Select', + data_option: { + options: { + 'Incident' => { + 'Hardware' => { + 'Monitor' => {}, + 'Mouse' => {}, + 'Keyboard' => {}, + }, + 'Softwareproblem' => { + 'CRM' => {}, + 'EDI' => {}, + 'SAP' => { + 'Authentication' => {}, + 'Not reachable' => {}, + }, + 'MS Office' => { + 'Excel' => {}, + 'PowerPoint' => {}, + 'Word' => {}, + 'Outlook' => {}, + }, + }, + }, + 'Service request' => { + 'New software requirement' => {}, + 'New hardware' => {}, + 'Consulting' => {}, + }, + 'Change request' => {}, + }, + }, + }, + ) + + correct_options = [ + { + 'name' => 'Incident', + 'value' => 'Incident', + 'children' => [ + { + 'name' => 'Hardware', + 'value' => 'Incident::Hardware', + 'children' => [ + { + 'name' => 'Monitor', + 'value' => 'Incident::Hardware::Monitor' + }, + { + 'name' => 'Mouse', + 'value' => 'Incident::Hardware::Mouse' + }, + { + 'name' => 'Keyboard', + 'value' => 'Incident::Hardware::Keyboard' + } + ] + }, + { + 'name' => 'Softwareproblem', + 'value' => 'Incident::Softwareproblem', + 'children' => [ + { + 'name' => 'CRM', + 'value' => 'Incident::Softwareproblem::CRM' + }, + { + 'name' => 'EDI', + 'value' => 'Incident::Softwareproblem::EDI' + }, + { + 'name' => 'SAP', + 'value' => 'Incident::Softwareproblem::SAP', + 'children' => [ + { + 'name' => 'Authentication', + 'value' => 'Incident::Softwareproblem::SAP::Authentication' + }, + { + 'name' => 'Not reachable', + 'value' => 'Incident::Softwareproblem::SAP::Not reachable' + } + ] + }, + { + 'name' => 'MS Office', + 'value' => 'Incident::Softwareproblem::MS Office', + 'children' => [ + { + 'name' => 'Excel', + 'value' => 'Incident::Softwareproblem::MS Office::Excel' + }, + { + 'name' => 'PowerPoint', + 'value' => 'Incident::Softwareproblem::MS Office::PowerPoint' + }, + { + 'name' => 'Word', + 'value' => 'Incident::Softwareproblem::MS Office::Word' + }, + { + 'name' => 'Outlook', + 'value' => 'Incident::Softwareproblem::MS Office::Outlook' + } + ] + } + ] + } + ] + }, + { + 'name' => 'Service request', + 'value' => 'Service request', + 'children' => [ + { + 'name' => 'New software requirement', + 'value' => 'Service request::New software requirement' + }, + { + 'name' => 'New hardware', + 'value' => 'Service request::New hardware' + }, + { + 'name' => 'Consulting', + 'value' => 'Service request::Consulting' + } + ] + }, + { + 'name' => 'Change request', + 'value' => 'Change request' + } + ] + + created_attribute = ObjectManager::Attribute.last + assert_equal(correct_options, created_attribute.data_option[:options]) + + watch_for( + css: '.content.active', + value: 'Database Update required', + ) + click(css: '.content.active .tab-pane.active div.js-execute') + watch_for( + css: '.modal', + value: 'restart', + ) + watch_for_disappear( + css: '.modal', + timeout: 120, + ) + sleep 5 + watch_for( + css: '.content.active', + ) + + # discard new attribute + click(css: 'a[href="#manage"]') + click(css: 'a[href="#system/object_manager"]') + watch_for( + css: '.content.active table', + value: 'browser_test_tree_select1', + ) + match_not( + css: '.content.active', + value: 'Database Update required', + ) + object_manager_attribute_delete( + data: { + name: 'browser_test_tree_select1', + }, + ) + watch_for( + css: '.content.active', + value: 'Database Update required', + ) + watch_for( + css: '.content.active table', + value: 'browser_test_tree_select1', + ) + click(css: '.content.active .tab-pane.active div.js-execute') + watch_for( + css: '.modal', + value: 'restart', + ) + watch_for_disappear( + css: '.modal', + timeout: 120, + ) + sleep 5 + watch_for( + css: '.content.active', + ) + match_not( + css: '.content.active', + value: 'Database Update required', + ) + match_not( + css: '.content.active table', + value: 'browser_test_tree_select1', + ) + end + +end diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index ccbdec8f4..edb20d00d 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -3362,6 +3362,11 @@ wait untill text in selector disabppears 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 @@ -3695,4 +3700,67 @@ wait untill text in selector disabppears return if params[:mute_log] puts "#{Time.zone.now}/#{method}: #{params.inspect}" end + + private + + def add_tree_options(instance:, options:) + + # first level entries have to get added in regular order + options.each_key.with_index do |option, index| + + if index != 0 + element = instance.find_elements(css: '.modal .js-treeTable .js-addRow')[index - 1] + element.click + end + + element = instance.find_elements(css: '.modal .js-treeTable .js-key')[index] + element.clear + element.send_keys(option) + end + + add_sub_tree_recursion( + instance: instance, + options: options, + ) + end + + def add_sub_tree_recursion(instance:, options:, offset: 0) + options.each_value.inject(offset) do |child_offset, children| + + child_offset += 1 + + # put your recursion glasses on 8-) + add_sub_tree_options( + instance: instance, + options: children, + offset: child_offset, + ) + end + end + + def add_sub_tree_options(instance:, options:, offset:) + + # sub level entries have to get added in reversed order + level_options = options.to_a.reverse.to_h.keys + + level_options.each do |option| + + # sub level entries have to get added via 'add child row' link + click_index = offset - 1 + + element = instance.find_elements(css: '.modal .js-treeTable .js-addChild')[click_index] + element.click + + element = instance.find_elements(css: '.modal .js-treeTable .js-key')[offset] + element.clear + element.send_keys(option) + sleep 0.25 + end + + add_sub_tree_recursion( + instance: instance, + options: options, + offset: offset, + ) + end end