aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--activemodel/CHANGELOG.md4
-rw-r--r--activemodel/lib/active_model/callbacks.rb1
-rw-r--r--activemodel/lib/active_model/validations/callbacks.rb1
-rw-r--r--activemodel/test/cases/callbacks_test.rb9
-rw-r--r--activemodel/test/cases/validations/callbacks_test.rb14
-rw-r--r--activerecord/CHANGELOG.md4
-rw-r--r--activerecord/lib/active_record/transactions.rb1
-rw-r--r--activerecord/test/cases/callbacks_test.rb110
-rw-r--r--activerecord/test/cases/transactions_test.rb10
-rw-r--r--activesupport/CHANGELOG.md4
-rw-r--r--activesupport/lib/active_support/callbacks.rb24
-rw-r--r--activesupport/test/callbacks_test.rb30
12 files changed, 24 insertions, 188 deletions
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index 41051b1315..edaac8c7cd 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Remove deprecated behavior that halts callbacks when the return is false.
+
+ *Rafael Mendonça França*
+
* Remove unused `ActiveModel::TestCase` class.
*Yuji Yaginuma*
diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb
index e99bfab6fd..eac2761433 100644
--- a/activemodel/lib/active_model/callbacks.rb
+++ b/activemodel/lib/active_model/callbacks.rb
@@ -103,7 +103,6 @@ module ActiveModel
def define_model_callbacks(*callbacks)
options = callbacks.extract_options!
options = {
- terminator: deprecated_false_terminator,
skip_after_callbacks_if_terminated: true,
scope: [:kind, :name],
only: [:before, :around, :after]
diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb
index 70bc1a0624..4e94422cf1 100644
--- a/activemodel/lib/active_model/validations/callbacks.rb
+++ b/activemodel/lib/active_model/validations/callbacks.rb
@@ -23,7 +23,6 @@ module ActiveModel
included do
include ActiveSupport::Callbacks
define_callbacks :validation,
- terminator: deprecated_false_terminator,
skip_after_callbacks_if_terminated: true,
scope: [:kind, :name]
end
diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb
index a6f0948376..f85cd7dec4 100644
--- a/activemodel/test/cases/callbacks_test.rb
+++ b/activemodel/test/cases/callbacks_test.rb
@@ -63,12 +63,10 @@ class CallbacksTest < ActiveModel::TestCase
assert_equal model.callbacks.last, :final_callback
end
- test "the callback chain is halted when a before callback returns false (deprecated)" do
+ test "the callback chain is not halted when a before callback returns false)" do
model = ModelCallbacks.new(before_create_returns: false)
- assert_deprecated do
- model.create
- assert_equal model.callbacks.last, :before_create
- end
+ model.create
+ assert_equal model.callbacks.last, :final_callback
end
test "the callback chain is halted when a callback throws :abort" do
@@ -127,6 +125,7 @@ class CallbacksTest < ActiveModel::TestCase
test "after_create callbacks with both callbacks declared in one line" do
assert_equal ["callback1", "callback2"], Violin1.new.create.history
end
+
test "after_create callbacks with both callbacks declared in different lines" do
assert_equal ["callback1", "callback2"], Violin2.new.create.history
end
diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb
index 83e8ac9522..f2e4a5946d 100644
--- a/activemodel/test/cases/validations/callbacks_test.rb
+++ b/activemodel/test/cases/validations/callbacks_test.rb
@@ -29,7 +29,7 @@ class DogWithTwoValidators < Dog
before_validation { history << "before_validation_marker2" }
end
-class DogDeprecatedBeforeValidatorReturningFalse < Dog
+class DogBeforeValidatorReturningFalse < Dog
before_validation { false }
before_validation { history << "before_validation_marker2" }
end
@@ -121,13 +121,11 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase
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
+ def test_further_callbacks_should_be_called_if_before_validation_returns_false
+ d = DogBeforeValidatorReturningFalse.new
+ output = d.valid?
+ assert_equal ["before_validation_marker2"], d.history
+ assert_equal true, output
end
def test_further_callbacks_should_be_called_if_after_validation_returns_false
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
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index afc5c5d78b..da78a0e057 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Remove deprecated behavior that halts callbacks when the return is false.
+
+ *Rafael Mendonça França*
+
* Deprecate passing string to `:if` and `:unless` conditional options
on `set_callback` and `skip_callback`.
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 9a2be1afc0..2449cb06d8 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -856,30 +856,6 @@ module ActiveSupport
def set_callbacks(name, callbacks) # :nodoc:
self.__callbacks = __callbacks.merge(name.to_sym => callbacks)
end
-
- def deprecated_false_terminator # :nodoc:
- Proc.new do |target, result_lambda|
- terminate = true
- catch(:abort) do
- result = result_lambda.call if result_lambda.is_a?(Proc)
- if Callbacks.halt_and_display_warning_on_return_false && result == false
- display_deprecation_warning_for_false_terminator
- else
- terminate = false
- end
- end
- terminate
- end
- end
-
- private
-
- def display_deprecation_warning_for_false_terminator
- ActiveSupport::Deprecation.warn(<<-MSG.squish)
- Returning `false` in Active Record and Active Model callbacks will not implicitly halt a callback chain in Rails 5.1.
- To explicitly halt the callback chain, please use `throw :abort` instead.
- MSG
- end
end
end
end
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index ed26081d4e..4f00afb581 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -869,34 +869,8 @@ module CallbacksTest
end
end
- class CallbackFalseTerminatorWithoutConfigTest < ActiveSupport::TestCase
- def test_returning_false_does_not_halt_callback_if_config_variable_is_not_set
- obj = CallbackFalseTerminator.new
- obj.save
- assert_nil obj.halted
- assert obj.saved
- end
- end
-
- class CallbackFalseTerminatorWithConfigTrueTest < ActiveSupport::TestCase
- def setup
- ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = true
- end
-
- def test_returning_false_does_not_halt_callback_if_config_variable_is_true
- obj = CallbackFalseTerminator.new
- obj.save
- assert_nil obj.halted
- assert obj.saved
- end
- end
-
- class CallbackFalseTerminatorWithConfigFalseTest < ActiveSupport::TestCase
- def setup
- ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = false
- end
-
- def test_returning_false_does_not_halt_callback_if_config_variable_is_false
+ class CallbackFalseTerminatorTest < ActiveSupport::TestCase
+ def test_returning_false_does_not_halt_callback
obj = CallbackFalseTerminator.new
obj.save
assert_nil obj.halted
generated by cgit v1.2.3 (git 2.25.1) at 2025-03-28 16:15:08 +0000