Browse Source

Fixed #1564 by using proper localeCompare for string comparisons

Billy Zhou 6 years ago
parent
commit
b4bbe26313

+ 3 - 5
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

+ 4 - 0
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) =>

+ 2 - 2
app/assets/javascripts/app/views/object_manager/attribute/select.jst.eco

@@ -8,8 +8,8 @@
         <th style="width: 30px"><%- @T('Action') %>
     </thead>
     <tbody>
-      <% 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: %>
         <tr>
           <td class="settings-list-control-cell">
             <input class="form-control form-control--small js-key" type="text" value="<%= key %>" required/>

+ 77 - 0
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