mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix regression in inverse_of on through associations
`inverse_of` on through associations was accidently removed/caused to
stop working in commit f8d2899
which was part of a refactoring on
`ThroughReflection`.
To fix we moved `inverse_of` and `check_validity_of_inverse!` to the
`AbstractReflection` so it's available to the `ThroughReflection`
without having to dup any methods. We then need to delegate `inverse_name`
method in `ThroughReflection`. `inverse_name` can't be moved to
`AbstractReflection` without moving methods that set the instance
variable `@automatic_inverse_of`.
This adds a test that ensures that `inverse_of` on a `ThroughReflection`
returns the correct class name, and the correct record for the inverse
relationship.
Fixes #21692
This commit is contained in:
parent
7f18ea14c8
commit
ee824c8858
6 changed files with 39 additions and 14 deletions
|
@ -175,6 +175,20 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def inverse_of
|
||||||
|
return unless inverse_name
|
||||||
|
|
||||||
|
@inverse_of ||= klass._reflect_on_association inverse_name
|
||||||
|
end
|
||||||
|
|
||||||
|
def check_validity_of_inverse!
|
||||||
|
unless polymorphic?
|
||||||
|
if has_inverse? && inverse_of.nil?
|
||||||
|
raise InverseOfAssociationNotFoundError.new(self)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# This shit is nasty. We need to avoid the following situation:
|
# This shit is nasty. We need to avoid the following situation:
|
||||||
#
|
#
|
||||||
# * An associated record is deleted via record.destroy
|
# * An associated record is deleted via record.destroy
|
||||||
|
@ -377,14 +391,6 @@ module ActiveRecord
|
||||||
check_validity_of_inverse!
|
check_validity_of_inverse!
|
||||||
end
|
end
|
||||||
|
|
||||||
def check_validity_of_inverse!
|
|
||||||
unless polymorphic?
|
|
||||||
if has_inverse? && inverse_of.nil?
|
|
||||||
raise InverseOfAssociationNotFoundError.new(self)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def check_preloadable!
|
def check_preloadable!
|
||||||
return unless scope
|
return unless scope
|
||||||
|
|
||||||
|
@ -436,12 +442,6 @@ module ActiveRecord
|
||||||
inverse_name
|
inverse_name
|
||||||
end
|
end
|
||||||
|
|
||||||
def inverse_of
|
|
||||||
return unless inverse_name
|
|
||||||
|
|
||||||
@inverse_of ||= klass._reflect_on_association inverse_name
|
|
||||||
end
|
|
||||||
|
|
||||||
def polymorphic_inverse_of(associated_class)
|
def polymorphic_inverse_of(associated_class)
|
||||||
if has_inverse?
|
if has_inverse?
|
||||||
if inverse_relationship = associated_class._reflect_on_association(options[:inverse_of])
|
if inverse_relationship = associated_class._reflect_on_association(options[:inverse_of])
|
||||||
|
@ -924,6 +924,8 @@ module ActiveRecord
|
||||||
klass.primary_key || raise(UnknownPrimaryKey.new(klass))
|
klass.primary_key || raise(UnknownPrimaryKey.new(klass))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def inverse_name; delegate_reflection.send(:inverse_name); end
|
||||||
|
|
||||||
private
|
private
|
||||||
def derive_class_name
|
def derive_class_name
|
||||||
# get the class_name of the belongs_to association of the through reflection
|
# get the class_name of the belongs_to association of the through reflection
|
||||||
|
|
|
@ -13,6 +13,9 @@ require 'models/mixed_case_monkey'
|
||||||
require 'models/admin'
|
require 'models/admin'
|
||||||
require 'models/admin/account'
|
require 'models/admin/account'
|
||||||
require 'models/admin/user'
|
require 'models/admin/user'
|
||||||
|
require 'models/developer'
|
||||||
|
require 'models/company'
|
||||||
|
require 'models/project'
|
||||||
|
|
||||||
class AutomaticInverseFindingTests < ActiveRecord::TestCase
|
class AutomaticInverseFindingTests < ActiveRecord::TestCase
|
||||||
fixtures :ratings, :comments, :cars
|
fixtures :ratings, :comments, :cars
|
||||||
|
@ -198,6 +201,16 @@ class InverseAssociationTests < ActiveRecord::TestCase
|
||||||
belongs_to_ref = Sponsor.reflect_on_association(:sponsor_club)
|
belongs_to_ref = Sponsor.reflect_on_association(:sponsor_club)
|
||||||
assert_nil belongs_to_ref.inverse_of
|
assert_nil belongs_to_ref.inverse_of
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_this_inverse_stuff
|
||||||
|
firm = Firm.create!(name: 'Adequate Holdings')
|
||||||
|
Project.create!(name: 'Project 1', firm: firm)
|
||||||
|
Developer.create!(name: 'Gorbypuff', firm: firm)
|
||||||
|
|
||||||
|
new_project = Project.last
|
||||||
|
assert Project.reflect_on_association(:lead_developer).inverse_of.present?, "Expected inverse of to be present"
|
||||||
|
assert new_project.lead_developer.present?, "Expected lead developer to be present on the project"
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
class InverseHasOneTests < ActiveRecord::TestCase
|
class InverseHasOneTests < ActiveRecord::TestCase
|
||||||
|
|
|
@ -86,6 +86,9 @@ class Firm < Company
|
||||||
|
|
||||||
has_many :association_with_references, -> { references(:foo) }, :class_name => 'Client'
|
has_many :association_with_references, -> { references(:foo) }, :class_name => 'Client'
|
||||||
|
|
||||||
|
has_one :lead_developer, class_name: "Developer"
|
||||||
|
has_many :projects
|
||||||
|
|
||||||
def log
|
def log
|
||||||
@log ||= []
|
@log ||= []
|
||||||
end
|
end
|
||||||
|
|
|
@ -54,6 +54,9 @@ class Developer < ActiveRecord::Base
|
||||||
has_many :ratings, through: :comments
|
has_many :ratings, through: :comments
|
||||||
has_one :ship, dependent: :nullify
|
has_one :ship, dependent: :nullify
|
||||||
|
|
||||||
|
belongs_to :firm
|
||||||
|
has_many :contracted_projects, class_name: "Project"
|
||||||
|
|
||||||
scope :jamises, -> { where(:name => 'Jamis') }
|
scope :jamises, -> { where(:name => 'Jamis') }
|
||||||
|
|
||||||
validates_inclusion_of :salary, :in => 50000..200000
|
validates_inclusion_of :salary, :in => 50000..200000
|
||||||
|
|
|
@ -11,6 +11,8 @@ class Project < ActiveRecord::Base
|
||||||
:before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"},
|
:before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"},
|
||||||
:after_remove => Proc.new {|o, r| o.developers_log << "after_removing#{r.id}"}
|
:after_remove => Proc.new {|o, r| o.developers_log << "after_removing#{r.id}"}
|
||||||
has_and_belongs_to_many :well_payed_salary_groups, -> { group("developers.salary").having("SUM(salary) > 10000").select("SUM(salary) as salary") }, :class_name => "Developer"
|
has_and_belongs_to_many :well_payed_salary_groups, -> { group("developers.salary").having("SUM(salary) > 10000").select("SUM(salary) as salary") }, :class_name => "Developer"
|
||||||
|
belongs_to :firm
|
||||||
|
has_one :lead_developer, through: :firm, inverse_of: :contracted_projects
|
||||||
|
|
||||||
attr_accessor :developers_log
|
attr_accessor :developers_log
|
||||||
after_initialize :set_developers_log
|
after_initialize :set_developers_log
|
||||||
|
|
|
@ -252,6 +252,7 @@ ActiveRecord::Schema.define do
|
||||||
t.string :name
|
t.string :name
|
||||||
t.string :first_name
|
t.string :first_name
|
||||||
t.integer :salary, default: 70000
|
t.integer :salary, default: 70000
|
||||||
|
t.integer :firm_id
|
||||||
if subsecond_precision_supported?
|
if subsecond_precision_supported?
|
||||||
t.datetime :created_at, precision: 6
|
t.datetime :created_at, precision: 6
|
||||||
t.datetime :updated_at, precision: 6
|
t.datetime :updated_at, precision: 6
|
||||||
|
@ -658,6 +659,7 @@ ActiveRecord::Schema.define do
|
||||||
create_table :projects, force: true do |t|
|
create_table :projects, force: true do |t|
|
||||||
t.string :name
|
t.string :name
|
||||||
t.string :type
|
t.string :type
|
||||||
|
t.integer :firm_id
|
||||||
end
|
end
|
||||||
|
|
||||||
create_table :randomly_named_table1, force: true do |t|
|
create_table :randomly_named_table1, force: true do |t|
|
||||||
|
|
Loading…
Reference in a new issue