From f7061a7d28e028db45bd1bc4ccd29ecb7870d28c Mon Sep 17 00:00:00 2001 From: Nate Matykiewicz Date: Thu, 14 Jan 2021 16:46:15 -0600 Subject: [PATCH] Fix FK CPK (#1396) --- .../active_record/association_matcher.rb | 29 +++++++++--- .../active_record/association_matcher_spec.rb | 44 ++++++++++++++++++- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index 249efc2af..48371cdb5 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -1397,7 +1397,7 @@ def class_has_foreign_key?(klass) if options.key?(:foreign_key) && !foreign_key_correct? @missing = foreign_key_failure_message(klass, options[:foreign_key]) false - elsif !column_names_for(klass).include?(foreign_key) + elsif !has_column?(klass, foreign_key) @missing = foreign_key_failure_message(klass, foreign_key) false else @@ -1405,6 +1405,15 @@ def class_has_foreign_key?(klass) end end + def has_column?(klass, column) + case column + when Array + column.all? { |c| has_column?(klass, c) } + else + column_names_for(klass).include?(column) + end + end + def foreign_key_correct? option_verifier.correct_for_string?( :foreign_key, @@ -1434,12 +1443,18 @@ def primary_key_correct?(klass) end def foreign_key - if foreign_key_reflection - if foreign_key_reflection.respond_to?(:foreign_key) - foreign_key_reflection.foreign_key.to_s - else - foreign_key_reflection.primary_key_name.to_s - end + key = if foreign_key_reflection + if foreign_key_reflection.respond_to?(:foreign_key) + foreign_key_reflection.foreign_key + else + foreign_key_reflection.primary_key_name + end + end + + if key.is_a?(Array) + key.map(&:to_s) + else + key.to_s end 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 23b7e38b2..1136fdf60 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -1414,7 +1414,7 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) expect(having_one_non_existent(:person, :detail, class_name: 'Detail')).not_to have_one(:detail) end - it 'rejects an association with a valid :class_name and a bad :primary_key option' do + it 'rejects an association with a valid :class_name and a bad :foreign_key option' do define_model :person_detail define_model :person do has_one :detail, class_name: 'PersonDetail', foreign_key: :person_detail_id @@ -1428,6 +1428,48 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) and_fail_with(expected_message) end + it 'rejects an association with a valid :class_name and a bad :foreign_key option (CPK)' do + define_model :person_detail + define_model :person do + has_one :detail, class_name: 'PersonDetail', foreign_key: %i[company_id person_detail_id] + end + + expected_message = 'Expected Person to have a has_one association called detail ' \ + '(PersonDetail does not have a [:foo_id, :bar_id] foreign key.)' + + expect do + have_one(:detail).class_name('PersonDetail'). + with_foreign_key(%i[foo_id bar_id]) + end.not_to match_against(Person.new).and_fail_with(expected_message) + end + + it 'rejects an association with a valid :class_name and :foreign_key option (CPK), but no columns' do + define_model :person_detail + define_model :person do + has_one :detail, class_name: 'PersonDetail', foreign_key: %i[company_id person_detail_id] + end + + expected_message = 'Expected Person to have a has_one association called detail ' \ + '(PersonDetail does not have a ["company_id", "person_detail_id"] foreign key.)' + + expect do + have_one(:detail).class_name('PersonDetail'). + with_foreign_key(%i[company_id person_detail_id]) + end.not_to match_against(Person.new).and_fail_with(expected_message) + end + + it 'accepts an association with a valid :class_name and :foreign_key option (CPK)' do + define_model :person_detail, company_id: :integer, person_detail_id: :integer + define_model :person do + has_one :detail, class_name: 'PersonDetail', foreign_key: %i[company_id person_detail_id] + end + + expect(Person.new). + to have_one(:detail). + class_name('PersonDetail'). + with_foreign_key(%i[company_id person_detail_id]) + end + it 'adds error message when rejecting an association with non-existent class' do message = 'Expected Person to have a has_one association called detail (Detail2 does not exist)' expect {