fix: standardize error message of association matcher

thoughtbot · Apr 3, 2021 · 3eaa16c · 3eaa16c
1 parent c79819f
commit 3eaa16c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 22 deletions.
diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb
@@ -1394,23 +1394,25 @@ def touch_correct?
         end
 
         def class_has_foreign_key?(klass)
+          @missing = validate_foreign_key(klass)
+
+          @missing.nil?
+        end
+
+        def validate_foreign_key(klass)
           if options.key?(:foreign_key) && !foreign_key_correct?
-            @missing = foreign_key_failure_message(klass, options[:foreign_key])
-            false
-          elsif !has_column?(klass, foreign_key)
-            @missing = foreign_key_failure_message(klass, foreign_key)
-            false
-          else
-            true
+            foreign_key_failure_message(klass, options[:foreign_key])
+          elsif !has_column?(klass, actual_foreign_key)
+            foreign_key_failure_message(klass, actual_foreign_key)
           end
         end
 
         def has_column?(klass, column)
           case column
           when Array
-            column.all? { |c| has_column?(klass, c) }
+            column.all? { |c| has_column?(klass, c.to_s) }
           else
-            column_names_for(klass).include?(column)
+            column_names_for(klass).include?(column.to_s)
           end
         end
 
@@ -1442,19 +1444,13 @@ def primary_key_correct?(klass)
           end
         end
 
-        def foreign_key
-          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)
+        def actual_foreign_key
+          return unless foreign_key_reflection
+
+          if foreign_key_reflection.respond_to?(:foreign_key)
+            foreign_key_reflection.foreign_key
           else
-            key.to_s
+            foreign_key_reflection.primary_key_name
           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
@@ -1350,7 +1350,7 @@ def having_many_non_existent_class(model_name, assoc_name, options = {})
       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.)'
+        '(PersonDetail does not have a [:company_id, :person_detail_id] foreign key.)'
 
       expect do
         have_one(:detail).class_name('PersonDetail').