Merge branch 'meineerde/issues/250-allow-empty-groups-in-queries' into chiliproject/master

5592794d · Holger Just · f2285fa9 · ca9ff0e4 · 5592794d · 5592794d
Commit 5592794d authored Mar 02, 2011 by Holger Just
Hide whitespace changes
Inline Side-by-side

Showing with 46 additions and 4 deletions

query.rb app/models/query.rb +12 -2

query_test.rb test/unit/query_test.rb +34 -2

No files found.
--- 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

--- 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