Remove deprecated behavior that halts callbacks when the return is false

4 files changed, 4 insertions, 121 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index e4fb363da5..7546b7edac 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Remove deprecated behavior that halts callbacks when the return is false.
+
+    *Rafael Mendonça França*
+
 *   Deprecate `ColumnDumper#migration_keys`.
 
     *Ryuta Kamizono*
diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 56b75540e3..08417aaa0f 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -11,7 +11,6 @@ module ActiveRecord
                        :before_commit_without_transaction_enrollment,
                        :commit_without_transaction_enrollment,
                        :rollback_without_transaction_enrollment,
-                       terminator: deprecated_false_terminator,
                        scope: [:kind, :name]
     end
 
diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb
index 1de5309bd3..b3c86586d0 100644
--- a/activerecord/test/cases/callbacks_test.rb
+++ b/activerecord/test/cases/callbacks_test.rb
@@ -39,11 +39,6 @@ class CallbackDeveloper < ActiveRecord::Base
   end
 end
 
-class CallbackDeveloperWithFalseValidation < CallbackDeveloper
-  before_validation proc { |model| model.history << [:before_validation, :returning_false]; false }
-  before_validation proc { |model| model.history << [:before_validation, :should_never_get_here] }
-end
-
 class CallbackDeveloperWithHaltedValidation < CallbackDeveloper
   before_validation proc { |model| model.history << [:before_validation, :throwing_abort]; throw(:abort) }
   before_validation proc { |model| model.history << [:before_validation, :should_never_get_here] }
@@ -132,23 +127,6 @@ class ContextualCallbacksDeveloper < ActiveRecord::Base
   end
 end
 
-class CallbackCancellationDeveloper < ActiveRecord::Base
-  self.table_name = "developers"
-
-  attr_reader   :after_save_called, :after_create_called, :after_update_called, :after_destroy_called
-  attr_accessor :cancel_before_save, :cancel_before_create, :cancel_before_update, :cancel_before_destroy
-
-  before_save    { defined?(@cancel_before_save) ? !@cancel_before_save : false }
-  before_create  { !@cancel_before_create  }
-  before_update  { !@cancel_before_update  }
-  before_destroy { !@cancel_before_destroy }
-
-  after_save    { @after_save_called    = true }
-  after_update  { @after_update_called  = true }
-  after_create  { @after_create_called  = true }
-  after_destroy { @after_destroy_called = true }
-end
-
 class CallbackHaltedDeveloper < ActiveRecord::Base
   self.table_name = "developers"
 
@@ -404,70 +382,6 @@ class CallbacksTest < ActiveRecord::TestCase
     ], david.history
   end
 
-  def test_deprecated_before_save_returning_false
-    david = ImmutableDeveloper.find(1)
-    assert_deprecated do
-      assert david.valid?
-      assert !david.save
-      exc = assert_raise(ActiveRecord::RecordNotSaved) { david.save! }
-      assert_equal david, exc.record
-      assert_equal "Failed to save the record", exc.message
-    end
-
-    david = ImmutableDeveloper.find(1)
-    david.salary = 10_000_000
-    assert !david.valid?
-    assert !david.save
-    assert_raise(ActiveRecord::RecordInvalid) { david.save! }
-
-    someone = CallbackCancellationDeveloper.find(1)
-    someone.cancel_before_save = true
-    assert_deprecated do
-      assert someone.valid?
-      assert !someone.save
-    end
-    assert_save_callbacks_not_called(someone)
-  end
-
-  def test_deprecated_before_create_returning_false
-    someone = CallbackCancellationDeveloper.new
-    someone.cancel_before_create = true
-    assert_deprecated do
-      assert someone.valid?
-      assert !someone.save
-    end
-    assert_save_callbacks_not_called(someone)
-  end
-
-  def test_deprecated_before_update_returning_false
-    someone = CallbackCancellationDeveloper.find(1)
-    someone.cancel_before_update = true
-    assert_deprecated do
-      assert someone.valid?
-      assert !someone.save
-    end
-    assert_save_callbacks_not_called(someone)
-  end
-
-  def test_deprecated_before_destroy_returning_false
-    david = ImmutableDeveloper.find(1)
-    assert_deprecated do
-      assert !david.destroy
-      exc = assert_raise(ActiveRecord::RecordNotDestroyed) { david.destroy! }
-      assert_equal david, exc.record
-      assert_equal "Failed to destroy the record", exc.message
-    end
-    assert_not_nil ImmutableDeveloper.find_by_id(1)
-
-    someone = CallbackCancellationDeveloper.find(1)
-    someone.cancel_before_destroy = true
-    assert_deprecated do
-      assert !someone.destroy
-      assert_raise(ActiveRecord::RecordNotDestroyed) { someone.destroy! }
-    end
-    assert !someone.after_destroy_called
-  end
-
   def assert_save_callbacks_not_called(someone)
     assert !someone.after_save_called
     assert !someone.after_create_called
@@ -525,30 +439,6 @@ class CallbacksTest < ActiveRecord::TestCase
     assert !someone.after_destroy_called
   end
 
-  def test_callback_returning_false
-    david = CallbackDeveloperWithFalseValidation.find(1)
-    assert_deprecated { david.save }
-    assert_equal [
-      [ :after_find,            :method ],
-      [ :after_find,            :proc   ],
-      [ :after_find,            :object ],
-      [ :after_find,            :block  ],
-      [ :after_initialize,            :method ],
-      [ :after_initialize,            :proc   ],
-      [ :after_initialize,            :object ],
-      [ :after_initialize,            :block  ],
-      [ :before_validation,           :method ],
-      [ :before_validation,           :proc   ],
-      [ :before_validation,           :object ],
-      [ :before_validation,           :block  ],
-      [ :before_validation, :returning_false  ],
-      [ :after_rollback, :block  ],
-      [ :after_rollback, :object ],
-      [ :after_rollback, :proc   ],
-      [ :after_rollback, :method ],
-    ], david.history
-  end
-
   def test_callback_throwing_abort
     david = CallbackDeveloperWithHaltedValidation.find(1)
     david.save
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 8f9980f168..111495c481 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -206,16 +206,6 @@ class TransactionTest < ActiveRecord::TestCase
     assert_equal posts_count, author.posts.reload.size
   end
 
-  def test_cancellation_from_returning_false_in_before_filter
-    def @first.before_save_for_transaction
-      false
-    end
-
-    assert_deprecated do
-      @first.save
-    end
-  end
-
   def test_cancellation_from_before_destroy_rollbacks_in_destroy
     add_cancelling_before_destroy_with_db_side_effect_to_topic @first
     nbooks_before_destroy = Book.count