Skip to content
This repository
Browse code

Fix various issues with the :primary_key option in :through associati…

…ons [#2421 state:resolved]
  • Loading branch information...
commit 14b880fd035fcdf807051398674c9aa89bd3b4d3 1 parent 09ddca6
Jon Leighton authored December 15, 2010 tenderlove committed December 15, 2010
26  activerecord/lib/active_record/associations/through_association_scope.rb
@@ -39,22 +39,22 @@ def aliased_through_table
39 39
       # Build SQL conditions from attributes, qualified by table name.
40 40
       def construct_conditions
41 41
         table = aliased_through_table
42  
-        conditions = construct_quoted_owner_attributes(@reflection.through_reflection).map do |attr, value|
  42
+        conditions = construct_owner_attributes(@reflection.through_reflection).map do |attr, value|
43 43
           table[attr].eq(value)
44 44
         end
45 45
         conditions << Arel.sql(sql_conditions) if sql_conditions
46 46
         table.create_and(conditions)
47 47
       end
48 48
 
49  
-      # Associate attributes pointing to owner, quoted.
50  
-      def construct_quoted_owner_attributes(reflection)
  49
+      # Associate attributes pointing to owner
  50
+      def construct_owner_attributes(reflection)
51 51
         if as = reflection.options[:as]
52  
-          { "#{as}_id" => @owner.id,
  52
+          { "#{as}_id"   => @owner[reflection.active_record_primary_key],
53 53
             "#{as}_type" => @owner.class.base_class.name }
54 54
         elsif reflection.macro == :belongs_to
55 55
           { reflection.klass.primary_key => @owner[reflection.primary_key_name] }
56 56
         else
57  
-          { reflection.primary_key_name => @owner.id }
  57
+          { reflection.primary_key_name => @owner[reflection.active_record_primary_key] }
58 58
         end
59 59
       end
60 60
 
@@ -74,7 +74,8 @@ def construct_joins
74 74
         conditions = []
75 75
 
76 76
         if @reflection.source_reflection.macro == :belongs_to
77  
-          reflection_primary_key = @reflection.klass.primary_key
  77
+          reflection_primary_key = @reflection.source_reflection.options[:primary_key] ||
  78
+                                   @reflection.klass.primary_key
78 79
           source_primary_key     = @reflection.source_reflection.primary_key_name
79 80
           if @reflection.options[:source_type]
80 81
             column = @reflection.source_reflection.options[:foreign_type]
@@ -83,7 +84,8 @@ def construct_joins
83 84
           end
84 85
         else
85 86
           reflection_primary_key = @reflection.source_reflection.primary_key_name
86  
-          source_primary_key     = @reflection.through_reflection.klass.primary_key
  87
+          source_primary_key     = @reflection.source_reflection.options[:primary_key] ||
  88
+                                   @reflection.through_reflection.klass.primary_key
87 89
           if @reflection.source_reflection.options[:as]
88 90
             column = "#{@reflection.source_reflection.options[:as]}_type"
89 91
             conditions <<
@@ -99,16 +101,6 @@ def construct_joins
99 101
           right.create_on(right.create_and(conditions)))
100 102
       end
101 103
 
102  
-      # Construct attributes for associate pointing to owner.
103  
-      def construct_owner_attributes(reflection)
104  
-        if as = reflection.options[:as]
105  
-          { "#{as}_id" => @owner.id,
106  
-            "#{as}_type" => @owner.class.base_class.name }
107  
-        else
108  
-          { reflection.primary_key_name => @owner.id }
109  
-        end
110  
-      end
111  
-
112 104
       # Construct attributes for :through pointing to owner and associate.
113 105
       def construct_join_attributes(associate)
114 106
         # TODO: revisit this to allow it for deletion, supposing dependent option is supported
6  activerecord/lib/active_record/reflection.rb
@@ -205,7 +205,11 @@ def primary_key_column
205 205
       end
206 206
 
207 207
       def association_foreign_key
208  
-        @association_foreign_key ||= @options[:association_foreign_key] || class_name.foreign_key
  208
+        @association_foreign_key ||= options[:association_foreign_key] || class_name.foreign_key
  209
+      end
  210
+
  211
+      def active_record_primary_key
  212
+        @active_record_primary_key ||= options[:primary_key] || active_record.primary_key
209 213
       end
210 214
 
211 215
       def counter_cache_column
16  activerecord/test/cases/associations/join_model_test.rb
@@ -298,6 +298,22 @@ def test_has_many_going_through_join_model_with_custom_foreign_key
298 298
     assert_equal [authors(:mary)], posts(:authorless).authors
299 299
   end
300 300
 
  301
+  def test_has_many_going_through_join_model_with_custom_primary_key
  302
+    assert_equal [authors(:david)], posts(:thinking).authors_using_author_id
  303
+  end
  304
+
  305
+  def test_has_many_going_through_polymorphic_join_model_with_custom_primary_key
  306
+    assert_equal [tags(:general)], posts(:eager_other).tags_using_author_id
  307
+  end
  308
+
  309
+  def test_has_many_through_with_custom_primary_key_on_belongs_to_source
  310
+    assert_equal [authors(:david), authors(:david)], posts(:thinking).author_using_custom_pk
  311
+  end
  312
+
  313
+  def test_has_many_through_with_custom_primary_key_on_has_many_source
  314
+    assert_equal [authors(:david)], posts(:thinking).authors_using_custom_pk
  315
+  end
  316
+
301 317
   def test_both_scoped_and_explicit_joins_should_be_respected
302 318
     assert_nothing_raised do
303 319
       Post.send(:with_scope, :find => {:joins => "left outer join comments on comments.id = posts.id"}) do
5  activerecord/test/cases/reflection_test.rb
@@ -191,6 +191,11 @@ def test_has_many_through_reflection
191 191
     assert_kind_of ThroughReflection, Subscriber.reflect_on_association(:books)
192 192
   end
193 193
 
  194
+  def test_active_record_primary_key
  195
+    assert_equal "nick", Subscriber.reflect_on_association(:subscriptions).active_record_primary_key.to_s
  196
+    assert_equal "name", Author.reflect_on_association(:essay).active_record_primary_key.to_s
  197
+  end
  198
+
194 199
   def test_collection_association
195 200
     assert Pirate.reflect_on_association(:birds).collection?
196 201
     assert Pirate.reflect_on_association(:parrots).collection?
3  activerecord/test/models/categorization.rb
@@ -2,6 +2,9 @@ class Categorization < ActiveRecord::Base
2 2
   belongs_to :post
3 3
   belongs_to :category
4 4
   belongs_to :author
  5
+
  6
+  belongs_to :author_using_custom_pk,  :class_name => 'Author', :foreign_key => :author_id, :primary_key => :author_address_extra_id
  7
+  has_many   :authors_using_custom_pk, :class_name => 'Author', :foreign_key => :id,        :primary_key => :category_id
5 8
 end
6 9
 
7 10
 class SpecialCategorization < ActiveRecord::Base
10  activerecord/test/models/post.rb
@@ -69,6 +69,16 @@ def add_joins_and_select
69 69
   has_many :categorizations, :foreign_key => :category_id
70 70
   has_many :authors, :through => :categorizations
71 71
 
  72
+  has_many :categorizations_using_author_id, :primary_key => :author_id, :foreign_key => :post_id, :class_name => 'Categorization'
  73
+  has_many :authors_using_author_id, :through => :categorizations_using_author_id, :source => :author
  74
+
  75
+  has_many :taggings_using_author_id, :primary_key => :author_id, :as => :taggable, :class_name => 'Tagging'
  76
+  has_many :tags_using_author_id, :through => :taggings_using_author_id, :source => :tag
  77
+
  78
+  has_many :standard_categorizations, :class_name => 'Categorization', :foreign_key => :post_id
  79
+  has_many :author_using_custom_pk,  :through => :standard_categorizations
  80
+  has_many :authors_using_custom_pk, :through => :standard_categorizations
  81
+
72 82
   has_many :readers
73 83
   has_many :readers_with_person, :include => :person, :class_name => "Reader"
74 84
   has_many :people, :through => :readers

0 notes on commit 14b880f

Please sign in to comment.
Something went wrong with that request. Please try again.