diff options
author | Yves Senn <yves.senn@gmail.com> | 2013-05-21 17:59:37 +0200 |
---|---|---|
committer | Yves Senn <yves.senn@gmail.com> | 2013-05-23 07:38:33 +0200 |
commit | ef99c1147592e91bb256952986470592ea0e5f6c (patch) | |
tree | 50f10c0b8e4d40314f0324b5286400e1f608a6fb /activerecord | |
parent | 30d28b19584783218e842ce2fd7bfe2bc1dccf66 (diff) | |
download | rails-ef99c1147592e91bb256952986470592ea0e5f6c.tar.gz rails-ef99c1147592e91bb256952986470592ea0e5f6c.tar.bz2 rails-ef99c1147592e91bb256952986470592ea0e5f6c.zip |
Fix the `:primary_key` option for `has_many` associations.
When removing records from a `has_many` association it used
the `primary_key` defined on the association.
Our test suite didn't fail because on all occurences of `:primary_key`,
the specified column was available in both tables. This prevented the
code from raising an exception but it still behaved badly.
I added a test-case to prevent regressions that failed with:
```
1) Error:
HasManyAssociationsTest#test_has_many_assignment_with_custom_primary_key:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: essays.first_name: UPDATE "essays" SET "writer_id" = NULL WHERE "essays"."writer_id" = ? AND "essays"."first_name" IS NULL
```
Diffstat (limited to 'activerecord')
4 files changed, 15 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 09ed2bdc0c..3efd473627 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix the `:primary_key` option for `has_many` associations. + Fixes #10693. + + *Yves Senn* + * Fix bug where tiny types are incorectly coerced as booleand when the length is more than 1. Fixes #10620. diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 29fae809da..cf8a589496 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -115,8 +115,7 @@ module ActiveRecord if records == :all scope = self.scope else - keys = records.map { |r| r[reflection.association_primary_key] } - scope = self.scope.where(reflection.association_primary_key => keys) + scope = self.scope.where(reflection.klass.primary_key => records) end if method == :delete_all diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index d85570236f..058c812dca 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1472,6 +1472,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal david.essays, Essay.where(writer_id: "David") end + def test_has_many_assignment_with_custom_primary_key + david = people(:david) + + assert_equal ["A Modest Proposal"], david.essays.map(&:name) + david.essays = [Essay.create!(name: "Remote Work" )] + assert_equal ["Remote Work"], david.essays.map(&:name) + end + def test_blank_custom_primary_key_on_new_record_should_not_run_queries author = Author.new assert !author.essays.loaded? diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 2985160d28..1a282dbce4 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -32,6 +32,7 @@ class Person < ActiveRecord::Base has_many :agents_posts, :through => :agents, :source => :posts has_many :agents_posts_authors, :through => :agents_posts, :source => :author + has_many :essays, primary_key: "first_name", foreign_key: "writer_id" scope :males, -> { where(:gender => 'M') } scope :females, -> { where(:gender => 'F') } |