Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

We shouldn't be using scoped.scoping { ... } to build associated reco…

…rds, as this can affect validations/callbacks/etc inside the record itself [#6252 state:resolved]
  • Loading branch information...
commit 63c73dd0214188dc91442db538e141e30ec3b1b9 1 parent 8adfede
@jonleighton jonleighton authored
View
13 activerecord/lib/active_record/associations/association_collection.rb
@@ -466,17 +466,14 @@ def save_record(record, force, validate)
force ? record.save! : record.save(:validate => validate)
end
- def create_record(attrs, &block)
+ def create_record(attributes, &block)
ensure_owner_is_persisted!
-
- transaction do
- scoped.scoping { build_record(attrs, &block) }
- end
+ transaction { build_record(attributes, &block) }
end
- def build_record(attrs, &block)
- attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
- record = @reflection.build_association(attrs)
+ def build_record(attributes, &block)
+ attributes = scoped.scope_for_create.merge(attributes)
+ record = @reflection.build_association(attributes)
add_record_to_target_with_callbacks(record, &block)
end
View
15 activerecord/lib/active_record/associations/singular_association.rb
@@ -2,9 +2,7 @@ module ActiveRecord
module Associations
class SingularAssociation < AssociationProxy #:nodoc:
def create(attributes = {})
- record = scoped.scoping { @reflection.create_association(attributes) }
- set_new_record(record)
- record
+ new_record(:create, attributes)
end
def create!(attributes = {})
@@ -12,9 +10,7 @@ def create!(attributes = {})
end
def build(attributes = {})
- record = scoped.scoping { @reflection.build_association(attributes) }
- set_new_record(record)
- record
+ new_record(:build, attributes)
end
private
@@ -36,6 +32,13 @@ def check_record(record)
raise_on_type_mismatch(record) if record
record
end
+
+ def new_record(method, attributes)
+ attributes = scoped.scope_for_create.merge(attributes || {})
+ record = @reflection.send("#{method}_association", attributes)
+ set_new_record(record)
+ record
+ end
end
end
end
View
18 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -43,7 +43,7 @@ def test_should_fail
class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :categories, :companies, :developers, :projects,
:developers_projects, :topics, :authors, :comments,
- :people, :posts, :readers, :taggings
+ :people, :posts, :readers, :taggings, :cars
def setup
Client.destroyed_client_ids.clear
@@ -66,6 +66,22 @@ def test_create_from_association_should_respect_default_scope
assert_equal 'exotic', bulb.name
end
+ # When creating objects on the association, we must not do it within a scope (even though it
+ # would be convenient), because this would cause that scope to be applied to any callbacks etc.
+ def test_build_and_create_should_not_happen_within_scope
+ car = cars(:honda)
+ original_scoped_methods = Bulb.scoped_methods
+
+ bulb = car.bulbs.build
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+
+ bulb = car.bulbs.create
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+
+ bulb = car.bulbs.create!
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ end
+
def test_no_sql_should_be_fired_if_association_already_loaded
Car.create(:name => 'honda')
bulbs = Car.first.bulbs
View
15 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -4,6 +4,7 @@
require 'models/company'
require 'models/ship'
require 'models/pirate'
+require 'models/bulb'
class HasOneAssociationsTest < ActiveRecord::TestCase
self.use_transactional_fixtures = false unless supports_savepoints?
@@ -167,6 +168,20 @@ def test_successful_build_association
assert_equal account, firm.account
end
+ def test_build_and_create_should_not_happen_within_scope
+ pirate = pirates(:blackbeard)
+ original_scoped_methods = Bulb.scoped_methods.dup
+
+ bulb = pirate.build_bulb
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+
+ bulb = pirate.create_bulb
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+
+ bulb = pirate.create_bulb!
+ assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ end
+
def test_create_association
firm = Firm.create(:name => "GlobalMegaCorp")
account = firm.create_account(:credit_limit => 1000)
View
11 activerecord/test/models/bulb.rb
@@ -1,7 +1,14 @@
class Bulb < ActiveRecord::Base
-
+
default_scope :conditions => {:name => 'defaulty' }
-
+
belongs_to :car
+ attr_reader :scoped_methods_after_initialize
+
+ after_initialize :record_scoped_methods_after_initialize
+ def record_scoped_methods_after_initialize
+ @scoped_methods_after_initialize = self.class.scoped_methods.dup
+ end
+
end
View
2  activerecord/test/models/pirate.rb
@@ -34,6 +34,8 @@ class Pirate < ActiveRecord::Base
:after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"}
has_many :birds_with_reject_all_blank, :class_name => "Bird"
+ has_one :bulb, :foreign_key => :car_id
+
accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
accepts_nested_attributes_for :update_only_ship, :update_only => true
Please sign in to comment.
Something went wrong with that request. Please try again.