From 085fdfc9394b02943478c74896c7ed6ce6bb0676 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Mon, 10 May 2021 15:06:42 +0000 Subject: [PATCH] Fixes #3494 - Status code 500 when trying to sort by groups in text module management --- .../_application_controller/table.coffee | 5 ++- .../javascripts/app/models/macro.coffee | 2 +- .../javascripts/app/models/text_module.coffee | 2 +- .../app/views/generic/table.jst.eco | 2 +- public/assets/tests/table.js | 39 +++++++++++++++---- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_application_controller/table.coffee b/app/assets/javascripts/app/controllers/_application_controller/table.coffee index 9ca732ac2..5ee35e98b 100644 --- a/app/assets/javascripts/app/controllers/_application_controller/table.coffee +++ b/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' ) diff --git a/app/assets/javascripts/app/models/macro.coffee b/app/assets/javascripts/app/models/macro.coffee index aa3e4b1d7..ff3ecd180 100644 --- a/app/assets/javascripts/app/models/macro.coffee +++ b/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 diff --git a/app/assets/javascripts/app/models/text_module.coffee b/app/assets/javascripts/app/models/text_module.coffee index 02e32425b..9e9f6b350 100644 --- a/app/assets/javascripts/app/models/text_module.coffee +++ b/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 diff --git a/app/assets/javascripts/app/views/generic/table.jst.eco b/app/assets/javascripts/app/views/generic/table.jst.eco index 33ef145d1..6e5663bc6 100644 --- a/app/assets/javascripts/app/views/generic/table.jst.eco +++ b/app/assets/javascripts/app/views/generic/table.jst.eco @@ -20,7 +20,7 @@ <% end %> <% for header, i in @headers: %> -
+
<%- @T(header.display) %>
<% if !@sortable: %>
<% if header.sortOrderIcon: %><%- @Icon(header.sortOrderIcon[0], header.sortOrderIcon[1]) %><% end %>
diff --git a/public/assets/tests/table.js b/public/assets/tests/table.js index 4634aacbd..b7eac8557 100644 --- a/public/assets/tests/table.js +++ b/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() - el_not_head_sortable.find('table > thead > tr > th:nth-child(3) > .js-sort').click() + ok(_.isEqual(list_items(el_head_sortable, 1), ['a item', 'b item', 'c item', 'd item']), 'sortable table is rendered correctly') + 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') - 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_head_sortable, 2) + 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()) +}