Clear up confusion w/ presence around belongs_to associations

Now that `belongs_to` associations add a presence validation
automatically, if you have a test such as the following:

    class StudentBook < ApplicationRecord
      belongs_to :student
    end

    RSpec.describe StudentBook do
      it { is_expected.not_to be_valid }
      it { is_expected.to validate_presence_of(:student) }
    end

then the test for presence of :student will fail, because the validation
message on the automatic presence validation is different than the usual
message. The solution here, of course, is to just use `belong_to`, but
that is not very obvious. So this commit updates the presence matcher to
remind users about using `belong_to` in a case such as this.
This commit is contained in:
Elliot Winkler 2019-06-01 16:39:10 -06:00
parent 32f6a35e39
commit 7cad5feaa7
6 changed files with 587 additions and 48 deletions

View File

@ -181,6 +181,27 @@ module Shoulda
"validate that :#{@attribute} cannot be empty/falsy"
end
def failure_message
message = super
if should_add_footnote_about_belongs_to?
message << "\n\n"
message << Shoulda::Matchers.word_wrap(<<-MESSAGE.strip, indent: 2)
You're getting this error because #{reason_for_existing_presence_validation}.
*This* presence validation doesn't use "can't be blank", the usual validation
message, but "must exist" instead.
With that said, did you know that the `belong_to` matcher can test this
validation for you? Instead of using `validate_presence_of`, try the following
instead:
it { should #{representation_of_belongs_to} }
MESSAGE
end
message
end
private
def secure_password_being_validated?
@ -197,7 +218,7 @@ module Shoulda
def allows_and_double_checks_value_of!(value)
allows_value_of(value, @expected_message)
rescue ActiveModel::AllowValueMatcher::AttributeChangedValueError
raise ActiveModel::CouldNotSetPasswordError.create(@subject.class)
raise ActiveModel::CouldNotSetPasswordError.create(model)
end
def allows_original_or_typecast_value?(value)
@ -207,7 +228,7 @@ module Shoulda
def disallows_and_double_checks_value_of!(value)
disallows_value_of(value, @expected_message)
rescue ActiveModel::AllowValueMatcher::AttributeChangedValueError
raise ActiveModel::CouldNotSetPasswordError.create(@subject.class)
raise ActiveModel::CouldNotSetPasswordError.create(model)
end
def disallows_original_or_typecast_value?(value)
@ -218,25 +239,79 @@ module Shoulda
if collection?
[Array.new]
else
[''].tap do |disallowed|
if !expects_to_allow_nil?
disallowed << nil
end
values = []
if !association_being_validated?
values << ''
end
if !expects_to_allow_nil?
values << nil
end
values
end
end
def collection?
if reflection
[:has_many, :has_and_belongs_to_many].include?(reflection.macro)
if association_reflection
[:has_many, :has_and_belongs_to_many].include?(
association_reflection.macro,
)
else
false
end
end
def reflection
@subject.class.respond_to?(:reflect_on_association) &&
@subject.class.reflect_on_association(@attribute)
def should_add_footnote_about_belongs_to?
belongs_to_association_being_validated? &&
presence_validation_exists_on_attribute?
end
def reason_for_existing_presence_validation
if belongs_to_association_configured_to_be_required?
"you've instructed your `belongs_to` association to add a " +
'presence validation to the attribute'
else
# assume ::ActiveRecord::Base.belongs_to_required_by_default == true
'ActiveRecord is configured to add a presence validation to all ' +
'`belongs_to` associations, and this includes yours'
end
end
def representation_of_belongs_to
'belong_to(:parent)'.tap do |str|
if association_reflection.options.include?(:optional)
str << ".optional(#{association_reflection.options[:optional]})"
end
end
end
def belongs_to_association_configured_to_be_required?
association_reflection.options[:optional] == false ||
association_reflection.options[:required] == true
end
def belongs_to_association_being_validated?
association_being_validated? &&
association_reflection.macro == :belongs_to
end
def association_being_validated?
!!association_reflection
end
def association_reflection
model.respond_to?(:reflect_on_association) &&
model.reflect_on_association(@attribute)
end
def presence_validation_exists_on_attribute?
model._validators.include?(@attribute)
end
def model
@subject.class
end
end
end

View File

@ -2,6 +2,8 @@ module Shoulda
module Matchers
# @private
module WordWrap
TERMINAL_WIDTH = 72
def word_wrap(document, options = {})
Document.new(document, options).wrap
end
@ -112,7 +114,6 @@ module Shoulda
# @private
class Line
TERMINAL_WIDTH = 72
OFFSETS = { left: -1, right: +1 }
def initialize(line, indent: 0)
@ -171,7 +172,7 @@ module Shoulda
def wrap_line(line, direction: :left)
index = nil
if line.length > TERMINAL_WIDTH
if line.length > Shoulda::Matchers::WordWrap::TERMINAL_WIDTH
index = determine_where_to_break_line(line, direction: :left)
if index == -1
@ -192,7 +193,7 @@ module Shoulda
def determine_where_to_break_line(line, args)
direction = args.fetch(:direction)
index = TERMINAL_WIDTH
index = Shoulda::Matchers::WordWrap::TERMINAL_WIDTH
offset = OFFSETS.fetch(direction)
while line[index] !~ /\s/ && (0...line.length).cover?(index)

View File

@ -0,0 +1,31 @@
module UnitTests
module ApplicationConfigurationHelpers
def with_belongs_to_as_required_by_default(&block)
configuring_application(
::ActiveRecord::Base,
:belongs_to_required_by_default,
true,
&block
)
end
def with_belongs_to_as_optional_by_default(&block)
configuring_application(
::ActiveRecord::Base,
:belongs_to_required_by_default,
false,
&block
)
end
private
def configuring_application(config, name, value)
previous_value = config.send(name)
config.send("#{name}=", value)
yield
ensure
config.send("#{name}=", previous_value)
end
end
end

View File

@ -0,0 +1,151 @@
module UnitTests
module Matchers
def match_against(object)
MatchAgainstMatcher.new(object)
end
class MatchAgainstMatcher
DIVIDER = ('-' * Shoulda::Matchers::WordWrap::TERMINAL_WIDTH).freeze
attr_reader :failure_message, :failure_message_when_negated
def initialize(object)
@object = object
@failure_message = nil
@failure_message_when_negated = nil
end
def and_fail_with(message)
@message = message.strip
self
end
alias_method :or_fail_with, :and_fail_with
def matches?(generate_matcher)
@positive_matcher = generate_matcher.call
@negative_matcher = generate_matcher.call
if positive_matcher.matches?(object)
!message || matcher_fails_in_negative?
else
@failure_message = <<-MESSAGE
Expected the matcher to match in the positive, but it failed with this message:
#{DIVIDER}
#{positive_matcher.failure_message}
#{DIVIDER}
MESSAGE
false
end
end
def does_not_match?(generate_matcher)
@positive_matcher = generate_matcher.call
@negative_matcher = generate_matcher.call
if negative_matcher.does_not_match?(object)
!message || matcher_fails_in_positive?
else
@failure_message_when_negated = <<-MESSAGE
Expected the matcher to match in the negative, but it failed with this message:
#{DIVIDER}
#{negative_matcher.failure_message_when_negated}
#{DIVIDER}
MESSAGE
false
end
end
def supports_block_expectations?
true
end
private
attr_reader :object, :message, :positive_matcher, :negative_matcher
def matcher_fails_in_negative?
if !negative_matcher.does_not_match?(object)
if message == negative_matcher.failure_message_when_negated.strip
true
else
diff_result = diff(
message,
negative_matcher.failure_message_when_negated.strip,
)
@failure_message = <<-MESSAGE
Expected the negative version of the matcher not to match and for the failure
message to be:
#{DIVIDER}
#{message.chomp}
#{DIVIDER}
However, it was:
#{DIVIDER}
#{negative_matcher.failure_message_when_negated}
#{DIVIDER}
Diff:
#{Shoulda::Matchers::Util.indent(diff_result, 2)}
MESSAGE
false
end
else
@failure_message =
'Expected the negative version of the matcher not to match, ' +
'but it did.'
false
end
end
def matcher_fails_in_positive?
if !positive_matcher.matches?(object)
if message == positive_matcher.failure_message.strip
true
else
diff_result = diff(
message,
positive_matcher.failure_message.strip,
)
@failure_message_when_negated = <<-MESSAGE
Expected the positive version of the matcher not to match and for the failure
message to be:
#{DIVIDER}
#{message.chomp}
#{DIVIDER}
However, it was:
#{DIVIDER}
#{positive_matcher.failure_message}
#{DIVIDER}
Diff:
#{Shoulda::Matchers::Util.indent(diff_result, 2)}
MESSAGE
false
end
else
@failure_message_when_negated =
'Expected the positive version of the matcher not to match, ' +
'but it did.'
false
end
end
def diff(expected, actual)
differ.diff(expected, actual)[1..-1]
end
def differ
@_differ ||= RSpec::Support::Differ.new
end
end
end
end

View File

@ -1,6 +1,8 @@
require 'unit_spec_helper'
describe Shoulda::Matchers::ActiveModel::ValidatePresenceOfMatcher, type: :model do
include UnitTests::ApplicationConfigurationHelpers
context 'a model with a presence validation' do
it 'accepts' do
expect(validating_presence).to matcher
@ -176,7 +178,7 @@ could not be proved.
end
end
context 'a required has_and_belongs_to_many association' do
context 'a has_and_belongs_to_many association with a presence validation on it' do
it 'accepts' do
expect(build_record_having_and_belonging_to_many).
to validate_presence_of(:children)
@ -228,7 +230,7 @@ could not be proved.
end
end
context 'an optional has_and_belongs_to_many association' do
context 'a has_and_belongs_to_many association without a presence validation on it' do
before do
define_model :child
@model = define_model :parent do
@ -256,6 +258,300 @@ this could not be proved.
end
end
context 'against a belongs_to association' do
if active_record_supports_optional_for_associations?
context 'declared with optional: true' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
record = record_belonging_to(
:parent,
optional: true,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match' do
record = record_belonging_to(
:parent,
optional: true,
validate_presence: false,
model_name: 'Child',
parent_model_name: 'Parent',
)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid, but it was valid instead.
MESSAGE
end
end
end
context 'declared with optional: false' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
record = record_belonging_to(
:parent,
optional: false,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match, instructing the user to use belong_to instead' do
record = record_belonging_to(
:parent,
optional: false,
validate_presence: false,
model_name: 'Child',
parent_model_name: 'Parent',
)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid and to produce the validation error "can't be blank" on
:parent. The record was indeed invalid, but it produced these
validation errors instead:
* parent: ["must exist"]
You're getting this error because you've instructed your `belongs_to`
association to add a presence validation to the attribute. *This*
presence validation doesn't use "can't be blank", the usual validation
message, but "must exist" instead.
With that said, did you know that the `belong_to` matcher can test
this validation for you? Instead of using `validate_presence_of`, try
the following instead:
it { should belong_to(:parent).optional(false) }
MESSAGE
end
end
end
context 'not declared with an optional or required option' do
context 'when belongs_to is configured to be required by default' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
with_belongs_to_as_required_by_default do
record = record_belonging_to(
:parent,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.
to match_against(record)
end
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match, instructing the user to use belong_to instead' do
with_belongs_to_as_required_by_default do
record = record_belonging_to(
:parent,
validate_presence: false,
model_name: 'Child',
parent_model_name: 'Parent',
)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid and to produce the validation error "can't be blank" on
:parent. The record was indeed invalid, but it produced these
validation errors instead:
* parent: ["must exist"]
You're getting this error because ActiveRecord is configured to add a
presence validation to all `belongs_to` associations, and this
includes yours. *This* presence validation doesn't use "can't be
blank", the usual validation message, but "must exist" instead.
With that said, did you know that the `belong_to` matcher can test
this validation for you? Instead of using `validate_presence_of`, try
the following instead:
it { should belong_to(:parent) }
MESSAGE
end
end
end
end
context 'when belongs_to is configured to be optional by default' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
with_belongs_to_as_optional_by_default do
record = record_belonging_to(
:parent,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.
to match_against(record)
end
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match' do
with_belongs_to_as_optional_by_default do
record = record_belonging_to(
:parent,
validate_presence: false,
model_name: 'Child',
parent_model_name: 'Parent',
)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid, but it was valid instead.
MESSAGE
end
end
end
end
end
else
context 'declared with required: true' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
record = record_belonging_to(
:parent,
required: true,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
context 'and an explicit presence validation is not on the association' do
it 'still matches' do
record = record_belonging_to(
:parent,
required: true,
validate_presence: false,
)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
end
context 'declared with required: false' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
record = record_belonging_to(
:parent,
required: false,
validate_presence: true,
)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match' do
record = record_belonging_to(
:parent,
required: false,
validate_presence: false,
model_name: 'Child',
parent_model_name: 'Parent',
)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid, but it was valid instead.
MESSAGE
end
end
end
context 'not declared with a required option' do
context 'and an explicit presence validation is on the association' do
it 'matches' do
record = record_belonging_to(:parent, validate_presence: true)
expect { validate_presence_of(:parent) }.to match_against(record)
end
end
context 'and an explicit presence validation is not on the association' do
it 'does not match' do
record = record_belonging_to(:parent, validate_presence: false)
expect { validate_presence_of(:parent) }.
not_to match_against(record).
and_fail_with(<<-MESSAGE)
Expected Child to validate that :parent cannot be empty/falsy, but this
could not be proved.
After setting :parent to nil, the matcher expected the Child to be
invalid, but it was valid instead.
MESSAGE
end
end
end
end
def record_belonging_to(
attribute_name,
model_name: 'Child',
parent_model_name: 'Parent',
column_name: "#{attribute_name}_id",
validate_presence: false,
**association_options,
&block
)
define_model(parent_model_name)
child_model = define_model(model_name, column_name => :integer) do
belongs_to(attribute_name, **association_options)
if validate_presence
validates_presence_of(attribute_name)
end
if block
instance_eval(&block)
end
end
child_model.new
end
end
context "an i18n translation containing %{attribute} and %{model}" do
before do
stub_translation(

View File

@ -1,6 +1,8 @@
require 'unit_spec_helper'
describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
include UnitTests::ApplicationConfigurationHelpers
context 'belong_to' do
it 'accepts a good association with the default foreign key' do
expect(belonging_to_parent).to belong_to(:parent)
@ -284,7 +286,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
if active_record_supports_optional_for_associations?
context 'when belongs_to is configured to be required by default' do
it 'passes' do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(belonging_to_parent).to belong_to(:parent).required(true)
end
end
@ -292,7 +294,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'when belongs_to is not configured to be required by default' do
it 'fails with an appropriate message' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
assertion = lambda do
expect(belonging_to_parent).
to belong_to(:parent).required(true)
@ -333,7 +335,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
if active_record_supports_optional_for_associations?
context 'when belongs_to is configured to be required by default' do
it 'fails with an appropriate message' do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
assertion = lambda do
expect(belonging_to_parent).
to belong_to(:parent).required(false)
@ -354,7 +356,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'when belongs_to is not configured to be required by default' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
expect(belonging_to_parent).to belong_to(:parent).required(false)
end
end
@ -370,7 +372,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
if active_record_supports_optional_for_associations?
context 'when belongs_to is configured to be required by default' do
it 'fails with an appropriate message' do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
assertion = lambda do
expect(belonging_to_parent).
to belong_to(:parent).optional
@ -391,7 +393,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'when belongs_to is not configured to be required by default' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
expect(belonging_to_parent).to belong_to(:parent).optional
end
end
@ -407,7 +409,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
if active_record_supports_optional_for_associations?
context 'when belongs_to is configured to be required by default' do
it 'passes' do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(belonging_to_parent).to belong_to(:parent)
end
end
@ -415,14 +417,14 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'when belongs_to is not configured to be required by default' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
expect(belonging_to_parent).to belong_to(:parent)
end
end
context 'and a presence validation is on the attribute instead of using required: true' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
record = belonging_to_parent do
validates_presence_of :parent
end
@ -435,7 +437,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'and a presence validation is on the attribute with a condition' do
context 'and the condition is true' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
child_model = create_child_model_belonging_to_parent do
attr_accessor :condition
validates_presence_of :parent, if: :condition
@ -450,7 +452,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
context 'and the condition is false' do
it 'passes' do
configuring_default_belongs_to_requiredness(false) do
with_belongs_to_as_optional_by_default do
child_model = create_child_model_belonging_to_parent do
attr_accessor :condition
validates_presence_of :parent, if: :condition
@ -630,7 +632,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
assertion = lambda do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(model.new).to belong_to(:parent)
end
end
@ -656,7 +658,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
end
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(model.new).
to belong_to(:parent).
without_validating_presence
@ -677,7 +679,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
assertion = lambda do
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(model.new).to belong_to(:parent).required
end
end
@ -703,7 +705,7 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
end
configuring_default_belongs_to_requiredness(true) do
with_belongs_to_as_required_by_default do
expect(model.new).
to belong_to(:parent).
required.
@ -1765,21 +1767,4 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
[:destroy, :delete, :nullify, :restrict]
end
end
def configuring_default_belongs_to_requiredness(value, &block)
configuring_application(
ActiveRecord::Base,
:belongs_to_required_by_default,
value,
&block
)
end
def configuring_application(config, name, value)
previous_value = config.send(name)
config.send("#{name}=", value)
yield
ensure
config.send("#{name}=", previous_value)
end
end