diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/association_collection.rb | 29 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_one_association.rb | 33 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/validations.rb | 6 | ||||
| -rwxr-xr-x | activerecord/test/validations_test.rb | 25 |
5 files changed, 71 insertions, 24 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index de864e15e8..ddfb883e5f 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Consolidated different create and create! versions to call through to the base class with scope. This fixes inconsistencies, especially related to protected attribtues. Closes #5847 [Alexander Dymo, Tobias Luetke] + * find supports :lock with :include. Check whether your database allows SELECT ... FOR UPDATE with outer joins before using. #6764 [vitaly, Jeremy Kemper] * Add AssociationCollection#create! to be consistent with AssociationCollection#create when dealing with a foreign key that is a protected attribute [Cody Fauser] diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 66b4079f55..80b7658a95 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -84,13 +84,19 @@ module ActiveRecord reset_target! end - - def create(attributes = {}) - build_and_save_with(attributes, :save) + + def create(attrs = {}) + record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create(attrs) } + @target ||= [] unless loaded? + @target << record + record end - def create!(attributes = {}) - build_and_save_with(attributes, :save!) + def create!(attrs = {}) + record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create!(attrs) } + @target ||= [] unless loaded? + @target << record + record end # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and @@ -201,18 +207,7 @@ module ActiveRecord def callbacks_for(callback_name) full_callback_name = "#{callback_name}_for_#{@reflection.name}" @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || [] - end - - def build_and_save_with(attributes, method) - # Can't use Base.create since the foreign key may be a protected attribute. - if attributes.is_a?(Array) - attributes.collect { |attr| create(attr) } - else - record = build(attributes) - record.send(method) unless @owner.new_record? - record - end - end + end end end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 4ccd725ebd..9beddb2ac9 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -6,9 +6,29 @@ module ActiveRecord construct_sql end - def create(attributes = {}, replace_existing = true) - record = build(attributes, replace_existing) - record.save + def create(attrs = {}, replace_existing = true) + record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create(attrs) } + + if replace_existing + replace(record, true) + else + record[@reflection.primary_key_name] = @owner.id unless @owner.new_record? + self.target = record + end + + record + end + + def create!(attrs = {}, replace_existing = true) + record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create!(attrs) } + + if replace_existing + replace(record, true) + else + record[@reflection.primary_key_name] = @owner.id unless @owner.new_record? + self.target = record + end + record end @@ -75,6 +95,13 @@ module ActiveRecord end @finder_sql << " AND (#{conditions})" if conditions end + + def construct_scope + create_scoping = {} + set_belongs_to_association_for(create_scoping) + { :create => create_scoping } + end + end end end diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 3bc0e8fa37..342a4844cc 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -717,12 +717,10 @@ module ActiveRecord # so an exception is raised if the record is invalid. def create!(attributes = nil) if attributes.is_a?(Array) - attributes.collect { |attr| create!(attr) } + attributes.collect { |attr| create(attr) } else - attributes ||= {} - attributes.reverse_merge!(scope(:create)) if scoped?(:create) - object = new(attributes) + scope(:create).each { |att,value| object.send("#{att}=", value) } if scoped?(:create) object.save! object end diff --git a/activerecord/test/validations_test.rb b/activerecord/test/validations_test.rb index 92a11778ef..f193261eb8 100755 --- a/activerecord/test/validations_test.rb +++ b/activerecord/test/validations_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'fixtures/topic' require 'fixtures/reply' +require 'fixtures/person' require 'fixtures/developer' # The following methods in Topic are used in test_conditional_validation_* @@ -14,6 +15,12 @@ class Topic end end +class ProtectedPerson < ActiveRecord::Base + set_table_name 'people' + attr_accessor :addon + attr_protected :first_name +end + class ValidationsTest < Test::Unit::TestCase fixtures :topics, :developers @@ -94,6 +101,24 @@ class ValidationsTest < Test::Unit::TestCase end end + def test_create_with_exceptions_using_scope_for_protected_attributes + assert_nothing_raised do + ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do + person = ProtectedPerson.create! :addon => "Addon" + assert_equal person.first_name, "Mary", "scope should ignore attr_protected" + end + end + end + + def test_create_with_exceptions_using_scope_and_empty_attributes + assert_nothing_raised do + ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do + person = ProtectedPerson.create! + assert_equal person.first_name, "Mary", "should be ok when no attributes are passed to create!" + end + end + end + def test_single_error_per_attr_iteration r = Reply.new r.save |