Skip to content

Commit

Permalink
Sort 'new_' records before initializing new records, this is so they …
Browse files Browse the repository at this point in the history
…will retain their position in forms if validations fail.
  • Loading branch information
alloy committed Sep 17, 2008
1 parent 623563f commit 4c740d5
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 23 deletions.
Expand Up @@ -95,25 +95,25 @@ def has_one_with_nested_params(*args)
def define_nested_params_for_has_many_association(attr, destroy_missing, reject_empty)
class_eval do
define_method("#{attr}_with_nested_params=") do |value|
if value.is_a? Hash
if value.is_a?(Hash) || value.is_a?(ActiveSupport::OrderedHash)
if destroy_missing
association = send(attr)
# Get all ids and subtract the ones we received, destroy the remainder
keys = value.keys.map { |key| key.to_s }
association.reject { |x| keys.include? x.id.to_s }.each { |record| record.destroy }
send(attr).reject { |x| keys.include? x.id.to_s }.each { |record| record.destroy }
end

# For existing records and new records that are marked by an id that starts with 'new_'
new_records = []
value.each do |id, attributes|
association ||= send(attr)
if id.is_a?(String) && id.starts_with?('new_')
next if reject_empty && attributes.values.all? { |v| v.blank? }
association.build attributes
# Collect new records marked by an id that starts with 'new_
new_records << [id, attributes] unless reject_empty && attributes.values.all? { |v| v.blank? }
else
# Find the record for this id and assign the attributes
association.detect { |x| x.id == id.to_i }.attributes = attributes
# Find the existing record for this id and assign the attributes
send(attr).detect { |x| x.id == id.to_i }.attributes = attributes
end
end
# Sort and build new records
new_records.sort_by { |id, _| id }.each { |_, attributes| send(attr).build attributes }
else
if value.is_a?(Array) && value.all? { |x| x.is_a?(Hash) }
# For an array full of new record hashes
Expand Down
Expand Up @@ -5,12 +5,21 @@ def fields_for(record_or_name_or_array, *args, &block)
if reflection = @object.class.reflect_on_all_associations.detect { |r| r.name == record_or_name_or_array.to_sym }
if reflection.macro == :has_many
record = args.first
# In the case of a new object use a fictive id which is composited with "new_" and the object_id.
name = "#{object_name}[#{record_or_name_or_array}][#{ record.new_record? ? "new_#{record.object_id}" : record.id}]"
# In the case of a new object use a fictive id which is composited with "new_" and the @child_counter.
name = "#{object_name}[#{record_or_name_or_array}][#{ record.new_record? ? "new_#{child_counter}" : record.id}]"
return @template.fields_for(name, *args, &block)
end
end
end
super
end

private

def child_counter
@child_counter ||= 1
value = @child_counter
@child_counter += 1
value
end
end
Expand Up @@ -49,7 +49,7 @@ def test_should_build_a_form_for_existing_records
assert_dom_equal expected, _erbout
end

def test_should_build_a_form_for_new_records_using_their_object_id_as_a_composited_id
def test_should_build_a_form_for_new_records_using_a_incremental_counter_as_a_composited_id
paco = @visitor.artists.build(:name => 'paco')
poncho = @visitor.artists.build(:name => 'poncho')

Expand All @@ -64,8 +64,8 @@ def test_should_build_a_form_for_new_records_using_their_object_id_as_a_composit
end

expected = '<form action="http://www.example.com" method="post">' +
"<input id=\"visitor_artists__new_#{paco.object_id}_name\" name=\"visitor[artists][new_#{paco.object_id}][name]\" size=\"30\" type=\"text\" value=\"paco\" />" +
"<input id=\"visitor_artists__new_#{poncho.object_id}_name\" name=\"visitor[artists][new_#{poncho.object_id}][name]\" size=\"30\" type=\"text\" value=\"poncho\" />" +
"<input id=\"visitor_artists__new_1_name\" name=\"visitor[artists][new_1][name]\" size=\"30\" type=\"text\" value=\"paco\" />" +
"<input id=\"visitor_artists__new_2_name\" name=\"visitor[artists][new_2][name]\" size=\"30\" type=\"text\" value=\"poncho\" />" +
'</form>'

assert_dom_equal expected, _erbout
Expand All @@ -87,7 +87,7 @@ def test_should_build_a_form_for_existing_and_new_records

expected = '<form action="http://www.example.com" method="post">' +
'<input id="visitor_artists__1_name" name="visitor[artists][1][name]" size="30" type="text" value="paco" />' +
"<input id=\"visitor_artists__new_#{poncho.object_id}_name\" name=\"visitor[artists][new_#{poncho.object_id}][name]\" size=\"30\" type=\"text\" value=\"poncho\" />" +
"<input id=\"visitor_artists__new_1_name\" name=\"visitor[artists][new_1][name]\" size=\"30\" type=\"text\" value=\"poncho\" />" +
'</form>'

assert_dom_equal expected, _erbout
Expand All @@ -98,12 +98,4 @@ def test_should_build_a_form_for_existing_and_new_records
def protect_against_forgery?
false
end

# def method_missing(method, *args, &block)
# if @controller.respond_to? method
# @controller.send(method, *args, &block)
# else
# super
# end
# end
end
Expand Up @@ -76,6 +76,20 @@
@visitor.artists.map(&:name).sort.should == %w{ jack jill joe }
end

it "should sort 'new_' records before adding them to the association" do
Artist.delete_all

artists = ActiveSupport::OrderedHash.new
artists['new_456'] = { :name => 'new_456' }
artists['new_789'] = { :name => 'new_789' }
artists['new_123'] = { :name => 'new_123' }

@valid_alt_params[:artists] = artists
@visitor.update_attributes @valid_alt_params

@visitor.reload.artists.sort_by(&:id).map(&:name).should == %w{ new_123 new_456 new_789 }
end

it "should automatically reject any new record which is empty" do
@valid_alt_params[:artists]["new_12345"] = { :name => '' }

Expand Down

0 comments on commit 4c740d5

Please sign in to comment.