Skip to content

Commit

Permalink
Fix validation callbacks on multiple context
Browse files Browse the repository at this point in the history
I found a bug that validation callbacks don't fire on multiple context.
So I've fixed it.

Example:

```ruby
class Dog
  include ActiveModel::Validations
  include ActiveModel::Validations::Callbacks

  attr_accessor :history

  def initialize
    @history = []
  end

  before_validation :set_before_validation_on_a, on: :a
  before_validation :set_before_validation_on_b, on: :b
  after_validation :set_after_validation_on_a, on: :a
  after_validation :set_after_validation_on_b, on: :b

  def set_before_validation_on_a; history << "before_validation on a"; end
  def set_before_validation_on_b; history << "before_validation on b"; end
  def set_after_validation_on_a;  history << "after_validation on a" ; end
  def set_after_validation_on_b;  history << "after_validation on b" ; end
end
```

Before:

```
d = Dog.new
d.valid?([:a, :b])
d.history # []
```

After:

```
d = Dog.new
d.valid?([:a, :b])
d.history # ["before_validation on a", "before_validation on b", "after_validation on a", "after_validation on b"]
```
  • Loading branch information
yhirano55 committed Dec 19, 2017
1 parent 011f76e commit 470d0e4
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 9 deletions.
4 changes: 4 additions & 0 deletions activemodel/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* Fix to working before/after validation callbacks on multiple contexts.

*Yoshiyuki Hirano*

## Rails 5.2.0.beta2 (November 28, 2017) ##

* No changes.
Expand Down
22 changes: 13 additions & 9 deletions activemodel/lib/active_model/validations/callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,16 @@ module ClassMethods
# person.valid? # => true
# person.name # => "bob"
def before_validation(*args, &block)
options = args.last
if options.is_a?(Hash) && options[:on]
options[:if] = Array(options[:if])
options[:on] = Array(options[:on])
options = args.extract_options!
options[:if] = Array(options[:if])

if options.key?(:on)
options[:if].unshift ->(o) {
options[:on].include? o.validation_context
!(Array(options[:on]) & Array(o.validation_context)).empty?
}
end

args << options
set_callback(:validation, :before, *args, &block)
end

Expand Down Expand Up @@ -95,13 +97,15 @@ def after_validation(*args, &block)
options = args.extract_options!
options[:prepend] = true
options[:if] = Array(options[:if])
if options[:on]
options[:on] = Array(options[:on])

if options.key?(:on)
options[:if].unshift ->(o) {
options[:on].include? o.validation_context
!(Array(options[:on]) & Array(o.validation_context)).empty?
}
end
set_callback(:validation, :after, *(args << options), &block)

args << options
set_callback(:validation, :after, *args, &block)
end
end

Expand Down
43 changes: 43 additions & 0 deletions activemodel/test/cases/validations/callbacks_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ def set_before_validation_marker; history << "before_validation_marker"; end
def set_after_validation_marker; history << "after_validation_marker" ; end
end

class DogValidatorWithOnMultipleCondition < Dog
before_validation :set_before_validation_marker_on_context_a, on: :context_a
before_validation :set_before_validation_marker_on_context_b, on: :context_b
after_validation :set_after_validation_marker_on_context_a, on: :context_a
after_validation :set_after_validation_marker_on_context_b, on: :context_b

def set_before_validation_marker_on_context_a; history << "before_validation_marker on context_a"; end
def set_before_validation_marker_on_context_b; history << "before_validation_marker on context_b"; end
def set_after_validation_marker_on_context_a; history << "after_validation_marker on context_a" ; end
def set_after_validation_marker_on_context_b; history << "after_validation_marker on context_b" ; end
end

class DogValidatorWithIfCondition < Dog
before_validation :set_before_validation_marker1, if: -> { true }
before_validation :set_before_validation_marker2, if: -> { false }
Expand Down Expand Up @@ -98,6 +110,37 @@ def test_on_condition_is_respected_for_validation_without_context
assert_equal [], d.history
end

def test_on_multiple_condition_is_respected_for_validation_with_matching_context
d = DogValidatorWithOnMultipleCondition.new
d.valid?(:context_a)
assert_equal ["before_validation_marker on context_a", "after_validation_marker on context_a"], d.history

d = DogValidatorWithOnMultipleCondition.new
d.valid?(:context_b)
assert_equal ["before_validation_marker on context_b", "after_validation_marker on context_b"], d.history

d = DogValidatorWithOnMultipleCondition.new
d.valid?([:context_a, :context_b])
assert_equal([
"before_validation_marker on context_a",
"before_validation_marker on context_b",
"after_validation_marker on context_a",
"after_validation_marker on context_b"
], d.history)
end

def test_on_multiple_condition_is_respected_for_validation_without_matching_context
d = DogValidatorWithOnMultipleCondition.new
d.valid?(:save)
assert_equal [], d.history
end

def test_on_multiple_condition_is_respected_for_validation_without_context
d = DogValidatorWithOnMultipleCondition.new
d.valid?
assert_equal [], d.history
end

def test_before_validation_and_after_validation_callbacks_should_be_called
d = DogWithMethodCallbacks.new
d.valid?
Expand Down

0 comments on commit 470d0e4

Please sign in to comment.