From 378493d7a9e37654c1af6a6e4fada7fffdf8c6a8 Mon Sep 17 00:00:00 2001 From: Rafael Santos Date: Thu, 12 Jan 2017 11:08:23 +1300 Subject: [PATCH] Pass correct object type to association reflection It's currently not possible to test a `has_many` association that is defined with a scope block where the block takes an instance of the association source. The `have_many` matcher attempts to call this scope and when it does so it does not pass that instance. This commit fixes that. Co-authored-by: Elliot Winkler --- .../association_matchers/model_reflection.rb | 4 +- .../association_matchers/model_reflector.rb | 5 +- .../active_record/association_matcher_spec.rb | 70 +++++++++++++++---- .../model_reflection_spec.rb | 21 +++++- 4 files changed, 83 insertions(+), 17 deletions(-) diff --git a/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb b/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb index 7a27ccae1..4247f146a 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/model_reflection.rb @@ -35,12 +35,12 @@ def join_table_name join_table_name.to_s end - def association_relation + def association_relation(related_instance) relation = associated_class.all if reflection.scope # Source: AR::Associations::AssociationScope#eval_scope - relation.instance_exec(subject, &reflection.scope) + relation.instance_exec(related_instance, &reflection.scope) else relation end diff --git a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb index 8927bc328..f72ea9a5d 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb @@ -7,7 +7,6 @@ class ModelReflector delegate( :associated_class, :association_foreign_key, - :association_relation, :foreign_key, :has_and_belongs_to_many_name, :join_table_name, @@ -27,6 +26,10 @@ def initialize(subject, name) @name = name end + def association_relation + reflection.association_relation(subject) + end + def reflection @reflection ||= reflect_on_association(name) end diff --git a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb index eb32d2654..0d7621362 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -1004,22 +1004,68 @@ def belonging_to_non_existent_class(model_name, assoc_name, options = {}) expect(matcher.failure_message).to match(/children should be ordered by id/) end - it 'accepts an association with a valid :conditions option' do - define_model :child, parent_id: :integer, adopted: :boolean - define_model(:parent).tap do |model| - define_association_with_conditions(model, :has_many, :children, adopted: true) - end + context 'if the association has a scope block' do + context 'and the block does not take an argument' do + context 'and the matcher is given conditions that match the conditions used in the scope' do + it 'matches' do + define_model :Child, parent_id: :integer, adopted: :boolean + define_model(:Parent) do + has_many :children, -> { where(adopted: true) } + end - expect(Parent.new).to have_many(:children).conditions(adopted: true) - end + expect(Parent.new). + to have_many(:children). + conditions(adopted: true) + end + end - it 'rejects an association with a bad :conditions option' do - define_model :child, parent_id: :integer, adopted: :boolean - define_model :parent do - has_many :children + context 'and the matcher is given conditions that do not match the conditions used in the scope' do + it 'rejects an association with a bad :conditions option' do + define_model :Child, parent_id: :integer, adopted: :boolean + define_model :Parent do + has_many :children + end + + expect(Parent.new). + not_to have_many(:children). + conditions(adopted: true) + end + end end - expect(Parent.new).not_to have_many(:children).conditions(adopted: true) + context 'and the block takes an argument' do + context 'and the matcher is given conditions that match the scope' do + it 'matches' do + define_model :Wheel, bike_id: :integer, tire_id: :integer + define_model :Tire + define_model :Bike, default_tire_id: :integer do + has_many :wheels, -> (bike) do + where(tire_id: bike.default_tire_id) + end + belongs_to :default_tire, class_name: 'Tire' + end + + expect(Bike.new(default_tire_id: 42)). + to have_many(:wheels).conditions(tire_id: 42) + end + end + + context 'and the matcher is given conditions that do not match the scope' do + it 'matches' do + define_model :Wheel, bike_id: :integer, tire_id: :integer + define_model :Tire + define_model :Bike, default_tire_id: :integer do + has_many :wheels, -> (bike) do + where(tire_id: bike.default_tire_id) + end + belongs_to :default_tire, class_name: 'Tire' + end + + expect(Bike.new(default_tire_id: 42)). + not_to have_many(:wheels).conditions(tire_id: 10) + end + end + end end it 'accepts an association without a :class_name option' do diff --git a/spec/unit/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb index 8cd4dac98..59b574e55 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matchers/model_reflection_spec.rb @@ -95,12 +95,28 @@ belongs_to :country, -> { where(mood: 'nice') } end delegate_reflection = person_model.reflect_on_association(:country) + person = person_model.new reflection = described_class.new(delegate_reflection) - actual_sql = reflection.association_relation.to_sql + actual_sql = reflection.association_relation(person).to_sql expected_sql = Country.where(mood: 'nice').to_sql expect(actual_sql).to eq expected_sql end + + it 'passes the object that has the association to the block' do + spy = spy('spy') + define_model(:country) + person_model = define_model(:person, country_id: :integer) do + belongs_to :country, -> (person) { spy.receive(person) } + end + delegate_reflection = person_model.reflect_on_association(:country) + person = person_model.new + reflection = described_class.new(delegate_reflection) + + reflection.association_relation(person) + + expect(spy).to have_received(:receive).with(person) + end end context 'when the scope is nil' do @@ -110,9 +126,10 @@ belongs_to :country end delegate_reflection = person_model.reflect_on_association(:country) + person = person_model.new reflection = described_class.new(delegate_reflection) - actual_sql = reflection.association_relation.to_sql + actual_sql = reflection.association_relation(person).to_sql expected_sql = Country.all.to_sql expect(actual_sql).to eq expected_sql end