From 59b935aa469c44d7c8c00f199af31168ce4af4c7 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Fri, 13 Sep 2013 17:41:54 +0000 Subject: [PATCH] Filters show issues with unused custom fields (#13537). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@12133 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue_custom_field.rb | 9 +++++ test/fixtures/custom_fields_trackers.yml | 9 +++++ test/unit/query_test.rb | 46 +++++++++++++++++++----- 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/app/models/issue_custom_field.rb b/app/models/issue_custom_field.rb index e811ccaae..513b96a54 100644 --- a/app/models/issue_custom_field.rb +++ b/app/models/issue_custom_field.rb @@ -28,6 +28,15 @@ class IssueCustomField < CustomField super || (roles & user.roles_for_project(project)).present? end + def visibility_by_project_condition(*args) + sql = super + additional_sql = "#{Issue.table_name}.tracker_id IN (SELECT tracker_id FROM #{table_name_prefix}custom_fields_trackers#{table_name_suffix} WHERE custom_field_id = #{id})" + unless is_for_all? + additional_sql << " AND #{Issue.table_name}.project_id IN (SELECT project_id FROM #{table_name_prefix}custom_fields_projects#{table_name_suffix} WHERE custom_field_id = #{id})" + end + "((#{sql}) AND (#{additional_sql}))" + end + def validate_custom_field super errors.add(:base, l(:label_role_plural) + ' ' + l('activerecord.errors.messages.blank')) unless visible? || roles.present? diff --git a/test/fixtures/custom_fields_trackers.yml b/test/fixtures/custom_fields_trackers.yml index a356e4b85..2a7a07381 100644 --- a/test/fixtures/custom_fields_trackers.yml +++ b/test/fixtures/custom_fields_trackers.yml @@ -17,3 +17,12 @@ custom_fields_trackers_005: custom_fields_trackers_006: custom_field_id: 6 tracker_id: 3 +custom_fields_trackers_007: + custom_field_id: 8 + tracker_id: 1 +custom_fields_trackers_008: + custom_field_id: 8 + tracker_id: 2 +custom_fields_trackers_009: + custom_field_id: 8 + tracker_id: 3 diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 12580bfd0..f4b765577 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -219,7 +219,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_is_on_integer_custom_field - f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_for_all => true, :is_filter => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_for_all => true, :is_filter => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '7') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '12') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -232,7 +232,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_is_on_integer_custom_field_should_accept_negative_value - f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_for_all => true, :is_filter => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_for_all => true, :is_filter => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '7') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '-12') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -246,7 +246,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_is_on_float_custom_field - f = IssueCustomField.create!(:name => 'filter', :field_format => 'float', :is_filter => true, :is_for_all => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'float', :is_filter => true, :is_for_all => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '7.3') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '12.7') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -259,7 +259,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_is_on_float_custom_field_should_accept_negative_value - f = IssueCustomField.create!(:name => 'filter', :field_format => 'float', :is_filter => true, :is_for_all => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'float', :is_filter => true, :is_for_all => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '7.3') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '-12.7') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -274,7 +274,7 @@ class QueryTest < ActiveSupport::TestCase def test_operator_is_on_multi_list_custom_field f = IssueCustomField.create!(:name => 'filter', :field_format => 'list', :is_filter => true, :is_for_all => true, - :possible_values => ['value1', 'value2', 'value3'], :multiple => true) + :possible_values => ['value1', 'value2', 'value3'], :multiple => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value1') CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value2') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => 'value1') @@ -292,7 +292,7 @@ class QueryTest < ActiveSupport::TestCase def test_operator_is_not_on_multi_list_custom_field f = IssueCustomField.create!(:name => 'filter', :field_format => 'list', :is_filter => true, :is_for_all => true, - :possible_values => ['value1', 'value2', 'value3'], :multiple => true) + :possible_values => ['value1', 'value2', 'value3'], :multiple => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value1') CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value2') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => 'value1') @@ -355,7 +355,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_greater_than_on_int_custom_field - f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_filter => true, :is_for_all => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'int', :is_filter => true, :is_for_all => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '7') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '12') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -383,7 +383,7 @@ class QueryTest < ActiveSupport::TestCase end def test_operator_lesser_than_on_date_custom_field - f = IssueCustomField.create!(:name => 'filter', :field_format => 'date', :is_filter => true, :is_for_all => true) + f = IssueCustomField.create!(:name => 'filter', :field_format => 'date', :is_filter => true, :is_for_all => true, :trackers => Tracker.all) CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => '2013-04-11') CustomValue.create!(:custom_field => f, :customized => Issue.find(2), :value => '2013-05-14') CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => '') @@ -459,7 +459,7 @@ class QueryTest < ActiveSupport::TestCase def test_operator_date_between query = IssueQuery.new(:name => '_') query.add_filter('due_date', '><', ['2011-06-23', '2011-07-10']) - assert_match /issues\.due_date > '2011-06-22 23:59:59(\.9+)?' AND issues\.due_date <= '2011-07-10 23:59:59(\.9+)?/, query.statement + assert_match /issues\.due_date > '2011-06-22 23:59:59(\.9+)?' AND issues\.due_date <= '2011-07-10 23:59:59(\.9+)?'/, query.statement find_issues_with_query(query) end @@ -660,6 +660,34 @@ class QueryTest < ActiveSupport::TestCase User.current = nil end + def test_filter_on_custom_field_should_ignore_projects_with_field_disabled + field = IssueCustomField.generate!(:trackers => Tracker.all, :project_ids => [1, 3, 4], :is_filter => true) + Issue.generate!(:project_id => 3, :tracker_id => 2, :custom_field_values => {field.id.to_s => 'Foo'}) + Issue.generate!(:project_id => 4, :tracker_id => 2, :custom_field_values => {field.id.to_s => 'Foo'}) + + query = IssueQuery.new(:name => '_', :project => Project.find(1)) + query.filters = {"cf_#{field.id}" => {:operator => '=', :values => ['Foo']}} + assert_equal 2, find_issues_with_query(query).size + + field.project_ids = [1, 3] # Disable the field for project 4 + field.save! + assert_equal 1, find_issues_with_query(query).size + end + + def test_filter_on_custom_field_should_ignore_trackers_with_field_disabled + field = IssueCustomField.generate!(:tracker_ids => [1, 2], :is_for_all => true, :is_filter => true) + Issue.generate!(:project_id => 1, :tracker_id => 1, :custom_field_values => {field.id.to_s => 'Foo'}) + Issue.generate!(:project_id => 1, :tracker_id => 2, :custom_field_values => {field.id.to_s => 'Foo'}) + + query = IssueQuery.new(:name => '_', :project => Project.find(1)) + query.filters = {"cf_#{field.id}" => {:operator => '=', :values => ['Foo']}} + assert_equal 2, find_issues_with_query(query).size + + field.tracker_ids = [1] # Disable the field for tracker 2 + field.save! + assert_equal 1, find_issues_with_query(query).size + end + def test_filter_on_project_custom_field field = ProjectCustomField.create!(:name => 'Client', :is_filter => true, :field_format => 'string') CustomValue.create!(:custom_field => field, :customized => Project.find(3), :value => 'Foo') -- 2.39.5