Rails 7.0 Compatibility (#1365)

dfurber · dfurber-truecar · web-flow · commit c10a8573f108 · 2022-01-21T00:10:53.000-05:00
* 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 &lt;dfurber@truecar.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -32,6 +32,12 @@ def deserialize(attr, val)
         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
@@ -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
@@ -16,7 +16,7 @@ module VersionConcern
     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
@@ -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
@@ -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
@@ -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
@@ -131,10 +131,10 @@ def up
       %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
@@ -12,4 +12,23 @@
       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
@@ -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 @@
         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
@@ -5,7 +5,7 @@
 # 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
@@ -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
@@ -260,6 +260,8 @@
         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
@@ -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
@@ -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
