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.

Bug in nested_attributes when the primary key is not "id"

#4395

In 2ff5f38abb4a44ed5356c34b40d30d446fb63408 a bug was introduced on line 357, in the case the primary key is not called "id".

Example :

create_table 'customer' do |t|
  t.string :name
end
create_table 'invoice', :primary_key => 'id_invoice' do |t|
  t.integer :client_id
  t.string :number
end
class Customer < ActiveRecord::Base
  has_many :invoices
  accepts_nested_attributes_for :invoices
end
class Invoice < ActiveRecord::Base
  set_primary_key 'id_invoice'
  belongs_to :customer
end

> @customer = Customer.create
> @customer.invoices.create
> @customer.update_attributes({invoices_attributes => {"0" => {"id" => "1", "number" => "1523"}}})

will fail because the query generated will be :

SELECT invoices.* FROM invoices WHERE invoice.client_id = 1 AND invoice.id IN ('1')

which is wrong, it should be invoice.id_invoice.

Changing line 357 to do

association.find(attribute_ids)

and not do a all(:conditions ...).

Reported by Mathieu Arnold · April 14th, 2010 @ 04:48 PM

State: resolved
Milestone: none
Assigned to: Pratik Pratik
Importance: Low

Activity

  1. Pratik
    Pratik

    Hey Mathieu,

    We can't use find() as it'll raise an exception on the missing record. I think the fix it to make sure {:id => ..} condition hash uses the appropriate primary key.

    Do you wanna submit a patch for it ? http://guides.rubyonrails.org/contributing_to_rails.html might help if you haven't done it before !

    Thanks.

    April 14th, 2010 @ 04:52 PM

  2. Pratik
    Pratik

    Hey Mathieu,

    We can't use find() as it'll raise an exception on the missing record. I think the fix it to make sure {:id => ..} condition hash uses the appropriate primary key.

    Do you wanna submit a patch for it ? http://guides.rubyonrails.org/contributing_to_rails.html might help if you haven't done it before !

    Thanks.

    April 14th, 2010 @ 04:53 PM

  3. Mathieu Arnold
    Mathieu Arnold

    I had not thought of missing records, in that case, you just need to do a :

    association.all(:conditions => {association.primary_key => attribute_ids})

    As for a patch, well, this fixes it, but for a proper patch with tests, I'll have to think about it :-)

    April 14th, 2010 @ 09:48 PM

  4. Mathieu Arnold
    Mathieu Arnold

    Here is a patch, with a test.

    April 14th, 2010 @ 11:39 PM

  5. Pratik
    Pratik

    The patch looks good. Do you want to upload another patch using git format-patch so that you are attributed properly for your contribution ?

    Thanks.

    April 15th, 2010 @ 12:19 AM

  6. Mathieu Arnold
    Mathieu Arnold

    Here it is.

    April 15th, 2010 @ 11:06 AM

  7. Mathieu Arnold
    Mathieu Arnold

    Any news on this ?

    May 3rd, 2010 @ 01:32 PM

  8. Repository
    Repository
    • State changed from new to resolved

    (from [f194d65f36be29971601664c880cb9b23ee2a95b]) Use primary key in conditions, not 'id' [#4395 Bug in nested_attributes when the primary key is not "id" state:resolved]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com

    Conflicts:

    activerecord/test/cases/nested_attributes_test.rb

    http://github.com/rails/rails/commit/f194d65f36be29971601664c880cb9...

    May 4th, 2010 @ 10:45 PM

  9. Repository
    Repository

    May 4th, 2010 @ 10:45 PM