From b4bbe26313d85a3498b2fb52d8750fcfa5bee0b4 Mon Sep 17 00:00:00 2001 From: Billy Zhou Date: Wed, 18 Jul 2018 15:24:18 +0800 Subject: [PATCH] Fixed #1564 by using proper localeCompare for string comparisons --- .../_application_ui_element.coffee | 8 +- .../object_manager_attribute.coffee | 4 + .../object_manager/attribute/select.jst.eco | 4 +- test/browser/admin_object_manager_test.rb | 77 +++++++++++++++++++ 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_ui_element/_application_ui_element.coffee b/app/assets/javascripts/app/controllers/_ui_element/_application_ui_element.coffee index cc272b47f..546d3e29b 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/_application_ui_element.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/_application_ui_element.coffee @@ -52,11 +52,9 @@ class App.UiElement.ApplicationUiElement row.name = App.i18n.translateInline(row.name) attribute.options.push row else - order = _.sortBy( - _.keys(selection), (item) -> - return '' if !selection[item] || !selection[item].toString - selection[item].toString().toLowerCase() - ) + forceString = (s) -> + return if !selection[s] || !selection[s].toString then '' else selection[s].toString() + order = _.keys(selection).sort( (a, b) -> forceString(a).localeCompare(forceString(b)) ) for key in order name_new = selection[key] if attribute.translate diff --git a/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee b/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee index 71fd11be6..99ae033f6 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee @@ -7,6 +7,10 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi if params.data_option_new && !_.isEmpty(params.data_option_new) params.data_option = params.data_option_new + if attribute.value == 'select' && params.data_option? && params.data_option.options? + sorted = _.map params.data_option.options, (value, key) -> [key.toString(), value.toString()] + params.data_option.sorted = sorted.sort( (a, b) -> a[1].localeCompare(b[1]) ) + item = $(App.view('object_manager/attribute')(attribute: attribute)) updateDataMap = (localParams, localAttribute, localAttributes, localClassname, localForm, localA) => diff --git a/app/assets/javascripts/app/views/object_manager/attribute/select.jst.eco b/app/assets/javascripts/app/views/object_manager/attribute/select.jst.eco index 448859052..3328f631e 100644 --- a/app/assets/javascripts/app/views/object_manager/attribute/select.jst.eco +++ b/app/assets/javascripts/app/views/object_manager/attribute/select.jst.eco @@ -8,8 +8,8 @@ <%- @T('Action') %> - <% if @params.data_option && @params.data_option.options: %> - <% for key, display of @params.data_option.options: %> + <% if @params.data_option && @params.data_option.sorted: %> + <% for [key, display] in @params.data_option.sorted: %> diff --git a/test/browser/admin_object_manager_test.rb b/test/browser/admin_object_manager_test.rb index d979a4b11..450196352 100644 --- a/test/browser/admin_object_manager_test.rb +++ b/test/browser/admin_object_manager_test.rb @@ -594,4 +594,81 @@ class AdminObjectManagerTest < TestCase assert(undeletable_attribute_html.include?('cannot be deleted')) assert(undeletable_attribute_html.exclude?('href="#"')) end + + def test_proper_sorting_of_select_attributes + @browser = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + tasks_close_all() + + # lexicographically ordered list of option strings + options = %w[0 000.000 1 100.100 100.200 2 200.100 200.200 3 ä b n ö p sr ß st t ü v] + options_hash = Hash[options.reverse.collect { |o| [o, o] }] + + object_manager_attribute_create( + data: { + name: 'select_attributes_sorting_test', + display: 'Select Attributes Sorting Test', + data_type: 'Select', + data_option: { options: options_hash }, + }, + ) + sleep 2 + + # open the select attribute that we just created + execute(js: "$(\".content.active td:contains('select_attributes_sorting_test')\").first().click()") + sleep 3 + + unsorted_locations = options.map do |key| + [get_location(xpath: "//input[@value='#{key}']").y, key] + end + log("unsorted_locations = #{unsorted_locations.inspect}") + sorted_locations = unsorted_locations.sort_by(&:first).map(&:second) + log("sorted_locations = #{sorted_locations.inspect}") + assert_equal options, sorted_locations + + # close the attribute modal + click(css: '.modal button.js-submit') + + watch_for( + css: '.content.active', + value: 'Database Update required', + ) + watch_for( + css: '.content.active table', + value: 'select_attributes_sorting_test', + ) + + click(css: '.content.active .tab-pane.active div.js-execute') + watch_for( + css: '.modal', + value: 'restart', + ) + watch_for_disappear( + css: '.modal', + timeout: 7.minutes, + ) + sleep 5 + watch_for( + css: '.content.active', + ) + + # create a new ticket and check whether the select attributes are correctly sorted or not + click( + css: 'a[href="#ticket/create"]', + mute_log: true, + ) + + watch_for( + css: 'select[name="select_attributes_sorting_test"]', + ) + + select_element = @browser.find_elements(css: 'select[name="select_attributes_sorting_test"]')[0] + unsorted_options = select_element.find_elements(xpath: './*').map(&:text).reject { |x| x == '-' } + log unsorted_options.inspect + assert_equal options, unsorted_options + end end