diff --git a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb index 49ef78d8..65c47baf 100644 --- a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb @@ -239,7 +239,7 @@ validation for you? Instead of using `validate_presence_of`, try else values = [] - if !association_being_validated? + if attribute_accepts_string_values? values << '' end @@ -316,12 +316,16 @@ validation for you? Instead of using `validate_presence_of`, try end def belongs_to_association_being_validated? - association_being_validated? && + !!association_reflection && association_reflection.macro == :belongs_to end - def association_being_validated? - !!association_reflection + def attribute_accepts_string_values? + !association_reflection && ( + !attribute_type.respond_to?(:coder) || + !attribute_type.coder || + attribute_type.coder.object_class == String + ) end def association_name @@ -337,6 +341,14 @@ validation for you? Instead of using `validate_presence_of`, try model.reflect_on_association(@attribute) end + def attribute_type + if model.respond_to?(:attribute_types) + model.attribute_types[@attribute.to_s] + else + LegacyAttributeType.new(model, @attribute) + end + end + def presence_validation_exists_on_attribute? model._validators.include?(@attribute) end @@ -344,6 +356,25 @@ validation for you? Instead of using `validate_presence_of`, try def model @subject.class end + + class LegacyAttributeType + def initialize(model, attribute_name) + @model = model + @attribute_name = attribute_name + end + + def coder + if model.respond_to?(:serialized_attributes) + ActiveSupport::Deprecation.silence do + model.serialized_attributes[attribute_name.to_s] + end + end + end + + private + + attr_reader :model, :attribute_name + end end end end diff --git a/spec/support/unit/helpers/active_model_versions.rb b/spec/support/unit/helpers/active_model_versions.rb index db7037de..15468cf2 100644 --- a/spec/support/unit/helpers/active_model_versions.rb +++ b/spec/support/unit/helpers/active_model_versions.rb @@ -28,5 +28,9 @@ module UnitTests def active_model_supports_strict? active_model_version >= 3.2 end + + def active_model_supports_full_attributes_api? + active_model_version >= '5.2' + end end end diff --git a/spec/support/unit/helpers/model_builder.rb b/spec/support/unit/helpers/model_builder.rb index fe6058df..e2e44ebb 100644 --- a/spec/support/unit/helpers/model_builder.rb +++ b/spec/support/unit/helpers/model_builder.rb @@ -56,13 +56,9 @@ module UnitTests def define_active_model_class(class_name, options = {}, &block) attribute_names = options.delete(:accessors) { [] } - columns = attribute_names.reduce({}) do |hash, attribute_name| - hash.merge(attribute_name => nil) - end - UnitTests::ModelCreationStrategies::ActiveModel.call( class_name, - columns, + attribute_names, options, &block ) diff --git a/spec/support/unit/model_creation_strategies/active_model.rb b/spec/support/unit/model_creation_strategies/active_model.rb index 5f51bec3..45b5f755 100644 --- a/spec/support/unit/model_creation_strategies/active_model.rb +++ b/spec/support/unit/model_creation_strategies/active_model.rb @@ -1,13 +1,19 @@ module UnitTests module ModelCreationStrategies class ActiveModel - def self.call(name, columns = {}, options = {}, &block) - new(name, columns, options, &block).call + def self.call(name, attribute_names = [], options = {}, &block) + new(name, attribute_names, options, &block).call end - def initialize(name, columns = {}, options = {}, &block) + def initialize(name, attribute_names = [], options = {}, &block) @name = name - @columns = columns + @attribute_names = + if attribute_names.is_a?(Hash) + # mimicking columns + attribute_names.keys + else + attribute_names + end @options = options @model_customizers = [] @@ -22,7 +28,9 @@ module UnitTests def call ClassBuilder.define_class(name, Model).tap do |model| - model.columns = columns.keys + attribute_names.each do |attribute_name| + model.attribute(attribute_name) + end model_customizers.each do |block| run_block(model, block) @@ -30,12 +38,10 @@ module UnitTests end end - protected - - attr_reader :name, :columns, :model_customizers, :options - private + attr_reader :name, :attribute_names, :model_customizers, :options + def run_block(model, block) if block if block.arity == 0 @@ -46,29 +52,40 @@ module UnitTests end end - class Model - class << self - def columns - @_columns ||= [] - end + module PoorMansAttributes + extend ActiveSupport::Concern - def columns=(columns) - existing_columns = self.columns - new_columns = columns - existing_columns + included do + class_attribute :attribute_names - @_columns += new_columns + self.attribute_names = Set.new + end + module ClassMethods + def attribute(name) include attributes_module - attributes_module.module_eval do - new_columns.each do |column| - define_method(column) do - attributes[column] - end + name = name.to_sym - define_method("#{column}=") do |value| - attributes[column] = value - end + if ( + attribute_names.include?(name) && + attributes_module.instance_methods.include?(name) + ) + attributes_module.module_eval do + remove_method(name) + remove_method("#{name}=") + end + end + + self.attribute_names += [name] + + attributes_module.module_eval do + define_method(name) do + attributes[name] + end + + define_method("#{name}=") do |value| + attributes[name] = value end end end @@ -80,8 +97,6 @@ module UnitTests end end - include ::ActiveModel::Model - attr_reader :attributes def initialize(attributes = {}) @@ -92,7 +107,7 @@ module UnitTests middle = '%s:0x%014x%s' % [ self.class, object_id * 2, - ' ' + inspected_attributes.join(' ') + ' ' + inspected_attributes.join(' '), ] "#<#{middle.strip}>" @@ -101,11 +116,21 @@ module UnitTests private def inspected_attributes - self.class.columns.map do |key, value| - "#{key}: #{attributes[key].inspect}" + self.class.attribute_names.map do |name| + "#{name}: #{attributes[name].inspect}" end end end + + class Model + include ::ActiveModel::Model + + if defined?(::ActiveModel::Attributes) + include ::ActiveModel::Attributes + else + include PoorMansAttributes + end + end end end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb index 57f80be3..76301d27 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb @@ -55,6 +55,50 @@ this could not be proved. expect(&assertion).to fail_with_message(message) end + + context 'when the attribute is decorated with serialize' do + context 'and the type is a string' do + it 'still works' do + record = record_validating_presence_of(:traits) do + serialize :traits, String + end + + expect(record).to validate_presence_of(:traits) + end + end + + context 'and the type is not a string' do + it 'still works' do + record = record_validating_presence_of(:traits) do + serialize :traits, Array + end + + expect(record).to validate_presence_of(:traits) + end + end + end + + context 'when the column backing the attribute is a scalar, but not a string' do + it 'still works' do + record = record_validating_presence_of( + :pinned_on, + column_options: { type: :date }, + ) + + expect(record).to validate_presence_of(:pinned_on) + end + end + + context 'when the column backing the attribute is an array' do + it 'still works' do + record = record_validating_presence_of( + :possible_meeting_dates, + column_options: { type: :date, array: true }, + ) + + expect(record).to validate_presence_of(:possible_meeting_dates) + end + end end context 'a model without a presence validation' do @@ -112,6 +156,30 @@ could not be proved. } ) + if active_model_supports_full_attributes_api? + context 'when the attribute has been configured with a type' do + context 'and it is a string' do + it 'works' do + record = active_model_object_validating_presence_of(:age) do + attribute :age, :string + end + + expect(record).to validate_presence_of(:age) + end + end + + context 'and it is not a string' do + it 'still works' do + record = active_model_object_validating_presence_of(:age) do + attribute :age, :time + end + + expect(record).to validate_presence_of(:age) + end + end + end + end + def model_creator :active_model end @@ -120,7 +188,9 @@ could not be proved. context 'an ActiveModel class without a presence validation' do it 'rejects with the correct failure message' do assertion = lambda do - expect(active_model).to matcher + record = plain_active_model_object_with(:attr, model_name: 'Example') + + expect(record).to matcher end message = <<-MESSAGE @@ -826,22 +896,58 @@ could not be proved. validate_presence_of(:attr) end - def validating_presence(options = {}) - define_model :example, attr: :string do - validates_presence_of :attr, options - end.new + def record_validating_presence_of( + attribute_name = :attr, + column_options: { type: :string }, + **options, + &block + ) + model = define_model 'Example', attribute_name => column_options do + validates_presence_of(attribute_name, options) + + if block + class_eval(&block) + end + end + + model.new end + alias_method :validating_presence, :record_validating_presence_of def without_validating_presence define_model(:example, attr: :string).new end - def active_model(&block) - define_active_model_class('Example', accessors: [:attr], &block).new - end + def active_model_object_validating_presence_of( + attribute_name = :attr, + **options, + &block + ) + plain_active_model_object_with(attribute_name, **options) do + validates_presence_of(attribute_name) - def active_model_validating_presence - active_model { validates_presence_of :attr } + if block + class_eval(&block) + end + end + end + alias_method :active_model_validating_presence, + :active_model_object_validating_presence_of + + def plain_active_model_object_with( + attribute_name = :attr, + model_name: 'Example', + **options, + &block + ) + model = define_active_model_class( + model_name, + accessors: [attribute_name], + **options, + &block + ) + + model.new end def has_many_children(options = {})