Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for composite primary key in foreign key #1396

Merged
merged 1 commit into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 22 additions & 7 deletions lib/shoulda/matchers/active_record/association_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1397,14 +1397,23 @@ 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
true
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,
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down