diff options
Diffstat (limited to 'activemodel')
31 files changed, 370 insertions, 71 deletions
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index f86b4804c8..5830a00bc5 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,56 @@ +* Assigning an unknown attribute key to an `ActiveModel` instance during initialization + will now raise `ActiveModel::AttributeAssignment::UnknownAttributeError` instead of + `NoMethodError` + + Example: + + User.new(foo: 'some value') + # => ActiveModel::AttributeAssignment::UnknownAttributeError: unknown attribute 'foo' for User. + + *Eugene Gilburg* + +* Extracted `ActiveRecord::AttributeAssignment` to `ActiveModel::AttributeAssignment` + allowing to use it for any object as an includable module. + + Example: + + class Cat + include ActiveModel::AttributeAssignment + attr_accessor :name, :status + end + + cat = Cat.new + cat.assign_attributes(name: "Gorby", status: "yawning") + cat.name # => 'Gorby' + cat.status => 'yawning' + cat.assign_attributes(status: "sleeping") + cat.name # => 'Gorby' + cat.status => 'sleeping' + + *Bogdan Gusiev* + +* Add `ActiveModel::Errors#details` + + To be able to return type of used validator, one can now call `details` + on errors instance. + + Example: + + class User < ActiveRecord::Base + validates :name, presence: true + end + + user = User.new; user.valid?; user.errors.details + => {name: [{error: :blank}]} + + *Wojciech Wnętrzak* + +* Change validates_acceptance_of to accept true by default. + + The default for validates_acceptance_of is now "1" and true. + In the past, only "1" was the default and you were required to add + accept: true. + * Remove deprecated `ActiveModel::Dirty#reset_#{attribute}` and `ActiveModel::Dirty#reset_changes`. diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index 46d60db756..58fe08cc11 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -28,6 +28,7 @@ require 'active_model/version' module ActiveModel extend ActiveSupport::Autoload + autoload :AttributeAssignment autoload :AttributeMethods autoload :BlockValidator, 'active_model/validator' autoload :Callbacks diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb new file mode 100644 index 0000000000..356421476c --- /dev/null +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -0,0 +1,63 @@ +require 'active_support/core_ext/hash/keys' + +module ActiveModel + module AttributeAssignment + include ActiveModel::ForbiddenAttributesProtection + + # Allows you to set all the attributes by passing in a hash of attributes with + # keys matching the attribute names. + # + # If the passed hash responds to <tt>permitted?</tt> method and the return value + # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> + # exception is raised. + # + # class Cat + # include ActiveModel::AttributeAssignment + # attr_accessor :name, :status + # end + # + # cat = Cat.new + # cat.assign_attributes(name: "Gorby", status: "yawning") + # cat.name # => 'Gorby' + # cat.status => 'yawning' + # cat.assign_attributes(status: "sleeping") + # cat.name # => 'Gorby' + # cat.status => 'sleeping' + def assign_attributes(new_attributes) + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end + return if new_attributes.blank? + + attributes = new_attributes.stringify_keys + _assign_attributes(sanitize_for_mass_assignment(attributes)) + end + + private + + def _assign_attributes(attributes) + attributes.each do |k, v| + _assign_attribute(k, v) + end + end + + def _assign_attribute(k, v) + if respond_to?("#{k}=") + public_send("#{k}=", v) + else + raise UnknownAttributeError.new(self, k) + end + end + + # Raised when unknown attributes are supplied via mass assignment. + class UnknownAttributeError < NoMethodError + attr_reader :record, :attribute + + def initialize(record, attribute) + @record = record + @attribute = attribute + super("unknown attribute '#{attribute}' for #{@record.class}.") + end + end + end +end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index afba9bab0d..ca5dac272f 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -52,10 +52,10 @@ module ActiveModel # end # end # - # A newly instantiated object is unchanged: + # A newly instantiated +Person+ object is unchanged: # - # person = Person.find_by(name: 'Uncle Bob') - # person.changed? # => false + # person = Person.new + # person.changed? # => false # # Change the name: # @@ -71,8 +71,8 @@ module ActiveModel # Save the changes: # # person.save - # person.changed? # => false - # person.name_changed? # => false + # person.changed? # => false + # person.name_changed? # => false # # Reset the changes: # @@ -84,20 +84,20 @@ module ActiveModel # # person.name = "Uncle Bob" # person.rollback! - # person.name # => "Bill" - # person.name_changed? # => false + # person.name # => "Bill" + # person.name_changed? # => false # # Assigning the same value leaves the attribute unchanged: # # person.name = 'Bill' - # person.name_changed? # => false - # person.name_change # => nil + # person.name_changed? # => false + # person.name_change # => nil # # Which attributes have changed? # # person.name = 'Bob' - # person.changed # => ["name"] - # person.changes # => {"name" => ["Bill", "Bob"]} + # person.changed # => ["name"] + # person.changes # => {"name" => ["Bill", "Bob"]} # # If an attribute is modified in-place then make use of # +[attribute_name]_will_change!+ to mark that the attribute is changing. @@ -106,9 +106,9 @@ module ActiveModel # not need to call +[attribute_name]_will_change!+ on Active Record models. # # person.name_will_change! - # person.name_change # => ["Bill", "Bill"] + # person.name_change # => ["Bill", "Bill"] # person.name << 'y' - # person.name_change # => ["Bill", "Billy"] + # person.name_change # => ["Bill", "Billy"] module Dirty extend ActiveSupport::Concern include ActiveModel::AttributeMethods @@ -189,6 +189,7 @@ module ActiveModel def changes_include?(attr_name) attributes_changed_by_setter.include?(attr_name) end + alias attribute_changed_by_setter? changes_include? # Removes current changes and makes them accessible through +previous_changes+. def changes_applied # :doc: diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 55687cb3c7..a809c72ccd 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/array/conversions' require 'active_support/core_ext/string/inflections' +require 'active_support/core_ext/object/deep_dup' module ActiveModel # == Active \Model \Errors @@ -23,7 +24,7 @@ module ActiveModel # attr_reader :errors # # def validate! - # errors.add(:name, "cannot be nil") if name.nil? + # errors.add(:name, :blank, message: "cannot be nil") if name.nil? # end # # # The following methods are needed to be minimally implemented @@ -58,8 +59,9 @@ module ActiveModel include Enumerable CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] + MESSAGE_OPTIONS = [:message] - attr_reader :messages + attr_reader :messages, :details # Pass in the instance of the object that is using the errors object. # @@ -71,10 +73,12 @@ module ActiveModel def initialize(base) @base = base @messages = {} + @details = Hash.new { |details, attribute| details[attribute] = [] } end def initialize_dup(other) # :nodoc: @messages = other.messages.dup + @details = other.details.deep_dup super end @@ -85,6 +89,7 @@ module ActiveModel # person.errors.full_messages # => [] def clear messages.clear + details.clear end # Returns +true+ if the error messages include an error for the given key @@ -126,6 +131,7 @@ module ActiveModel # person.errors.get(:name) # => nil def delete(key) messages.delete(key) + details.delete(key) end # When passed a symbol or a name of a method, returns an array of errors @@ -149,12 +155,12 @@ module ActiveModel # Yields the attribute and the error for that attribute. If the attribute # has more than one error message, yields once for each error message. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.each do |attribute, error| # # Will yield :name and "can't be blank" # end # - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.each do |attribute, error| # # Will yield :name and "can't be blank" # # then yield :name and "must be specified" @@ -167,9 +173,9 @@ module ActiveModel # Returns the number of error messages. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.size # => 1 - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.size # => 2 def size values.flatten.size @@ -193,8 +199,8 @@ module ActiveModel # Returns an array of error messages, with the attribute name included. # - # person.errors.add(:name, "can't be blank") - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :blank, message: "can't be blank") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.to_a # => ["name can't be blank", "name must be specified"] def to_a full_messages @@ -202,9 +208,9 @@ module ActiveModel # Returns the number of error messages. # - # person.errors.add(:name, "can't be blank") + # person.errors.add(:name, :blank, message: "can't be blank") # person.errors.count # => 1 - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.count # => 2 def count to_a.size @@ -223,8 +229,8 @@ module ActiveModel # Returns an xml formatted representation of the Errors hash. # - # person.errors.add(:name, "can't be blank") - # person.errors.add(:name, "must be specified") + # person.errors.add(:name, :blank, message: "can't be blank") + # person.errors.add(:name, :not_specified, message: "must be specified") # person.errors.to_xml # # => # # <?xml version=\"1.0\" encoding=\"UTF-8\"?> @@ -261,17 +267,20 @@ module ActiveModel end end - # Adds +message+ to the error messages on +attribute+. More than one error - # can be added to the same +attribute+. If no +message+ is supplied, - # <tt>:invalid</tt> is assumed. + # Adds +message+ to the error messages and used validator type to +details+ on +attribute+. + # More than one error can be added to the same +attribute+. + # If no +message+ is supplied, <tt>:invalid</tt> is assumed. # # person.errors.add(:name) # # => ["is invalid"] - # person.errors.add(:name, 'must be implemented') + # person.errors.add(:name, :not_implemented, message: "must be implemented") # # => ["is invalid", "must be implemented"] # # person.errors.messages - # # => {:name=>["must be implemented", "is invalid"]} + # # => {:name=>["is invalid", "must be implemented"]} + # + # person.errors.details + # # => {:name=>[{error: :not_implemented}, {error: :invalid}]} # # If +message+ is a symbol, it will be translated using the appropriate # scope (see +generate_message+). @@ -293,16 +302,22 @@ module ActiveModel # +attribute+ should be set to <tt>:base</tt> if the error is not # directly associated with a single attribute. # - # person.errors.add(:base, "either name or email must be present") + # person.errors.add(:base, :name_or_email_blank, + # message: "either name or email must be present") # person.errors.messages # # => {:base=>["either name or email must be present"]} + # person.errors.details + # # => {:base=>[{error: :name_or_email_blank}]} def add(attribute, message = :invalid, options = {}) + message = message.call if message.respond_to?(:call) + detail = normalize_detail(attribute, message, options) message = normalize_message(attribute, message, options) if exception = options[:strict] exception = ActiveModel::StrictValidationFailed if exception == true raise exception, full_message(attribute, message) end + details[attribute.to_sym] << detail self[attribute] << message end @@ -339,6 +354,7 @@ module ActiveModel # person.errors.add :name, :blank # person.errors.added? :name, :blank # => true def added?(attribute, message = :invalid, options = {}) + message = message.call if message.respond_to?(:call) message = normalize_message(attribute, message, options) self[attribute].include? message end @@ -388,8 +404,8 @@ module ActiveModel # Translates an error message in its default scope # (<tt>activemodel.errors.messages</tt>). # - # Error messages are first looked up in <tt>models.MODEL.attributes.ATTRIBUTE.MESSAGE</tt>, - # if it's not there, it's looked up in <tt>models.MODEL.MESSAGE</tt> and if + # Error messages are first looked up in <tt>activemodel.errors.models.MODEL.attributes.ATTRIBUTE.MESSAGE</tt>, + # if it's not there, it's looked up in <tt>activemodel.errors.models.MODEL.MESSAGE</tt> and if # that is not there also, it returns the translation of the default message # (e.g. <tt>activemodel.errors.messages.MESSAGE</tt>). The translated model # name, translated attribute name and the value are available for @@ -447,12 +463,14 @@ module ActiveModel case message when Symbol generate_message(attribute, message, options.except(*CALLBACKS_OPTIONS)) - when Proc - message.call else message end end + + def normalize_detail(attribute, message, options) + { error: message }.merge(options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)) + end end # Raised when a validation cannot be corrected by end users and are considered diff --git a/activemodel/lib/active_model/locale/en.yml b/activemodel/lib/active_model/locale/en.yml index bf07945fe1..b11c8f53b4 100644 --- a/activemodel/lib/active_model/locale/en.yml +++ b/activemodel/lib/active_model/locale/en.yml @@ -16,7 +16,7 @@ en: present: "must be blank" too_long: one: "is too long (maximum is 1 character)" - other: "is too long (maximum is %{count} characters)" + other: "is too long (maximum is %{count} characters)" too_short: one: "is too short (minimum is 1 character)" other: "is too short (minimum is %{count} characters)" diff --git a/activemodel/lib/active_model/model.rb b/activemodel/lib/active_model/model.rb index d51d6ddcc9..dac8d549a7 100644 --- a/activemodel/lib/active_model/model.rb +++ b/activemodel/lib/active_model/model.rb @@ -57,6 +57,7 @@ module ActiveModel # (see below). module Model extend ActiveSupport::Concern + include ActiveModel::AttributeAssignment include ActiveModel::Validations include ActiveModel::Conversion @@ -75,10 +76,8 @@ module ActiveModel # person = Person.new(name: 'bob', age: '18') # person.name # => "bob" # person.age # => "18" - def initialize(params={}) - params.each do |attr, value| - self.public_send("#{attr}=", value) - end if params + def initialize(attributes={}) + assign_attributes(attributes) if attributes super() end diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index ada1f9a4f3..2bc3eeaa19 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/module/introspection' +require 'active_support/core_ext/module/remove_method' module ActiveModel class Name diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 96e88f1b6c..871031ece4 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -77,13 +77,6 @@ module ActiveModel validates_length_of :password, maximum: ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED validates_confirmation_of :password, allow_blank: true end - - # This code is necessary as long as the protected_attributes gem is supported. - if respond_to?(:attributes_protected_by_default) - def self.attributes_protected_by_default #:nodoc: - super + ['password_digest'] - end - end end end diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index ac5e79859b..ee160fb483 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -3,12 +3,12 @@ module ActiveModel module Validations class AcceptanceValidator < EachValidator # :nodoc: def initialize(options) - super({ allow_nil: true, accept: "1" }.merge!(options)) + super({ allow_nil: true, accept: ["1", true] }.merge!(options)) setup!(options[:class]) end def validate_each(record, attribute, value) - unless value == options[:accept] + unless acceptable_option?(value) record.errors.add(attribute, :accepted, options.except(:accept, :allow_nil)) end end @@ -20,6 +20,10 @@ module ActiveModel klass.send(:attr_reader, *attr_readers) klass.send(:attr_writer, *attr_writers) end + + def acceptable_option?(value) + Array(options[:accept]).include?(value) + end end module HelperMethods diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index ac32750946..b98585912e 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -163,6 +163,10 @@ module ActiveModel # +ArgumentError+ when invalid options are supplied. def check_validity! end + + def should_validate?(record) # :nodoc: + !record.persisted? || record.changed? || record.marked_for_destruction? + end end # +BlockValidator+ is a special +EachValidator+ which receives a block on initialization diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb new file mode 100644 index 0000000000..402caf21f7 --- /dev/null +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -0,0 +1,107 @@ +require "cases/helper" +require "active_support/hash_with_indifferent_access" + +class AttributeAssignmentTest < ActiveModel::TestCase + class Model + include ActiveModel::AttributeAssignment + + attr_accessor :name, :description + + def initialize(attributes = {}) + assign_attributes(attributes) + end + + def broken_attribute=(value) + raise ErrorFromAttributeWriter + end + + protected + + attr_writer :metadata + end + + class ErrorFromAttributeWriter < StandardError + end + + class ProtectedParams < ActiveSupport::HashWithIndifferentAccess + def permit! + @permitted = true + end + + def permitted? + @permitted ||= false + end + + def dup + super.tap do |duplicate| + duplicate.instance_variable_set :@permitted, permitted? + end + end + end + + test "simple assignment" do + model = Model.new + + model.assign_attributes(name: "hello", description: "world") + assert_equal "hello", model.name + assert_equal "world", model.description + end + + test "assign non-existing attribute" do + model = Model.new + error = assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + model.assign_attributes(hz: 1) + end + + assert_equal model, error.record + assert_equal "hz", error.attribute + end + + test "assign private attribute" do + model = Model.new + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + model.assign_attributes(metadata: { a: 1 }) + end + end + + test "does not swallow errors raised in an attribute writer" do + assert_raises(ErrorFromAttributeWriter) do + Model.new(broken_attribute: 1) + end + end + + test "an ArgumentError is raised if a non-hash-like obejct is passed" do + assert_raises(ArgumentError) do + Model.new(1) + end + end + + test "forbidden attributes cannot be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "m") + + assert_raises(ActiveModel::ForbiddenAttributesError) do + Model.new(params) + end + end + + test "permitted attributes can be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "desc") + params.permit! + model = Model.new(params) + + assert_equal "Guille", model.name + assert_equal "desc", model.description + end + + test "regular hash should still be used for mass assignment" do + model = Model.new(name: "Guille", description: "m") + + assert_equal "Guille", model.name + assert_equal "m", model.description + end + + test "assigning no attributes should not raise, even if the hash is un-permitted" do + model = Model.new + assert_nil model.assign_attributes(ProtectedParams.new({})) + end +end diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index 8ffd62fd86..66ed8a350a 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -45,10 +45,6 @@ class DirtyTest < ActiveModel::TestCase def reload clear_changes_information end - - def deprecated_reload - reset_changes - end end setup do diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index efedd9055f..17dbb817d0 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -323,4 +323,46 @@ class ErrorsTest < ActiveModel::TestCase person.errors.expects(:generate_message).with(:name, :blank, { message: 'custom' }) person.errors.add_on_blank :name, message: 'custom' end + + test "details returns added error detail" do + person = Person.new + person.errors.add(:name, :invalid) + assert_equal({ name: [{ error: :invalid }] }, person.errors.details) + end + + test "details returns added error detail with custom option" do + person = Person.new + person.errors.add(:name, :greater_than, count: 5) + assert_equal({ name: [{ error: :greater_than, count: 5 }] }, person.errors.details) + end + + test "details do not include message option" do + person = Person.new + person.errors.add(:name, :invalid, message: "is bad") + assert_equal({ name: [{ error: :invalid }] }, person.errors.details) + end + + test "dup duplicates details" do + errors = ActiveModel::Errors.new(Person.new) + errors.add(:name, :invalid) + errors_dup = errors.dup + errors_dup.add(:name, :taken) + assert_not_equal errors_dup.details, errors.details + end + + test "delete removes details on given attribute" do + errors = ActiveModel::Errors.new(Person.new) + errors.add(:name, :invalid) + errors.delete(:name) + assert_empty errors.details[:name] + end + + test "clear removes details" do + person = Person.new + person.errors.add(:name, :invalid) + + assert_equal 1, person.errors.details.count + person.errors.clear + assert person.errors.details.empty? + end end diff --git a/activemodel/test/cases/model_test.rb b/activemodel/test/cases/model_test.rb index ee0fa26546..9a8d873ec9 100644 --- a/activemodel/test/cases/model_test.rb +++ b/activemodel/test/cases/model_test.rb @@ -70,6 +70,8 @@ class ModelTest < ActiveModel::TestCase end def test_mixin_initializer_when_args_dont_exist - assert_raises(NoMethodError) { SimpleModel.new(hello: 'world') } + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do + SimpleModel.new(hello: 'world') + end end end diff --git a/activemodel/test/cases/validations/absence_validation_test.rb b/activemodel/test/cases/validations/absence_validation_test.rb index ebfe1cf4e4..9cbc77dfb5 100644 --- a/activemodel/test/cases/validations/absence_validation_test.rb +++ b/activemodel/test/cases/validations/absence_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' require 'models/person' diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index e78aa1adaf..9c2114d83d 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' @@ -65,4 +64,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase ensure Person.clear_validators! end + + def test_validates_acceptance_of_true + Topic.validates_acceptance_of(:terms_of_service) + + assert Topic.new(terms_of_service: true).valid? + end end diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index 4b0dd58efb..75eb18e795 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' class Dog @@ -50,6 +49,14 @@ class DogWithMissingName < Dog validates_presence_of :name end +class DogValidatorWithOnCondition < Dog + before_validation :set_before_validation_marker, on: :create + after_validation :set_after_validation_marker, on: :create + + def set_before_validation_marker; self.history << 'before_validation_marker'; end + def set_after_validation_marker; self.history << 'after_validation_marker' ; end +end + class DogValidatorWithIfCondition < Dog before_validation :set_before_validation_marker1, if: -> { true } before_validation :set_before_validation_marker2, if: -> { false } @@ -73,6 +80,24 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase assert_equal ["before_validation_marker1", "after_validation_marker1"], d.history end + def test_on_condition_is_respected_for_validation_with_matching_context + d = DogValidatorWithOnCondition.new + d.valid?(:create) + assert_equal ["before_validation_marker", "after_validation_marker"], d.history + end + + def test_on_condition_is_respected_for_validation_without_matching_context + d = DogValidatorWithOnCondition.new + d.valid?(:save) + assert_equal [], d.history + end + + def test_on_condition_is_respected_for_validation_without_context + d = DogValidatorWithOnCondition.new + d.valid? + assert_equal [], d.history + end + def test_before_validation_and_after_validation_callbacks_should_be_called d = DogWithMethodCallbacks.new d.valid? diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb index 1261937b56..296d3b4407 100644 --- a/activemodel/test/cases/validations/conditional_validation_test.rb +++ b/activemodel/test/cases/validations/conditional_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index 65a2a1eb49..c1431548f7 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb index 1ce41f9bc9..b269c3691a 100644 --- a/activemodel/test/cases/validations/exclusion_validation_test.rb +++ b/activemodel/test/cases/validations/exclusion_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb index 0f91b73cd7..86bbbe6ebe 100644 --- a/activemodel/test/cases/validations/format_validation_test.rb +++ b/activemodel/test/cases/validations/format_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index 96084a32ba..70ee7afecc 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - require "cases/helper" require 'models/person' diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index 3a8f3080e1..55d1fb4dcb 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'active_support/all' diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 046ffcb16f..29130ce46c 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index 12a22f9c40..05432abaff 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/presence_validation_test.rb b/activemodel/test/cases/validations/presence_validation_test.rb index ecf16d1e16..59b9db0795 100644 --- a/activemodel/test/cases/validations/presence_validation_test.rb +++ b/activemodel/test/cases/validations/presence_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/validates_test.rb b/activemodel/test/cases/validations/validates_test.rb index 8d4b74ee49..04101f3545 100644 --- a/activemodel/test/cases/validations/validates_test.rb +++ b/activemodel/test/cases/validations/validates_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/person' require 'models/topic' diff --git a/activemodel/test/cases/validations/validations_context_test.rb b/activemodel/test/cases/validations/validations_context_test.rb index 005bf118c6..150dce379f 100644 --- a/activemodel/test/cases/validations/validations_context_test.rb +++ b/activemodel/test/cases/validations/validations_context_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations/with_validation_test.rb b/activemodel/test/cases/validations/with_validation_test.rb index 736c2deea8..01804032f0 100644 --- a/activemodel/test/cases/validations/with_validation_test.rb +++ b/activemodel/test/cases/validations/with_validation_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 2b932683ea..dc125bc884 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -1,4 +1,3 @@ -# encoding: utf-8 require 'cases/helper' require 'models/topic' |