From 2649b175d350ff0ecdc88b09347d3c1a59938c54 Mon Sep 17 00:00:00 2001
From: Pedro Paiva <pedro.v.s.paiva@gmail.com>
Date: Sun, 15 Nov 2020 11:56:39 -0300
Subject: [PATCH] fix: with_foreign_key modifier fails with wrong error message

---
 .../active_record/association_matcher.rb      | 27 ++++++++++++-------
 .../active_record/association_matcher_spec.rb | 14 ++++++++++
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb
index 851471ca5..249efc2af 100644
--- a/lib/shoulda/matchers/active_record/association_matcher.rb
+++ b/lib/shoulda/matchers/active_record/association_matcher.rb
@@ -1394,19 +1394,28 @@ def touch_correct?
         end
 
         def class_has_foreign_key?(klass)
-          if options.key?(:foreign_key)
-            option_verifier.correct_for_string?(
-              :foreign_key,
-              options[:foreign_key],
-            )
-          elsif column_names_for(klass).include?(foreign_key)
-            true
-          else
-            @missing = "#{klass} does not have a #{foreign_key} foreign key."
+          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)
+            @missing = foreign_key_failure_message(klass, foreign_key)
+            false
+          else
+            true
           end
         end
 
+        def foreign_key_correct?
+          option_verifier.correct_for_string?(
+            :foreign_key,
+            options[:foreign_key],
+          )
+        end
+
+        def foreign_key_failure_message(klass, foreign_key)
+          "#{klass} does not have a #{foreign_key} foreign key."
+        end
+
         def primary_key_correct?(klass)
           if options.key?(:primary_key)
             if option_verifier.correct_for_string?(
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 5fe336a70..1d2e28586 100644
--- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb
+++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb
@@ -1394,6 +1394,20 @@ 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
+      define_model :person_detail
+      define_model :person do
+        has_one :detail, class_name: 'PersonDetail', foreign_key: :person_detail_id
+      end
+
+      expected_message = 'Expected Person to have a has_one association called detail ' \
+        '(PersonDetail does not have a custom_primary_id foreign key.)'
+
+      expect { have_one(:detail).class_name('PersonDetail').with_foreign_key(:custom_primary_id) }.
+        not_to match_against(Person.new).
+        and_fail_with(expected_message)
+    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 {