From f767981286b4c7dcb96e061a6f3edcc334008ea8 Mon Sep 17 00:00:00 2001 From: claudiob Date: Mon, 8 Dec 2014 06:35:25 -0800 Subject: Deprecate `false` as the way to halt AM validation callbacks Before this commit, returning `false` in an ActiveModel validation callback such as `before_validation` would halt the callback chain. After this commit, the behavior is deprecated: will still work until the next release of Rails but will also display a deprecation warning. The preferred way to halt a callback chain is to explicitly `throw(:abort)`. --- activemodel/test/cases/validations/callbacks_test.rb | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index 5d6d48b824..4b0dd58efb 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -30,11 +30,16 @@ class DogWithTwoValidators < Dog before_validation { self.history << 'before_validation_marker2' } end -class DogBeforeValidatorReturningFalse < Dog +class DogDeprecatedBeforeValidatorReturningFalse < Dog before_validation { false } before_validation { self.history << 'before_validation_marker2' } end +class DogBeforeValidatorThrowingAbort < Dog + before_validation { throw :abort } + before_validation { self.history << 'before_validation_marker2' } +end + class DogAfterValidatorReturningFalse < Dog after_validation { false } after_validation { self.history << 'after_validation_marker' } @@ -86,13 +91,22 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase assert_equal ['before_validation_marker1', 'before_validation_marker2'], d.history end - def test_further_callbacks_should_not_be_called_if_before_validation_returns_false - d = DogBeforeValidatorReturningFalse.new + def test_further_callbacks_should_not_be_called_if_before_validation_throws_abort + d = DogBeforeValidatorThrowingAbort.new output = d.valid? assert_equal [], d.history assert_equal false, output end + def test_deprecated_further_callbacks_should_not_be_called_if_before_validation_returns_false + d = DogDeprecatedBeforeValidatorReturningFalse.new + assert_deprecated do + output = d.valid? + assert_equal [], d.history + assert_equal false, output + end + end + def test_further_callbacks_should_be_called_if_after_validation_returns_false d = DogAfterValidatorReturningFalse.new d.valid? -- cgit v1.2.3 From 91b8129320522f664801f122daea4a7628db90a7 Mon Sep 17 00:00:00 2001 From: claudiob Date: Mon, 8 Dec 2014 06:53:24 -0800 Subject: Deprecate `false` as the way to halt AM callbacks Before this commit, returning `false` in an ActiveModel `before_` callback such as `before_create` would halt the callback chain. After this commit, the behavior is deprecated: will still work until the next release of Rails but will also display a deprecation warning. The preferred way to halt a callback chain is to explicitly `throw(:abort)`. --- activemodel/test/cases/callbacks_test.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb index 2ac681b8d8..85455c112c 100644 --- a/activemodel/test/cases/callbacks_test.rb +++ b/activemodel/test/cases/callbacks_test.rb @@ -33,11 +33,13 @@ class CallbacksTest < ActiveModel::TestCase def initialize(options = {}) @callbacks = [] @valid = options[:valid] - @before_create_returns = options[:before_create_returns] + @before_create_returns = options.fetch(:before_create_returns, true) + @before_create_throws = options[:before_create_throws] end def before_create @callbacks << :before_create + throw(@before_create_throws) if @before_create_throws @before_create_returns end @@ -62,10 +64,18 @@ class CallbacksTest < ActiveModel::TestCase assert_equal model.callbacks.last, :final_callback end - test "the callback chain is halted when a before callback returns false" do + test "the callback chain is halted when a before callback returns false (deprecated)" do model = ModelCallbacks.new(before_create_returns: false) + assert_deprecated do + model.create + assert_equal model.callbacks.last, :before_create + end + end + + test "the callback chain is halted when a callback throws :abort" do + model = ModelCallbacks.new(before_create_throws: :abort) model.create - assert_equal model.callbacks.last, :before_create + assert_equal model.callbacks, [:before_create] end test "after callbacks are not executed if the block returns false" do -- cgit v1.2.3 From 37175a24bd508e2983247ec5d011d57df836c743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Sat, 3 Jan 2015 18:20:30 -0300 Subject: Remove deprecated `ActiveModel::Dirty#reset_#{attribute}` and `ActiveModel::Dirty#reset_changes`. --- activemodel/test/cases/dirty_test.rb | 17 ----------------- 1 file changed, 17 deletions(-) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index db2cd885e2..8ffd62fd86 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -181,23 +181,6 @@ class DirtyTest < ActiveModel::TestCase assert_equal ActiveSupport::HashWithIndifferentAccess.new, @model.changed_attributes end - 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' -- cgit v1.2.3 From 140557e85fc43ef5cff81a3af748605a6870f45b Mon Sep 17 00:00:00 2001 From: mo khan Date: Sat, 10 Jan 2015 09:35:58 -0700 Subject: allow '1' or true for acceptance validation. --- activemodel/test/cases/validations/acceptance_validation_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index e78aa1adaf..b7872ea6bf 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -65,4 +65,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 -- cgit v1.2.3 From 684cbee4733d47fe242b5637d6bdc48ec16ac536 Mon Sep 17 00:00:00 2001 From: claudiob Date: Sun, 11 Jan 2015 12:25:53 -0800 Subject: Add test for AM::Validation::Callbacks with :on `before_validation` and `after_validation` from ActiveModel::Validation::Callbacks accept an optional `:on` parameter that was not previously documented or tested. For instance given before_validation :do_something, on: :create then `object.valid?(:create)` will invoke `:do_something` while `object.valid?` or `object.valid?(:anything_else)` will not. --- .../test/cases/validations/callbacks_test.rb | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index 4b0dd58efb..cc50ffbbef 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -50,6 +50,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 +81,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? -- cgit v1.2.3 From d3927c8d180574657b50dfc3ca40b07d48b3545c Mon Sep 17 00:00:00 2001 From: claudiob Date: Sun, 11 Jan 2015 12:32:26 -0800 Subject: Remove unused "deprecated_reload" method The method was introduced in https://github.com/rails/rails/commit/66d0a0153578ce760d822580c5b8c0b726042ac2#diff-8cec05860729a3851ceb756f4dd90370R49 for the "reset_changes is deprecated" test, but this test was successively removed in https://github.com/rails/rails/commit/37175a24bd508e2983247ec5d011d57df836c743 --- activemodel/test/cases/dirty_test.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'activemodel/test') 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 -- cgit v1.2.3 From cb74473db68900d336844d840dda6e10dc03fde1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Wn=C4=99trzak?= Date: Sun, 4 Jan 2015 09:18:03 +0100 Subject: Add ActiveModel::Errors#details To be able to return type of validator, one can now call `details` on Errors instance: ```ruby class User < ActiveRecord::Base validates :name, presence: true end ``` ```ruby user = User.new; user.valid?; user.errors.details => {name: [{error: :blank}]} ``` --- activemodel/test/cases/errors_test.rb | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index efedd9055f..9cfba9812b 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_same 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 -- cgit v1.2.3 From 2606fb339797a99c50e531105fc92071cef3db01 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Fri, 23 Jan 2015 13:57:34 +0200 Subject: Extracted `ActiveRecord::AttributeAssignment` to `ActiveModel::AttributesAssignment` Allows to use it for any object as an includable module. --- .../test/cases/attribute_assignment_test.rb | 106 +++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 activemodel/test/cases/attribute_assignment_test.rb (limited to 'activemodel/test') diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb new file mode 100644 index 0000000000..5c1be6a456 --- /dev/null +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -0,0 +1,106 @@ +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) + non_existing_method(value) + end + + private + def metadata=(data) + @metadata = data + end + 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 "raises NoMethodError if raised in attribute writer" do + assert_raises NoMethodError do + Model.new(broken_attribute: 1) + end + end + + test "raises ArgumentError if non-hash object 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 'blank attributes should not raise' do + model = Model.new + assert_nil model.assign_attributes(ProtectedParams.new({})) + end + +end -- cgit v1.2.3 From a225d4bec51778d99ccba5f0d6700dd00d2474f4 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 23 Jan 2015 14:51:59 -0700 Subject: =?UTF-8?q?=E2=9C=82=EF=B8=8F=20and=20=F0=9F=92=85=20for=20#10776?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Minor style changes across the board. Changed an alias to an explicit method declaration, since the alias will not be documented otherwise. --- .../test/cases/attribute_assignment_test.rb | 59 +++++++++++----------- 1 file changed, 30 insertions(+), 29 deletions(-) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb index 5c1be6a456..402caf21f7 100644 --- a/activemodel/test/cases/attribute_assignment_test.rb +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -1,8 +1,7 @@ -require 'cases/helper' -require 'active_support/hash_with_indifferent_access' +require "cases/helper" +require "active_support/hash_with_indifferent_access" class AttributeAssignmentTest < ActiveModel::TestCase - class Model include ActiveModel::AttributeAssignment @@ -13,13 +12,15 @@ class AttributeAssignmentTest < ActiveModel::TestCase end def broken_attribute=(value) - non_existing_method(value) + raise ErrorFromAttributeWriter end - private - def metadata=(data) - @metadata = data - end + protected + + attr_writer :metadata + end + + class ErrorFromAttributeWriter < StandardError end class ProtectedParams < ActiveSupport::HashWithIndifferentAccess @@ -41,14 +42,14 @@ class AttributeAssignmentTest < ActiveModel::TestCase test "simple assignment" do model = Model.new - model.assign_attributes(name: 'hello', description: 'world') - assert_equal 'hello', model.name - assert_equal 'world', model.description + 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 + error = assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do model.assign_attributes(hz: 1) end @@ -58,49 +59,49 @@ class AttributeAssignmentTest < ActiveModel::TestCase test "assign private attribute" do model = Model.new - assert_raises ActiveModel::AttributeAssignment::UnknownAttributeError do + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do model.assign_attributes(metadata: { a: 1 }) end end - test "raises NoMethodError if raised in attribute writer" do - assert_raises NoMethodError do + test "does not swallow errors raised in an attribute writer" do + assert_raises(ErrorFromAttributeWriter) do Model.new(broken_attribute: 1) end end - test "raises ArgumentError if non-hash object passed" do - assert_raises ArgumentError do + 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') + 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') + 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 + 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') + 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 + assert_equal "Guille", model.name + assert_equal "m", model.description end - test 'blank attributes should not raise' do + 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 -- cgit v1.2.3 From 5bdb42159ec461d678652319da14b4a59bfafd27 Mon Sep 17 00:00:00 2001 From: Eugene Gilburg Date: Fri, 23 Jan 2015 14:36:43 -0800 Subject: use attribute assignment module logic during active model initialization --- activemodel/test/cases/model_test.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activemodel/test') 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 -- cgit v1.2.3 From fb7d95b21222f762eb4dded727a5999fb198fdfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Wn=C4=99trzak?= Date: Sat, 24 Jan 2015 11:58:52 +0100 Subject: Fixed duplicating ActiveModel::Errors#details --- activemodel/test/cases/errors_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/test') diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index 9cfba9812b..17dbb817d0 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -347,7 +347,7 @@ class ErrorsTest < ActiveModel::TestCase errors.add(:name, :invalid) errors_dup = errors.dup errors_dup.add(:name, :taken) - assert_not_same errors_dup.details, errors.details + assert_not_equal errors_dup.details, errors.details end test "delete removes details on given attribute" do -- cgit v1.2.3