Make employees and absences evaluator tables sortable (65435)

app/controllers/evaluator_controller.rb

@@ -132,7 +132,9 @@ def set_management_evaluation
     @evaluation =
       case params[:evaluation].downcase
       when 'clients'                   then Evaluations::ClientsEval.new
-      when 'employees'                 then Evaluations::EmployeesEval.new(params.slice(:department_id, :member_coach_id))
+      when 'employees'                 then Evaluations::EmployeesEval.new(
+        params.slice(:department_id, :member_coach_id), sort_conditions
+      )
       when 'departments'               then Evaluations::DepartmentsEval.new
       when 'clientworkitems'           then Evaluations::ClientWorkItemsEval.new(params[:category_id])
       when 'employeeworkitems'         then Evaluations::EmployeeWorkItemsEval.new(params[:category_id])

app/domain/evaluations/absences_eval.rb

@@ -9,13 +9,16 @@ module Evaluations
   class AbsencesEval < Evaluations::Evaluation
     include EvaluatorHelper
     include FormatHelper
+    include Sortable
 
     self.sub_evaluation   = 'employeeabsences'
     self.division_column  = :employee_id
     self.label            = 'Members Absenzen'
     self.absences         = true
     self.detail_columns   = detail_columns.reject { |i| i == :billable }
     self.detail_labels    = detail_labels.merge(account: 'Absenz')
+    self.sortable_division_header = true
+    self.sortable_period_header   = true
 
     attr_reader :department_id, :sort_conditions
 
