From c10a8573f1080d8835c5efca1411235135bcddc9 Mon Sep 17 00:00:00 2001
From: David Furber <furberd@gmail.com>
Date: Fri, 21 Jan 2022 00:10:53 -0500
Subject: [PATCH] Rails 7.0 Compatibility (#1365)

* Make paper_trail work with Rails 7.0

* from class_methods do back to module ClassMethods

* add spec for PostgresArraySerializer to boost coverage

* lint the spec for PostgresArraySerializer

* lint the spec for PostgresArraySerializer again

* and now make that linted spec pass again

* test object change scopes a bit

* round out json and jsonb testing of object scopes

* test some other code paths to increase coverage

* linting

* linting

* mess with yaml loading in test

* oddball cop for double quotes

* use Rails public API for compatibility rather than instance_variable_set

Co-authored-by: dfurber <dfurber@truecar.com>
---
 Appraisals                                    |  5 +++
 CHANGELOG.md                                  |  2 +
 Rakefile                                      | 13 +++---
 gemfiles/rails_7.0.gemfile                    |  8 ++++
 lib/paper_trail.rb                            |  2 +
 .../cast_attribute_serializer.rb              |  6 +++
 lib/paper_trail/compatibility.rb              |  2 +-
 lib/paper_trail/version_concern.rb            |  2 +-
 spec/dummy_app/app/models/car.rb              |  2 +
 spec/dummy_app/app/models/vegetable.rb        |  6 +--
 spec/dummy_app/app/versions/jsonb_version.rb  |  4 +-
 .../20110208155312_set_up_test_tables.rb      |  8 ++--
 spec/models/car_spec.rb                       | 19 ++++++++
 spec/models/fruit_spec.rb                     | 37 +++++++++++++++-
 spec/models/json_version_spec.rb              |  2 +-
 spec/models/vegetable_spec.rb                 | 43 +++++++++++++++++++
 spec/models/widget_spec.rb                    |  2 +
 spec/paper_trail/compatibility_spec.rb        |  2 +-
 .../postgres_array_serializer_spec.rb         | 32 ++++++++++++++
 19 files changed, 179 insertions(+), 18 deletions(-)
 create mode 100644 gemfiles/rails_7.0.gemfile
 create mode 100644 spec/models/vegetable_spec.rb
 create mode 100644 spec/paper_trail/type_serializers/postgres_array_serializer_spec.rb

diff --git a/Appraisals b/Appraisals
index b4051e70..7d16c71c 100644
--- a/Appraisals
+++ b/Appraisals
@@ -24,3 +24,8 @@ appraise "rails-6.1" do
   gem "rails", "~> 6.1.0"
   gem "rails-controller-testing", "~> 1.0.5"
 end
+
+appraise "rails-7.0" do
+  gem "rails", "~> 7.0.0"
+  gem "rails-controller-testing", "~> 1.0.5"
+end
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e255068e..7541e3fa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
+- [#1365](https://github.com/paper-trail-gem/paper_trail/pull/1365) -
+  Support Rails 7.0
 - [#1349](https://github.com/paper-trail-gem/paper_trail/pull/1349) -
   `if:` and `unless:` work with `touch` events now.
 
diff --git a/Rakefile b/Rakefile
index 3f814c5f..0c7ed0c0 100644
--- a/Rakefile
+++ b/Rakefile
@@ -38,11 +38,8 @@ task :clean do
   end
 end
 
-desc <<~EOS
-  Write a database.yml for the specified RDBMS, and create database. Does not
-  migrate. Migration happens later in spec_helper.
-EOS
-task prepare: %i[clean install_database_yml] do
+desc "Create the database."
+task :create_db do
   puts format("creating %s database", ENV["DB"])
   case ENV["DB"]
   when "mysql"
@@ -59,6 +56,12 @@ task prepare: %i[clean install_database_yml] do
   end
 end
 
+desc <<~EOS
+  Write a database.yml for the specified RDBMS, and create database. Does not
+  migrate. Migration happens later in spec_helper.
+EOS
+task prepare: %i[clean install_database_yml create_db]
+
 require "rspec/core/rake_task"
 desc "Run tests on PaperTrail with RSpec"
 task(:spec).clear
diff --git a/gemfiles/rails_7.0.gemfile b/gemfiles/rails_7.0.gemfile
new file mode 100644
index 00000000..ed927c64
--- /dev/null
+++ b/gemfiles/rails_7.0.gemfile
@@ -0,0 +1,8 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "rails", "~> 7.0.0"
+gem "rails-controller-testing", "~> 1.0.5"
+
+gemspec path: "../"
diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb
index 5df47023..8559f556 100644
--- a/lib/paper_trail.rb
+++ b/lib/paper_trail.rb
@@ -26,6 +26,8 @@ module PaperTrail
     named created_at.
   EOS
 
+  RAILS_GTE_7_0 = ::ActiveRecord.gem_version >= ::Gem::Version.new("7.0.0")
+
   extend PaperTrail::Cleaner
 
   class << self
diff --git a/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb b/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb
index 5fcdb05c..471a2ce8 100644
--- a/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb
+++ b/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb
@@ -32,6 +32,12 @@ module PaperTrail
         if defined_enums[attr] && val.is_a?(::String)
           # Because PT 4 used to save the string version of enums to `object_changes`
           val
+        elsif PaperTrail::RAILS_GTE_7_0 && val.is_a?(ActiveRecord::Type::Time::Value)
+          # Because Rails 7 time attribute throws a delegation error when you deserialize
+          # it with the factory.
+          # See ActiveRecord::Type::Time::Value crashes when loaded from YAML on rails 7.0
+          # https://github.com/rails/rails/issues/43966
+          val.instance_variable_get(:@time)
         else
           AttributeSerializerFactory.for(@klass, attr).deserialize(val)
         end
diff --git a/lib/paper_trail/compatibility.rb b/lib/paper_trail/compatibility.rb
index fb8163db..5a5ee0df 100644
--- a/lib/paper_trail/compatibility.rb
+++ b/lib/paper_trail/compatibility.rb
@@ -18,7 +18,7 @@ module PaperTrail
   # versions.
   module Compatibility
     ACTIVERECORD_GTE = ">= 5.2" # enforced in gemspec
-    ACTIVERECORD_LT = "< 7.0" # not enforced in gemspec
+    ACTIVERECORD_LT = "< 7.1" # not enforced in gemspec
 
     E_INCOMPATIBLE_AR = <<-EOS
       PaperTrail %s is not compatible with ActiveRecord %s. We allow PT
diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb
index fc6181c9..1e4a96c0 100644
--- a/lib/paper_trail/version_concern.rb
+++ b/lib/paper_trail/version_concern.rb
@@ -16,7 +16,7 @@ module PaperTrail
     extend ::ActiveSupport::Concern
 
     included do
-      belongs_to :item, polymorphic: true, optional: true
+      belongs_to :item, polymorphic: true, optional: true, inverse_of: false
       validates_presence_of :event
       after_create :enforce_version_limit!
     end
diff --git a/spec/dummy_app/app/models/car.rb b/spec/dummy_app/app/models/car.rb
index ea3faa5d..a6064a83 100644
--- a/spec/dummy_app/app/models/car.rb
+++ b/spec/dummy_app/app/models/car.rb
@@ -2,4 +2,6 @@
 
 class Car < Vehicle
   has_paper_trail
+  attribute :color, type: ActiveModel::Type::String
+  attr_accessor :top_speed
 end
diff --git a/spec/dummy_app/app/models/vegetable.rb b/spec/dummy_app/app/models/vegetable.rb
index 56659373..a07a24c6 100644
--- a/spec/dummy_app/app/models/vegetable.rb
+++ b/spec/dummy_app/app/models/vegetable.rb
@@ -2,7 +2,7 @@
 
 # See also `Fruit` which uses `JsonVersion`.
 class Vegetable < ActiveRecord::Base
-  if ENV["DB"] == "postgres"
-    has_paper_trail versions: { class_name: "JsonbVersion" }
-  end
+  has_paper_trail versions: {
+    class_name: ENV["DB"] == "postgres" ? "JsonbVersion" : "PaperTrail::Version"
+  }, on: %i[create update]
 end
diff --git a/spec/dummy_app/app/versions/jsonb_version.rb b/spec/dummy_app/app/versions/jsonb_version.rb
index eaf83bdf..59040471 100644
--- a/spec/dummy_app/app/versions/jsonb_version.rb
+++ b/spec/dummy_app/app/versions/jsonb_version.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
-class JsonbVersion < PaperTrail::Version
+class JsonbVersion < ActiveRecord::Base
+  include PaperTrail::VersionConcern
+
   self.table_name = "jsonb_versions"
 end
diff --git a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
index 9731c85b..377889ad 100644
--- a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
+++ b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb
@@ -131,10 +131,10 @@ class SetUpTestTables < ::ActiveRecord::Migration::Current
       %w[json jsonb].each do |j|
         table_name = j + "_versions"
         create_table table_name, force: true do |t|
-          t.string :item_type, null: false
-          t.integer :item_id, null: false
-          t.string :event, null: false
-          t.string :whodunnit
+          t.string   :item_type, null: false
+          t.bigint   :item_id, null: false
+          t.string   :event, null: false
+          t.string   :whodunnit
           t.public_send j, :object
           t.public_send j, :object_changes
           t.datetime :created_at, limit: 6
diff --git a/spec/models/car_spec.rb b/spec/models/car_spec.rb
index 9cab7305..37f98c88 100644
--- a/spec/models/car_spec.rb
+++ b/spec/models/car_spec.rb
@@ -12,4 +12,23 @@ RSpec.describe Car, type: :model do
       assert_includes car.versions.last.changeset.keys, "name"
     end
   end
+
+  describe "attributes and accessors", versioning: true do
+    it "reifies attributes that are not AR attributes" do
+      car = described_class.create name: "Pinto", color: "green"
+      car.update color: "yellow"
+      car.update color: "brown"
+      expect(car.versions.second.reify.color).to eq("yellow")
+    end
+
+    it "reifies attributes that once were attributes but now just attr_accessor" do
+      car = described_class.create name: "Pinto", color: "green"
+      car.update color: "yellow"
+      changes = PaperTrail::Serializers::YAML.load(car.versions.last.attributes["object"])
+      changes[:top_speed] = 80
+      car.versions.first.update object: changes.to_yaml
+      car.reload
+      expect(car.versions.first.reify.top_speed).to eq(80)
+    end
+  end
 end
diff --git a/spec/models/fruit_spec.rb b/spec/models/fruit_spec.rb
index 66a9c5b2..10e12bc9 100644
--- a/spec/models/fruit_spec.rb
+++ b/spec/models/fruit_spec.rb
@@ -2,7 +2,7 @@
 
 require "spec_helper"
 
-if ENV["DB"] == "postgres" || JsonVersion.table_exists?
+if ENV["DB"] == "postgres" && JsonVersion.table_exists?
   RSpec.describe Fruit, type: :model, versioning: true do
     describe "have_a_version_with_changes matcher" do
       it "works with Fruit because Fruit uses JsonVersion" do
@@ -16,5 +16,40 @@ if ENV["DB"] == "postgres" || JsonVersion.table_exists?
         expect(banana).not_to have_a_version_with_changes(color: "Yellow", name: "Kiwi")
       end
     end
+
+    describe "queries of versions", versioning: true do
+      let!(:fruit) { described_class.create(name: "Apple", mass: 1, color: "green") }
+
+      before do
+        described_class.create(name: "Pear")
+        fruit.update(name: "Fidget")
+        fruit.update(name: "Digit")
+      end
+
+      it "return the fruit whose name has changed" do
+        result = JsonVersion.where_attribute_changes(:name).map(&:item)
+        expect(result).to include(fruit)
+      end
+
+      it "returns the fruit whose name was Fidget" do
+        result = JsonVersion.where_object_changes_from({ name: "Fidget" }).map(&:item)
+        expect(result).to include(fruit)
+      end
+
+      it "returns the fruit whose name became Digit" do
+        result = JsonVersion.where_object_changes_to({ name: "Digit" }).map(&:item)
+        expect(result).to include(fruit)
+      end
+
+      it "returns the fruit where the object was named Fidget before it changed" do
+        result = JsonVersion.where_object({ name: "Fidget" }).map(&:item)
+        expect(result).to include(fruit)
+      end
+
+      it "returns the fruit that changed to Fidget" do
+        result = JsonVersion.where_object_changes({ name: "Fidget" }).map(&:item)
+        expect(result).to include(fruit)
+      end
+    end
   end
 end
diff --git a/spec/models/json_version_spec.rb b/spec/models/json_version_spec.rb
index b4649b21..72400e3f 100644
--- a/spec/models/json_version_spec.rb
+++ b/spec/models/json_version_spec.rb
@@ -5,7 +5,7 @@ require "spec_helper"
 # The `json_versions` table tests postgres' `json` data type. So, that
 # table is only created when testing against postgres.
 if JsonVersion.table_exists?
-  RSpec.describe JsonVersion, type: :model do
+  RSpec.describe JsonVersion, type: :model, versioning: true do
     it "includes the VersionConcern module" do
       expect(described_class).to include(PaperTrail::VersionConcern)
     end
diff --git a/spec/models/vegetable_spec.rb b/spec/models/vegetable_spec.rb
new file mode 100644
index 00000000..7ab422a8
--- /dev/null
+++ b/spec/models/vegetable_spec.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "support/performance_helpers"
+
+if ENV["DB"] == "postgres" && JsonbVersion.table_exists?
+  ::RSpec.describe Vegetable do
+    describe "queries of versions", versioning: true do
+      let!(:vegetable) { described_class.create(name: "Veggie", mass: 1, color: "green") }
+
+      before do
+        vegetable.update(name: "Fidget")
+        vegetable.update(name: "Digit")
+        described_class.create(name: "Cucumber")
+      end
+
+      it "return the vegetable whose name has changed" do
+        result = JsonbVersion.where_attribute_changes(:name).map(&:item)
+        expect(result).to include(vegetable)
+      end
+
+      it "returns the vegetable whose name was Fidget" do
+        result = JsonbVersion.where_object_changes_from({ name: "Fidget" }).map(&:item)
+        expect(result).to include(vegetable)
+      end
+
+      it "returns the vegetable whose name became Digit" do
+        result = JsonbVersion.where_object_changes_to({ name: "Digit" }).map(&:item)
+        expect(result).to include(vegetable)
+      end
+
+      it "returns the vegetable where the object was named Fidget before it changed" do
+        result = JsonbVersion.where_object({ name: "Fidget" }).map(&:item)
+        expect(result).to include(vegetable)
+      end
+
+      it "returns the vegetable that changed to Fidget" do
+        result = JsonbVersion.where_object_changes({ name: "Fidget" }).map(&:item)
+        expect(result).to include(vegetable)
+      end
+    end
+  end
+end
diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb
index 7c22cbbd..72e2393a 100644
--- a/spec/models/widget_spec.rb
+++ b/spec/models/widget_spec.rb
@@ -260,6 +260,8 @@ RSpec.describe Widget, type: :model, versioning: true do
         widget = described_class.create(name: "Henry")
         widget.update(name: "Harry")
         widget.destroy
+        expect(widget.versions.first.item.object_id).not_to eq(widget.object_id)
+        expect(widget.versions.last.item.object_id).not_to eq(widget.object_id)
         expect(widget.versions.last.item).to be_nil
       end
     end
diff --git a/spec/paper_trail/compatibility_spec.rb b/spec/paper_trail/compatibility_spec.rb
index dae02398..79c65976 100644
--- a/spec/paper_trail/compatibility_spec.rb
+++ b/spec/paper_trail/compatibility_spec.rb
@@ -14,7 +14,7 @@ module PaperTrail
 
       context "when incompatible" do
         it "writes a warning to stderr" do
-          ar_version = ::Gem::Version.new("7.0.0")
+          ar_version = ::Gem::Version.new("7.1.0")
           expect {
             described_class.check_activerecord(ar_version)
           }.to output(/not compatible/).to_stderr
diff --git a/spec/paper_trail/type_serializers/postgres_array_serializer_spec.rb b/spec/paper_trail/type_serializers/postgres_array_serializer_spec.rb
new file mode 100644
index 00000000..4bec2e84
--- /dev/null
+++ b/spec/paper_trail/type_serializers/postgres_array_serializer_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module PaperTrail
+  module TypeSerializers
+    ::RSpec.describe PostgresArraySerializer do
+      let(:word_array) { [].fill(0, rand(4..8)) { ::FFaker::Lorem.word } }
+      let(:word_array_as_string) { word_array.join("|") }
+
+      let(:the_thing) { described_class.new("foo", "bar") }
+
+      describe ".deserialize" do
+        it "deserializes array to Ruby" do
+          expect(the_thing.deserialize(word_array)).to eq(word_array)
+        end
+
+        it "deserializes string to Ruby array" do
+          allow(the_thing).to receive(:deserialize_with_ar).and_return(word_array)
+          expect(the_thing.deserialize(word_array_as_string)).to eq(word_array)
+          expect(the_thing).to have_received(:deserialize_with_ar)
+        end
+      end
+
+      describe ".dump" do
+        it "serializes Ruby to JSON" do
+          expect(the_thing.serialize(word_array)).to eq(word_array)
+        end
+      end
+    end
+  end
+end