Skip to content

Commit

Permalink
Fix collisions with before and after validation callbacks.
Browse files Browse the repository at this point in the history
This commit allows a user to do something like:
    before_validation :do_stuff, :on => [ :create, :update ]
    after_validation :do_more, :on => [ :create, :update ]
  • Loading branch information
John Foley committed Sep 23, 2012
1 parent 09d298e commit d66d607
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
8 changes: 6 additions & 2 deletions activemodel/lib/active_model/validations/callbacks.rb
Expand Up @@ -56,7 +56,8 @@ def before_validation(*args, &block)
options = args.last
if options.is_a?(Hash) && options[:on]
options[:if] = Array(options[:if])
options[:if].unshift("self.validation_context == :#{options[:on]}")
options[:on] = Array(options[:on])
options[:if].unshift("self.validation_context.in? #{options[:on]}")
end
set_callback(:validation, :before, *args, &block)
end
Expand Down Expand Up @@ -92,7 +93,10 @@ def after_validation(*args, &block)
options = args.extract_options!
options[:prepend] = true
options[:if] = Array(options[:if])
options[:if].unshift("self.validation_context == :#{options[:on]}") if options[:on]
if options[:on]
options[:on] = Array(options[:on])
options[:if].unshift("self.validation_context.in? #{options[:on]}")
end
set_callback(:validation, :after, *(args << options), &block)
end
end
Expand Down
49 changes: 49 additions & 0 deletions activerecord/test/cases/callbacks_test.rb
Expand Up @@ -137,6 +137,32 @@ def history
end
end

class ContextualCallbacksDeveloper < ActiveRecord::Base
self.table_name = 'developers'

before_validation { history << :before_validation }
before_validation :before_validation_on_create_and_update, :on => [ :create, :update ]

validate do
history << :validate
end

after_validation { history << :after_validation }
after_validation :after_validation_on_create_and_update, :on => [ :create, :update ]

def before_validation_on_create_and_update
history << "before_validation_on_#{self.validation_context}".to_sym
end

def after_validation_on_create_and_update
history << "after_validation_on_#{self.validation_context}".to_sym
end

def history
@history ||= []
end
end

class CallbackCancellationDeveloper < ActiveRecord::Base
self.table_name = 'developers'

Expand Down Expand Up @@ -285,6 +311,17 @@ def test_validate_on_create
], david.history
end

def test_validate_on_contextual_create
david = ContextualCallbacksDeveloper.create('name' => 'David', 'salary' => 1000000)
assert_equal [
:before_validation,
:before_validation_on_create,
:validate,
:after_validation,
:after_validation_on_create
], david.history
end

def test_update
david = CallbackDeveloper.find(1)
david.save
Expand Down Expand Up @@ -344,6 +381,18 @@ def test_validate_on_update
], david.history
end

def test_validate_on_contextual_update
david = ContextualCallbacksDeveloper.find(1)
david.save
assert_equal [
:before_validation,
:before_validation_on_update,
:validate,
:after_validation,
:after_validation_on_update
], david.history
end

def test_destroy
david = CallbackDeveloper.find(1)
david.destroy
Expand Down

2 comments on commit d66d607

@skojin
Copy link
Contributor

@skojin skojin commented on d66d607 Sep 24, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's sense of this? we have only two state :create and :update, and if not specify :on option, callbacks will be called for both case.

@mhuggins
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skojin - See my comment here.

Please sign in to comment.