Skip to content

Commit

Permalink
Update messaging around presence + belongs_to
Browse files Browse the repository at this point in the history
  • Loading branch information
mcmire committed Jun 8, 2019
1 parent 7cad5fe commit 8697b01
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 17 deletions.
51 changes: 41 additions & 10 deletions lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,8 @@ def failure_message
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} }
validation for you? Instead of using `validate_presence_of`, try
#{suggestions_for_belongs_to}
MESSAGE
end

Expand Down Expand Up @@ -279,17 +277,42 @@ def reason_for_existing_presence_validation
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]})"
def suggestions_for_belongs_to
if belongs_to_association_configured_to_be_required?
<<~MESSAGE
one of the following instead, depending on your use case:
#{example_of_belongs_to(with: [:optional, false])}
#{example_of_belongs_to(with: [:required, true])}
MESSAGE
else
<<~MESSAGE
the following instead:
#{example_of_belongs_to}
MESSAGE
end
end

def example_of_belongs_to(with: nil)
initial_call = "should belong_to(:#{association_name})"
inside =
if with
"#{initial_call}.#{with.first}(#{with.second})"
else
initial_call
end

if Shoulda::Matchers.integrations.test_frameworks.any?(&:n_unit?)
inside
else
"it { #{inside} }"
end
end

def belongs_to_association_configured_to_be_required?
association_reflection.options[:optional] == false ||
association_reflection.options[:required] == true
association_options[:optional] == false ||
association_options[:required] == true
end

def belongs_to_association_being_validated?
Expand All @@ -301,6 +324,14 @@ def association_being_validated?
!!association_reflection
end

def association_name
association_reflection.name
end

def association_options
association_reflection&.options
end

def association_reflection
model.respond_to?(:reflect_on_association) &&
model.reflect_on_association(@attribute)
Expand Down
13 changes: 12 additions & 1 deletion lib/shoulda/matchers/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,26 @@ def self.configure
yield configuration
end

# @private
def self.integrations
configuration.integrations
end

# @private
def self.configuration
@_configuration ||= Configuration.new
end

# @private
class Configuration
attr_reader :integrations

def initialize
@integrations = nil
end

def integrate(&block)
Integrations::Configuration.apply(self, &block)
@integrations = Integrations::Configuration.apply(&block)
end
end
end
Expand Down
10 changes: 7 additions & 3 deletions lib/shoulda/matchers/integrations/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ module Matchers
module Integrations
# @private
class Configuration
def self.apply(configuration, &block)
new(configuration, &block).apply
def self.apply(&block)
new(&block).apply
end

def initialize(configuration, &block)
attr_reader :test_frameworks

def initialize(&block)
@test_frameworks = Set.new
@libraries = Set.new

Expand Down Expand Up @@ -47,6 +49,8 @@ def apply
test_framework.include(Shoulda::Matchers::Independent)
@libraries.each { |library| library.integrate_with(test_framework) }
end

self
end

private
Expand Down
2 changes: 1 addition & 1 deletion spec/support/unit/matchers/match_against.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def matcher_fails_in_positive?
end

def diff(expected, actual)
differ.diff(expected, actual)[1..-1]
differ.diff(actual, expected).strip
end

def differ
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def model_creator
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
it 'does not match, instructing the user to use belongs_to instead' do
record = record_belonging_to(
:parent,
optional: false,
Expand Down Expand Up @@ -337,9 +337,96 @@ def model_creator
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:
one of the following instead, depending on your use case:
it { should belong_to(:parent).optional(false) }
it { should belong_to(:parent).required(true) }
MESSAGE
end
end
end

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 'does not match, instructing the user to use belongs_to instead' do
record = record_belonging_to(
:parent,
required: 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 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
one of the following instead, depending on your use case:
it { should belong_to(:parent).optional(false) }
it { should belong_to(:parent).required(true) }
MESSAGE
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
Expand Down

0 comments on commit 8697b01

Please sign in to comment.