Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor association.scoping not to rely on klass.all #37523

Merged
merged 1 commit into from Oct 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions activerecord/lib/active_record/association_relation.rb
Expand Up @@ -17,23 +17,23 @@ def ==(other)

def build(attributes = nil, &block)
block = _deprecated_scope_block("new", &block)
@association.enable_scoping do
scoping { @association.build(attributes, &block) }
@association.scoping(self) do
@association.build(attributes, &block)
end
end
alias new build

def create(attributes = nil, &block)
block = _deprecated_scope_block("create", &block)
@association.enable_scoping do
scoping { @association.create(attributes, &block) }
@association.scoping(self) do
@association.create(attributes, &block)
end
end

def create!(attributes = nil, &block)
block = _deprecated_scope_block("create!", &block)
@association.enable_scoping do
scoping { @association.create!(attributes, &block) }
@association.scoping(self) do
@association.create!(attributes, &block)
end
end

Expand Down
18 changes: 7 additions & 11 deletions activerecord/lib/active_record/associations/association.rb
Expand Up @@ -41,7 +41,7 @@ def initialize(owner, reflection)
reflection.check_validity!

@owner, @reflection = owner, reflection
@enable_scoping = false
@_scope = nil

reset
reset_scope
Expand Down Expand Up @@ -98,7 +98,7 @@ def target=(target)
end

def scope
target_scope.merge!(association_scope)
@_scope&.spawn || target_scope.merge!(association_scope)
end

def reset_scope
Expand Down Expand Up @@ -198,11 +198,11 @@ def create!(attributes = nil, &block)
_create_record(attributes, true, &block)
end

def enable_scoping
@enable_scoping = true
yield
def scoping(relation, &block)
@_scope = relation
relation.scoping(&block)
ensure
@enable_scoping = false
@_scope = nil
end

private
Expand Down Expand Up @@ -235,11 +235,7 @@ def association_scope
# Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
# through association's scope)
def target_scope
AssociationRelation.create(klass, self).merge!(scope_for_association)
end

def scope_for_association
@enable_scoping ? klass.all : klass.scope_for_association
AssociationRelation.create(klass, self).merge!(klass.scope_for_association)
end

def scope_for_create
Expand Down
Expand Up @@ -254,6 +254,15 @@ def test_build_and_create_from_association_should_respect_unscope_over_default_s

bulb = car.bulbs.unscope(where: :name).create!
assert_nil bulb.name

bulb = car.awesome_bulbs.unscope(where: :frickinawesome).build
assert_equal false, bulb.frickinawesome

bulb = car.awesome_bulbs.unscope(where: :frickinawesome).create
assert_equal false, bulb.frickinawesome

bulb = car.awesome_bulbs.unscope(where: :frickinawesome).create!
assert_equal false, bulb.frickinawesome
end

def test_build_from_association_should_respect_scope
Expand Down
Expand Up @@ -688,8 +688,10 @@ def test_associate_with_create

def test_through_record_is_built_when_created_with_where
assert_difference("posts(:thinking).readers.count", 1) do
posts(:thinking).people.where(first_name: "Jeb").create
posts(:thinking).people.where(readers: { skimmer: true }).create(first_name: "Jeb")
end
reader = posts(:thinking).readers.last
assert_equal true, reader.skimmer
end

def test_associate_with_create_and_no_options
Expand Down