Skip to content

Commit

Permalink
Allow you to pass :all_blank to :reject_if option to automatically cr…
Browse files Browse the repository at this point in the history
…eate a Proc that will reject any record with blank attributes.
  • Loading branch information
hardbap authored and NZKoz committed May 10, 2009
1 parent 026b78f commit 9010ed2
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 2 deletions.
11 changes: 10 additions & 1 deletion activerecord/lib/active_record/nested_attributes.rb
Expand Up @@ -180,10 +180,14 @@ module ClassMethods
# and the Proc should return either +true+ or +false+. When no Proc
# is specified a record will be built for all attribute hashes that
# do not have a <tt>_delete</tt> 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.
#
# Examples:
# # creates avatar_attributes=
# accepts_nested_attributes_for :avatar, :reject_if => proc { |attributes| attributes['name'].blank? }
# # creates avatar_attributes=
# accepts_nested_attributes_for :avatar, :reject_if => :all_blank
# # creates avatar_attributes= and posts_attributes=
# accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true
def accepts_nested_attributes_for(*attr_names)
Expand All @@ -201,7 +205,12 @@ def accepts_nested_attributes_for(*attr_names)
end

reflection.options[:autosave] = true
self.reject_new_nested_attributes_procs[association_name.to_sym] = options[:reject_if]

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]
end

# def pirate_attributes=(attributes)
# assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false)
Expand Down
18 changes: 17 additions & 1 deletion activerecord/test/cases/nested_attributes_test.rb
Expand Up @@ -31,11 +31,27 @@ def test_base_should_have_an_empty_reject_new_nested_attributes_procs
end

def test_should_add_a_proc_to_reject_new_nested_attributes_procs
[:parrots, :birds].each do |name|
[:parrots, :birds, :birds_with_reject_all_blank].each do |name|
assert_instance_of Proc, Pirate.reject_new_nested_attributes_procs[name]
end
end

def test_should_not_build_a_new_record_if_reject_all_blank_returns_false
pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
pirate.birds_with_reject_all_blank_attributes = [{:name => '', :color => ''}]
pirate.save!

assert pirate.birds_with_reject_all_blank.empty?
end

def test_should_build_a_new_record_if_reject_all_blank_does_not_return_false
pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
pirate.birds_with_reject_all_blank_attributes = [{:name => 'Tweetie', :color => ''}]
pirate.save!

assert_equal 1, pirate.birds_with_reject_all_blank.count
end

def test_should_raise_an_ArgumentError_for_non_existing_associations
assert_raise_with_message ArgumentError, "No association found for name `honesty'. Has it been defined yet?" do
Pirate.accepts_nested_attributes_for :honesty
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/pirate.rb
Expand Up @@ -28,11 +28,13 @@ class Pirate < ActiveRecord::Base
:after_add => proc {|p,b| p.ship_log << "after_adding_proc_bird_#{b.id || '<new>'}"},
:before_remove => proc {|p,b| p.ship_log << "before_removing_proc_bird_#{b.id}"},
:after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"}
has_many :birds_with_reject_all_blank, :class_name => "Bird"

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 :parrots_with_method_callbacks, :parrots_with_proc_callbacks,
:birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true
accepts_nested_attributes_for :birds_with_reject_all_blank, :reject_if => :all_blank

validates_presence_of :catchphrase

Expand Down
1 change: 1 addition & 0 deletions activerecord/test/schema/schema.rb
Expand Up @@ -57,6 +57,7 @@ def create_table(*args, &block)

create_table :birds, :force => true do |t|
t.string :name
t.string :color
t.integer :pirate_id
end

Expand Down

5 comments on commit 9010ed2

@bwlang
Copy link

@bwlang bwlang commented on 9010ed2 Jun 8, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's pretty typical to end up with something like this
"choices_attributes"=>{"0"=>{"upperlimit"=>"", "_delete"=>"0", "lowerlimit"=>"", "value"=>""}}

which will fail the :all_blank test since _delete => 0

maybe all except hidden field should be blank? (detecting hiddenness by the _? - ugh)

@hardbap
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well you could do proc { |attributes| attributes.reject {|k,v| k == '_delete'}.all? {|k,v| v.blank?} }

@hardbap
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if you follow this convention you shouldn't have a problem

<% unless order_form.object.new_record? %> 
  <div>
    <%= order_form.label :_delete, 'Remove:' %>
    <%= order_form.check_box :_delete %>
 </div>
<% end %>

@watson
Copy link

@watson watson commented on 9010ed2 Jul 30, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this feature... Any reason why this haven't made it into 2-3-stable yet?

@carter-thaxton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the following:

lambda { |attrs| attrs.all? { |k,v| k == '_delete' || v.blank? } }

It uses lambda, instead of proc, which has a different meaning in ruby 1.9
I don't like to have the new_record? check in my views. Why is that necessary?

Please sign in to comment.