From faab8678d440908a7190a1a4300fdfbca52efe47 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 24 Jul 2012 17:39:30 +0000 Subject: [PATCH] Ability to group and sort the issue list by user/version custom field (#9419). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10073 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/custom_field.rb | 44 +++++++++++++++++++ app/models/query.rb | 28 +++++++++--- test/functional/issues_controller_test.rb | 52 +++++++++++++++++++++++ test/unit/custom_field_test.rb | 10 +++++ test/unit/query_test.rb | 14 ++++++ 5 files changed, 142 insertions(+), 6 deletions(-) diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 408b54afb..3373fb8d8 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -146,6 +146,8 @@ class CustomField < ActiveRecord::Base " WHERE cv_sort.customized_type='#{self.class.customized_class.base_class.name}'" + " AND cv_sort.customized_id=#{self.class.customized_class.table_name}.id" + " AND cv_sort.custom_field_id=#{id} AND cv_sort.value <> '' AND cv_sort.value IS NOT NULL LIMIT 1)" + when 'user', 'version' + value_class.fields_for_order_statement(value_join_alias) else nil end @@ -158,15 +160,57 @@ class CustomField < ActiveRecord::Base case field_format when 'list', 'date', 'bool', 'int' order_statement + when 'user', 'version' + "COALESCE((SELECT cv_sort.value FROM #{CustomValue.table_name} cv_sort" + + " WHERE cv_sort.customized_type='#{self.class.customized_class.base_class.name}'" + + " AND cv_sort.customized_id=#{self.class.customized_class.table_name}.id" + + " AND cv_sort.custom_field_id=#{id} LIMIT 1), '')" + else + nil + end + end + + def join_for_order_statement + case field_format + when 'user', 'version' + "LEFT OUTER JOIN #{CustomValue.table_name} #{join_alias}" + + " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" + + " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" + + " AND #{join_alias}.custom_field_id = #{id}" + + " AND #{join_alias}.value <> ''" + + " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" + + " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" + + " AND #{join_alias}_2.customized_id = #{join_alias}.customized_id" + + " AND #{join_alias}_2.custom_field_id = #{join_alias}.custom_field_id)" + + " LEFT OUTER JOIN #{value_class.table_name} #{value_join_alias}" + + " ON CAST(#{join_alias}.value as decimal(60,0)) = #{value_join_alias}.id" else nil end end + def join_alias + "cf_#{id}" + end + + def value_join_alias + join_alias + "_" + field_format + end + def <=>(field) position <=> field.position end + # Returns the class that values represent + def value_class + case field_format + when 'user', 'version' + field_format.classify.constantize + else + nil + end + end + def self.customized_class self.name =~ /^(.+)CustomField$/ begin; $1.constantize; rescue nil; end diff --git a/app/models/query.rb b/app/models/query.rb index 5e76b17fe..2d9ab303c 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -604,13 +604,11 @@ class Query < ActiveRecord::Base def issues(options={}) order_option = [group_by_sort_order, options[:order]].reject {|s| s.blank?}.join(',') order_option = nil if order_option.blank? - - joins = (order_option && order_option.include?('authors')) ? "LEFT OUTER JOIN users authors ON authors.id = #{Issue.table_name}.author_id" : nil issues = Issue.visible.scoped(:conditions => options[:conditions]).find :all, :include => ([:status, :project] + (options[:include] || [])).uniq, :conditions => statement, :order => order_option, - :joins => joins, + :joins => joins_for_order_statement(order_option), :limit => options[:limit], :offset => options[:offset] @@ -626,13 +624,11 @@ class Query < ActiveRecord::Base def issue_ids(options={}) order_option = [group_by_sort_order, options[:order]].reject {|s| s.blank?}.join(',') order_option = nil if order_option.blank? - - joins = (order_option && order_option.include?('authors')) ? "LEFT OUTER JOIN users authors ON authors.id = #{Issue.table_name}.author_id" : nil Issue.visible.scoped(:conditions => options[:conditions]).scoped(:include => ([:status, :project] + (options[:include] || [])).uniq, :conditions => statement, :order => order_option, - :joins => joins, + :joins => joins_for_order_statement(order_option), :limit => options[:limit], :offset => options[:offset]).find_ids rescue ::ActiveRecord::StatementInvalid => e @@ -895,4 +891,24 @@ class Query < ActiveRecord::Base def relative_date_clause(table, field, days_from, days_to) date_clause(table, field, (days_from ? Date.today + days_from : nil), (days_to ? Date.today + days_to : nil)) end + + # Additional joins required for the given sort options + def joins_for_order_statement(order_options) + joins = [] + + if order_options + if order_options.include?('authors') + joins << "LEFT OUTER JOIN #{User.table_name} authors ON authors.id = #{Issue.table_name}.author_id" + end + order_options.scan(/cf_\d+/).uniq.each do |name| + column = available_columns.detect {|c| c.name.to_s == name} + join = column.custom_field.join_for_order_statement + if join + joins << join + end + end + end + + joins.any? ? joins.join(' ') : nil + end end diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index b7ab6d0fe..b554e9e5f 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -254,6 +254,27 @@ class IssuesControllerTest < ActionController::TestCase assert_not_nil assigns(:issue_count_by_group) end + def test_index_with_query_grouped_by_user_custom_field + cf = IssueCustomField.create!(:name => 'User', :is_for_all => true, :tracker_ids => [1,2,3], :field_format => 'user') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(1), :value => '2') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(2), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(3), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(5), :value => '') + + get :index, :project_id => 1, :set_filter => 1, :group_by => "cf_#{cf.id}" + assert_response :success + + assert_select 'tr.group', 3 + assert_select 'tr.group' do + assert_select 'a', :text => 'John Smith' + assert_select 'span.count', :text => '(1)' + end + assert_select 'tr.group' do + assert_select 'a', :text => 'Dave Lopper' + assert_select 'span.count', :text => '(2)' + end + end + def test_index_with_query_id_and_project_id_should_set_session_query get :index, :project_id => 1, :query_id => 4 assert_response :success @@ -619,6 +640,19 @@ class IssuesControllerTest < ActionController::TestCase assert_equal hours.sort.reverse, hours end + def test_index_sort_by_user_custom_field + cf = IssueCustomField.create!(:name => 'User', :is_for_all => true, :tracker_ids => [1,2,3], :field_format => 'user') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(1), :value => '2') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(2), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(3), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(5), :value => '') + + get :index, :project_id => 1, :set_filter => 1, :sort => "cf_#{cf.id},id" + assert_response :success + + assert_equal [2, 3, 1], assigns(:issues).select {|issue| issue.custom_field_value(cf).present?}.map(&:id) + end + def test_index_with_columns columns = ['tracker', 'subject', 'assigned_to'] get :index, :set_filter => 1, :c => columns @@ -1114,6 +1148,24 @@ class IssuesControllerTest < ActionController::TestCase assert_no_tag 'a', :content => /Next/ end + def test_show_show_should_display_prev_next_links_with_query_sort_by_user_custom_field + cf = IssueCustomField.create!(:name => 'User', :is_for_all => true, :tracker_ids => [1,2,3], :field_format => 'user') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(1), :value => '2') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(2), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(3), :value => '3') + CustomValue.create!(:custom_field => cf, :customized => Issue.find(5), :value => '') + + query = Query.create!(:name => 'test', :is_public => true, :user_id => 1, :filters => {}, + :sort_criteria => [["cf_#{cf.id}", 'asc'], ['id', 'asc']]) + @request.session[:query] = {:id => query.id, :project_id => nil} + + get :show, :id => 3 + assert_response :success + + assert_equal 2, assigns(:prev_issue_id) + assert_equal 1, assigns(:next_issue_id) + end + def test_show_should_display_link_to_the_assignee get :show, :id => 2 assert_response :success diff --git a/test/unit/custom_field_test.rb b/test/unit/custom_field_test.rb index 4509dc8ed..3d98c95a5 100644 --- a/test/unit/custom_field_test.rb +++ b/test/unit/custom_field_test.rb @@ -202,4 +202,14 @@ class CustomFieldTest < ActiveSupport::TestCase assert f.valid_field_value?(['value1', 'value2']) assert !f.valid_field_value?(['value1', 'abc']) end + + def test_value_class_should_return_the_class_used_for_fields_values + assert_equal User, CustomField.new(:field_format => 'user') + assert_equal Version, CustomField.new(:field_format => 'version') + end + + def test_value_class_should_return_nil_for_other_fields + assert_nil CustomField.new(:field_format => 'text') + assert_nil CustomField.new + end end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 522d8d0a3..de6765f2c 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -621,6 +621,20 @@ class QueryTest < ActiveSupport::TestCase assert_nil column end + def test_groupable_columns_should_include_user_custom_fields + cf = IssueCustomField.create!(:name => 'User', :is_for_all => true, :tracker_ids => [1], :field_format => 'user') + + q = Query.new + assert q.groupable_columns.detect {|c| c.name == "cf_#{cf.id}".to_sym} + end + + def test_groupable_columns_should_include_version_custom_fields + cf = IssueCustomField.create!(:name => 'User', :is_for_all => true, :tracker_ids => [1], :field_format => 'version') + + q = Query.new + assert q.groupable_columns.detect {|c| c.name == "cf_#{cf.id}".to_sym} + end + def test_grouped_with_valid_column q = Query.new(:group_by => 'status') assert q.grouped? -- 2.39.5