From b9c9e792f5a9022d33ac98b6a8ecd24a687aab07 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Wed, 1 Aug 2018 22:45:50 -0400 Subject: [PATCH] touch now always inserts null in object_changes [Fixes #1121] --- CHANGELOG.md | 2 ++ lib/paper_trail/events/update.rb | 15 ++++++++- lib/paper_trail/record_trail.rb | 7 +++-- spec/paper_trail/events/update_spec.rb | 42 ++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 spec/paper_trail/events/update_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index f7e035e6..5f5f8ac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). - This change fixes a long-standing issue with reification of STI subclasses, [#594](https://github.com/paper-trail-gem/paper_trail/issues/594) - Removed `touch_with_version`, was deprecated in 9.0.0 +- [#1121](https://github.com/paper-trail-gem/paper_trail/issues/1121) - + `touch` now always inserts `null` in `object_changes`. ### Added diff --git a/lib/paper_trail/events/update.rb b/lib/paper_trail/events/update.rb index a22d144b..a63ebf6c 100644 --- a/lib/paper_trail/events/update.rb +++ b/lib/paper_trail/events/update.rb @@ -11,7 +11,7 @@ module PaperTrail # - is_touch - [boolean] - Used in the two situations that are touch-like: # - `after_touch` we call `RecordTrail#record_update` # - force_changes - [Hash] - Only used by `RecordTrail#update_columns`, - # because there dirty tracking is off, so it has to track its own changes. + # because there dirty-tracking is off, so it has to track its own changes. # # @api private def initialize(record, in_after_callback, is_touch, force_changes) @@ -37,6 +37,19 @@ module PaperTrail end merge_metadata_into(data) end + + private + + # `touch` cannot record `object_changes` because rails' `touch` does not + # perform dirty-tracking. Specifically, methods from `Dirty`, like + # `saved_changes`, return the same values before and after `touch`. + # + # See https://github.com/rails/rails/issues/33429 + # + # @api private + def record_object_changes? + !@is_touch && super + end end end end diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index 0526ee5e..634f6181 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -268,9 +268,10 @@ module PaperTrail # creates a version to record those changes. # @api public def update_columns(attributes) - # `@record.update_columns` skips dirty tracking, so we can't just use `@record.changes` or - # @record.saved_changes` from `ActiveModel::Dirty`. We need to build our own hash with the - # changes that will be made directly to the database. + # `@record.update_columns` skips dirty-tracking, so we can't just use + # `@record.changes` or @record.saved_changes` from `ActiveModel::Dirty`. + # We need to build our own hash with the changes that will be made + # directly to the database. changes = {} attributes.each do |k, v| changes[k] = [@record[k], v] diff --git a/spec/paper_trail/events/update_spec.rb b/spec/paper_trail/events/update_spec.rb new file mode 100644 index 00000000..d16564bc --- /dev/null +++ b/spec/paper_trail/events/update_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "spec_helper" + +module PaperTrail + module Events + ::RSpec.describe Update do + describe "#data", versioning: true do + context "is_touch false" do + it "object_changes is present" do + carter = Family::CelebrityFamily.create( + name: "Carter", + path_to_stardom: "Mexican radio" + ) + carter.path_to_stardom = "Johnny" + data = PaperTrail::Events::Update.new(carter, false, false, nil).data + expect(data[:object_changes]).to eq( + <<~YAML + --- + path_to_stardom: + - Mexican radio + - Johnny + YAML + ) + end + end + + context "is_touch true" do + it "object_changes is nil" do + carter = Family::CelebrityFamily.create( + name: "Carter", + path_to_stardom: "Mexican radio" + ) + carter.path_to_stardom = "Johnny" + data = PaperTrail::Events::Update.new(carter, false, true, nil).data + expect(data[:object_changes]).to be_nil + end + end + end + end + end +end