Skip to content

Commit

Permalink
Move ActiveRecord::Base.delete in Relation
Browse files Browse the repository at this point in the history
Ref: #50396
  • Loading branch information
byroot authored and dhh committed May 14, 2024
1 parent 4b51f4b commit b1381c3
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 32 deletions.
24 changes: 0 additions & 24 deletions activerecord/lib/active_record/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -544,30 +544,6 @@ def destroy(id)
end
end

# Deletes the row with a primary key matching the +id+ argument, using an
# SQL +DELETE+ statement, and returns the number of rows deleted. Active
# Record objects are not instantiated, so the object's callbacks are not
# executed, including any <tt>:dependent</tt> association options.
#
# You can delete multiple rows at once by passing an Array of <tt>id</tt>s.
#
# Note: Although it is often much faster than the alternative, #destroy,
# skipping callbacks might bypass business logic in your application
# that ensures referential integrity or performs other essential jobs.
#
# ==== Examples
#
# # Delete a single row
# Todo.delete(1)
#
# # Delete multiple rows
# Todo.delete([2,3,4])
def delete(id_or_array)
return 0 if id_or_array.nil? || (id_or_array.is_a?(Array) && id_or_array.empty?)

delete_by(primary_key => id_or_array)
end

def _insert_record(connection, values, returning) # :nodoc:
primary_key = self.primary_key
primary_key_value = nil
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/querying.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Querying
:first_or_create, :first_or_create!, :first_or_initialize,
:find_or_create_by, :find_or_create_by!, :find_or_initialize_by,
:create_or_find_by, :create_or_find_by!,
:destroy_all, :delete_all, :update_all, :touch_all, :destroy_by, :delete_by,
:destroy_all, :delete, :delete_all, :update_all, :touch_all, :destroy_by, :delete_by,
:find_each, :find_in_batches, :in_batches,
:select, :reselect, :order, :regroup, :in_order_of, :reorder, :group, :limit, :offset, :joins, :left_joins, :left_outer_joins,
:where, :rewhere, :invert_where, :preload, :extract_associated, :eager_load, :includes, :from, :lock, :readonly,
Expand Down
24 changes: 24 additions & 0 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,30 @@ def delete_all
end
end

# Deletes the row with a primary key matching the +id+ argument, using an
# SQL +DELETE+ statement, and returns the number of rows deleted. Active
# Record objects are not instantiated, so the object's callbacks are not
# executed, including any <tt>:dependent</tt> association options.
#
# You can delete multiple rows at once by passing an Array of <tt>id</tt>s.
#
# Note: Although it is often much faster than the alternative, #destroy,
# skipping callbacks might bypass business logic in your application
# that ensures referential integrity or performs other essential jobs.
#
# ==== Examples
#
# # Delete a single row
# Todo.delete(1)
#
# # Delete multiple rows
# Todo.delete([2,3,4])
def delete(id_or_array)
return 0 if id_or_array.nil? || (id_or_array.is_a?(Array) && id_or_array.empty?)

where(model.primary_key => id_or_array).delete_all
end

# Finds and destroys all records matching the specified conditions.
# This is short-hand for <tt>relation.where(condition).destroy_all</tt>.
# Returns the collection of objects that were destroyed.
Expand Down
14 changes: 7 additions & 7 deletions activerecord/test/cases/relation/delegation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class QueryingMethodsDelegationTest < ActiveRecord::TestCase
:first_or_create, :first_or_create!, :first_or_initialize,
:find_or_create_by, :find_or_create_by!, :find_or_initialize_by,
:create_or_find_by, :create_or_find_by!,
:destroy_all, :delete_all, :update_all, :touch_all, :delete_by, :destroy_by
:destroy_all, :delete, :delete_all, :update_all, :touch_all, :delete_by, :destroy_by
]

def test_delegate_querying_methods
Expand All @@ -89,14 +89,14 @@ class DelegationCachingTest < ActiveRecord::TestCase

test "delegation doesn't override methods defined in other relation subclasses" do
# precondition, some methods are available on ActiveRecord::Relation subclasses
# but not ActiveRecord::Relation itself. Here `delete` is just an example.
assert_equal false, ActiveRecord::Relation.method_defined?(:delete)
assert_equal true, ActiveRecord::Associations::CollectionProxy.method_defined?(:delete)
# but not ActiveRecord::Relation itself. Here `target` is just an example.
assert_equal false, ActiveRecord::Relation.method_defined?(:target)
assert_equal true, ActiveRecord::Associations::CollectionProxy.method_defined?(:target)

project = projects(:active_record)
original_owner = project.developers_with_callbacks.method(:delete).owner
Developer.all.delete(12345)
assert_equal original_owner, project.developers_with_callbacks.method(:delete).owner
original_owner = project.developers_with_callbacks.method(:target).owner
assert_equal :__target__, Developer.all.target
assert_equal original_owner, project.developers_with_callbacks.method(:target).owner
end
end
end
4 changes: 4 additions & 0 deletions activerecord/test/models/developer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ def find_most_recent
end
end

def self.target
:__target__ # Used by delegation_test.rb
end

belongs_to :mentor
belongs_to :strict_loading_mentor, strict_loading: true, foreign_key: :mentor_id, class_name: "Mentor"
belongs_to :strict_loading_off_mentor, strict_loading: false, foreign_key: :mentor_id, class_name: "Mentor"
Expand Down

0 comments on commit b1381c3

Please sign in to comment.