This project is archived and is in readonly mode.

#3052 ✓stale
yanosz

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

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

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

Comments and changes to this ticket

  • George Montana Harkin

    George Montana Harkin June 9th, 2010 @ 06:56 PM

    • 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.

  • Subba

    Subba June 9th, 2010 @ 09:51 PM

    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.

  • George Montana Harkin

    George Montana Harkin June 9th, 2010 @ 10:06 PM

    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!

  • Subba

    Subba June 10th, 2010 @ 11:54 AM

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

  • George Montana Harkin

    George Montana Harkin June 11th, 2010 @ 07:34 PM

    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.

  • George Montana Harkin
  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:37 PM

    • 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.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:37 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages