From 207747ec2d07699c30cd06c1bcf5bacd567c9417 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Thu, 24 Jun 2021 10:08:43 -0400 Subject: [PATCH] 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. --- .../disable_joins_association_scope.rb | 6 +++++ ...through_disable_joins_associations_test.rb | 23 ++++++++++++++++++- activerecord/test/models/member.rb | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/disable_joins_association_scope.rb b/activerecord/lib/active_record/associations/disable_joins_association_scope.rb index 87ab8aef92..8ae89539a5 100644 --- a/activerecord/lib/active_record/associations/disable_joins_association_scope.rb +++ b/activerecord/lib/active_record/associations/disable_joins_association_scope.rb @@ -32,6 +32,12 @@ module ActiveRecord def add_constraints(reflection, key, join_ids, owner, ordered) scope = reflection.build_scope(reflection.aliased_table).where(key => join_ids) + + relation = reflection.klass.scope_for_association + scope.merge!( + relation.except(:select, :create_with, :includes, :preload, :eager_load, :joins, :left_outer_joins) + ) + scope = reflection.constraints.inject(scope) do |memo, scope_chain_item| item = eval_scope(reflection, scope_chain_item, owner) scope.unscope!(*item.unscope_values) diff --git a/activerecord/test/cases/associations/has_one_through_disable_joins_associations_test.rb b/activerecord/test/cases/associations/has_one_through_disable_joins_associations_test.rb index a1994fa819..28c544321e 100644 --- a/activerecord/test/cases/associations/has_one_through_disable_joins_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_disable_joins_associations_test.rb @@ -8,9 +8,11 @@ require "models/project" require "models/developer" require "models/company" require "models/computer" +require "models/club" +require "models/membership" class HasOneThroughDisableJoinsAssociationsTest < ActiveRecord::TestCase - fixtures :members, :organizations, :projects, :developers, :companies + fixtures :members, :organizations, :projects, :developers, :companies, :clubs, :memberships def setup @member = members(:groucho) @@ -61,4 +63,23 @@ class HasOneThroughDisableJoinsAssociationsTest < ActiveRecord::TestCase assert_no_match(/INNER JOIN/, nj) end end + + def test_disable_joins_through_with_enum_type + joins = capture_sql { @member.club } + no_joins = capture_sql { @member.club_without_joins } + + assert_equal 1, joins.size + assert_equal 2, no_joins.size + + assert_match(/INNER JOIN/, joins.first) + no_joins.each do |nj| + assert_no_match(/INNER JOIN/, nj) + end + + if current_adapter?(:Mysql2Adapter) + assert_match(/`memberships`.`type`/, no_joins.first) + else + assert_match(/"memberships"."type"/, no_joins.first) + end + end end diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb index 40e88e7621..f69aac061e 100644 --- a/activerecord/test/models/member.rb +++ b/activerecord/test/models/member.rb @@ -5,6 +5,7 @@ class Member < ActiveRecord::Base has_one :selected_membership has_one :membership has_one :club, through: :current_membership + has_one :club_without_joins, through: :current_membership, source: :club, disable_joins: true has_one :selected_club, through: :selected_membership, source: :club has_one :favorite_club, -> { where "memberships.favorite = ?", true }, through: :membership, source: :club has_one :hairy_club, -> { where clubs: { name: "Moustache and Eyebrow Fancier Club" } }, through: :membership, source: :club