diff options
Diffstat (limited to 'activerecord')
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 65 | ||||
| -rw-r--r-- | activerecord/lib/active_record/nested_attributes.rb | 83 | ||||
| -rwxr-xr-x | activerecord/test/cases/base_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/cases/nested_attributes_test.rb | 66 | ||||
| -rw-r--r-- | activerecord/test/models/pirate.rb | 4 |
5 files changed, 168 insertions, 52 deletions
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 502fe0442e..150e3fc263 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2427,6 +2427,29 @@ module ActiveRecord #:nodoc: result end + # Cloned objects have no id assigned and are treated as new records. Note that this is a "shallow" clone + # as it copies the object's attributes only, not its associations. The extent of a "deep" clone is + # application specific and is therefore left to the application to implement according to its need. + def initialize_copy(other) + # Think the assertion which fails if the after_initialize callback goes at the end of the method is wrong. The + # deleted clone method called new which therefore called the after_initialize callback. It then went on to copy + # over the attributes. But if it's copying the attributes afterwards then it hasn't finished initializing right? + # For example in the test suite the topic model's after_initialize method sets the author_email_address to + # test@test.com. I would have thought this would mean that all cloned models would have an author email address + # of test@test.com. However the test_clone test method seems to test that this is not the case. As a result the + # after_initialize callback has to be run *before* the copying of the atrributes rather than afterwards in order + # for all tests to pass. This makes no sense to me. + callback(:after_initialize) if respond_to_without_attributes?(:after_initialize) + cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast) + cloned_attributes.delete(self.class.primary_key) + @attributes = cloned_attributes + clear_aggregation_cache + @attributes_cache = {} + @new_record = true + ensure_proper_type + self.class.send(:scope, :create).each { |att, value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create) + end + # Returns a String, which Action Pack uses for constructing an URL to this # object. The default implementation returns this record's id as a String, # or nil if this record's unsaved. @@ -2555,19 +2578,6 @@ module ActiveRecord #:nodoc: freeze end - # Returns a clone of the record that hasn't been assigned an id yet and - # is treated as a new record. Note that this is a "shallow" clone: - # it copies the object's attributes only, not its associations. - # The extent of a "deep" clone is application-specific and is therefore - # left to the application to implement according to its need. - def clone - attrs = clone_attributes(:read_attribute_before_type_cast) - attrs.delete(self.class.primary_key) - record = self.class.new - record.send :instance_variable_set, '@attributes', attrs - record - end - # Returns an instance of the specified +klass+ with the attributes of the current record. This is mostly useful in relation to # single-table inheritance structures where you want a subclass to appear as the superclass. This can be used along with record # identification in Action Pack to allow, say, <tt>Client < Company</tt> to do something like render <tt>:partial => @client.becomes(Company)</tt> @@ -2831,6 +2841,21 @@ module ActiveRecord #:nodoc: "#<#{self.class} #{attributes_as_nice_string}>" end + protected + def clone_attributes(reader_method = :read_attribute, attributes = {}) + self.attribute_names.inject(attributes) do |attrs, name| + attrs[name] = clone_attribute_value(reader_method, name) + attrs + end + end + + def clone_attribute_value(reader_method, attribute_name) + value = send(reader_method, attribute_name) + value.duplicable? ? value.clone : value + rescue TypeError, NoMethodError + value + end + private def create_or_update raise ReadOnlyRecord if readonly? @@ -3093,20 +3118,6 @@ module ActiveRecord #:nodoc: return string unless string.is_a?(String) && string =~ /^---/ YAML::load(string) rescue string end - - def clone_attributes(reader_method = :read_attribute, attributes = {}) - self.attribute_names.inject(attributes) do |attrs, name| - attrs[name] = clone_attribute_value(reader_method, name) - attrs - end - end - - def clone_attribute_value(reader_method, attribute_name) - value = send(reader_method, attribute_name) - value.duplicable? ? value.clone : value - rescue TypeError, NoMethodError - value - end end Base.class_eval do diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 9c29f32639..f65ee9465e 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -3,11 +3,14 @@ require 'active_support/core_ext/object/try' module ActiveRecord module NestedAttributes #:nodoc: + class TooManyRecords < ActiveRecordError + end + extend ActiveSupport::Concern included do - class_inheritable_accessor :reject_new_nested_attributes_procs, :instance_writer => false - self.reject_new_nested_attributes_procs = {} + class_inheritable_accessor :nested_attributes_options, :instance_writer => false + self.nested_attributes_options = {} end # == Nested Attributes @@ -127,6 +130,22 @@ module ActiveRecord # member.posts.first.title # => 'Kari, the awesome Ruby documentation browser!' # member.posts.second.title # => 'The egalitarian assumption of the modern citizen' # + # Alternatively, :reject_if also accepts a symbol for using methods: + # + # class Member < ActiveRecord::Base + # has_many :posts + # accepts_nested_attributes_for :posts, :reject_if => :new_record? + # end + # + # class Member < ActiveRecord::Base + # has_many :posts + # accepts_nested_attributes_for :posts, :reject_if => :reject_posts + # + # def reject_posts(attributed) + # attributed['title].blank? + # end + # end + # # If the hash contains an <tt>id</tt> key that matches an already # associated record, the matching record will be modified: # @@ -179,13 +198,20 @@ module ActiveRecord # <tt>_destroy</tt> key and a value that evaluates to +true+ # (eg. 1, '1', true, or 'true'). This option is off by default. # [:reject_if] - # Allows you to specify a Proc that checks whether a record should be - # built for a certain attribute hash. The hash is passed to the Proc - # and the Proc should return either +true+ or +false+. When no Proc - # is specified a record will be built for all attribute hashes that + # Allows you to specify a Proc or a Symbol pointing to a method + # that checks whether a record should be built for a certain attribute + # hash. The hash is passed to the supplied Proc or the method + # and it should return either +true+ or +false+. When no :reject_if + # is specified, a record will be built for all attribute hashes that # do not have a <tt>_destroy</tt> value that evaluates to true. # Passing <tt>:all_blank</tt> instead of a Proc will create a proc # that will reject a record where all the attributes are blank. + # [:limit] + # Allows you to specify the maximum number of the associated records that + # can be processes with the nested attributes. If the size of the + # nested attributes array exceeds the specified limit, NestedAttributes::TooManyRecords + # exception is raised. If omitted, any number associations can be processed. + # Note that the :limit option is only applicable to one-to-many associations. # # Examples: # # creates avatar_attributes= @@ -197,7 +223,7 @@ module ActiveRecord def accepts_nested_attributes_for(*attr_names) options = { :allow_destroy => false } options.update(attr_names.extract_options!) - options.assert_valid_keys(:allow_destroy, :reject_if) + options.assert_valid_keys(:allow_destroy, :reject_if, :limit) attr_names.each do |association_name| if reflection = reflect_on_association(association_name) @@ -210,10 +236,10 @@ module ActiveRecord reflection.options[:autosave] = true - self.reject_new_nested_attributes_procs[association_name.to_sym] = if options[:reject_if] == :all_blank - proc { |attributes| attributes.all? {|k,v| v.blank?} } - else - options[:reject_if] + self.nested_attributes_options[association_name.to_sym] = options + + if options[:reject_if] == :all_blank + self.nested_attributes_options[association_name.to_sym][:reject_if] = proc { |attributes| attributes.all? {|k,v| v.blank?} } end # def pirate_attributes=(attributes) @@ -221,7 +247,7 @@ module ActiveRecord # end class_eval %{ def #{association_name}_attributes=(attributes) - assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes, #{options[:allow_destroy]}) + assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) end }, __FILE__, __LINE__ else @@ -265,8 +291,9 @@ module ActiveRecord # If the given attributes include a matching <tt>:id</tt> attribute _and_ a # <tt>:_destroy</tt> key set to a truthy value, then the existing record # will be marked for destruction. - def assign_nested_attributes_for_one_to_one_association(association_name, attributes, allow_destroy) - attributes = attributes.stringify_keys + def assign_nested_attributes_for_one_to_one_association(association_name, attributes) + options = self.nested_attributes_options[association_name] + attributes = attributes.with_indifferent_access if attributes['id'].blank? unless reject_new_record?(association_name, attributes) @@ -278,7 +305,7 @@ module ActiveRecord end end elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s - assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy) + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) end end @@ -309,24 +336,30 @@ module ActiveRecord # { :name => 'John' }, # { :id => '2', :_destroy => true } # ]) - def assign_nested_attributes_for_collection_association(association_name, attributes_collection, allow_destroy) + def assign_nested_attributes_for_collection_association(association_name, attributes_collection) + options = self.nested_attributes_options[association_name] + unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" end + if options[:limit] && attributes_collection.size > options[:limit] + raise TooManyRecords, "Maximum #{options[:limit]} records are allowed. Got #{attributes_collection.size} records instead." + end + if attributes_collection.is_a? Hash attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes } end attributes_collection.each do |attributes| - attributes = attributes.stringify_keys + attributes = attributes.with_indifferent_access if attributes['id'].blank? unless reject_new_record?(association_name, attributes) send(association_name).build(attributes.except(*UNASSIGNABLE_KEYS)) end elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s } - assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy) + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) end end end @@ -351,8 +384,18 @@ module ActiveRecord # has_destroy_flag? or if a <tt>:reject_if</tt> proc exists for this # association and evaluates to +true+. def reject_new_record?(association_name, attributes) - has_destroy_flag?(attributes) || - self.class.reject_new_nested_attributes_procs[association_name].try(:call, attributes) + has_destroy_flag?(attributes) || call_reject_if(association_name, attributes) + end + + def call_reject_if(association_name, attributes) + callback = self.nested_attributes_options[association_name][:reject_if] + + case callback + when Symbol + method(callback).arity == 0 ? send(callback) : send(callback, attributes) + when Proc + callback.try(:call, attributes) + end end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8421a8fb07..1e2d903492 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1352,7 +1352,7 @@ class BasicsTest < ActiveRecord::TestCase cloned_topic.title["a"] = "c" assert_equal "b", topic.title["a"] - #test if attributes set as part of after_initialize are cloned correctly + # test if attributes set as part of after_initialize are cloned correctly assert_equal topic.author_email_address, cloned_topic.author_email_address # test if saved clone object differs from original diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 721792132c..53fd168e1b 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -29,13 +29,13 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } end - def test_base_should_have_an_empty_reject_new_nested_attributes_procs - assert_equal Hash.new, ActiveRecord::Base.reject_new_nested_attributes_procs + def test_base_should_have_an_empty_nested_attributes_options + assert_equal Hash.new, ActiveRecord::Base.nested_attributes_options end - def test_should_add_a_proc_to_reject_new_nested_attributes_procs + def test_should_add_a_proc_to_nested_attributes_options [:parrots, :birds, :birds_with_reject_all_blank].each do |name| - assert_instance_of Proc, Pirate.reject_new_nested_attributes_procs[name] + assert_instance_of Proc, Pirate.nested_attributes_options[name][:reject_if] end end @@ -84,6 +84,34 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase ship = Ship.create!(:name => 'Nights Dirty Lightning') ship._delete end + + def test_reject_if_method_without_arguments + Pirate.accepts_nested_attributes_for :ship, :reject_if => :new_record? + + pirate = Pirate.new(:catchphrase => "Stop wastin' me time") + pirate.ship_attributes = { :name => 'Black Pearl' } + assert_no_difference('Ship.count') { pirate.save! } + end + + def test_reject_if_method_with_arguments + Pirate.accepts_nested_attributes_for :ship, :reject_if => :reject_empty_ships_on_create + + pirate = Pirate.new(:catchphrase => "Stop wastin' me time") + pirate.ship_attributes = { :name => 'Red Pearl', :_reject_me_if_new => true } + assert_no_difference('Ship.count') { pirate.save! } + + # pirate.reject_empty_ships_on_create returns false for saved records + pirate.ship_attributes = { :name => 'Red Pearl', :_reject_me_if_new => true } + assert_difference('Ship.count') { pirate.save! } + end + + def test_reject_if_with_indifferent_keys + Pirate.accepts_nested_attributes_for :ship, :reject_if => proc {|attributes| attributes[:name].blank? } + + pirate = Pirate.new(:catchphrase => "Stop wastin' me time") + pirate.ship_attributes = { :name => 'Hello Pearl' } + assert_difference('Ship.count') { pirate.save! } + end end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase @@ -575,3 +603,33 @@ class TestNestedAttributesOnAHasAndBelongsToManyAssociation < ActiveRecord::Test include NestedAttributesOnACollectionAssociationTests end + +class TestNestedAttributesLimit < ActiveRecord::TestCase + def setup + Pirate.accepts_nested_attributes_for :parrots, :limit => 2 + + @pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") + end + + def teardown + Pirate.accepts_nested_attributes_for :parrots, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + end + + def test_limit_with_less_records + @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Big Big Love' } } } + assert_difference('Parrot.count') { @pirate.save! } + end + + def test_limit_with_number_exact_records + @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Lovely Day' }, 'bar' => { :name => 'Blown Away' } } } + assert_difference('Parrot.count', 2) { @pirate.save! } + end + + def test_limit_with_exceeding_records + assert_raises(ActiveRecord::NestedAttributes::TooManyRecords) do + @pirate.attributes = { :parrots_attributes => { 'foo' => { :name => 'Lovely Day' }, + 'bar' => { :name => 'Blown Away' }, + 'car' => { :name => 'The Happening' }} } + end + end +end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 3d7c4bc48a..05c5b666ae 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -45,6 +45,10 @@ class Pirate < ActiveRecord::Base @ship_log ||= [] end + def reject_empty_ships_on_create(attributes) + attributes.delete('_reject_me_if_new').present? && new_record? + end + private def log_before_add(record) log(record, "before_adding_method") |