Skip to content

Commit 566c403

Browse files
committed
Fix disable_joins when foreign key is not ID
If you have a `has_one through` assocation where the `through` has a `belongs_to` the foreign key won't be `id` for the joins. This change ensures we use the correct foreign key when querying with disable joins turned off. Prior to this change the call to `project.lead_developer` would return `nil` if `disable_joins` was set. Project has one lead developer through firm. Project also belongs to firm. So when we call `project.lead_developer` it needs to find the lead developer through the firm. In these cases we don't want to read the owners id, we want to read the attribute that will give us the value of the first chain item's `join_foreign_key`.
1 parent 9bb2512 commit 566c403

File tree

3 files changed

+25
-2
lines changed

3 files changed

+25
-2
lines changed

activerecord/lib/active_record/associations/disable_joins_association_scope.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ def scope(association)
1616

1717
private
1818
def last_scope_chain(reverse_chain, owner)
19-
first_scope = [reverse_chain.shift, false, [owner.id]]
19+
first_item = reverse_chain.shift
20+
first_scope = [first_item, false, [owner._read_attribute(first_item.join_foreign_key)]]
2021

2122
reverse_chain.inject(first_scope) do |(reflection, ordered, join_ids), next_reflection|
2223
key = reflection.join_primary_key

activerecord/test/cases/associations/has_one_through_disable_joins_associations_test.rb

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
require "models/member"
55
require "models/member_detail"
66
require "models/organization"
7+
require "models/project"
8+
require "models/developer"
9+
require "models/company"
10+
require "models/computer"
711

812
class HasOneThroughDisableJoinsAssociationsTest < ActiveRecord::TestCase
9-
fixtures :members, :organizations
13+
fixtures :members, :organizations, :projects, :developers, :companies
1014

1115
def setup
1216
@member = members(:groucho)
@@ -40,4 +44,21 @@ def test_preload_on_disable_joins_through
4044
assert_no_queries { members[0].organization }
4145
assert_no_queries { members[0].organization_without_joins }
4246
end
47+
48+
def test_has_one_through_with_belongs_to_on_disable_joins
49+
firm = Firm.create!(name: "Adequate Holdings")
50+
project = Project.create!(name: "Project 1", firm: firm)
51+
Developer.create!(name: "Gorbypuff", firm: firm)
52+
53+
joins = capture_sql { project.lead_developer }
54+
no_joins = capture_sql { project.lead_developer_disable_joins }
55+
56+
assert_equal project.lead_developer, project.lead_developer_disable_joins
57+
assert_equal 2, no_joins.count
58+
assert_equal 1, joins.count
59+
assert_match(/INNER JOIN/, joins.first)
60+
no_joins.each do |nj|
61+
assert_no_match(/INNER JOIN/, nj)
62+
end
63+
end
4364
end

activerecord/test/models/project.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class Project < ActiveRecord::Base
1616
has_and_belongs_to_many :well_paid_salary_groups, -> { group("developers.salary").having("SUM(salary) > 10000").select("SUM(salary) as salary") }, class_name: "Developer"
1717
belongs_to :firm
1818
has_one :lead_developer, through: :firm, inverse_of: :contracted_projects
19+
has_one :lead_developer_disable_joins, through: :firm, inverse_of: :contracted_projects, source: :lead_developer, disable_joins: true
1920

2021
begin
2122
previous_value, ActiveRecord::Base.belongs_to_required_by_default =

0 commit comments

Comments
 (0)