@@ -26,12 +29,11 @@ def initialize(department_id = nil, sort_conditions = nil, **worktime_search_con
     end
 
     def divisions(period = nil, times = nil)
-      employees_with_absences(period, times).map do |e|
-        unformatted_vacations = remaining_vacations(e, format: false)
-        e.remaining_vacations = format_days(unformatted_vacations)
-        e.sort_col = unformatted_vacations * descending
+      employees = employees_with_absences(period, times).map do |e|
+        e.remaining_vacations = format_days(vacations_value(e))
         e
-      end.sort_by(&:sort_col)
+      end
+      sort_divisions(employees, times)
     end
 
     def employees_with_absences(period, times)
@@ -67,8 +69,25 @@ def division_supplement(_user)
 
     private
 
-    def descending
-      sort_conditions && sort_conditions['sort_dir'] == 'desc' ? -1 : 1
+    def sort_divisions(employees, times)
+      case sort_conditions && sort_conditions['sort']
+      when 'name' then sort_by_name(employees)
+      when 'period_hours' then sort_by_value(employees) { |e| period_hours_value(e, times) }
+      else sort_by_value(employees) { |e| vacations_value(e) }
+      end
+    end
+
+    def sort_by_name(employees)
+      sorted = employees.sort_by { |e| [e.lastname, e.firstname] }
+      sort_direction == :desc ? sorted.reverse : sorted
+    end
+
+    def sort_by_value(employees)
+      employees.sort_by { |e| yield(e) * sort_multiplier }
+    end
+
+    def vacations_value(employee)
+      remaining_vacations(employee, format: false)
     end
   end
 end

app/domain/evaluations/employees_eval.rb

@@ -7,18 +7,25 @@
 
 module Evaluations
   class EmployeesEval < Evaluations::Evaluation
+    include Sortable
+
     self.division_column = 'employee_id'
     self.sub_evaluation   = 'employeeworkitems'
     self.label            = 'Member Zeit'
     self.total_details    = false
+    self.sortable_division_header = true
+    self.sortable_period_header   = true
+
+    attr_reader :sort_conditions
 
-    def initialize(emp_search_conditions = {})
+    def initialize(emp_search_conditions = {}, sort_conditions = nil)
       @department_id = emp_search_conditions[:department_id].to_i
       @member_coach_id = emp_search_conditions[:member_coach_id].to_i
+      @sort_conditions = sort_conditions
       super(Employee)
     end
 
-    def divisions(period = nil, _times = nil)
+    def divisions(period = nil, times = nil)
       employees = if period
                     Employee.list
                   else
@@ -27,7 +34,7 @@ def divisions(period = nil, _times = nil)
       employees = employees.where(member_coach_id: @member_coach_id) unless @member_coach_id.zero?
       employees = employees.where(department_id: @department_id) unless @department_id.zero?
 
-      employees
+      sort_divisions(employees, period, times)
     end
 
     def employee_id
@@ -52,12 +59,42 @@ def sum_total_times(period = nil)
 
     def division_supplement(_user)
       {
-        overtime: { title: 'Überstunden', align: 'right' },
-        vacations: { title: 'Ferien', align: 'right' },
+        overtime: { title: 'Überstunden', align: 'right', sortable: true },
+        vacations: { title: 'Ferien', align: 'right', sortable: true },
         overtime_vacations_tooltip: {},
-        worktime_commits: { title: 'Freigabe' },
-        worktime_reviews: { title: 'Kontrolle' }
+        worktime_commits: { title: 'Freigabe', sortable: true },
+        worktime_reviews: { title: 'Kontrolle', sortable: true }
       }
     end
+
+    private
+
+    def sort_divisions(employees, period, times)
+      case sort_conditions && sort_conditions['sort']
+      when 'name' then employees.reorder(lastname: sort_direction, firstname: sort_direction)
+      when 'worktime_commits' then employees.reorder(committed_worktimes_at: sort_direction)
+      when 'worktime_reviews' then employees.reorder(reviewed_worktimes_at: sort_direction)
+      when 'overtime' then sort_by_value(employees) { |e| overtime_value(e, period) }
+      when 'vacations' then sort_by_value(employees) { |e| vacations_value(e, period) }
+      when 'period_hours' then sort_by_value(employees) { |e| period_hours_value(e, times) }
+      else employees
+      end
+    end
+
+    # Built as a fresh relation rather than chained onto `employees`, since the latter may
+    # carry a `distinct` and Postgres rejects an in_order_of ORDER BY on that.
+    def sort_by_value(employees)
+      ranked_ids = employees.sort_by { |e| yield(e) * sort_multiplier }.map(&:id)
+      Employee.where(id: ranked_ids).in_order_of(:id, ranked_ids)
+    end
+
+    def overtime_value(employee, period)
+      period ? employee.statistics.overtime(period) : employee.statistics.current_overtime
+    end
+
+    def vacations_value(employee, period)
+      date = period&.end_date || Time.zone.today
+      employee.statistics.remaining_vacations(date.end_of_year)
+    end
   end
 end

app/domain/evaluations/evaluation.rb

@@ -16,7 +16,14 @@ class Evaluation
     class_attribute :division_method, :division_column, :division_join, :division_planning_join,
                     :sub_evaluation, :sub_work_items_eval, :label, :absences,
                     :total_details, :billable_hours, :planned_hours, :category_ref, :detail_columns,
-                    :detail_labels
+                    :detail_labels, :sortable_division_header, :sortable_period_header
+
+    # Whether the division header column can be sorted.
+    self.sortable_division_header = false
+
+    # Whether the period column can be sorted. Only offered for a single selected period,
+    # since with several period columns there'd be no single value to sort by.
+    self.sortable_period_header = false
 
     # The method to send to the category object to retrieve a list of divisions.
     self.division_method   = :list
@@ -176,12 +183,7 @@ def header_info_labels_custom
     # The header name of the division column to be displayed.
     # Returns the class name of the division objects.
     def division_header
-      divs = divisions
-      if divs.respond_to?(:klass)
-        divs.klass.model_name.human
-      else
-        respond_to?(:division_header) ? division_header : ''
-      end
+      divisions.klass.model_name.human
     end
 
     # Returns a two-dimensional Array with helper methods of the evaluator

app/domain/evaluations/sortable.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+#  Copyright (c) 2006-2017, Puzzle ITC GmbH. This file is part of
+#  PuzzleTime and licensed under the Affero General Public License version 3
+#  or later. See the COPYING file at the top-level directory or at
+#  https://github.com/puzzle/puzzletime.
+
+module Evaluations
+  # Shared sort_dir handling for Evaluations backed by a sort_conditions hash
+  # ({'sort' => ..., 'sort_dir' => ...}), e.g. from EvaluatorController#sort_conditions.
+  module Sortable
+    def sort_direction
+      sort_conditions && sort_conditions['sort_dir'] == 'desc' ? :desc : :asc
+    end
+
+    def sort_multiplier
+      sort_direction == :desc ? -1 : 1
+    end
+
+    private
+
+    # times has exactly one entry: this sort is only offered for a single period.
+    def period_hours_value(employee, times)
+      times&.first&.dig(employee.id).to_f
+    end
+  end
+end

app/models/employee.rb

@@ -122,7 +122,7 @@ class Employee < ApplicationRecord
                                    }
   scope :active_employed_current_month, -> { joins(:employments).merge(Employment.active.during(Period.current_month)) }
 
