Skip to content

Commit

Permalink
Don't track implicit touch mutation
Browse files Browse the repository at this point in the history
This partly reverts the effect of d1107f4.

d1107f4 makes `touch` tracks the mutation whether the `touch` is
occurred by explicit or not.

Existing apps expects that the previous changes tracks only the changes
which is explicit action by users.

I'd revert the implicit `touch` mutation tracking since I'd not like to
break existing apps.

Fixes #36219.
  • Loading branch information
kamipo committed May 13, 2019
1 parent 9854efd commit dcb8259
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
7 changes: 6 additions & 1 deletion activerecord/lib/active_record/attribute_methods/dirty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ def _touch_row(attribute_names, time)

affected_rows = super

if @_skip_dirty_tracking ||= false
clear_attribute_changes(@_touch_attr_names)
return affected_rows
end

changes = {}
@attributes.keys.each do |attr_name|
next if @_touch_attr_names.include?(attr_name)
Expand All @@ -193,7 +198,7 @@ def _touch_row(attribute_names, time)

affected_rows
ensure
@_touch_attr_names = nil
@_touch_attr_names, @_skip_dirty_tracking = nil, nil
end

def _update_record(attribute_names = attribute_names_for_partial_writes)
Expand Down
1 change: 1 addition & 0 deletions activerecord/lib/active_record/touch_later.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def surreptitiously_touch(attrs)

def touch_deferred_attributes
if has_defer_touch_attrs? && persisted?
@_skip_dirty_tracking = true
touch(*@_defer_touch_attrs, time: @_touch_time)
@_defer_touch_attrs, @_touch_time = nil, nil
end
Expand Down
20 changes: 15 additions & 5 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2711,18 +2711,22 @@ def test_association_with_rewhere_doesnt_set_inverse_instance_key
bulb = Bulb.create!
tyre = Tyre.create!

car = Car.create! do |c|
car = Car.create!(name: "honda") do |c|
c.bulbs << bulb
c.tyres << tyre
end

assert_equal [nil, "honda"], car.saved_change_to_name

assert_equal 1, car.bulbs.count
assert_equal 1, car.tyres.count
end

test "associations replace in memory when records have the same id" do
bulb = Bulb.create!
car = Car.create!(bulbs: [bulb])
car = Car.create!(name: "honda", bulbs: [bulb])

assert_equal [nil, "honda"], car.saved_change_to_name

new_bulb = Bulb.find(bulb.id)
new_bulb.name = "foo"
Expand All @@ -2733,7 +2737,9 @@ def test_association_with_rewhere_doesnt_set_inverse_instance_key

test "in memory replacement executes no queries" do
bulb = Bulb.create!
car = Car.create!(bulbs: [bulb])
car = Car.create!(name: "honda", bulbs: [bulb])

assert_equal [nil, "honda"], car.saved_change_to_name

new_bulb = Bulb.find(bulb.id)

Expand Down Expand Up @@ -2765,7 +2771,9 @@ def self.name

test "in memory replacements sets inverse instance" do
bulb = Bulb.create!
car = Car.create!(bulbs: [bulb])
car = Car.create!(name: "honda", bulbs: [bulb])

assert_equal [nil, "honda"], car.saved_change_to_name

new_bulb = Bulb.find(bulb.id)
car.bulbs = [new_bulb]
Expand All @@ -2785,7 +2793,9 @@ def self.name
test "in memory replacement maintains order" do
first_bulb = Bulb.create!
second_bulb = Bulb.create!
car = Car.create!(bulbs: [first_bulb, second_bulb])
car = Car.create!(name: "honda", bulbs: [first_bulb, second_bulb])

assert_equal [nil, "honda"], car.saved_change_to_name

same_bulb = Bulb.find(first_bulb.id)
car.bulbs = [second_bulb, same_bulb]
Expand Down

0 comments on commit dcb8259

Please sign in to comment.