This project is archived and is in readonly mode.
Bug in nested_attributes when the primary key is not "id"
Reported by Mathieu Arnold | April 14th, 2010 @ 04:48 PM
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 ...).
Comments and changes to this ticket
-
Pratik April 14th, 2010 @ 04:52 PM
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.
-
Pratik April 14th, 2010 @ 04:53 PM
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.
-
Mathieu Arnold April 14th, 2010 @ 09:48 PM
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 :-)
-
Pratik April 15th, 2010 @ 12:19 AM
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.
-
Repository May 4th, 2010 @ 10:45 PM
- State changed from new to resolved
(from [f194d65f36be29971601664c880cb9b23ee2a95b]) Use primary key in conditions, not 'id' [#4395 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...
-
Repository May 4th, 2010 @ 10:45 PM
(from [38da0ace772e6f9f5e2fff74db76237ab31790fa]) Use primary key in conditions, not 'id' [#4395 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/38da0ace772e6f9f5e2fff74db7623...
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>
People watching this ticket
Attachments
Referenced by
- 4395 Bug in nested_attributes when the primary key is not "id" (from [f194d65f36be29971601664c880cb9b23ee2a95b]) Use pri...
- 4395 Bug in nested_attributes when the primary key is not "id" (from [38da0ace772e6f9f5e2fff74db76237ab31790fa]) Use pri...