object_changes inserted by #touch is not correct value

[fukayatsu]

Splited from #1119

#1119 (comment)
We've been talking in #1099 about storing object_changes on destroy. Maybe we should also store object_changes on create. If we did, your test would pass. It's a little wasteful of disk space, but it makes the code a lot simpler.

---

bug_report_pt_9_touch.rb

# frozen_string_literal: true

# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "2.5.1"
  source "https://rubygems.org"
  gem "activerecord", "5.2.0"
  gem "minitest", "5.11.3"
  gem "paper_trail", "9.2.0", require: false
  gem "sqlite3", "1.3.13"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Please use sqlite for your bug reports, if possible.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :users, force: true do |t|
    t.text :first_name, null: false
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.text :object, limit: 1_073_741_823
    t.text :object_changes, limit: 1_073_741_823
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail"

# STEP FOUR: Define your AR models here.
class User < ActiveRecord::Base
  has_paper_trail
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_touch_1
    user = User.create(first_name: "Jane")
    user.touch

    assert_equal 2, user.versions.size
    assert_equal ['updated_at'], YAML.load(user.versions.last.object_changes).keys
  end

  def test_touch_2
    user = User.create(first_name: "Jane")
    user.reload
    user.touch

    assert_equal 2, user.versions.size
    assert_equal ['updated_at'], YAML.load(user.versions.last.object_changes).keys
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`

result

  1) Failure:
BugTest#test_touch_1 [bug_report_pt_9_touch.rb:57]:
--- expected
+++ actual
@@ -1 +1 @@
-["updated_at"]
+["id", "first_name", "created_at", "updated_at"]


  2) Failure:
BugTest#test_touch_2 [bug_report_pt_9_touch.rb:66]:
Expected: ["updated_at"]
  Actual: []

2 runs, 4 assertions, 2 failures, 0 errors, 0 skips

[jaredbeck]

Maybe we should also store object_changes on create.

Hmmm. Never mind, I was wrong, we are already doing that.

Actually, this seems like a bug in rails.

class User < ActiveRecord::Base
  after_touch :inspect_saved_changes

  def inspect_saved_changes
    puts saved_changes.inspect
  end
end

user = User.create(first_name: "Jane")
user.touch

# output
{"id"=>[nil, 1], "first_name"=>[nil, "Jane"], "created_at"=>[nil, 2018-07-24 04:22:55 UTC], "updated_at"=>[nil, 2018-07-24 04:22:55 UTC]}

It seems that saved_changes does not work correctly in after_touch.

[jaredbeck]

Actually, this seems like a bug in rails.

I opened an issue in rails about this: rails/rails#33429

[jaredbeck]

Closing via b9c9e79

It is not perfect, but is an improvement. Further PRs on this subject are welcome.

Thanks for the bug report, Fukaya-san.