-  attr_accessor :remaining_vacations, :sort_col # used for multiabsence and not persisted
+  attr_accessor :remaining_vacations # used for multiabsence and not persisted
 
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable

app/views/evaluator/_category.html.haml

@@ -14,16 +14,20 @@
 %table#evaluation.table.table-striped.table-condensed.table-hover.table-inline
   %thead
     %tr
-      %th= evaluation.division_header
+      %th
+        = evaluation.class.sortable_division_header ? sort_link(:name, evaluation.division_header) : evaluation.division_header
       -if evaluation.sub_work_items_eval
         %th
       - @periods.each do |p|
         %th.periodHead{colspan: evaluation.class.planned_hours ? 3 : nil}
-          = p ? p.label : 'Total'
+          - if evaluation.class.sortable_period_header && @period
+            = sort_link(:period_hours, p ? p.label : 'Total')
+          - else
+            = p ? p.label : 'Total'
              
-      - evaluation.division_supplement(@user).each do |_key, supp|
+      - evaluation.division_supplement(@user).each do |key, supp|
         %th
-          = supp[:sortable] ? sort_link(:vacation, supp[:title]) : supp[:title]
+          = supp[:sortable] ? sort_link(key, supp[:title]) : supp[:title]
     - if evaluation.class.planned_hours
       %tr
         %th.subhead

test/domain/evaluations/absences_eval_test.rb

@@ -84,6 +84,35 @@ def test_absences_detail_pascal
       assert_count_times 0, 1, 2, 2
     end
 
