Fail have_many :through when inverse is invalid

When used with a `:through` association where a corresponding
`belongs_to` association has not been set up on the inverse model, the
`have_many` matcher raises this error:

    Failures:

      1) User should have many projects through clients
         Failure/Error: it {  should have_many(:projects).through :clients }
         NoMethodError:
           undefined method `class_name' for nil:NilClass
         # /Users/<USER>/.rvm/gems/ruby-2.2.0/gems/activerecord-4.2.0/lib/active_record/reflection.rb:871:in `derive_class_name'
         # /Users/<USER>/.rvm/gems/ruby-2.2.0/gems/activerecord-4.2.0/lib/active_record/reflection.rb:147:in `class_name'
         # /Users/<USER>/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:1067:in `rescue in class_exists?'
         # /Users/<USER>/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:1064:in `class_exists?'
         # /Users/<USER>/.rvm/gems/ruby-2.2.0/gems/shoulda-matchers-2.8.0/lib/shoulda/matchers/active_record/association_matcher.rb:928:in `matches?'
         # ./spec/models/user_spec.rb:5:in `block (2 levels) in <top (required)>'

Fortunately, ActiveRecord has a `check_validity!` method we can call on
an association. For `:through` associations, this will run through a
litany of checks, one of which is to check for the inverse association,
which we want in this case. We rescue the error that is produced and
include this in the failure message.

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
This commit is contained in:
Mauro George 2015-05-07 20:01:57 -03:00 committed by Elliot Winkler
parent f50f70e9c8
commit c0a1578435
4 changed files with 68 additions and 8 deletions

View File

@ -1144,6 +1144,7 @@ module Shoulda
@subject = subject @subject = subject
association_exists? && association_exists? &&
macro_correct? && macro_correct? &&
validate_inverse_of_through_association &&
(polymorphic? || class_exists?) && (polymorphic? || class_exists?) &&
foreign_key_exists? && foreign_key_exists? &&
primary_key_exists? && primary_key_exists? &&
@ -1198,7 +1199,14 @@ module Shoulda
end end
def expectation def expectation
"#{model_class.name} to have a #{macro} association called #{name}" expectation =
"#{model_class.name} to have a #{macro} association called #{name}"
if through?
expectation << " through #{reflector.has_and_belongs_to_many_name}"
end
expectation
end end
def missing_options def missing_options
@ -1241,6 +1249,14 @@ module Shoulda
end end
end end
def validate_inverse_of_through_association
reflector.validate_inverse_of_through_association!
true
rescue ::ActiveRecord::ActiveRecordError => error
@missing = error.message
false
end
def macro_supports_primary_key? def macro_supports_primary_key?
macro == :belongs_to || macro == :belongs_to ||
([:has_many, :has_one].include?(macro) && !through?) ([:has_many, :has_one].include?(macro) && !through?)

View File

@ -65,16 +65,22 @@ module Shoulda
end end
end end
def validate_inverse_of_through_association!
if through?
reflection.check_validity!
end
end
def has_and_belongs_to_many_name
reflection.options[:through]
end
protected protected
attr_reader :reflection, :subject attr_reader :reflection, :subject
private private
def has_and_belongs_to_many_name
reflection.options[:through]
end
def has_and_belongs_to_many_name_table_name def has_and_belongs_to_many_name_table_name
if has_and_belongs_to_many_reflection if has_and_belongs_to_many_reflection
has_and_belongs_to_many_reflection.table_name has_and_belongs_to_many_reflection.table_name

View File

@ -4,9 +4,23 @@ module Shoulda
module AssociationMatchers module AssociationMatchers
# @private # @private
class ModelReflector class ModelReflector
delegate :associated_class, :through?, :join_table_name, delegate(
:association_relation, :polymorphic?, :foreign_key, :associated_class,
:association_foreign_key, to: :reflection :association_foreign_key,
:association_relation,
:foreign_key,
:has_and_belongs_to_many_name,
:join_table_name,
:polymorphic?,
:validate_inverse_of_through_association!,
to: :reflection
)
delegate(
:through?,
to: :reflection,
allow_nil: true
)
def initialize(subject, name) def initialize(subject, name)
@subject = subject @subject = subject

View File

@ -847,6 +847,15 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
expect(having_many_children).to have_many(:children) expect(having_many_children).to have_many(:children)
end end
it 'does not reject a non-:through association where there is no belongs_to in the inverse model' do
define_model :Child, parent_id: :integer
parent_class = define_model :Parent do
has_many :children
end
expect { have_many(:children) }.to match_against(parent_class.new)
end
it 'accepts a valid association with a :through option' do it 'accepts a valid association with a :through option' do
define_model :child define_model :child
define_model :conception, child_id: :integer, define_model :conception, child_id: :integer,
@ -860,6 +869,21 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
expect(Parent.new).to have_many(:children) expect(Parent.new).to have_many(:children)
end end
it 'rejects a :through association where there is no belongs_to in the inverse model' do
define_model :Child
define_model :Conception, child_id: :integer, parent_id: :integer
parent_class = define_model :Parent do
has_many :conceptions
has_many :children, through: :conceptions
end
expect { have_many(:children) }
.not_to match_against(parent_class.new)
.and_fail_with(<<-MESSAGE)
Expected Parent to have a has_many association called children through conceptions (Could not find the source association(s) "child" or :children in model Conception. Try 'has_many :children, :through => :conceptions, :source => <name>'. Is it one of ?)
MESSAGE
end
it 'accepts a valid association with an :as option' do it 'accepts a valid association with an :as option' do
define_model :child, guardian_type: :string, guardian_id: :integer define_model :child, guardian_type: :string, guardian_id: :integer
define_model :parent do define_model :parent do