Skip to content

Commit

Permalink
Merge pull request #49264 from Shopify/explicitly-configured-primary-…
Browse files Browse the repository at this point in the history
…key-option-must-take-priority-in-association-primary-key

Explicit `primary_key:` option should always take priority in associations
  • Loading branch information
eileencodes committed Sep 14, 2023
2 parents ff30934 + dcfddc1 commit 910eb47
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 4 deletions.
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -856,10 +856,10 @@ def association_class

# klass option is necessary to support loading polymorphic associations
def association_primary_key(klass = nil)
if !polymorphic? && ((klass || self.klass).has_query_constraints? || options[:query_constraints])
(klass || self.klass).composite_query_constraints_list
elsif primary_key = options[:primary_key]
if primary_key = options[:primary_key]
@association_primary_key ||= -primary_key.to_s
elsif !polymorphic? && ((klass || self.klass).has_query_constraints? || options[:query_constraints])
(klass || self.klass).composite_query_constraints_list
elsif (klass || self.klass).composite_primary_key?
# If klass has composite primary key of shape [:<tenant_key>, :id], infer primary_key as :id
primary_key = (klass || self.klass).primary_key
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/cases/associations_test.rb
Expand Up @@ -260,6 +260,13 @@ def test_belongs_to_association_does_not_use_parent_query_constraints_if_not_con
assert_equal(blog_post, comment.blog_post_by_id)
end

def test_preloads_model_with_query_constraints_by_explicitly_configured_fk_and_pk
comment = sharded_comments(:great_comment_blog_post_one)
comments = Sharded::Comment.where(id: comment.id).preload(:blog_post_by_id).to_a
comment = comments.first
assert_equal(comment.blog_post_by_id, comment.blog_post)
end

def test_append_composite_foreign_key_has_many_association
blog_post = sharded_blog_posts(:great_post_blog_one)
comment = Sharded::Comment.new(body: "Great post! :clap:")
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/reflection_test.rb
Expand Up @@ -29,6 +29,7 @@
require "models/recipe"
require "models/user_with_invalid_relation"
require "models/hardback"
require "models/sharded/comment"

class ReflectionTest < ActiveRecord::TestCase
include ActiveRecord::Reflection
Expand Down Expand Up @@ -614,6 +615,11 @@ def test_automatic_inverse_does_not_suppress_name_error_from_incidental_code
end
end

def test_association_primary_key_uses_explicit_primary_key_option_as_first_priority
actual = Sharded::Comment.reflect_on_association(:blog_post_by_id).association_primary_key
assert_equal "id", actual
end

private
def assert_reflection(klass, association, options)
assert reflection = klass.reflect_on_association(association)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/models/sharded/comment.rb
Expand Up @@ -6,7 +6,7 @@ class Comment < ActiveRecord::Base
query_constraints :blog_id, :id

belongs_to :blog_post
belongs_to :blog_post_by_id, class_name: "Sharded::BlogPost", foreign_key: :blog_post_id
belongs_to :blog_post_by_id, class_name: "Sharded::BlogPost", foreign_key: :blog_post_id, primary_key: :id
belongs_to :blog
end
end

0 comments on commit 910eb47

Please sign in to comment.