Add PaperTrail.config.association_reify_error_behaviour

.rubocop.yml

@@ -95,6 +95,12 @@ Naming/PredicateName:
 Naming/UncommunicativeMethodParamName:
   Enabled: false
 
+RSpec/FilePath:
+  Exclude:
+    - spec/paper_trail/association_reify_error_behaviour/error.rb
+    - spec/paper_trail/association_reify_error_behaviour/warn.rb
+    - spec/paper_trail/association_reify_error_behaviour/ignore.rb
+
 # In an ideal world, each example has a single expectation. In the real world,
 # sometimes setup is a pain and you don't want to duplicate setup in multiple
 # examples, or make the specs more confusing with many nested `context`s, and

CHANGELOG.md

@@ -11,7 +11,7 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
-- None
+- [#1089] Sometimes the has_one association will find more than one possible candidate and will raise a `PaperTrail::Reifiers::HasOne::FoundMoreThanOne` when NOT using STI. You may want to just assume the first result (of multiple) is the correct one and continue. Versions pre v8.1.2 and below did this without error or warning. To do so add the following line to your initializer: `PaperTrail.config.association_reify_error_behaviour = :warn`. Valid options are: `[:error, :warn, :ignore]`
 
 ### Fixed
 

README.md

@@ -948,8 +948,9 @@ Associations are an **experimental feature** and have the following known
 issues, in order of descending importance.
 
 1. PaperTrail only reifies the first level of associations.
-1. Does not fully support STI (For example, see `spec/models/person_spec.rb` and
-   `PaperTrail::Reifiers::HasOne::FoundMoreThanOne` error)
+1. Sometimes the has_one association will find more than one possible candidate and will raise a `PaperTrail::Reifiers::HasOne::FoundMoreThanOne` error. For example, see `spec/models/person_spec.rb`
+  - If you are not using STI, you may want to just assume the first result (of multiple) is the correct one and continue. Versions pre v8.1.2 and below did this without error or warning. To do so add the following line to your initializer: `PaperTrail.config.association_reify_error_behaviour = :warn`. Valid options are: `[:error, :warn, :ignore]`
+  - When using STI, even if you enable `:warn` you will likely still end up recieving an `ActiveRecord::AssociationTypeMismatch` error.
 1. [#542](https://github.com/paper-trail-gem/paper_trail/issues/542) -
    Not compatible with [transactional tests][34], aka. transactional fixtures.
 1. Requires database timestamp columns with fractional second precision.

lib/generators/paper_trail/install_generator.rb

@@ -45,7 +45,8 @@ def create_migration_file
     def create_initializer
       create_file(
         "config/initializers/paper_trail.rb",
-        "PaperTrail.config.track_associations = #{!!options.with_associations?}\n"
+        "PaperTrail.config.track_associations = true\n",
+        "PaperTrail.config.association_reify_error_behaviour = :error"
       )
     end
 

lib/paper_trail/config.rb

@@ -26,7 +26,7 @@ class Config
     STR
 
     include Singleton
-    attr_accessor :serializer, :version_limit
+    attr_accessor :serializer, :version_limit, :association_reify_error_behaviour
 
     def initialize
       # Variables which affect all threads, whose access is synchronized.

lib/paper_trail/reifiers/has_one.rb

@@ -74,7 +74,18 @@ def load_version(assoc, model, transaction_id, version_at)
           when 1
             versions.first
           else
-            raise FoundMoreThanOne.new(base_class_name, versions.length)
+            case PaperTrail.config.association_reify_error_behaviour.to_s
+            when "warn"
+              version = versions.first
+              version.logger&.warn(
+                FoundMoreThanOne.new(base_class_name, versions.length).message
+              )
+              version
+            when "ignore"
+              versions.first
+            else # "error"
+              raise FoundMoreThanOne.new(base_class_name, versions.length)
+            end
           end
         end
 

spec/dummy_app/app/models/person.rb

@@ -12,6 +12,9 @@ class Person < ActiveRecord::Base
   has_one :car, foreign_key: :owner_id
   has_one :bicycle, foreign_key: :owner_id
 
+  has_one :thing
+  has_one :thing_2, class_name: "Thing"
+
   if ActiveRecord.gem_version >= Gem::Version.new("5.0")
     belongs_to :mentor, class_name: "Person", foreign_key: :mentor_id, optional: true
   else

spec/dummy_app/app/models/thing.rb

@@ -2,4 +2,10 @@
 
 class Thing < ActiveRecord::Base
   has_paper_trail save_changes: false
+
+  if ActiveRecord.gem_version >= Gem::Version.new("5.0")
+    belongs_to :person, optional: true
+  else
+    belongs_to :person
+  end
 end

spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb

@@ -238,6 +238,7 @@ def up
 
     create_table :things, force: true do |t|
       t.string    :name
+      t.references :person
     end
 
     create_table :translations, force: true do |t|

spec/models/person_spec.rb → spec/paper_trail/association_reify_error_behaviour/error.rb

@@ -2,13 +2,16 @@
 
 require "spec_helper"
 
-RSpec.describe Person, type: :model, versioning: true do
+RSpec.describe PaperTrail, versioning: true do
   it "baseline test setup" do
     expect(Person.new).to be_versioned
   end
 
-  describe "#cars and bicycles" do
-    it "can be reified" do
+  # See https://github.com/paper-trail-gem/paper_trail/issues/594
+  describe "#association reify error behaviour" do
+    it "association reify error behaviour = :error" do
+      ::PaperTrail.config.association_reify_error_behaviour = :error
+
       person = Person.create(name: "Frank")
       car = Car.create(name: "BMW 325")
       bicycle = Bicycle.create(name: "BMX 1.0")
@@ -23,7 +26,6 @@
 
       expect(person.reload.versions.length).to(eq(3))
 
-      # See https://github.com/paper-trail-gem/paper_trail/issues/594
       expect {
         person.reload.versions.second.reify(has_one: true)
       }.to(

spec/paper_trail/association_reify_error_behaviour/ignore.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe PaperTrail, versioning: true do
+  it "baseline test setup" do
+    expect(Person.new).to be_versioned
+  end
+
+  # See https://github.com/paper-trail-gem/paper_trail/issues/594
+  describe "#association reify error behaviour" do
+    it "association reify error behaviour = :ignore" do
+      ::PaperTrail.config.association_reify_error_behaviour = :ignore
+
+      person = Person.create(name: "Frank")
+      thing = Thing.create(name: "BMW 325")
+      thing2 = Thing.create(name: "BMX 1.0")
+
+      person.thing = thing
+      person.thing_2 = thing2
+      person.update_attributes(name: "Steve")
+
+      thing.update_attributes(name: "BMW 330")
+      thing.update_attributes(name: "BMX 2.0")
+      person.update_attributes(name: "Peter")
+
+      expect(person.reload.versions.length).to(eq(3))
+
+      logger = person.versions.first.logger
+
+      allow(logger).to receive(:warn)
+
+      person.reload.versions.second.reify(has_one: true)
+
+      expect(logger).not_to(
+        have_received(:warn).with(/Unable to reify has_one association/)
+      )
+    end
+  end
+end

spec/paper_trail/association_reify_error_behaviour/warn.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe PaperTrail, versioning: true do
+  it "baseline test setup" do
+    expect(Person.new).to be_versioned
+  end
+
+  # See https://github.com/paper-trail-gem/paper_trail/issues/594
+  describe "#association reify error behaviour" do
+    it "association reify error behaviour = :warn" do
+      ::PaperTrail.config.association_reify_error_behaviour = :warn
+
+      person = Person.create(name: "Frank")
+      thing = Thing.create(name: "BMW 325")
+      thing2 = Thing.create(name: "BMX 1.0")
+
+      person.thing = thing
+      person.thing_2 = thing2
+      person.update_attributes(name: "Steve")
+
+      thing.update_attributes(name: "BMW 330")
+      thing.update_attributes(name: "BMX 2.0")
+      person.update_attributes(name: "Peter")
+
+      expect(person.reload.versions.length).to(eq(3))
+
+      logger = person.versions.first.logger
+
+      allow(logger).to receive(:warn)
+
+      person.reload.versions.second.reify(has_one: true)
+
+      expect(logger).to(
+        have_received(:warn).with(/Unable to reify has_one association/).twice
+      )
+    end
+  end
+end