Merge pull request #16353 from eileencodes/refactor-join_keys-usage-in-reflection

Refactor join_keys to remove complex conditionals

2 files changed, 29 insertions, 29 deletions

diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
index 719eff9acc..a9d1099871 100644
--- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -37,14 +37,9 @@ module ActiveRecord
             table = tables.shift
             klass = reflection.klass
 
-            case reflection.source_macro
-            when :belongs_to
-              key         = reflection.association_primary_key
-              foreign_key = reflection.foreign_key
-            else
-              key         = reflection.foreign_key
-              foreign_key = reflection.active_record_primary_key
-            end
+            join_keys   = reflection.join_keys(klass)
+            key         = join_keys.key
+            foreign_key = join_keys.foreign_key
 
             constraint = build_constraint(klass, table, key, foreign_table, foreign_key)
 
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 21da011e8a..575e588f35 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -149,19 +149,10 @@ module ActiveRecord
       JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
 
       def join_keys(assoc_klass)
-        if source_macro == :belongs_to
-          if polymorphic?
-            reflection_key = association_primary_key(assoc_klass)
-          else
-            reflection_key = association_primary_key
-          end
-          reflection_foreign_key = foreign_key
-        else
-          reflection_foreign_key = active_record_primary_key
-          reflection_key = foreign_key
-        end
-        JoinKeys.new(reflection_key, reflection_foreign_key)
+        JoinKeys.new(foreign_key, active_record_primary_key)
       end
+
+      def source_macro; macro; end
     end
     # Base class for AggregateReflection and AssociationReflection. Objects of
     # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods.
@@ -354,9 +345,8 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
       end
       alias :check_eager_loadable! :check_preloadable!
 
-      def join_id_for(owner) #:nodoc:
-        key = (source_macro == :belongs_to) ? foreign_key : active_record_primary_key
-        owner[key]
+      def join_id_for(owner) # :nodoc:
+        owner[active_record_primary_key]
       end
 
       def through_reflection
@@ -383,8 +373,6 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
         scope ? [[scope]] : [[]]
       end
 
-      def source_macro; macro; end
-
       def has_inverse?
         inverse_name
       end
@@ -574,7 +562,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
         end
     end
 
-    class HasManyReflection < AssociationReflection #:nodoc:
+    class HasManyReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -584,7 +572,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
       def collection?; true; end
     end
 
-    class HasOneReflection < AssociationReflection #:nodoc:
+    class HasOneReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -594,7 +582,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
       def has_one?; true; end
     end
 
-    class BelongsToReflection < AssociationReflection #:nodoc:
+    class BelongsToReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -602,9 +590,18 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
       def macro; :belongs_to; end
 
       def belongs_to?; true; end
+
+      def join_keys(assoc_klass)
+        key = polymorphic? ? association_primary_key(assoc_klass) : association_primary_key
+        JoinKeys.new(key, foreign_key)
+      end
+
+      def join_id_for(owner) # :nodoc:
+        owner[foreign_key]
+      end
     end
 
-    class HasAndBelongsToManyReflection < AssociationReflection #:nodoc:
+    class HasAndBelongsToManyReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super
       end
@@ -735,6 +732,10 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
         end
       end
 
+      def join_keys(assoc_klass)
+        source_reflection.join_keys(assoc_klass)
+      end
+
       # The macro used by the source association
       def source_macro
         source_reflection.source_macro
@@ -802,6 +803,10 @@ directive on your declaration like:
         through_reflection.options
       end
 
+      def join_id_for(owner) # :nodoc:
+        source_reflection.join_id_for(owner)
+      end
+
       def check_validity!
         if through_reflection.nil?
           raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)