mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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.
This commit is contained in:
parent
f55c60f5ae
commit
bc6ac8609c
4 changed files with 16 additions and 6 deletions
|
@ -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`, ...)
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
)
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue