From c4ccbc46ff753d50f2405f7ca62e37201ee9ca5c Mon Sep 17 00:00:00 2001
From: Romit Choudhary <rc@zammad.com>
Date: Thu, 7 Oct 2021 15:43:44 +0200
Subject: [PATCH] Fixes #3737 - Bug Report 4.1.x Overview Sort - Grouped by
 user

---
 .../_application_controller/table.coffee      | 21 +++++-
 public/assets/tests/table.js                  |  4 +-
 spec/system/overview_spec.rb                  | 67 +++++++++++++++++++
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/_application_controller/table.coffee b/app/assets/javascripts/app/controllers/_application_controller/table.coffee
index 64e809fe3..7304710a0 100644
--- a/app/assets/javascripts/app/controllers/_application_controller/table.coffee
+++ b/app/assets/javascripts/app/controllers/_application_controller/table.coffee
@@ -493,6 +493,23 @@ class App.ControllerTable extends App.Controller
       sortable:   @dndCallback
     ))
 
+  getGroupByKeyName: (object, groupBy) ->
+    reference_key = groupBy + '_id'
+
+    if reference_key of object
+      return reference_key
+
+    groupBy
+
+  sortObjectKeys: (objects, direction) ->
+    sorted = Object.keys(objects).sort()
+
+    switch direction
+      when 'DESC'
+        sorted.reverse()
+      else
+        sorted
+
   renderTableRows: (sort = false) =>
     if sort is true
       @sortList()
@@ -506,11 +523,11 @@ class App.ControllerTable extends App.Controller
     objectsToShow = @objectsOfPage(@pagerShownPage)
     if @groupBy
       # group by raw (and not printable) value so dates work also
-      objectsGrouped = _.groupBy(objectsToShow, (object) => object[@groupBy])
+      objectsGrouped = _.groupBy(objectsToShow, (object) => object[@getGroupByKeyName(object, @groupBy)])
     else
       objectsGrouped = { '': objectsToShow }
 
-    for groupValue in Object.keys(objectsGrouped).sort()
+    for groupValue in @sortObjectKeys(objectsGrouped, @groupDirection)
       groupObjects = objectsGrouped[groupValue]
 
       for object in groupObjects
diff --git a/public/assets/tests/table.js b/public/assets/tests/table.js
index 02ca59876..b7eac8557 100644
--- a/public/assets/tests/table.js
+++ b/public/assets/tests/table.js
@@ -370,8 +370,8 @@ test('table test', function() {
     groupBy:        'priority',
     groupDirection: 'DESC',
   })
-  equal(el.find('tbody > tr:nth-child(1) > td:nth-child(1)').text().trim(), '1 niedrig', 'check row 1')
-  equal(el.find('tbody > tr:nth-child(4) > td:nth-child(1)').text().trim(), '2 normal', 'check row 3')
+  equal(el.find('tbody > tr:nth-child(1) > td:nth-child(1)').text().trim(), '2 normal', 'check row 1')
+  equal(el.find('tbody > tr:nth-child(3) > td:nth-child(1)').text().trim(), '1 niedrig', 'check row 3')
 
   $('#table').append('<hr><h1>table Group By Direction ASC</h1><div id="table7"></div>')
   el = $('#table7')
diff --git a/spec/system/overview_spec.rb b/spec/system/overview_spec.rb
index 50402cfe8..763619ec2 100644
--- a/spec/system/overview_spec.rb
+++ b/spec/system/overview_spec.rb
@@ -65,4 +65,71 @@ RSpec.describe 'Overview', type: :system do
       end
     end
   end
+
+  context 'sorting when group by is set' do
+    let(:user) { create(:customer) }
+    let(:ticket1) { create(:ticket, group: Group.find_by(name: 'Users'), priority_id: 1, customer: user) }
+    let(:ticket2) { create(:ticket, group: Group.find_by(name: 'Users'), priority_id: 2, customer: user) }
+    let(:ticket3) { create(:ticket, group: Group.find_by(name: 'Users'), priority_id: 3, customer: user) }
+    let(:overview) do
+      create(:overview, group_by: 'priority', group_direction: group_direction, condition: {
+               'ticket.customer_id' => {
+                 operator: 'is',
+                 value:    user.id
+               }
+             })
+    end
+
+    before do
+      ticket1 && ticket2 && ticket3
+
+      visit "ticket/view/#{overview.link}"
+    end
+
+    context 'when group direction is default' do
+      let(:group_direction) { nil }
+
+      it 'sorts groups 1 > 3' do
+        within :active_content do
+          expect(find('.table-overview table tbody tr:first-child td:nth-child(1)').text).to match('1 low')
+          expect(find('.table-overview table tbody tr:nth-child(5) td:nth-child(1)').text).to match('3 high')
+        end
+      end
+
+      it 'does not show duplicates when any ticket attribute is updated using bulk update' do
+        find("tr[data-id='#{ticket3.id}']").check('bulk', allow_label_click: true)
+        select '2 normal', from: 'priority_id'
+
+        click '.js-confirm'
+        find('.js-confirm-step textarea').fill_in with: 'test tickets ordering'
+        click '.js-submit'
+
+        within :active_content do
+          expect(page).to have_css("tr[data-id='#{ticket3.id}']", count: 1)
+        end
+      end
+    end
+
+    context 'when group direction is ASC' do
+      let(:group_direction) { 'ASC' }
+
+      it 'sorts groups 1 > 3' do
+        within :active_content do
+          expect(find('.table-overview table tbody tr:first-child td:nth-child(1)').text).to match('1 low')
+          expect(find('.table-overview table tbody tr:nth-child(5) td:nth-child(1)').text).to match('3 high')
+        end
+      end
+    end
+
+    context 'when group direction is DESC' do
+      let(:group_direction) { 'DESC' }
+
+      it 'sorts groups 3 > 1' do
+        within :active_content do
+          expect(find('.table-overview table tbody tr:first-child td:nth-child(1)').text).to match('3 high')
+          expect(find('.table-overview table tbody tr:nth-child(5) td:nth-child(1)').text).to match('1 low')
+        end
+      end
+    end
+  end
 end