From 06ecdf6aa0e6589df03ea7cabf24dd63a5031e2a Mon Sep 17 00:00:00 2001 From: Ken Collins Date: Tue, 6 Dec 2011 10:10:48 -0500 Subject: [PATCH] Use #ids_sql in all cases. Update docs. --- README.md | 43 +++++++++++++++++++ .../arish/associations/association_scope.rb | 2 +- .../arish/relation/predicate_builer.rb | 6 +-- lib/grouped_scope/self_grouping.rb | 1 - test/grouped_scope/has_many_test.rb | 16 +++---- test/grouped_scope/has_many_through_test.rb | 5 ++- test/grouped_scope/self_grouping_test.rb | 10 +++-- 7 files changed, 64 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 1b6529d..9d2d765 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,18 @@ defined on the original association. For instance: ## Advanced Usage +The group scoped object can respond to either `blank?` or `present?` which checks the group's +target `group_id` presence or not. We use this internally so that grouped scopes only use grouping +SQL when absolutely needed. + +```ruby +@employee_one = Employee.create :group_id => nil +@employee_two = Employee.create :group_id => 38 + +@employee_one.group.blank? # => true +@employee_two.group.present? # => true +``` + The object returned by the `#group` method is an ActiveRecord relation on the targets class, in this case `Employee`. Given this, you can further scope the grouped proxy if needed. Below, we use the `:email_present` scope to refine the group down. @@ -105,9 +117,40 @@ end @employee_one.group.email_present # => [# @employee.group).all +``` +If you need more control and you are working with the group at a lower level, you can always +use the `#ids` or `#ids_sql` methods on the group. +```ruby +# Returns primary key array. +@employee.group.ids # => [33, 58, 240] +# Returns a Arel::Nodes::SqlLiteral object. +@employee.group.ids_sql # => 'SELECT "employees"."id" FROM "employees" WHERE "employees"."group_id" = 33' +``` ## Todo List diff --git a/lib/grouped_scope/arish/associations/association_scope.rb b/lib/grouped_scope/arish/associations/association_scope.rb index b9057ec..29254ec 100644 --- a/lib/grouped_scope/arish/associations/association_scope.rb +++ b/lib/grouped_scope/arish/associations/association_scope.rb @@ -49,7 +49,7 @@ def add_constraints(scope) # GroupedScope changed this line. # scope = scope.where(table[key].eq(owner[foreign_key])) scope = if owner.group.present? - scope.where(table[key].in(owner.group.ids)) + scope.where(table[key].in(owner.group.ids_sql)) else scope.where(table[key].eq(owner[foreign_key])) end diff --git a/lib/grouped_scope/arish/relation/predicate_builer.rb b/lib/grouped_scope/arish/relation/predicate_builer.rb index ca0d44d..d46f179 100644 --- a/lib/grouped_scope/arish/relation/predicate_builer.rb +++ b/lib/grouped_scope/arish/relation/predicate_builer.rb @@ -11,9 +11,9 @@ module PredicateBuilder module ClassMethods def build_from_hash_with_grouped_scope(engine, attributes, default_table) - attributes.select{ |k,v| GroupedScope::SelfGroupping === v }.each do |kv| - k, v = kv - attributes[k] = v.ids + attributes.select{ |column, value| GroupedScope::SelfGroupping === value }.each do |column_value| + column, value = column_value + attributes[column] = value.arel_table[column.to_s].in(value.ids_sql) end build_from_hash_without_grouped_scope(engine, attributes, default_table) end diff --git a/lib/grouped_scope/self_grouping.rb b/lib/grouped_scope/self_grouping.rb index 1ac5469..b962da5 100644 --- a/lib/grouped_scope/self_grouping.rb +++ b/lib/grouped_scope/self_grouping.rb @@ -33,7 +33,6 @@ def ids_sql Arel.sql(grouped_scoped_ids.to_sql) end - # TODO: Note this. def quoted_ids ids.map { |id| quote_value(id,columns_hash[primary_key]) }.join(',') end diff --git a/test/grouped_scope/has_many_test.rb b/test/grouped_scope/has_many_test.rb index f564e97..d721750 100644 --- a/test/grouped_scope/has_many_test.rb +++ b/test/grouped_scope/has_many_test.rb @@ -22,7 +22,7 @@ class GroupedScope::HasManyTest < GroupedScope::TestCase it 'scopes group association to owner when group present' do @employee.update_attribute :group_id, 43 - assert_sql(/"employee_id" IN \(#{@employee.id}\)/) do + assert_sql(/"employee_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = 43\)/) do @employee.group.reports(true) end end @@ -41,7 +41,7 @@ class GroupedScope::HasManyTest < GroupedScope::TestCase end it 'scope count sql to group' do - assert_sql(/SELECT COUNT\(\*\)/,/"employee_id" IN \(#{@e1.id}, #{@e2.id}\)/) do + assert_sql(/SELECT COUNT\(\*\)/,/"employee_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = #{@e1.group_id}\)/) do @e1.group.reports(true).count end end @@ -68,8 +68,8 @@ class GroupedScope::HasManyTest < GroupedScope::TestCase assert_same_elements @urgent_reports, @e2.group.reports(true).urgent end - it 'use assoc extension SQL along with group reflection' do - assert_sql(select_from_reports, where_for_groups, where_for_urgent_title) do + it 'use association extension SQL along with group reflection' do + assert_sql(select_from_reports, where_for_groups(@e2.group_id), where_for_urgent_title) do @e2.group.reports.urgent end end @@ -96,7 +96,7 @@ class GroupedScope::HasManyTest < GroupedScope::TestCase end it 'use named scope SQL along with group reflection' do - assert_sql(select_from_reports, where_for_groups, where_for_urgent_body, where_for_urgent_title) do + assert_sql(select_from_reports, where_for_groups(@e2.group_id), where_for_urgent_body, where_for_urgent_title) do @e2.group.reports.with_urgent_title.with_urgent_body.inspect end end @@ -125,7 +125,7 @@ class GroupedScope::HasManyTest < GroupedScope::TestCase it 'scopes group association to owners group when present' do @employee.update_attribute :group_id, 43 - assert_sql(/"legacy_reports"."email" IN \('#{@employee.id}'\)/) do + assert_sql(/"legacy_reports"."email" IN \(SELECT "legacy_employees"\."email" FROM "legacy_employees" WHERE "legacy_employees"\."group_id" = 43\)/) do @employee.group.reports(true) end end @@ -139,8 +139,8 @@ def select_from_reports /SELECT "reports"\.\* FROM "reports"/ end - def where_for_groups - /WHERE "reports"."employee_id" IN \(2, 3\)/ + def where_for_groups(id) + /WHERE "reports"."employee_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = #{id}\)/ end def where_for_urgent_body diff --git a/test/grouped_scope/has_many_through_test.rb b/test/grouped_scope/has_many_through_test.rb index 21c9647..fc55859 100644 --- a/test/grouped_scope/has_many_through_test.rb +++ b/test/grouped_scope/has_many_through_test.rb @@ -29,13 +29,14 @@ class GroupedScope::HasManyThroughTest < GroupedScope::TestCase describe 'For grouped association' do it 'scope to group' do - assert_sql(/"employee_id" IN \(#{@e1.id}, #{@e2.id}\)/) do + + assert_sql(/"employee_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = #{@e1.group_id}\)/) do @e2.group.departments(true) end end it 'scope count to group' do - assert_sql(/"employee_id" IN \(#{@e1.id}, #{@e2.id}\)/) do + assert_sql(/"employee_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = #{@e1.group_id}\)/) do @e1.group.departments(true).count end end diff --git a/test/grouped_scope/self_grouping_test.rb b/test/grouped_scope/self_grouping_test.rb index 8155c66..73f71d4 100644 --- a/test/grouped_scope/self_grouping_test.rb +++ b/test/grouped_scope/self_grouping_test.rb @@ -28,10 +28,12 @@ class GroupedScope::SelfGrouppingTest < GroupedScope::TestCase lambda{ GroupedScope::SelfGroupping.new(FooBar.new) }.must_raise(GroupedScope::NoGroupIdError) end - it 'return correct attribute_condition for GroupedScope::SelfGroupping object' do - assert_sql(/"?group_id"? IN \(#{@employee.id}\)/) do - Employee.find :all, :conditions => {:group_id => @employee.group} - end + it 'return correct predicate for GroupedScope::SelfGroupping object' do + @employee.update_attribute :group_id, 82 + expected_sql = /"group_id" IN \(SELECT "employees"\."id" FROM "employees" WHERE "employees"\."group_id" = 82/ + assert_sql(expected_sql) { Employee.where(:group_id => @employee.group).all } + assert_sql(expected_sql) { Employee.all(:conditions => {:group_id => @employee.group}) } + assert_equal [@employee], Employee.where(:group_id => @employee.group).all end it 'allows you to ask if the group is present' do