Add required/optional qual's to belongs_to/has_one

Rails 5 made two changes to `belongs_to` associations:

* `required` and `optional` were added as options (which add and remove
  a presence validation on the association, respectively)
* `required` was made the default

In addition, a `required` option was also added to `has_one`.

These new qualifiers allow us to test these options appropriately.

Credit: Shia <rise.shia@gmail.com>
This commit is contained in:
Elliot Winkler 2017-10-02 00:19:03 -05:00
parent b310986f9a
commit 3af3d9f7ab
9 changed files with 375 additions and 32 deletions

View File

@ -48,6 +48,15 @@ is now:
* *PRs: [#989], [#964], [#917]*
* *Original issue: [#867]*
### Features
* Add `required` and `optional` qualifiers to `belong_to` and `have_one`
matchers. (When using the `belong_to` matcher under Rails 5+, `required` is
assumed unless overridden.)
* *Original PR: [#956]*
* *Original issues: [#870], [#861]*
[a6d09aa]: https://github.com/thoughtbot/shoulda-matchers/commit/a6d09aa5de0d546367e7b3d7177dfde6c66f7f05
[#943]: https://github.com/thoughtbot/shoulda-matchers/pulls/943
[#933]: https://github.com/thoughtbot/shoulda-matchers/issues/933

View File

@ -6,6 +6,8 @@ require "shoulda/matchers/active_record/association_matchers/join_table_matcher"
require "shoulda/matchers/active_record/association_matchers/order_matcher"
require "shoulda/matchers/active_record/association_matchers/through_matcher"
require "shoulda/matchers/active_record/association_matchers/dependent_matcher"
require "shoulda/matchers/active_record/association_matchers/required_matcher"
require "shoulda/matchers/active_record/association_matchers/optional_matcher"
require "shoulda/matchers/active_record/association_matchers/source_matcher"
require "shoulda/matchers/active_record/association_matchers/model_reflector"
require "shoulda/matchers/active_record/association_matchers/model_reflection"

View File

@ -255,6 +255,48 @@ module Shoulda
#
# @return [AssociationMatcher]
#
# ##### required
#
# Use `required` to assert that the association is not allowed to be nil.
# (Enabled by default in Rails 5+.)
#
# class Person < ActiveRecord::Base
# belongs_to :organization, required: true
# end
#
# # RSpec
# describe Person
# it { should belong_to(:organization).required }
# end
#
# # Minitest (Shoulda)
# class PersonTest < ActiveSupport::TestCase
# should belong_to(:organization).required
# end
#
# @return [AssociationMatcher]
#
# ##### optional
#
# Use `optional` to assert that the association is allowed to be nil.
# (Rails 5+ only.)
#
# class Person < ActiveRecord::Base
# belongs_to :organization, optional: true
# end
#
# # RSpec
# describe Person
# it { should belong_to(:organization).optional }
# end
#
# # Minitest (Shoulda)
# class PersonTest < ActiveSupport::TestCase
# should belong_to(:organization).optional
# end
#
# @return [AssociationMatcher]
#
def belong_to(name)
AssociationMatcher.new(:belongs_to, name)
end
@ -714,6 +756,25 @@ module Shoulda
# should have_one(:bank).autosave(true)
# end
#
# ##### required
#
# Use `required` to assert that the association is not allowed to be nil.
# (Rails 5+ only.)
#
# class Person < ActiveRecord::Base
# has_one :brain, required: true
# end
#
# # RSpec
# describe Person
# it { should have_one(:brain).required }
# end
#
# # Minitest (Shoulda)
# class PersonTest < ActiveSupport::TestCase
# should have_one(:brain).required
# end
#
# @return [AssociationMatcher]
#
def have_one(name)
@ -891,42 +952,67 @@ module Shoulda
@options = {}
@submatchers = []
@missing = ''
if macro == :belongs_to
if RailsShim.active_record_gte_5?
required
else
optional
end
end
end
def through(through)
through_matcher = AssociationMatchers::ThroughMatcher.new(through, name)
add_submatcher(through_matcher)
add_submatcher(
AssociationMatchers::ThroughMatcher,
through,
name,
)
self
end
def dependent(dependent)
dependent_matcher = AssociationMatchers::DependentMatcher.new(dependent, name)
add_submatcher(dependent_matcher)
add_submatcher(
AssociationMatchers::DependentMatcher,
dependent,
name,
)
self
end
def order(order)
order_matcher = AssociationMatchers::OrderMatcher.new(order, name)
add_submatcher(order_matcher)
add_submatcher(
AssociationMatchers::OrderMatcher,
order,
name,
)
self
end
def counter_cache(counter_cache = true)
counter_cache_matcher = AssociationMatchers::CounterCacheMatcher.new(counter_cache, name)
add_submatcher(counter_cache_matcher)
add_submatcher(
AssociationMatchers::CounterCacheMatcher,
counter_cache,
name,
)
self
end
def inverse_of(inverse_of)
inverse_of_matcher =
AssociationMatchers::InverseOfMatcher.new(inverse_of, name)
add_submatcher(inverse_of_matcher)
add_submatcher(
AssociationMatchers::InverseOfMatcher,
inverse_of,
name,
)
self
end
def source(source)
source_matcher = AssociationMatchers::SourceMatcher.new(source, name)
add_submatcher(source_matcher)
add_submatcher(
AssociationMatchers::SourceMatcher,
source,
name,
)
self
end
@ -955,6 +1041,26 @@ module Shoulda
self
end
def required(required = true)
remove_submatcher(AssociationMatchers::OptionalMatcher)
add_submatcher(
AssociationMatchers::RequiredMatcher,
name,
required,
)
self
end
def optional(optional = true)
remove_submatcher(AssociationMatchers::RequiredMatcher)
add_submatcher(
AssociationMatchers::OptionalMatcher,
name,
optional,
)
self
end
def validate(validate = true)
@options[:validate] = validate
self
@ -1016,8 +1122,15 @@ module Shoulda
@reflector ||= AssociationMatchers::ModelReflector.new(subject, name)
end
def add_submatcher(matcher)
@submatchers << matcher
def add_submatcher(matcher_class, *args)
remove_submatcher(matcher_class)
submatchers << matcher_class.new(*args)
end
def remove_submatcher(matcher_class)
submatchers.delete_if do |submatcher|
submatcher.is_a?(matcher_class)
end
end
def macro_description
@ -1039,7 +1152,7 @@ module Shoulda
def missing_options
missing_options = [missing, missing_options_for_failing_submatchers]
missing_options.flatten.compact.join(', ')
missing_options.flatten.select(&:present?).join(', ')
end
def failing_submatchers

View File

@ -0,0 +1,47 @@
module Shoulda
module Matchers
module ActiveRecord
module AssociationMatchers
# @private
class OptionalMatcher
attr_reader :missing_option
def initialize(attribute_name, optional)
@attribute_name = attribute_name
@missing_option = ''
@submatcher = submatcher_class_for(optional).new(nil).
for(attribute_name).
with_message(:required)
end
def description
'required: true'
end
def matches?(subject)
if submatcher.matches?(subject)
true
else
@missing_option =
'the association should have been defined ' +
'with `optional: true`, but was not'
false
end
end
private
attr_reader :subject, :submatcher
def submatcher_class_for(optional)
if optional
ActiveModel::AllowValueMatcher
else
ActiveModel::DisallowValueMatcher
end
end
end
end
end
end
end

View File

@ -0,0 +1,50 @@
module Shoulda
module Matchers
module ActiveRecord
module AssociationMatchers
# @private
class RequiredMatcher
attr_reader :missing_option
def initialize(attribute_name, required)
@missing_option = ''
@submatcher = submatcher_class_for(required).new(nil).
for(attribute_name).
with_message(validation_message_key)
end
def description
'required: true'
end
def matches?(subject)
if submatcher.matches?(subject)
true
else
@missing_option =
'the association should have been defined ' +
'with `required: true`, but was not'
false
end
end
private
attr_reader :subject, :submatcher
def submatcher_class_for(required)
if required
ActiveModel::DisallowValueMatcher
else
ActiveModel::AllowValueMatcher
end
end
def validation_message_key
RailsShim.validation_message_key_for_association_required_option
end
end
end
end
end
end

View File

@ -21,8 +21,12 @@ module Shoulda
Gem::Version.new('0')
end
def active_record_major_version
::ActiveRecord::VERSION::MAJOR
def active_record_gte_5?
Gem::Requirement.new('>= 5').satisfied_by?(active_record_version)
end
def active_record_version
Gem::Version.new(::ActiveRecord::VERSION::STRING)
rescue NameError
Gem::Version.new('0')
end
@ -92,7 +96,7 @@ module Shoulda
end
def tables_and_views(connection)
if active_record_major_version >= 5
if active_record_gte_5?
connection.data_sources
else
connection.tables
@ -107,6 +111,14 @@ module Shoulda
end
end
def validation_message_key_for_association_required_option
if active_record_gte_5?
:required
else
:blank
end
end
private
def simply_generate_validation_message(

View File

@ -32,5 +32,9 @@ module UnitTests
def active_record_uniqueness_supports_array_columns?
active_record_version < 5
end
def active_record_supports_required_for_associations?
active_record_version >= 5
end
end
end

View File

@ -24,13 +24,15 @@ module UnitTests
lines << Shoulda::Matchers::Util.indent(expected, 2)
if @actual
diff = differ.diff(@actual, expected)[1..-1]
lines << 'Actually failed with:'
lines << Shoulda::Matchers::Util.indent(@actual, 2)
lines << "Diff:"
lines << Shoulda::Matchers::Util.indent(
differ.diff(@actual, expected)[1..-1],
2
)
if diff
lines << 'Diff:'
lines << Shoulda::Matchers::Util.indent(diff, 2)
end
else
lines << 'However, the expectation did not fail at all.'
end

View File

@ -82,18 +82,18 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
it 'accepts an association with a valid :inverse_of option' do
expect(belonging_to_parent(inverse_of: :children))
.to belong_to(:parent).inverse_of(:children)
expect(belonging_to_with_inverse(:parent, :children)).
to belong_to(:parent).inverse_of(:children)
end
it 'rejects an association with a bad :inverse_of option' do
expect(belonging_to_parent(inverse_of: :other_children))
.not_to belong_to(:parent).inverse_of(:children)
expect(belonging_to_with_inverse(:parent, :other_children)).
not_to belong_to(:parent).inverse_of(:children)
end
it 'rejects an association that has no :inverse_of option' do
expect(belonging_to_parent)
.not_to belong_to(:parent).inverse_of(:children)
expect(belonging_to_parent).
not_to belong_to(:parent).inverse_of(:children)
end
it 'accepts an association with a valid :conditions option' do
@ -279,13 +279,92 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
end
def belonging_to_parent(options = {})
define_model :parent
context 'given an association with neither :required nor :optional specified' do
if active_record_supports_required_for_associations?
it 'assumes it is required' do
expect(belonging_to_parent).to belong_to(:parent).required
end
else
it 'assumes it is optional' do
expect(belonging_to_parent).to belong_to(:parent).optional
end
end
end
context 'given an association with a matching :required option' do
it 'passes' do
expect(belonging_to_parent(required: true)).
to belong_to(:parent).required
end
end
context 'given an association with a non-matching :required option' do
it 'fails with an appropriate message' do
assertion = lambda do
expect(belonging_to_parent(required: false)).
to belong_to(:parent).required
end
message =
'Expected Child to have a belongs_to association called parent ' +
'(the association should have been defined with `required: true`, ' +
'but was not)'
expect(&assertion).to fail_with_message(message)
end
end
if active_record_supports_required_for_associations?
context 'given an association with a matching :optional option' do
it 'passes' do
expect(belonging_to_parent(optional: true)).
to belong_to(:parent).optional
end
end
context 'given an association with a non-matching :optional option' do
it 'fails with an appropriate message' do
assertion = lambda do
expect(belonging_to_parent(optional: false)).
to belong_to(:parent).optional
end
message =
'Expected Child to have a belongs_to association called parent ' +
'(the association should have been defined with `optional: ' +
'true`, but was not)'
expect(&assertion).to fail_with_message(message)
end
end
end
def belonging_to_parent(options = {}, parent_options = {})
define_model :parent, parent_options
define_model :child, parent_id: :integer do
belongs_to :parent, options
end.new
end
def belonging_to_with_inverse(association, inverse_association)
parent_model_name = association.to_s.singularize
child_model_name = inverse_association.to_s.singularize
parent_foreign_key = "#{parent_model_name}_id"
define_model parent_model_name do
has_many inverse_association
end
child_model = define_model(
child_model_name,
parent_foreign_key => :integer,
) do
belongs_to association, inverse_of: inverse_association
end
child_model.new
end
def belonging_to_non_existent_class(model_name, assoc_name, options = {})
define_model model_name, "#{assoc_name}_id" => :integer do
belongs_to assoc_name, options
@ -826,6 +905,31 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do
end
end
if active_record_supports_required_for_associations?
context 'given an association with a matching :required option' do
it 'passes' do
expect(having_one_detail(required: true)).
to have_one(:detail).required
end
end
end
context 'given an association with a non-matching :required option' do
it 'fails with an appropriate message' do
assertion = lambda do
expect(having_one_detail(required: false)).
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)'
expect(&assertion).to fail_with_message(message)
end
end
def having_one_detail(options = {})
define_model :detail, person_id: :integer
define_model(:person).tap do |model|