diff options
Diffstat (limited to 'activemodel')
| -rw-r--r-- | activemodel/CHANGELOG.md | 20 | ||||
| -rw-r--r-- | activemodel/lib/active_model/dirty.rb | 34 | ||||
| -rw-r--r-- | activemodel/lib/active_model/errors.rb | 2 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations.rb | 6 | ||||
| -rw-r--r-- | activemodel/test/cases/dirty_test.rb | 43 | ||||
| -rw-r--r-- | activemodel/test/cases/validations_test.rb | 7 |
6 files changed, 95 insertions, 17 deletions
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2565b24c97..8d22e3ac46 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,4 +1,22 @@ -* Added `undo_changes` method to `ActiveModel::Dirty` API to restore all the +* Validate options passed to `ActiveModel::Validations.validate`. + + Preventing, in many cases, the simple mistake of using `validate` instead of `validates`. + + *Sonny Michaud* + +* Deprecate `reset_#{attribute}` in favor of `restore_#{attribute}`. + + These methods may cause confusion with the `reset_changes` that behaves differently + of them. + +* Deprecate `ActiveModel::Dirty#reset_changes` in favor of `#clear_changes_information`. + + This method name is causing confusion with the `reset_#{attribute}` + methods. While `reset_name` set the value of the name attribute for the + previous value `reset_changes` only discard the changes and previous + changes. + +* Added `restore_attributes` method to `ActiveModel::Dirty` API to restore all the changed values to the previous data. *Igor G.* diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index dc6088a39a..d11243c4c0 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -15,9 +15,9 @@ module ActiveModel # * Call <tt>attr_name_will_change!</tt> before each change to the tracked # attribute. # * Call <tt>changes_applied</tt> after the changes are persisted. - # * Call <tt>reset_changes</tt> when you want to reset the changes + # * Call <tt>clear_changes_information</tt> when you want to reset the changes # information. - # * Call <tt>undo_changes</tt> when you want to restore previous data. + # * Call <tt>restore_attributes</tt> when you want to restore previous data. # # A minimal implementation could be: # @@ -37,15 +37,18 @@ module ActiveModel # # def save # # do persistence work + # # changes_applied # end # # def reload! - # reset_changes + # # get the values from the persistence layer + # + # clear_changes_information # end # # def rollback! - # undo_changes + # restore_attributes # end # end # @@ -113,6 +116,7 @@ module ActiveModel included do attribute_method_suffix '_changed?', '_change', '_will_change!', '_was' attribute_method_affix prefix: 'reset_', suffix: '!' + attribute_method_affix prefix: 'restore_', suffix: '!' end # Returns +true+ if any attribute have unsaved changes, +false+ otherwise. @@ -176,6 +180,11 @@ module ActiveModel attribute_changed?(attr) ? changed_attributes[attr] : __send__(attr) end + # Restore all previous data of the provided attributes. + def restore_attributes(attributes = changed) + attributes.each { |attr| restore_attribute! attr } + end + private # Removes current changes and makes them accessible through +previous_changes+. @@ -184,15 +193,15 @@ module ActiveModel @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end - # Removes all dirty data: current changes and previous changes. - def reset_changes # :doc: + # Clear all dirty data: current changes and previous changes. + def clear_changes_information # :doc: @previously_changed = ActiveSupport::HashWithIndifferentAccess.new @changed_attributes = ActiveSupport::HashWithIndifferentAccess.new end - # Restore all previous data. - def undo_changes # :doc: - changed_attributes.each_key { |attr| reset_attribute! attr } + def reset_changes + ActiveSupport::Deprecation.warn "#reset_changes is deprecated and will be removed on Rails 5. Please use #clear_changes_information instead." + clear_changes_information end # Handle <tt>*_change</tt> for +method_missing+. @@ -215,6 +224,13 @@ module ActiveModel # Handle <tt>reset_*!</tt> for +method_missing+. def reset_attribute!(attr) + ActiveSupport::Deprecation.warn "#reset_#{attr}! is deprecated and will be removed on Rails 5. Please use #restore_#{attr}! instead." + + restore_attribute!(attr) + end + + # Handle <tt>restore_*!</tt> for +method_missing+. + def restore_attribute!(attr) if attribute_changed?(attr) __send__("#{attr}=", changed_attributes[attr]) changed_attributes.delete(attr) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 917d3b9142..1d025beeef 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -23,7 +23,7 @@ module ActiveModel # attr_reader :errors # # def validate! - # errors.add(:name, "cannot be nil") if name == nil + # errors.add(:name, "cannot be nil") if name.nil? # end # # # The following methods are needed to be minimally implemented diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index cf97f45dba..f67a3be5c1 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -141,6 +141,11 @@ module ActiveModel # value. def validate(*args, &block) options = args.extract_options! + + if args.all? { |arg| arg.is_a?(Symbol) } + options.assert_valid_keys([:on, :if, :unless]) + end + if options.key?(:on) options = options.dup options[:if] = Array(options[:if]) @@ -148,6 +153,7 @@ module ActiveModel Array(options[:on]).include?(o.validation_context) } end + args << options set_callback(:validate, *args, &block) end diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index 562fadbb85..db2cd885e2 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -43,11 +43,11 @@ class DirtyTest < ActiveModel::TestCase end def reload - reset_changes + clear_changes_information end - def rollback - undo_changes + def deprecated_reload + reset_changes end end @@ -111,7 +111,7 @@ class DirtyTest < ActiveModel::TestCase test "resetting attribute" do @model.name = "Bob" - @model.reset_name! + @model.restore_name! assert_nil @model.name assert !@model.name_changed? end @@ -181,17 +181,48 @@ class DirtyTest < ActiveModel::TestCase assert_equal ActiveSupport::HashWithIndifferentAccess.new, @model.changed_attributes end - test "undo_changes should restore all previous data" do + test "reset_changes is deprecated" do + @model.name = 'Dmitry' + @model.name_changed? + @model.save + @model.name = 'Bob' + + assert_equal [nil, 'Dmitry'], @model.previous_changes['name'] + assert_equal 'Dmitry', @model.changed_attributes['name'] + + assert_deprecated do + @model.deprecated_reload + end + + assert_equal ActiveSupport::HashWithIndifferentAccess.new, @model.previous_changes + assert_equal ActiveSupport::HashWithIndifferentAccess.new, @model.changed_attributes + end + + test "restore_attributes should restore all previous data" do @model.name = 'Dmitry' @model.color = 'Red' @model.save @model.name = 'Bob' @model.color = 'White' - @model.rollback + @model.restore_attributes assert_not @model.changed? assert_equal 'Dmitry', @model.name assert_equal 'Red', @model.color end + + test "restore_attributes can restore only some attributes" do + @model.name = 'Dmitry' + @model.color = 'Red' + @model.save + @model.name = 'Bob' + @model.color = 'White' + + @model.restore_attributes(['name']) + + assert @model.changed? + assert_equal 'Dmitry', @model.name + assert_equal 'White', @model.color + end end diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 4fee704ef5..38e54b7f17 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -167,6 +167,13 @@ class ValidationsTest < ActiveModel::TestCase end end + def test_invalid_options_to_validate + assert_raises(ArgumentError) do + # A common mistake -- we meant to call 'validates' + Topic.validate :title, presence: true + end + end + def test_errors_conversions Topic.validates_presence_of %w(title content) t = Topic.new |