From 21b8e8b5e586e8e0b96c9077047a2f3f260c5731 Mon Sep 17 00:00:00 2001 From: Holger Just Date: Wed, 2 Mar 2011 10:54:44 +0100 Subject: [PATCH 1/2] [#250] Allow empty sets in query value lists --- app/models/query.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 1f033048..1772d68f 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -566,9 +566,19 @@ class Query < ActiveRecord::Base sql = '' case operator when "=" - sql = "#{db_table}.#{db_field} IN (" + value.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ")" + if value.present? + sql = "#{db_table}.#{db_field} IN (" + value.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + ")" + else + # empty set of allowed values produces no result + sql = "0=1" + end when "!" - sql = "(#{db_table}.#{db_field} IS NULL OR #{db_table}.#{db_field} NOT IN (" + value.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + "))" + if value.present? + sql = "(#{db_table}.#{db_field} IS NULL OR #{db_table}.#{db_field} NOT IN (" + value.collect{|val| "'#{connection.quote_string(val)}'"}.join(",") + "))" + else + # empty set of forbidden values allows all results + sql = "1=1" + end when "!*" sql = "#{db_table}.#{db_field} IS NULL" sql << " OR #{db_table}.#{db_field} = ''" if is_custom_filter From ca9ff0e4712b27abe6528fc37148a0de3b2a1cc3 Mon Sep 17 00:00:00 2001 From: Holger Just Date: Wed, 2 Mar 2011 11:12:57 +0100 Subject: [PATCH 2/2] [#250] Add tests for empty sets of users in queries --- test/unit/query_test.rb | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 4d85a2b2..a20c14e9 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -467,6 +467,7 @@ class QueryTest < ActiveSupport::TestCase @group2 = Group.generate!.reload @group2.users << @user_in_group2 + @empty_group = Group.generate!.reload end should "search assigned to for users in the group" do @@ -484,7 +485,6 @@ class QueryTest < ActiveSupport::TestCase # Users not in a group assert_query_statement_includes @query, "#{Issue.table_name}.assigned_to_id IS NULL OR #{Issue.table_name}.assigned_to_id NOT IN ('#{@user_in_group.id}','#{@second_user_in_group.id}','#{@user_in_group2.id}')" assert_find_issues_with_query_is_successful @query - end should "search assigned to any group member (all)" do @@ -494,7 +494,22 @@ class QueryTest < ActiveSupport::TestCase # Only users in a group assert_query_statement_includes @query, "#{Issue.table_name}.assigned_to_id IN ('#{@user_in_group.id}','#{@second_user_in_group.id}','#{@user_in_group2.id}')" assert_find_issues_with_query_is_successful @query - + end + + should "return no results on empty set" do + @query = Query.new(:name => '_') + @query.add_filter('member_of_group', '=', [@empty_group.id.to_s]) + + assert_query_statement_includes @query, "(0=1)" + assert find_issues_with_query(@query).empty? + end + + should "return results on disallowed empty set" do + @query = Query.new(:name => '_') + @query.add_filter('member_of_group', '!', [@empty_group.id.to_s]) + + assert_query_statement_includes @query, "(1=1)" + assert_find_issues_with_query_is_successful @query end end @@ -507,6 +522,7 @@ class QueryTest < ActiveSupport::TestCase @manager_role = Role.generate!(:name => 'Manager') @developer_role = Role.generate!(:name => 'Developer') + @empty_role = Role.generate!(:name => 'Empty') @project = Project.generate! @manager = User.generate! @@ -540,6 +556,22 @@ class QueryTest < ActiveSupport::TestCase 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 end + + should "return no results on empty set" do + @query = Query.new(:name => '_') + @query.add_filter('assigned_to_role', '=', [@empty_role.id.to_s]) + + assert_query_statement_includes @query, "(0=1)" + assert find_issues_with_query(@query).empty? + end + + should "return results on disallowed empty set" do + @query = Query.new(:name => '_') + @query.add_filter('assigned_to_role', '!', [@empty_role.id.to_s]) + + assert_query_statement_includes @query, "(1=1)" + assert_find_issues_with_query_is_successful @query + end end end