Merge branch 'master' into pr/18316

Conflicts:
	activerecord/CHANGELOG.md

7 files changed, 214 insertions, 28 deletions

diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb
new file mode 100644
index 0000000000..402caf21f7
--- /dev/null
+++ b/activemodel/test/cases/attribute_assignment_test.rb
@@ -0,0 +1,107 @@
+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)
+      raise ErrorFromAttributeWriter
+    end
+
+    protected
+
+    attr_writer :metadata
+  end
+
+  class ErrorFromAttributeWriter < StandardError
+  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 "does not swallow errors raised in an attribute writer" do
+    assert_raises(ErrorFromAttributeWriter) do
+      Model.new(broken_attribute: 1)
+    end
+  end
+
+  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")
+
+    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 "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
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
diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb
index db2cd885e2..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
@@ -181,23 +177,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'
diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb
index efedd9055f..17dbb817d0 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_equal 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
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
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
diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb
index 5d6d48b824..cc50ffbbef 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' }
@@ -45,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 }
@@ -68,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?
@@ -86,13 +117,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?