Improve messaging of .required and .optional

When the `required` and `optional` qualifiers on `belong_to` fail, the
message can be a little confusing. There are actually two ways to
satisfy both qualifiers. One way, of course, is to match the option on
the association with the qualifier itself, so if the association is
defined with `required: true`, that will match
`belong_to(...).required`, and if it's `required: false`, that will
match `belong_to(...).required(false)` (and similar for `optional`). The
second way, though, is to manually add a presence validation to the
association (this will obviously only match `required(true)`). We need
to update the failure message to reflect these cases.
This commit is contained in:
Elliot Winkler 2019-01-29 00:00:48 -07:00
parent 0e7e250fb3
commit ca6a1ff9ee
3 changed files with 93 additions and 33 deletions

View File

@ -7,6 +7,7 @@ module Shoulda
attr_reader :missing_option
def initialize(attribute_name, optional)
@attribute_name = attribute_name
@optional = optional
@submatcher = ActiveModel::AllowValueMatcher.new(nil).
for(attribute_name)
@ -21,16 +22,38 @@ module Shoulda
if submatcher_passes?(subject)
true
else
@missing_option =
'the association should have been defined ' +
"with `optional: #{optional}`, but was not"
@missing_option = 'and for the record '
missing_option <<
if optional
'not to '
else
'to '
end
missing_option << (
'fail validation if ' +
":#{attribute_name} is unset; i.e., either the association " +
'should have been defined with `optional: ' +
"#{optional.inspect}`, or there "
)
missing_option <<
if optional
'should not '
else
'should '
end
missing_option << "be a presence validation on :#{attribute_name}"
false
end
end
private
attr_reader :optional, :submatcher
attr_reader :attribute_name, :optional, :submatcher
def submatcher_passes?(subject)
if optional

View File

@ -7,6 +7,7 @@ module Shoulda
attr_reader :missing_option
def initialize(attribute_name, required)
@attribute_name = attribute_name
@required = required
@submatcher = ActiveModel::DisallowValueMatcher.new(nil).
for(attribute_name).
@ -22,16 +23,38 @@ module Shoulda
if submatcher_passes?(subject)
true
else
@missing_option =
'the association should have been defined ' +
"with `required: #{required}`, but was not"
@missing_option = 'and for the record '
missing_option <<
if required
'to '
else
'not to '
end
missing_option << (
'fail validation if ' +
":#{attribute_name} is unset; i.e., either the association " +
'should have been defined with `required: ' +
"#{required.inspect}`, or there "
)
missing_option <<
if required
'should '
else
'should not '
end
missing_option << "be a presence validation on :#{attribute_name}"
false
end
end
private
attr_reader :required, :submatcher
attr_reader :attribute_name, :required, :submatcher
def submatcher_passes?(subject)
if required

View File

@ -300,8 +300,10 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent
(the association should have been defined with `required:
true`, but was not)
(and for the record to fail validation if :parent is unset;
i.e., either the association should have been defined with
`required: true`, or there should be a presence validation on
:parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -317,8 +319,9 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent
(the association should have been defined with `required:
true`, but was not)
(and for the record to fail validation if :parent is unset; i.e.,
either the association should have been defined with `required:
true`, or there should be a presence validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -338,8 +341,10 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent
(the association should have been defined with `required:
false`, but was not)
(and for the record not to fail validation if :parent is
unset; i.e., either the association should have been defined
with `required: false`, or there should not be a presence
validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -372,9 +377,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent (the
association should have been defined with `optional: true`, but
was not)
Expected Child to have a belongs_to association called parent
(and for the record not to fail validation if :parent is
unset; i.e., either the association should have been defined
with `optional: true`, or there should not be a presence
validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -519,9 +526,10 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent (the
association should have been defined with `required: false`, but
was not)
Expected Child to have a belongs_to association called parent (and
for the record not to fail validation if :parent is unset; i.e.,
either the association should have been defined with `required:
false`, or there should not be a presence validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -536,9 +544,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent (the
association should have been defined with `optional: true`, but
was not)
Expected Child to have a belongs_to association called parent
(and for the record not to fail validation if :parent is unset;
i.e., either the association should have been defined with
`optional: true`, or there should not be a presence validation on
:parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -562,9 +572,10 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent (the
association should have been defined with `required: true`, but
was not)
Expected Child to have a belongs_to association called parent
(and for the record to fail validation if :parent is unset; i.e.,
either the association should have been defined with `required:
true`, or there should be a presence validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -593,9 +604,10 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
message = format_message(<<-MESSAGE, one_line: true)
Expected Child to have a belongs_to association called parent (the
association should have been defined with `required: true`, but
was not)
Expected Child to have a belongs_to association called parent
(and for the record to fail validation if :parent is unset; i.e.,
either the association should have been defined with `required:
true`, or there should be a presence validation on :parent)
MESSAGE
expect(&assertion).to fail_with_message(message)
@ -1231,10 +1243,12 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
to have_one(:detail).required
end
message =
'Expected Person to have a has_one association called detail ' +
'(the association should have been defined with `required: true`, ' +
'but was not)'
message = format_message(<<-MESSAGE, one_line: true)
Expected Person to have a has_one association called detail (and for
the record to fail validation if :detail is unset; i.e., either the
association should have been defined with `required: true`, or there
should be a presence validation on :detail)
MESSAGE
expect(&assertion).to fail_with_message(message)
end