From 6bc745c960bae7f470e9bc3a7bde3dfcfcf51b5f Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Fri, 17 Jun 2016 16:37:29 +0100 Subject: [PATCH 1/2] The where_unscoping test case does apply to Rails 4.2 Perhaps the original author thought it didn't because there was no `where_unscoping` override defined for 4.2 but it inherits from 4.1. It doesn't hurt to do the test anyway, except that the unscope call had to be changed to `unscope!` in order for the SQL check to pass. It should have been `unscope!` all along and it only worked because of rails/rails#20722. --- .../active_record/relation_extensions_spec.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/spec/squeel/adapters/active_record/relation_extensions_spec.rb b/spec/squeel/adapters/active_record/relation_extensions_spec.rb index 0ea9667..b786113 100644 --- a/spec/squeel/adapters/active_record/relation_extensions_spec.rb +++ b/spec/squeel/adapters/active_record/relation_extensions_spec.rb @@ -1068,16 +1068,12 @@ module ActiveRecord describe '#where_unscoping' do it "doesn't ruin everything when predicate expression in where_values doesn't respond to :symbol method" do - unless activerecord_version_at_least '4.2.0' - if activerecord_version_at_least '4.0.0' - order_items = OrderItem.where{quantity == 0}.where{unit_price / 2 == 5} - expect { order_items.unscope(where: :quantity) }.should_not raise_error - order_items.to_sql.should_not match /#{Q}order_items#{Q}.#{Q}quantity#{Q} = 0/ - else - pending 'Unsupported on AR versions < 4.0.0' - end + if activerecord_version_at_least '4.0.0' + order_items = OrderItem.where{quantity == 0}.where{unit_price / 2 == 5} + expect { order_items.unscope!(where: :quantity) }.should_not raise_error + order_items.to_sql.should_not match /#{Q}order_items#{Q}.#{Q}quantity#{Q} = 0/ else - pending 'Not required in AR versions > 4.2.0' + pending 'Unsupported on AR versions < 4.0.0' end end From 12a8ed0c8428879fc8604c374d6be4e1cff5fb5f Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Fri, 17 Jun 2016 16:44:54 +0100 Subject: [PATCH 2/2] Fix where_unscoping on 4.0, 4.1, 4.2 Believe it or not, basic `unscope` functionality was broken. I don't know how but With Squeel in play, `where_values` somehow becomes a nested array. Flattening it first fixes the issue. I have adjusted the 4.1 override to use `super` so that less code is duplicated. This isn't possible for 4.0. I have added a new override for 4.2 because `where_values` should not be mutated in that version. --- .../active_record/4.0/relation_extensions.rb | 1 + .../active_record/4.1/relation_extensions.rb | 6 ++---- .../active_record/4.2/relation_extensions.rb | 15 +++++++++++++++ .../active_record/relation_extensions_spec.rb | 4 ++++ 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/squeel/adapters/active_record/4.0/relation_extensions.rb b/lib/squeel/adapters/active_record/4.0/relation_extensions.rb index b8e5c98..1fa85b7 100644 --- a/lib/squeel/adapters/active_record/4.0/relation_extensions.rb +++ b/lib/squeel/adapters/active_record/4.0/relation_extensions.rb @@ -32,6 +32,7 @@ def where(opts = :chain, *rest) def where_unscoping(target_value) target_value = target_value.to_s + where_values.flatten! where_values.reject! do |rel| case rel diff --git a/lib/squeel/adapters/active_record/4.1/relation_extensions.rb b/lib/squeel/adapters/active_record/4.1/relation_extensions.rb index 38e9335..c8cf7fa 100644 --- a/lib/squeel/adapters/active_record/4.1/relation_extensions.rb +++ b/lib/squeel/adapters/active_record/4.1/relation_extensions.rb @@ -44,12 +44,10 @@ def visited def where_unscoping(target_value) target_value = target_value.to_s + where_values.flatten! where_values.reject! do |rel| case rel - when Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual - subrelation = (rel.left.kind_of?(Arel::Attributes::Attribute) ? rel.left : rel.right) - subrelation.name == target_value when Hash rel.stringify_keys.has_key?(target_value) when Squeel::Nodes::Predicate @@ -57,7 +55,7 @@ def where_unscoping(target_value) end end - bind_values.reject! { |col,_| col.name == target_value } + super end def reverse_sql_order(order_query) diff --git a/lib/squeel/adapters/active_record/4.2/relation_extensions.rb b/lib/squeel/adapters/active_record/4.2/relation_extensions.rb index 866815f..b8cd12f 100644 --- a/lib/squeel/adapters/active_record/4.2/relation_extensions.rb +++ b/lib/squeel/adapters/active_record/4.2/relation_extensions.rb @@ -7,6 +7,21 @@ module RelationExtensions undef_method 'to_sql_with_binding_params' + def where_unscoping(target_value) + target_value = target_value.to_s + + self.where_values = where_values.flatten.reject do |rel| + case rel + when Hash + rel.stringify_keys.has_key?(target_value) + when Squeel::Nodes::Predicate + rel.expr.symbol.to_s == target_value if rel.expr.respond_to?(:symbol) + end + end + + super + end + attr_accessor :reverse_order_value private :reverse_order_value, :reverse_order_value= diff --git a/spec/squeel/adapters/active_record/relation_extensions_spec.rb b/spec/squeel/adapters/active_record/relation_extensions_spec.rb index b786113..b6fd005 100644 --- a/spec/squeel/adapters/active_record/relation_extensions_spec.rb +++ b/spec/squeel/adapters/active_record/relation_extensions_spec.rb @@ -1067,6 +1067,10 @@ module ActiveRecord describe '#where_unscoping' do + it "isn't broken" do + OrderItem.where(:quantity => 0).unscope(:where => :quantity).where_values.should be_empty + end + it "doesn't ruin everything when predicate expression in where_values doesn't respond to :symbol method" do if activerecord_version_at_least '4.0.0' order_items = OrderItem.where{quantity == 0}.where{unit_price / 2 == 5}