Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

assign_nested_attributes_for_collection_association ought to assign object's for collection if an id is given but the object is currently not associated

#3052

Hello,

in 2.3.3 in nested_attributes.rb starting at line 284:

def assign_nested_attributes_for_collection_association(association_name, attributes_collection, allow_destroy)
  #Some sanity-checks deleted 
  attributes_collection.each do |attributes|
    attributes = attributes.stringify_keys
    if attributes['id'].blank? #Case a
      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 } #Case b
      assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy)
    end
  end
end

What if attributes['id'] is not blank (Not Case #a), but send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s } is nil? (Not Case b)

Imho this that that either:
1. The record was deleted by a concurrent process (eg another user) - thus there is none or 2. The record is meant to be associated with the current object or 3. The record was removed from the association by a concurrent process (eg another user)

Since the current run is done after what ever happend in 1.,2.,3., I propose a third case

  elsif existing_record = record.class.find_by_id(attributes['id'])







send(association_name) << existing_record





  end

Reported by yanosz · August 14th, 2009 @ 10:24 PM

State: stale
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. George Montana Harkin
    George Montana Harkin
    • Tag set to accepts_nested_attributes_for, belongs_to

    I ran into this issue with 2.3.8. Currently it throws a nested_attributes_record_not_found error on line 288 of nested_attributes.rb.

    This might have already been fixed somewhere else.

    The use case which is causing our issue is we have Model 1 which is being created a new. Model 2 already exists and needs to be associated with Model 1 on create. Since the model_2_attributes params contains an id it throws this error.

    June 9th, 2010 @ 06:56 PM

  2. Subba
    Subba

    Hi George
    is below code explains your problem

    class Ship < ActiveRecord::Base
  has_one :pirate, :inverse_of => :ship
  accepts_nested_attributes_for :pirate, :allow_destroy => true
end
    

    



    
class Pirate < ActiveRecord::Base
  belongs_to :ship, :inverse_of => :pirate
end
    

    



    
pirate = Pirate.create(:name => 'Johnny')
    

    



    
ship = Ship.new
ship.pirate_attributes = { :id => pirate.id }
    

    



    
ActiveRecord::RecordNotFound: Couldn't find Pirate with ID=2 for Ship with ID=

    If so the correct way to assign attribute is

    ship.pirate_attributes = { :pirate_id => pirate.id }

    I wrote this block to create test case for your problem. please apologize me if i misunderstood you.

    June 9th, 2010 @ 09:51 PM

  3. George Montana Harkin
    George Montana Harkin

    thanks subbarao, that looks simliar to the issue we're running into.

    That fix is kind of a pain as we're using fields_for which automatically inserts an :id field into the attributes rather than an :pirate_id attribute.

    so our example is:

    class Ship < ActiveRecord::Base
    has_one :pirate accepts_nested_attributes_for :pirate, :allow_destroy => true end

    class Pirate < ActiveRecord::Base
    belongs_to :ship end

    params:
    { :ship => { :name => 'Titanic', :pirate_attributes => { :id => '2', :name => 'Leeroy' }}}

    ship = Ship.new(params[:ship])

    ActiveRecord::RecordNotFound: Couldn't find Pirate with ID=2 for Ship with ID=

    Should our fix be a patch to fields_for when dealing with a belongs_to relationship to use :pirate_id instead of :id? Will this even work?

    Thanks!

    June 9th, 2010 @ 10:06 PM

  4. Subba
    Subba

    it will fix your problem. nested attributes hash if contains id it believes existing record.

    June 10th, 2010 @ 11:54 AM

  5. George Montana Harkin
    George Montana Harkin

    We shouldn't have to named the id something specific. :id is the name of the field and it should create or assign the related Pirate automatically from that. If it doesn't then there is no point to the rails field name conventions.

    This is definitely a bug. It is now having issues with one to many. There must be other tickets with the same issue. Seems like it is 2.3.8 related.

    June 11th, 2010 @ 07:34 PM

  6. George Montana Harkin
    George Montana Harkin

    June 11th, 2010 @ 07:37 PM

  7. Santiago Pastorino
    Santiago Pastorino
    • State changed from new to open
    • Importance changed from to

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 2nd, 2011 @ 04:37 PM

  8. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:37 PM