From e53a5de918c1db1f409df8d72a0cb5f38b4c7671 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Wed, 23 Jan 2013 17:44:28 +0000 Subject: [PATCH] Adds "sorted" scope to Principal and User and sort users/groups properly. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11259 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/watchers_controller.rb | 2 +- app/helpers/application_helper.rb | 2 +- app/helpers/groups_helper.rb | 2 +- app/helpers/members_helper.rb | 2 +- app/models/principal.rb | 10 ++++++++++ app/models/user.rb | 1 + test/unit/principal_test.rb | 14 ++++++++++++++ test/unit/user_test.rb | 4 ++++ 8 files changed, 33 insertions(+), 4 deletions(-) diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index d9275b965..e564d64ec 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -64,7 +64,7 @@ class WatchersController < ApplicationController end def autocomplete_for_user - @users = User.active.like(params[:q]).limit(100).all + @users = User.active.sorted.like(params[:q]).limit(100).all if @watched @users -= @watched.watcher_users end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 90c0072a9..08141a25c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -316,7 +316,7 @@ module ApplicationHelper def principals_check_box_tags(name, principals) s = '' - principals.sort.each do |principal| + principals.each do |principal| s << "\n" end s.html_safe diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 5e6b1f7aa..aa58eb575 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -26,7 +26,7 @@ module GroupsHelper end def render_principals_for_new_group_users(group) - scope = User.active.not_in_group(group).like(params[:q]) + scope = User.active.sorted.not_in_group(group).like(params[:q]) principal_count = scope.count principal_pages = Redmine::Pagination::Paginator.new principal_count, 100, params['page'] principals = scope.offset(principal_pages.offset).limit(principal_pages.per_page).all diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index eb06a3977..afb90f3df 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -19,7 +19,7 @@ module MembersHelper def render_principals_for_new_members(project) - scope = Principal.active.not_member_of(project).like(params[:q]).order('type, login, lastname ASC') + scope = Principal.active.sorted.not_member_of(project).like(params[:q]) principal_count = scope.count principal_pages = Redmine::Pagination::Paginator.new principal_count, 100, params['page'] principals = scope.offset(principal_pages.offset).limit(principal_pages.per_page).all diff --git a/app/models/principal.rb b/app/models/principal.rb index 921677647..97babba1c 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -70,6 +70,7 @@ class Principal < ActiveRecord::Base where("#{Principal.table_name}.id NOT IN (SELECT DISTINCT user_id FROM #{Member.table_name} WHERE project_id IN (?))", ids) end } + scope :sorted, lambda { order(*Principal.fields_for_order_statement)} before_create :set_default_empty_values @@ -88,6 +89,15 @@ class Principal < ActiveRecord::Base end end + # Returns an array of fields names than can be used to make an order statement for principals. + # Users are sorted before Groups. + # Examples: + def self.fields_for_order_statement(table=nil) + table ||= table_name + columns = ['type DESC'] + (User.name_formatter[:order] - ['id']) + ['lastname', 'id'] + columns.uniq.map {|field| "#{table}.#{field}"} + end + protected # Make sure we don't try to insert NULL values (see #4632) diff --git a/app/models/user.rb b/app/models/user.rb index e4b54dc41..a4e1fe47e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -114,6 +114,7 @@ class User < Principal group_id = group.is_a?(Group) ? group.id : group.to_i where("#{User.table_name}.id NOT IN (SELECT gu.user_id FROM #{table_name_prefix}groups_users#{table_name_suffix} gu WHERE gu.group_id = ?)", group_id) } + scope :sorted, lambda { order(*User.fields_for_order_statement)} def set_mail_notification self.mail_notification = Setting.default_notification_option if self.mail_notification.blank? diff --git a/test/unit/principal_test.rb b/test/unit/principal_test.rb index b96e786e9..8cd511ea2 100644 --- a/test/unit/principal_test.rb +++ b/test/unit/principal_test.rb @@ -49,6 +49,20 @@ class PrincipalTest < ActiveSupport::TestCase assert_equal [], Principal.not_member_of([]).sort end + def test_sorted_scope_should_sort_users_before_groups + scope = Principal.where("type <> ?", 'AnonymousUser') + expected_order = scope.all.sort do |a, b| + if a.is_a?(User) && b.is_a?(Group) + -1 + elsif a.is_a?(Group) && b.is_a?(User) + 1 + else + a.name.downcase <=> b.name.downcase + end + end + assert_equal expected_order.map(&:name).map(&:downcase), scope.sorted.all.map(&:name).map(&:downcase) + end + context "#like" do setup do Principal.create!(:login => 'login') diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index bf74dd6da..c0e51d469 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -34,6 +34,10 @@ class UserTest < ActiveSupport::TestCase @dlopper = User.find(3) end + def test_sorted_scope_should_sort_user_by_display_name + assert_equal User.all.map(&:name).map(&:downcase).sort, User.sorted.all.map(&:name).map(&:downcase) + end + def test_generate User.generate!(:firstname => 'Testing connection') User.generate!(:firstname => 'Testing connection') -- 2.39.5