From 5a1fcf826ffe4c4d6e0f839138d442725cae2ab5 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 26 Nov 2011 17:37:20 +0000 Subject: [PATCH] Sort the issue list by author/assignee according to user display format (#9669). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@7938 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/query.rb | 10 ++++-- app/models/user.rb | 33 +++++++++++++++----- test/functional/issues_controller_test.rb | 38 ++++++++++++++++++++--- test/test_helper.rb | 4 +-- test/unit/query_test.rb | 16 ++++++++++ test/unit/user_test.rb | 24 ++++++++++++++ 6 files changed, 107 insertions(+), 18 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 55ac3d6a4..f3b83d0f4 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -36,7 +36,11 @@ class QueryColumn # Returns true if the column is sortable, otherwise false def sortable? - !sortable.nil? + !@sortable.nil? + end + + def sortable + @sortable.is_a?(Proc) ? @sortable.call : @sortable end def value(issue) @@ -136,8 +140,8 @@ class Query < ActiveRecord::Base QueryColumn.new(:status, :sortable => "#{IssueStatus.table_name}.position", :groupable => true), QueryColumn.new(:priority, :sortable => "#{IssuePriority.table_name}.position", :default_order => 'desc', :groupable => true), QueryColumn.new(:subject, :sortable => "#{Issue.table_name}.subject"), - QueryColumn.new(:author, :sortable => ["authors.lastname", "authors.firstname", "authors.id"], :groupable => true), - QueryColumn.new(:assigned_to, :sortable => ["#{User.table_name}.lastname", "#{User.table_name}.firstname", "#{User.table_name}.id"], :groupable => true), + QueryColumn.new(:author, :sortable => lambda {User.fields_for_order_statement("authors")}, :groupable => true), + QueryColumn.new(:assigned_to, :sortable => lambda {User.fields_for_order_statement}, :groupable => true), QueryColumn.new(:updated_on, :sortable => "#{Issue.table_name}.updated_on", :default_order => 'desc'), QueryColumn.new(:category, :sortable => "#{IssueCategory.table_name}.name", :groupable => true), QueryColumn.new(:fixed_version, :sortable => ["#{Version.table_name}.effective_date", "#{Version.table_name}.name"], :default_order => 'desc', :groupable => true), diff --git a/app/models/user.rb b/app/models/user.rb index 0ac0dcb74..8268d752e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,12 +26,13 @@ class User < Principal STATUS_REGISTERED = 2 STATUS_LOCKED = 3 + # Different ways of displaying/sorting users USER_FORMATS = { - :firstname_lastname => '#{firstname} #{lastname}', - :firstname => '#{firstname}', - :lastname_firstname => '#{lastname} #{firstname}', - :lastname_coma_firstname => '#{lastname}, #{firstname}', - :username => '#{login}' + :firstname_lastname => {:string => '#{firstname} #{lastname}', :order => %w(firstname lastname id)}, + :firstname => {:string => '#{firstname}', :order => %w(firstname id)}, + :lastname_firstname => {:string => '#{lastname} #{firstname}', :order => %w(lastname firstname id)}, + :lastname_coma_firstname => {:string => '#{lastname}, #{firstname}', :order => %w(lastname firstname id)}, + :username => {:string => '#{login}', :order => %w(login id)}, } MAIL_NOTIFICATION_OPTIONS = [ @@ -168,13 +169,29 @@ class User < Principal end end end - + + def self.name_formatter(formatter = nil) + USER_FORMATS[formatter || Setting.user_format] || USER_FORMATS[:firstname_lastname] + end + + # Returns an array of fields names than can be used to make an order statement for users + # according to how user names are displayed + # Examples: + # + # User.fields_for_order_statement => ['users.login', 'users.id'] + # User.fields_for_order_statement('authors') => ['authors.login', 'authors.id'] + def self.fields_for_order_statement(table=nil) + table ||= table_name + name_formatter[:order].map {|field| "#{table}.#{field}"} + end + # Return user's full name for display def name(formatter = nil) + f = self.class.name_formatter(formatter) if formatter - eval('"' + (USER_FORMATS[formatter] || USER_FORMATS[:firstname_lastname]) + '"') + eval('"' + f[:string] + '"') else - @name ||= eval('"' + (USER_FORMATS[Setting.user_format] || USER_FORMATS[:firstname_lastname]) + '"') + @name ||= eval('"' + f[:string] + '"') end end diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 8044ead12..ff89e5a85 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -276,11 +276,6 @@ class IssuesControllerTest < ActionController::TestCase assert_response :success end - def test_index_sort_by_field_not_included_in_columns - Setting.issue_list_default_columns = %w(subject author) - get :index, :sort => 'tracker' - end - def test_index_csv get :index, :format => 'csv' assert_response :success @@ -421,10 +416,43 @@ class IssuesControllerTest < ActionController::TestCase assert !issues.empty? assert_equal issues.sort {|a,b| a.tracker == b.tracker ? b.id <=> a.id : a.tracker <=> b.tracker }.collect(&:id), issues.collect(&:id) end + + def test_index_sort_by_field_not_included_in_columns + Setting.issue_list_default_columns = %w(subject author) + get :index, :sort => 'tracker' + end + + def test_index_sort_by_assigned_to + get :index, :sort => 'assigned_to' + assert_response :success + assignees = assigns(:issues).collect(&:assigned_to).compact + assert_equal assignees.sort, assignees + end + + def test_index_sort_by_assigned_to_desc + get :index, :sort => 'assigned_to:desc' + assert_response :success + assignees = assigns(:issues).collect(&:assigned_to).compact + assert_equal assignees.sort.reverse, assignees + end + + def test_index_group_by_assigned_to + get :index, :group_by => 'assigned_to', :sort => 'priority' + assert_response :success + end def test_index_sort_by_author get :index, :sort => 'author' assert_response :success + authors = assigns(:issues).collect(&:author) + assert_equal authors.sort, authors + end + + def test_index_sort_by_author_desc + get :index, :sort => 'author:desc' + assert_response :success + authors = assigns(:issues).collect(&:author) + assert_equal authors.sort.reverse, authors end def test_index_group_by_author diff --git a/test/test_helper.rb b/test/test_helper.rb index 78b83c06d..dcac221d4 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -98,11 +98,11 @@ class ActiveSupport::TestCase end def with_settings(options, &block) - saved_settings = options.keys.inject({}) {|h, k| h[k] = Setting[k].dup; h} + saved_settings = options.keys.inject({}) {|h, k| h[k] = Setting[k].is_a?(Symbol) ? Setting[k] : Setting[k].dup; h} options.each {|k, v| Setting[k] = v} yield ensure - saved_settings.each {|k, v| Setting[k] = v} + saved_settings.each {|k, v| Setting[k] = v} if saved_settings end def change_user_password(login, new_password) diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 6306e8721..e3f3c9249 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -443,6 +443,22 @@ class QueryTest < ActiveSupport::TestCase assert_nil q.group_by_column assert_nil q.group_by_statement end + + def test_sortable_columns_should_sort_assignees_according_to_user_format_setting + with_settings :user_format => 'lastname_coma_firstname' do + q = Query.new + assert q.sortable_columns.has_key?('assigned_to') + assert_equal %w(users.lastname users.firstname users.id), q.sortable_columns['assigned_to'] + end + end + + def test_sortable_columns_should_sort_authors_according_to_user_format_setting + with_settings :user_format => 'lastname_coma_firstname' do + q = Query.new + assert q.sortable_columns.has_key?('author') + assert_equal %w(authors.lastname authors.firstname authors.id), q.sortable_columns['author'] + end + end def test_default_sort q = Query.new diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 5f7150d6e..a772d0079 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -397,6 +397,30 @@ class UserTest < ActiveSupport::TestCase Setting.user_format = :username assert_equal 'jsmith', @jsmith.reload.name end + + def test_fields_for_order_statement_should_return_fields_according_user_format_setting + with_settings :user_format => 'lastname_coma_firstname' do + assert_equal ['users.lastname', 'users.firstname', 'users.id'], User.fields_for_order_statement + end + end + + def test_fields_for_order_statement_width_table_name_should_prepend_table_name + with_settings :user_format => 'lastname_firstname' do + assert_equal ['authors.lastname', 'authors.firstname', 'authors.id'], User.fields_for_order_statement('authors') + end + end + + def test_fields_for_order_statement_with_blank_format_should_return_default + with_settings :user_format => '' do + assert_equal ['users.firstname', 'users.lastname', 'users.id'], User.fields_for_order_statement + end + end + + def test_fields_for_order_statement_with_invalid_format_should_return_default + with_settings :user_format => 'foo' do + assert_equal ['users.firstname', 'users.lastname', 'users.id'], User.fields_for_order_statement + end + end def test_lock user = User.try_to_login("jsmith", "jsmith") -- 2.39.5