Skip to content

Commit

Permalink
Merge pull request rails#41012 from kamipo/fix_complicated_through_as…
Browse files Browse the repository at this point in the history
…sociation

Fix complicated has_many through with nested where condition
  • Loading branch information
kamipo authored Jan 5, 2021
2 parents 968de9c + cbb19eb commit 21b8d9b
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 12 deletions.
13 changes: 8 additions & 5 deletions activerecord/lib/active_record/associations/association_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,14 @@ def add_constraints(scope, owner, chain)
if scope_chain_item == chain_head.scope
scope.merge! item.except(:where, :includes, :unscope, :order)
elsif !item.references_values.empty?
join_dependency = item.construct_join_dependency(
item.eager_load_values | item.includes_values, Arel::Nodes::OuterJoin
)
scope.joins!(*item.joins_values, join_dependency)
scope.left_outer_joins!(*item.left_outer_joins_values)
scope.joins_values |= item.joins_values
scope.left_outer_joins_values |= item.left_outer_joins_values

associations = item.eager_load_values | item.includes_values

unless associations.empty?
scope.joins_values << item.construct_join_dependency(associations, Arel::Nodes::OuterJoin)
end
end

reflection.all_includes do
Expand Down
8 changes: 4 additions & 4 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ def build_where_clause(opts, rest = []) # :nodoc:
self.references_values |= references unless references.empty?

parts = predicate_builder.build_from_hash(opts) do |table_name|
lookup_reflection_from_join_dependencies(table_name)
lookup_table_klass_from_join_dependencies(table_name)
end
when Arel::Nodes::Node
parts = [opts]
Expand All @@ -1153,9 +1153,9 @@ def build_where_clause(opts, rest = []) # :nodoc:
alias :build_having_clause :build_where_clause

private
def lookup_reflection_from_join_dependencies(table_name)
def lookup_table_klass_from_join_dependencies(table_name)
each_join_dependencies do |join|
return join.reflection if table_name == join.table_name
return join.base_klass if table_name == join.table_name
end
nil
end
Expand Down Expand Up @@ -1366,7 +1366,7 @@ def arel_column(field)
elsif field.match?(/\A\w+\.\w+\z/)
table, column = field.split(".")
predicate_builder.resolve_arel_attribute(table, column) do
lookup_reflection_from_join_dependencies(table)
lookup_table_klass_from_join_dependencies(table)
end
else
yield field
Expand Down
9 changes: 6 additions & 3 deletions activerecord/lib/active_record/table_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,13 @@ def associated_table(table_name)
return self
end

reflection ||= yield table_name if block_given?
if reflection
association_klass = reflection.klass unless reflection.polymorphic?
elsif block_given?
association_klass = yield table_name
end

if reflection && !reflection.polymorphic?
association_klass = reflection.klass
if association_klass
arel_table = association_klass.arel_table
arel_table = arel_table.alias(table_name) if arel_table.name != table_name
TableMetadata.new(association_klass, arel_table, reflection)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ def test_through_association_with_left_joins
assert_equal [comments(:eager_other_comment1)], authors(:mary).comments.merge(Post.left_joins(:comments))
end

def test_through_association_with_through_scope_and_nested_where
company = Company.create!(name: "special")
developer = SpecialDeveloper.create!
SpecialContract.create!(company: company, special_developer: developer)

assert_equal [developer], company.special_developers.where.not("contracts.id": nil)
end

def test_preload_with_nested_association
posts = Post.where(id: [authors(:david).id, authors(:mary).id]).
preload(:author, :author_favorites_with_scope).order(:id).to_a
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/company.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class Company < AbstractCompany
has_one :dummy_account, foreign_key: "firm_id", class_name: "Account"
has_many :contracts
has_many :developers, through: :contracts
has_many :special_contracts, -> { includes(:special_developer).where.not("developers.id": nil) }
has_many :special_developers, through: :special_contracts

alias_attribute :new_name, :name
attribute :metadata, :json
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,9 @@ def update_metadata
class NewContract < Contract
validates :company_id, presence: true
end

class SpecialContract < ActiveRecord::Base
self.table_name = "contracts"
belongs_to :company
belongs_to :special_developer, foreign_key: "developer_id"
end
5 changes: 5 additions & 0 deletions activerecord/test/models/developer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ def track_instance_count
class SubDeveloper < Developer
end

class SpecialDeveloper < ActiveRecord::Base
self.table_name = "developers"
has_many :special_contracts, foreign_key: "developer_id"
end

class SymbolIgnoredDeveloper < ActiveRecord::Base
self.table_name = "developers"
self.ignored_columns = [:first_name, :last_name]
Expand Down

0 comments on commit 21b8d9b

Please sign in to comment.