Skip to content

Commit

Permalink
Avoid extra scoping when using Relation#update
Browse files Browse the repository at this point in the history
Since 9ac7dd4, class level `update`, `destroy`, and `delete` were placed
in the `Persistence` module as class methods.

But `Relation#update` without passing ids which was introduced at #11898
is not a class method, and it was caused the extra scoping regression
#33470.

I moved the relation method back into the `Relation` to fix the
regression.

Fixes #33470.
  • Loading branch information
kamipo committed Jul 30, 2018
1 parent e8dd2bf commit c83e30d
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
4 changes: 1 addition & 3 deletions activerecord/lib/active_record/persistence.rb
Expand Up @@ -107,13 +107,11 @@ def instantiate_instance_of(klass, attributes, column_types = {}, &block)
# When running callbacks is not needed for each record update,
# it is preferred to use {update_all}[rdoc-ref:Relation#update_all]
# for updating all records in a single query.
def update(id = :all, attributes)
def update(id, attributes)
if id.is_a?(Array)
id.map { |one_id| find(one_id) }.each_with_index { |object, idx|
object.update(attributes[idx])
}
elsif id == :all
all.each { |record| record.update(attributes) }
else
if ActiveRecord::Base === id
raise ArgumentError,
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/relation.rb
Expand Up @@ -375,6 +375,10 @@ def update_all(updates)
@klass.connection.update stmt, "#{@klass} Update All"
end

def update(attributes) # :nodoc:
each { |record| record.update(attributes) }
end

def update_counters(counters) # :nodoc:
touch = counters.delete(:touch)

Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/cases/relations_test.rb
Expand Up @@ -30,7 +30,9 @@ class RelationTest < ActiveRecord::TestCase

class TopicWithCallbacks < ActiveRecord::Base
self.table_name = :topics
cattr_accessor :topic_count
before_update { |topic| topic.author_name = "David" if topic.author_name.blank? }
after_update { |topic| topic.class.topic_count = topic.class.count }
end

def test_do_not_double_quote_string_id
Expand Down Expand Up @@ -1576,6 +1578,8 @@ def test_update_on_relation
topics = TopicWithCallbacks.where(id: [topic1.id, topic2.id])
topics.update(title: "adequaterecord")

assert_equal TopicWithCallbacks.count, TopicWithCallbacks.topic_count

assert_equal "adequaterecord", topic1.reload.title
assert_equal "adequaterecord", topic2.reload.title
# Testing that the before_update callbacks have run
Expand Down

0 comments on commit c83e30d

Please sign in to comment.