+    def test_absences_sorted_by_name
+      create_absences
+
+      ascending = Evaluations::AbsencesEval.new(nil, { 'sort' => 'name', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:mark), employees(:lucien), employees(:pascal)],
+                   ascending.divisions(@period_month, @times)
+
+      descending = Evaluations::AbsencesEval.new(nil, { 'sort' => 'name', 'sort_dir' => 'desc' })
+
+      assert_equal [employees(:pascal), employees(:lucien), employees(:mark)],
+                   descending.divisions(@period_month, @times)
+    end
+
+    def test_absences_sorted_by_period_hours
+      create_absences
+      times = [{ employees(:pascal).id => 3.0, employees(:lucien).id => 9.0, employees(:mark).id => 18.0 }]
+
+      ascending = Evaluations::AbsencesEval.new(nil, { 'sort' => 'period_hours', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:pascal), employees(:lucien), employees(:mark)],
+                   ascending.divisions(@period_month, times)
+
+      descending = Evaluations::AbsencesEval.new(nil, { 'sort' => 'period_hours', 'sort_dir' => 'desc' })
+
+      assert_equal [employees(:mark), employees(:lucien), employees(:pascal)],
+                   descending.divisions(@period_month, times)
+    end
+
     def create_absences
       %i[mark lucien pascal].each do |e|
         employees(e).employments.create!(start_date: @period_month.start_date,

test/domain/evaluations/employees_eval_test.rb

@@ -73,5 +73,98 @@ def test_employee_detail_pascal
       assert_sum_times 3, 3, 5, 6
       assert_count_times 1, 1, 2, 3
     end
+
+    def test_employees_sorted_by_worktime_commits
+      stub_employee_relation
+      employees(:pascal).update!(committed_worktimes_at: Date.new(2020, 6, 1))
+      employees(:mark).update!(committed_worktimes_at: Date.new(2021, 6, 1))
+      employees(:lucien).update!(committed_worktimes_at: Date.new(2022, 6, 1))
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'worktime_commits', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:pascal), employees(:mark), employees(:lucien)], ascending.divisions(@period_month)
+
+      descending = Evaluations::EmployeesEval.new({}, { 'sort' => 'worktime_commits', 'sort_dir' => 'desc' })
+
+      assert_equal [employees(:lucien), employees(:mark), employees(:pascal)], descending.divisions(@period_month)
+    end
+
+    def test_employees_sorted_by_worktime_reviews
+      stub_employee_relation
+      employees(:pascal).update!(reviewed_worktimes_at: Date.new(2020, 6, 1))
+      employees(:mark).update!(reviewed_worktimes_at: Date.new(2021, 6, 1))
+      employees(:lucien).update!(reviewed_worktimes_at: Date.new(2022, 6, 1))
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'worktime_reviews', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:pascal), employees(:mark), employees(:lucien)], ascending.divisions(@period_month)
+    end
+
+    def test_employees_sorted_by_overtime
+      stub_employee_objects
+      employees(:pascal).stubs(:statistics).returns(stub(overtime: 5.0))
+      employees(:mark).stubs(:statistics).returns(stub(overtime: -2.0))
+      employees(:lucien).stubs(:statistics).returns(stub(overtime: 10.0))
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'overtime', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:mark), employees(:pascal), employees(:lucien)], ascending.divisions(@period_month)
+
+      descending = Evaluations::EmployeesEval.new({}, { 'sort' => 'overtime', 'sort_dir' => 'desc' })
+
+      assert_equal [employees(:lucien), employees(:pascal), employees(:mark)], descending.divisions(@period_month)
+    end
+
+    def test_employees_sorted_by_vacations
+      stub_employee_objects
+      employees(:pascal).stubs(:statistics).returns(stub(remaining_vacations: 3.0))
+      employees(:mark).stubs(:statistics).returns(stub(remaining_vacations: 12.0))
+      employees(:lucien).stubs(:statistics).returns(stub(remaining_vacations: 7.0))
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'vacations', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:pascal), employees(:lucien), employees(:mark)], ascending.divisions(@period_month)
+    end
+
+    def test_employees_sorted_by_period_hours
+      stub_employee_relation
+      times = [{ employees(:pascal).id => 3.0, employees(:lucien).id => 9.0, employees(:mark).id => 18.0 }]
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'period_hours', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:pascal), employees(:lucien), employees(:mark)],
+                   ascending.divisions(@period_month, times)
+
+      descending = Evaluations::EmployeesEval.new({}, { 'sort' => 'period_hours', 'sort_dir' => 'desc' })
+
+      assert_equal [employees(:mark), employees(:lucien), employees(:pascal)],
+                   descending.divisions(@period_month, times)
+    end
+
+    def test_employees_sorted_by_period_hours_defaults_missing_employees_to_zero
+      stub_employee_relation
+      # lucien has no entry, i.e. no worktimes at all in the period
+      times = [{ employees(:pascal).id => 3.0, employees(:mark).id => 1.0 }]
+
+      ascending = Evaluations::EmployeesEval.new({}, { 'sort' => 'period_hours', 'sort_dir' => 'asc' })
+
+      assert_equal [employees(:lucien), employees(:mark), employees(:pascal)],
+                   ascending.divisions(@period_month, times)
+    end
+
+    private
+
+    # Restricts Employee.list to a real relation of exactly the 3 employees these
+    # tests care about, so a real SQL ORDER BY (reorder) can still run on it.
+    def stub_employee_relation
+      ids = [employees(:pascal).id, employees(:mark).id, employees(:lucien).id]
+      Employee.stubs(:list).returns(Employee.where(id: ids))
+    end
+
+    # Restricts Employee.list to the exact fixture instances, so a per-instance
+    # stub (e.g. on #statistics) set on them is still in effect when read back.
+    def stub_employee_objects
+      Employee.stubs(:list).returns([employees(:pascal), employees(:mark), employees(:lucien)])
+    end
   end
 end