Fixes #3494 - Status code 500 when trying to sort by groups in text module management

app/assets/javascripts/app/controllers/_application_controller/table.coffee

@@ -638,6 +638,7 @@ class App.ControllerTable extends App.Controller
         align:        'right'
         parentClass:  'noTruncate no-padding'
         unresizable:  true
+        unsortable:   true
 
       @bindCol['action'] =
         events:
@@ -707,7 +708,9 @@ class App.ControllerTable extends App.Controller
         list
         (item) ->
           res = iteratee(item)
-          return res if res
+          # Checking for a falsey value would overide 0 or false to placeholder.
+          # UnderscoreJS sorter breaks when \uFFFF is sorted together with non-string values.
+          return res if res != null and res != undefined and res != ''
           # null values are considered lexicographically "last"
           '\uFFFF'
       )

app/assets/javascripts/app/models/macro.coffee

@@ -12,7 +12,7 @@ class App.Macro extends App.Model
     },
     { name: 'updated_at',      display: 'Updated',  tag: 'datetime',      readonly: 1 },
     { name: 'note',            display: 'Note',     tag: 'textarea',      limit:   250,      null: true },
-    { name: 'group_ids',       display: 'Groups',   tag: 'column_select', relation: 'Group', null: true },
+    { name: 'group_ids',       display: 'Groups',   tag: 'column_select', relation: 'Group', null: true, unsortable: true },
     { name: 'active',          display: 'Active',   tag: 'active',        default: true },
   ]
   @configure_delete = true

app/assets/javascripts/app/models/text_module.coffee

@@ -24,7 +24,7 @@ class App.TextModule extends App.Model
       }
     ], note: 'To select placeholders from a list, just enter "::".'},
     { name: 'updated_at', display: 'Updated', tag: 'datetime', readonly: 1 },
-    { name: 'group_ids',  display: 'Groups',  tag: 'column_select', relation: 'Group', null: true },
+    { name: 'group_ids',  display: 'Groups',  tag: 'column_select', relation: 'Group', null: true, unsortable: true },
     { name: 'active',     display: 'Active',  tag: 'active',   default: true },
   ]
   @configure_delete = true

app/assets/javascripts/app/views/generic/table.jst.eco

@@ -20,7 +20,7 @@
     <% end %>
     <% for header, i in @headers: %>
       <th class="js-tableHead<% if header.className: %> <%= header.className %><% end %><% if header.align: %> align-<%= header.align %><% end %>" style="<% if header.displayWidth: %>width:<%= header.displayWidth %>px<% end %>" data-column-key="<%= header.name %>">
-        <div class="table-column-head<%= ' table-column-head-unclickable' if @sortable %><%= ' js-sort' if @tableId and !@sortable %>">
+        <div class="table-column-head<%= ' table-column-head-unclickable' if @sortable or header.unsortable %><%= ' js-sort' if @tableId and !@sortable and !header.unsortable %>">
           <div class="table-column-title"><%- @T(header.display) %></div>
           <% if !@sortable: %>
             <div class="table-column-sortIcon"><% if header.sortOrderIcon: %><%- @Icon(header.sortOrderIcon[0], header.sortOrderIcon[1]) %><% end %></div>

public/assets/tests/table.js

@@ -780,7 +780,8 @@ test('table test 6/7', function() {
   data = [
     { name: 'a item', data: 'g data', active: true },
     { name: 'b item', data: 'b data', active: false },
-    { name: 'c item', data: 'z data', active: true },
+    { name: 'c item', data: 'z data', active: false },
+    { name: 'd item', data: '', active: false },
   ]
 
   new App.ControllerTable({
@@ -788,7 +789,7 @@ test('table test 6/7', function() {
     el:       el_head_sortable,
     overview: ['name', 'data', 'active'],
     attribute_list: [
-      { name: 'name',     display: 'Name',      type: 'text', style: 'width: 10%' },
+      { name: 'name',     display: 'Name',      type: 'text', style: 'width: 10%', unsortable: true },
       { name: 'data',     display: 'Data',      type: 'text' },
       { name: 'active',   display: 'Active',    type: 'text' },
     ],
@@ -800,7 +801,7 @@ test('table test 6/7', function() {
     el:       el_not_head_sortable,
     overview: ['name', 'data', 'active'],
     attribute_list: [
-      { name: 'name',     display: 'Name',      type: 'text', style: 'width: 10%' },
+      { name: 'name',     display: 'Name',      type: 'text', style: 'width: 10%', unsortable: true },
       { name: 'data',     display: 'Data',      type: 'text' },
       { name: 'active',   display: 'Active',    type: 'text' },
     ],
@@ -811,9 +812,33 @@ test('table test 6/7', function() {
   equal(el_head_sortable.find('tbody > tr:nth-child(1) > td:first').text().trim(), 'a item', 'check row 1')
   equal(el_not_head_sortable.find('tbody > tr:nth-child(1) > td:nth-child(2)').text().trim(), 'a item', 'check row 1')
 
-  el_head_sortable.find('table > thead > tr > th:nth-child(2) > .js-sort').click()
+  ok(_.isEqual(list_items(el_head_sortable, 1), ['a item', 'b item', 'c item', 'd item']), 'sortable table is rendered correctly')
-  el_not_head_sortable.find('table > thead > tr > th:nth-child(3) > .js-sort').click()
+  ok(_.isEqual(list_items(el_not_head_sortable, 2), ['a item', 'b item', 'c item', 'd item']), 'unsortable table is rendered correctly')
 
-  equal(el_head_sortable.find('tbody > tr:nth-child(1) > td:first').text().trim(), 'b item', 'check row 1')
+  click_sort(el_head_sortable, 2)
-  equal(el_not_head_sortable.find('tbody > tr:nth-child(1) > td:nth-child(2)').text().trim(), 'a item', 'check row 1')
+  click_sort(el_not_head_sortable, 3)
+
+  ok(_.isEqual(list_items(el_head_sortable, 1), ['b item', 'a item', 'c item', 'd item']), 'sortable table is sorted')
+  ok(_.isEqual(list_items(el_not_head_sortable, 2), ['a item', 'b item', 'c item', 'd item']), 'unsortable table is not sorted')
+
+  click_sort(el_head_sortable, 3)
+  ok(_.isEqual(list_items(el_head_sortable, 1), ['b item', 'c item', 'd item', 'a item']), 'sorting by boolean column works')
+
+  click_sort(el_head_sortable, 1)
+  ok(_.isEqual(list_items(el_head_sortable, 1), ['b item', 'c item', 'd item', 'a item']), 'sorting on name column is disabled')
 });
+
+function click_sort(table, column_number) {
+  table
+    .find(`table > thead > tr > th:nth-child(${column_number}) > .js-sort`)
+    .click()
+}
+
+function list_items(table, column_number) {
+  return table
+    .find(`tbody > tr > td:nth-child(${column_number})`)
+    .text()
+    .split("\n")
+    .filter(elem => elem.match(/[\w]+/))
+    .map(elem => elem.trim())
+}