From d66d6076b2a723c72d97fb8e8f90d83249f9bfe5 Mon Sep 17 00:00:00 2001 From: John Foley Date: Mon, 2 Jul 2012 10:18:12 -0600 Subject: [PATCH 1/3] Fix collisions with before and after validation callbacks. This commit allows a user to do something like: before_validation :do_stuff, :on => [ :create, :update ] after_validation :do_more, :on => [ :create, :update ] --- .../lib/active_model/validations/callbacks.rb | 8 ++- activerecord/test/cases/callbacks_test.rb | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index bf3fe7ff04..c59babd831 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -56,7 +56,8 @@ module ActiveModel 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 @@ -92,7 +93,10 @@ module ActiveModel 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 diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index deeef3a3fd..7457bafd4e 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -137,6 +137,32 @@ class OnCallbacksDeveloper < ActiveRecord::Base 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' @@ -285,6 +311,17 @@ class CallbacksTest < ActiveRecord::TestCase ], 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 @@ -344,6 +381,18 @@ class CallbacksTest < ActiveRecord::TestCase ], 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 From 60c65ca8dfc197b83f3b74b7e6ddeede005d416b Mon Sep 17 00:00:00 2001 From: John Foley Date: Mon, 2 Jul 2012 19:23:34 -0600 Subject: [PATCH 2/3] Switch to using include? on validation callbacks --- activemodel/lib/active_model/validations/callbacks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index c59babd831..c153ef4309 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -57,7 +57,7 @@ module ActiveModel if options.is_a?(Hash) && options[:on] options[:if] = Array(options[:if]) options[:on] = Array(options[:on]) - options[:if].unshift("self.validation_context.in? #{options[:on]}") + options[:if].unshift("#{options[:on]}.include? self.validation_context") end set_callback(:validation, :before, *args, &block) end @@ -95,7 +95,7 @@ module ActiveModel options[:if] = Array(options[:if]) if options[:on] options[:on] = Array(options[:on]) - options[:if].unshift("self.validation_context.in? #{options[:on]}") + options[:if].unshift("#{options[:on]}.include? self.validation_context") end set_callback(:validation, :after, *(args << options), &block) end From f31ea4df3ac760ab7ff18ea439e9a9ce9b8c625a Mon Sep 17 00:00:00 2001 From: John Foley Date: Sun, 23 Sep 2012 12:49:34 -0600 Subject: [PATCH 3/3] Add CHANGELOG entry and update the guide --- activerecord/CHANGELOG.md | 4 ++++ .../active_record_validations_callbacks.md | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 04b67cdf3a..344ee6416d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,9 @@ ## Rails 4.0.0 (unreleased) ## +* Allow before and after validations to take an array of lifecycle events + + *John Foley* + * Support for specifying transaction isolation level If your database supports setting the isolation level for a transaction, you can set diff --git a/guides/source/active_record_validations_callbacks.md b/guides/source/active_record_validations_callbacks.md index e5957d8acb..f32c1050ce 100644 --- a/guides/source/active_record_validations_callbacks.md +++ b/guides/source/active_record_validations_callbacks.md @@ -995,6 +995,25 @@ class User < ActiveRecord::Base end ``` +Callbacks can also be registered to only fire on certain lifecycle events: + +class User < ActiveRecord::Base + before_validation :normalize_name, :on => :create + + # :on takes an array as well + after_validation :set_location, :on => [ :create, :update ] + + protected + def normalize_name + self.name = self.name.downcase.titleize + end + + def set_location + self.location = LocationService.query(self) + end +end + + It is considered good practice to declare callback methods as protected or private. If left public, they can be called from outside of the model and violate the principle of object encapsulation. Available Callbacks