Skip to content

Commit 207747e

Browse files
committed
Fix disable_joins when using an enum STI type
When a model has an STI enum type, the type wasn't properly applied when disable joins was set to true. In this case we need to apply the scope from the association in `add_constraints` so that `type` is included in the query. Otherwise in a `has_one :through` with `type` all records will be returned because we're not filtering on type. Long term I think this might be in the wrong place and that we want to do this in a new definition of `target_scope` but that's a future refactoring that needs to be done.
1 parent 25f2341 commit 207747e

File tree

3 files changed

+29
-1
lines changed

3 files changed

+29
-1
lines changed

activerecord/lib/active_record/associations/disable_joins_association_scope.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ def last_scope_chain(reverse_chain, owner)
3232

3333
def add_constraints(reflection, key, join_ids, owner, ordered)
3434
scope = reflection.build_scope(reflection.aliased_table).where(key => join_ids)
35+
36+
relation = reflection.klass.scope_for_association
37+
scope.merge!(
38+
relation.except(:select, :create_with, :includes, :preload, :eager_load, :joins, :left_outer_joins)
39+
)
40+
3541
scope = reflection.constraints.inject(scope) do |memo, scope_chain_item|
3642
item = eval_scope(reflection, scope_chain_item, owner)
3743
scope.unscope!(*item.unscope_values)

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
@@ -8,9 +8,11 @@
88
require "models/developer"
99
require "models/company"
1010
require "models/computer"
11+
require "models/club"
12+
require "models/membership"
1113

1214
class HasOneThroughDisableJoinsAssociationsTest < ActiveRecord::TestCase
13-
fixtures :members, :organizations, :projects, :developers, :companies
15+
fixtures :members, :organizations, :projects, :developers, :companies, :clubs, :memberships
1416

1517
def setup
1618
@member = members(:groucho)
@@ -61,4 +63,23 @@ def test_has_one_through_with_belongs_to_on_disable_joins
6163
assert_no_match(/INNER JOIN/, nj)
6264
end
6365
end
66+
67+
def test_disable_joins_through_with_enum_type
68+
joins = capture_sql { @member.club }
69+
no_joins = capture_sql { @member.club_without_joins }
70+
71+
assert_equal 1, joins.size
72+
assert_equal 2, no_joins.size
73+
74+
assert_match(/INNER JOIN/, joins.first)
75+
no_joins.each do |nj|
76+
assert_no_match(/INNER JOIN/, nj)
77+
end
78+
79+
if current_adapter?(:Mysql2Adapter)
80+
assert_match(/`memberships`.`type`/, no_joins.first)
81+
else
82+
assert_match(/"memberships"."type"/, no_joins.first)
83+
end
84+
end
6485
end

activerecord/test/models/member.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ class Member < ActiveRecord::Base
55
has_one :selected_membership
66
has_one :membership
77
has_one :club, through: :current_membership
8+
has_one :club_without_joins, through: :current_membership, source: :club, disable_joins: true
89
has_one :selected_club, through: :selected_membership, source: :club
910
has_one :favorite_club, -> { where "memberships.favorite = ?", true }, through: :membership, source: :club
1011
has_one :hairy_club, -> { where clubs: { name: "Moustache and Eyebrow Fancier Club" } }, through: :membership, source: :club

0 commit comments

Comments
 (0)