diff --git a/app/models/group_workload.rb b/app/models/group_workload.rb index d0d9332..7d2cb08 100644 --- a/app/models/group_workload.rb +++ b/app/models/group_workload.rb @@ -5,6 +5,8 @@ # the group user dummy. # class GroupWorkload + include RedmineWorkload::WlUserSorting + attr_reader :time_span, :user_workload ## @@ -52,11 +54,12 @@ def define_group_members end ## - # Sorting of users lastname and their class name in order to ensure that the - # GroupUserDummy will come first. + # Sorting by the class name first ensures that the GroupUserDummy comes + # before the real users. Within a class the name is used as rendered by + # Redmine, see RedmineWorkload::WlUserSorting. # def sorted_user_workload - user_workload_with_availabilities.sort_by { |user, _data| [user.class.name, user.lastname] }.to_h + user_workload_with_availabilities.sort_by { |user, _data| [user.class.name, *user_sort_key(user)] }.to_h end ## diff --git a/app/models/wl_user_selection.rb b/app/models/wl_user_selection.rb index dc11a42..93fa9f0 100644 --- a/app/models/wl_user_selection.rb +++ b/app/models/wl_user_selection.rb @@ -4,6 +4,8 @@ # Presenter organising users to be used in views/workloads/_filers.erb. # class WlUserSelection + include RedmineWorkload::WlUserSorting + attr_reader :groups ## @@ -36,7 +38,7 @@ def selected # Prepares users to be used in filters # @return [Array(User)] An array of user objects. def allowed_to_display - users_allowed_to_display.sort_by(&:lastname) + users_allowed_to_display.sort_by { |user| user_sort_key(user) } end def all_user_ids diff --git a/lib/redmine_workload.rb b/lib/redmine_workload.rb index 9f8265d..692edbc 100644 --- a/lib/redmine_workload.rb +++ b/lib/redmine_workload.rb @@ -11,3 +11,4 @@ require File.expand_path('redmine_workload/wl_issue_state', __dir__) require File.expand_path('redmine_workload/wl_user_data_finder', __dir__) require File.expand_path('redmine_workload/wl_user_data_defaults', __dir__) +require File.expand_path('redmine_workload/wl_user_sorting', __dir__) diff --git a/lib/redmine_workload/wl_user_sorting.rb b/lib/redmine_workload/wl_user_sorting.rb new file mode 100644 index 0000000..65b2899 --- /dev/null +++ b/lib/redmine_workload/wl_user_sorting.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module RedmineWorkload + ## + # Ordering of users and group dummies in the workload views. + # + # Redmine renders a user's name according to the 'Users display format' + # setting, so the list has to be ordered by that name rather than by the + # last name. Otherwise an installation using 'Firstname Lastname' shows an + # apparently unsorted list. + # + module WlUserSorting + ## + # @param user [User|GroupUserDummy] Object responding to name and id. + # @return [Array] Sort key following Setting.user_format. + # + def user_sort_key(user) + [user.name.to_s.downcase, user.id.to_i] + end + end +end diff --git a/test/unit/user_selection_test.rb b/test/unit/user_selection_test.rb index cb40dc2..03ab10f 100644 --- a/test/unit/user_selection_test.rb +++ b/test/unit/user_selection_test.rb @@ -43,6 +43,14 @@ def all_group_member_ids (@fixture_group_member_ids + @group_member_ids.flatten).uniq.sort end + ## + # Ids in the order WlUserSelection would render them. + # + def displayed_ids(current_user) + groups = WlGroupSelection.new(user: current_user, groups: [@group1.id, @group2.id, @group3.id]) + WlUserSelection.new(user: current_user, group_selection: groups).allowed_to_display.map(&:id) + end + test 'should return all users if the current user is admin' do current_user = User.generate!(admin: true) groups = WlGroupSelection.new(user: current_user, groups: [@group1.id, @group2.id, @group3.id]) @@ -80,6 +88,24 @@ def all_group_member_ids assert_equal expected, current end + test 'should order users by the display name format' do + current_user = User.generate!(admin: true) + zoe_adams = User.generate!(firstname: 'Zoe', lastname: 'Adams') + adam_zimmer = User.generate!(firstname: 'Adam', lastname: 'Zimmer') + [zoe_adams, adam_zimmer].each { |user| user.groups << @group1 } + ids = [zoe_adams.id, adam_zimmer.id] + + # A new selection per block, otherwise User#name serves its memoized value + # from before the setting changed. + with_settings user_format: 'lastname_comma_firstname' do + assert_equal [zoe_adams.id, adam_zimmer.id], displayed_ids(current_user) & ids + end + + with_settings user_format: 'firstname_lastname' do + assert_equal [adam_zimmer.id, zoe_adams.id], displayed_ids(current_user) & ids + end + end + test 'should return the current user if allowed to :view_own_workloads' do current_user = users :users_002 # jsmith manager = roles :roles_001 # manager