From 4faaa811614b408b32422692ce36024442d02ffb Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Tue, 5 Jun 2012 02:35:05 -0400 Subject: [PATCH] + ActiveRecord::Base#destroy! --- activerecord/CHANGELOG.md | 5 +++++ activerecord/lib/active_record/errors.rb | 4 ++++ activerecord/lib/active_record/persistence.rb | 16 ++++++++++++++++ activerecord/test/cases/callbacks_test.rb | 2 ++ activerecord/test/cases/persistence_test.rb | 7 +++++++ 5 files changed, 34 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b04c54ce7f369..55cc85a63d55a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Added `#destroy!` which acts like `#destroy` but will raise an + `ActiveRecord::RecordNotDestroyed` exception instead of returning `false`. + + *Marc-André Lafortune* + * Allow blocks for `count` with `ActiveRecord::Relation`, to work similar as `Array#count`: diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index fc80f3081e0bc..9b88bb81789cd 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -53,6 +53,10 @@ class RecordNotFound < ActiveRecordError class RecordNotSaved < ActiveRecordError end + # Raised by ActiveRecord::Base.destroy! when a call to destroy would return false. + class RecordNotDestroyed < ActiveRecordError + end + # Raised when SQL statement cannot be executed by the database (for example, it's often the case for # MySQL when Ruby driver used is too old). class StatementInvalid < ActiveRecordError diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index a1bc39a32d1cc..ec5670ba6ef76 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -122,6 +122,11 @@ def delete # Deletes the record in the database and freezes this instance to reflect # that no changes should be made (since they can't be persisted). + # + # There's a series of callbacks associated with destroy. If + # the before_destroy callback return +false+ the action is cancelled + # and destroy returns +false+. See + # ActiveRecord::Callbacks for further details. def destroy raise ReadOnlyRecord if readonly? destroy_associations @@ -130,6 +135,17 @@ def destroy freeze end + # Deletes the record in the database and freezes this instance to reflect + # that no changes should be made (since they can't be persisted). + # + # There's a series of callbacks associated with destroy!. If + # the before_destroy callback return +false+ the action is cancelled + # and destroy! raises ActiveRecord::RecordNotDestroyed. See + # ActiveRecord::Callbacks for further details. + def destroy! + destroy || raise(ActiveRecord::RecordNotDestroyed) + end + # Returns an instance of the specified +klass+ with the attributes of the # current record. This is mostly useful in relation to single-table # inheritance structures where you want a subclass to appear as the diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 769076922605f..deeef3a3fdbfe 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -426,11 +426,13 @@ def test_before_update_returning_false def test_before_destroy_returning_false david = ImmutableDeveloper.find(1) assert !david.destroy + assert_raise(ActiveRecord::RecordNotDestroyed) { david.destroy! } assert_not_nil ImmutableDeveloper.find_by_id(1) someone = CallbackCancellationDeveloper.find(1) someone.cancel_before_destroy = true assert !someone.destroy + assert_raise(ActiveRecord::RecordNotDestroyed) { someone.destroy! } assert !someone.after_destroy_called end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 0933a4ff3d196..fecdf2b7056b5 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -305,6 +305,13 @@ def test_destroy assert_raise(ActiveRecord::RecordNotFound) { Topic.find(topic.id) } end + def test_destroy! + topic = Topic.find(1) + assert_equal topic, topic.destroy!, 'topic.destroy! did not return self' + assert topic.frozen?, 'topic not frozen after destroy!' + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(topic.id) } + end + def test_record_not_found_exception assert_raise(ActiveRecord::RecordNotFound) { Topic.find(99999) } end