aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-06-30 09:53:56 -0700
committerSean Griffin <sean@thoughtbot.com>2015-06-30 10:00:30 -0700
commitbc6ac8609cf79f28047c0928b9433e00e6ea1f09 (patch)
treec7aba6513e16d0abb3e700ad87ee558929df4945 /activerecord
parentf55c60f5aec67aaa2b1614a59f9a3639f966ea30 (diff)
downloadrails-bc6ac8609cf79f28047c0928b9433e00e6ea1f09.tar.gz
rails-bc6ac8609cf79f28047c0928b9433e00e6ea1f09.tar.bz2
rails-bc6ac8609cf79f28047c0928b9433e00e6ea1f09.zip
Correct through associations using scopes
The changes introduced to through associations in c80487eb were quite interesting. Changing `relation.merge!(scope)` to `relation = relation.merge(scope)` should in theory never cause any changes in behavior. The subtle breakage led to a surprising conclusion. The old code wasn't doing anything! Since `merge!` calls `instance_exec` when given a proc, and most scopes will look something like `has_many :foos, -> { where(foo: :bar) }`, if we're not capturing the return value, it's a no-op. However, removing the `merge` causes `unscope` to break. While we're merging in the rest of the chain elsewhere, we were never merging in `unscope` values, causing a breakage on associations where a default scope was being unscoped in an association scope (yuk!). This is subtly related to #20722, since it appears we were previously relying on this mutability. Fixes #20721. Fixes #20727.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md8
-rw-r--r--activerecord/lib/active_record/associations/association_scope.rb1
-rw-r--r--activerecord/lib/active_record/associations/through_association.rb6
-rw-r--r--activerecord/test/cases/associations/has_many_through_associations_test.rb7
4 files changed, 16 insertions, 6 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 109f6f3cd4..6054b08f33 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+* Fix through associations using scopes having the scope merged multiple
+ times.
+
+ Fixes #20721.
+ Fixes #20727.
+
+ *Sean Griffin*
+
* `ActiveRecord::Base.dump_schema_after_migration` applies migration tasks
other than `db:migrate`. (eg. `db:rollback`, `db:migrate:dup`, ...)
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index 2416167834..a140dc239c 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -149,6 +149,7 @@ module ActiveRecord
scope.where_clause += item.where_clause
scope.order_values |= item.order_values
+ scope.unscope!(*item.unscope_values)
end
reflection = reflection.next
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index af1bce523c..55ee9f04e0 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -15,12 +15,6 @@ module ActiveRecord
scope = super
reflection.chain.drop(1).each do |reflection|
relation = reflection.klass.all
-
- reflection_scope = reflection.scope
- if reflection_scope && reflection_scope.arity.zero?
- relation = relation.merge(reflection_scope)
- end
-
scope.merge!(
relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load)
)
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 1d545af5a5..c34387f06d 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1158,4 +1158,11 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
post_through = organization.posts.build
assert_equal post_direct.author_id, post_through.author_id
end
+
+ def test_has_many_through_with_scope_that_should_not_be_fully_merged
+ Club.has_many :distinct_memberships, -> { distinct }, class_name: "Membership"
+ Club.has_many :special_favourites, through: :distinct_memberships, source: :member
+
+ assert_nil Club.new.special_favourites.distinct_value
+ end
end