From e4cda67cf48624d5f72732b080a10ef708c353c3 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 19 Nov 2011 15:11:20 +0000 Subject: [PATCH] Fixed: Issue filter by assigned_to_role is not project specific (#9540). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@7847 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/query.rb | 33 ++++++++--------- test/unit/query_test.rb | 79 ++++++++++++++++++++++++++--------------- 2 files changed, 66 insertions(+), 46 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 1ac18c7f5..2f99d0fe7 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -617,25 +617,22 @@ class Query < ActiveRecord::Base end def sql_for_assigned_to_role_field(field, operator, value) - if operator == "*" # Any Role - roles = Role.givable - operator = '=' # Override the operator since we want to find by assigned_to - elsif operator == "!*" # No role - roles = Role.givable - operator = '!' # Override the operator since we want to find by assigned_to - else - roles = Role.givable.find_all_by_id(value) + case operator + when "*", "!*" # Member / Not member + sw = operator == "!*" ? 'NOT' : '' + nl = operator == "!*" ? "#{Issue.table_name}.assigned_to_id IS NULL OR" : '' + "(#{nl} #{Issue.table_name}.assigned_to_id #{sw} IN (SELECT DISTINCT #{Member.table_name}.user_id FROM #{Member.table_name}" + + " WHERE #{Member.table_name}.project_id = #{Issue.table_name}.project_id))" + when "=", "!" + role_cond = value.any? ? + "#{MemberRole.table_name}.role_id IN (" + value.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ")" : + "1=0" + + sw = operator == "!" ? 'NOT' : '' + nl = operator == "!" ? "#{Issue.table_name}.assigned_to_id IS NULL OR" : '' + "(#{nl} #{Issue.table_name}.assigned_to_id #{sw} IN (SELECT DISTINCT #{Member.table_name}.user_id FROM #{Member.table_name}, #{MemberRole.table_name}" + + " WHERE #{Member.table_name}.project_id = #{Issue.table_name}.project_id AND #{Member.table_name}.id = #{MemberRole.table_name}.member_id AND #{role_cond}))" end - roles ||= [] - - members_of_roles = roles.inject([]) {|user_ids, role| - if role && role.members - user_ids << role.members.collect(&:user_id) - end - user_ids.flatten.uniq.compact - }.sort.collect(&:to_s) - - '(' + sql_for_field("assigned_to_id", operator, members_of_roles, Issue.table_name, "assigned_to_id", false) + ')' end private diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 11e6151b3..6306e8721 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -62,6 +62,13 @@ class QueryTest < ActiveSupport::TestCase def assert_query_statement_includes(query, condition) assert query.statement.include?(condition), "Query statement condition not found in: #{query.statement}" end + + def assert_query_result(expected, query) + assert_nothing_raised do + assert_equal expected.map(&:id).sort, query.issues.map(&:id).sort + assert_equal expected.size, query.issue_count + end + end def test_query_should_allow_shared_versions_for_a_project_query subproject_version = Version.find(4) @@ -723,61 +730,77 @@ class QueryTest < ActiveSupport::TestCase context "with 'assigned_to_role' filter" do setup do - # No fixtures - MemberRole.delete_all - Member.delete_all - Role.delete_all - - @manager_role = Role.generate!(:name => 'Manager') - @developer_role = Role.generate!(:name => 'Developer') + @manager_role = Role.find_by_name('Manager') + @developer_role = Role.find_by_name('Developer') @project = Project.generate! @manager = User.generate! @developer = User.generate! @boss = User.generate! + @guest = User.generate! User.add_to_project(@manager, @project, @manager_role) User.add_to_project(@developer, @project, @developer_role) User.add_to_project(@boss, @project, [@manager_role, @developer_role]) + + @issue1 = Issue.generate_for_project!(@project, :assigned_to_id => @manager.id) + @issue2 = Issue.generate_for_project!(@project, :assigned_to_id => @developer.id) + @issue3 = Issue.generate_for_project!(@project, :assigned_to_id => @boss.id) + @issue4 = Issue.generate_for_project!(@project, :assigned_to_id => @guest.id) + @issue5 = Issue.generate_for_project!(@project) end should "search assigned to for users with the Role" do - @query = Query.new(:name => '_') + @query = Query.new(:name => '_', :project => @project) @query.add_filter('assigned_to_role', '=', [@manager_role.id.to_s]) - assert_query_statement_includes @query, "#{Issue.table_name}.assigned_to_id IN ('#{@manager.id}','#{@boss.id}')" - assert_find_issues_with_query_is_successful @query + assert_query_result [@issue1, @issue3], @query end - should "search assigned to for users not assigned to any Role (none)" do - @query = Query.new(:name => '_') - @query.add_filter('assigned_to_role', '!*', ['']) + should "search assigned to for users with the Role on the issue project" do + other_project = Project.generate! + User.add_to_project(@developer, other_project, @manager_role) + + @query = Query.new(:name => '_', :project => @project) + @query.add_filter('assigned_to_role', '=', [@manager_role.id.to_s]) - assert_query_statement_includes @query, "#{Issue.table_name}.assigned_to_id IS NULL OR #{Issue.table_name}.assigned_to_id NOT IN ('#{@manager.id}','#{@developer.id}','#{@boss.id}')" - assert_find_issues_with_query_is_successful @query - end - - should "search assigned to for users assigned to any Role (all)" do - @query = Query.new(:name => '_') - @query.add_filter('assigned_to_role', '*', ['']) - - assert_query_statement_includes @query, "#{Issue.table_name}.assigned_to_id IN ('#{@manager.id}','#{@developer.id}','#{@boss.id}')" - assert_find_issues_with_query_is_successful @query + assert_query_result [@issue1, @issue3], @query end should "return an empty set with empty role" do @empty_role = Role.generate! - @query = Query.new(:name => '_') + @query = Query.new(:name => '_', :project => @project) @query.add_filter('assigned_to_role', '=', [@empty_role.id.to_s]) - assert_equal [], find_issues_with_query(@query) + assert_query_result [], @query + end + + should "search assigned to for users without the Role" do + @query = Query.new(:name => '_', :project => @project) + @query.add_filter('assigned_to_role', '!', [@manager_role.id.to_s]) + + assert_query_result [@issue2, @issue4, @issue5], @query + end + + should "search assigned to for users not assigned to any Role (none)" do + @query = Query.new(:name => '_', :project => @project) + @query.add_filter('assigned_to_role', '!*', ['']) + + assert_query_result [@issue4, @issue5], @query + end + + should "search assigned to for users assigned to any Role (all)" do + @query = Query.new(:name => '_', :project => @project) + @query.add_filter('assigned_to_role', '*', ['']) + + assert_query_result [@issue1, @issue2, @issue3], @query end should "return issues with ! empty role" do @empty_role = Role.generate! - @query = Query.new(:name => '_') - @query.add_filter('member_of_group', '!', [@empty_role.id.to_s]) + @query = Query.new(:name => '_', :project => @project) + @query.add_filter('assigned_to_role', '!', [@empty_role.id.to_s]) - assert_find_issues_with_query_is_successful @query + assert_query_result [@issue1, @issue2, @issue3, @issue4, @issue5], @query end end end