Add always_raise_on_error config option

CHANGELOG.md

@@ -11,7 +11,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
-- None
+- [#1422](https://github.com/paper-trail-gem/paper_trail/pull/1450) - Add `always_raise_on_error`
+  config option to consistently surface errors when creating `Version` records.
 
 ### Fixed
 

README.md

@@ -289,6 +289,7 @@ Global configuration options affect all threads.
 - object_changes_adapter
 - serializer
 - version_limit
+- always_raise_on_error
 
 Syntax example: (options described in detail later)
 

lib/paper_trail/config.rb

@@ -14,7 +14,8 @@ class Config
       :object_changes_adapter,
       :serializer,
       :version_limit,
-      :has_paper_trail_defaults
+      :has_paper_trail_defaults,
+      :always_raise_on_error
     )
 
     def initialize

lib/paper_trail/record_trail.rb

@@ -81,12 +81,13 @@ def record_destroy(recording_order)
       # `data_for_destroy` but PT-AT still does.
       data = event.data.merge(data_for_destroy)
 
-      version = @record.class.paper_trail.version_class.create(data)
-      if version.errors.any?
-        log_version_errors(version, :destroy)
-      else
+      version = @record.class.paper_trail.version_class.new(data)
+      begin
+        version.save!
         assign_and_reset_version_association(version)
         version
+      rescue StandardError => e
+        handle_version_errors e, version, :destroy
       end
     end
 
@@ -108,14 +109,15 @@ def record_update(force:, in_after_callback:, is_touch:)
       )
       return unless version
 
-      if version.save
+      begin
+        version.save!
         # Because the version object was created using version_class.new instead
         # of versions_assoc.build?, the association cache is unaware. So, we
         # invalidate the `versions` association cache with `reset`.
         versions.reset
         version
-      else
-        log_version_errors(version, :update)
+      rescue StandardError => e
+        handle_version_errors e, version, :update
       end
     end
 
@@ -286,6 +288,16 @@ def log_version_errors(version, action)
       )
     end
 
+    # Centralized handler for version errors
+    # @api private
+    def handle_version_errors(e, version, action)
+      if PaperTrail.config.always_raise_on_error
+        raise e
+      else
+        log_version_errors(version, action)
+      end
+    end
+
     # @api private
     # @return - The created version object, so that plugins can use it, e.g.
     # paper_trail-association_tracking
@@ -298,11 +310,12 @@ def record_update_columns(changes)
       data.merge!(data_for_update_columns)
 
       versions_assoc = @record.send(@record.class.versions_association_name)
-      version = versions_assoc.create(data)
-      if version.errors.any?
-        log_version_errors(version, :update)
-      else
+      version = versions_assoc.new(data)
+      begin
+        version.save!
         version
+      rescue StandardError => e
+        handle_version_errors e, version, :update
       end
     end
 

paper_trail.gemspec

@@ -52,7 +52,7 @@ has been destroyed.
   # PT supports request_store versions for 3 years.
   s.add_dependency "request_store", "~> 1.4"
 
-  s.add_development_dependency "appraisal", "~> 2.4.1"
+  s.add_development_dependency "appraisal", "~> 2.5.0"
   s.add_development_dependency "byebug", "~> 11.1"
   s.add_development_dependency "ffaker", "~> 2.20"
   s.add_development_dependency "generator_spec", "~> 0.9.4"

spec/dummy_app/app/models/comment.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class Comment < ApplicationRecord
+  has_paper_trail versions: { class_name: "CommentVersion" }
+end

spec/dummy_app/app/versions/comment_version.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class CommentVersion < PaperTrail::Version
+  self.table_name = "comment_versions"
+  # add rails validation to require whodunnit
+  validates :whodunnit, presence: true
+end

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

@@ -114,6 +114,17 @@ def up
     end
     add_index :post_versions, %i[item_type item_id]
 
+    # Requires whodunnit column.
+    create_table :comment_versions, force: true do |t|
+      t.string   :item_type, null: false
+      t.integer  :item_id,   null: false
+      t.string   :event,     null: false
+      t.string   :whodunnit, null: false
+      t.text     :object
+      t.datetime :created_at, limit: 6
+    end
+    add_index :comment_versions, %i[item_type item_id]
+
     # Uses custom versions table `no_object_versions`.
     create_table :no_objects, force: true do |t|
       t.string :letter, null: false, limit: 1
@@ -224,6 +235,10 @@ def up
       t.string :content
     end
 
+    create_table :comments, force: true do |t|
+      t.string :content
+    end
+
     create_table :post_with_statuses, force: true do |t|
       t.integer :status
       t.timestamps null: false, limit: 6

spec/paper_trail/config_spec.rb

@@ -17,6 +17,94 @@ module PaperTrail
       end
     end
 
+    describe ".always_raise_on_error", versioning: true do
+      context "when true" do
+        context "when version cannot be created" do
+          before { PaperTrail.config.always_raise_on_error = true }
+
+          after { PaperTrail.config.always_raise_on_error = false }
+
+          it "raises an error on create" do
+            expect {
+              Comment.create!(content: "Henry")
+            }.to raise_error(ActiveRecord::RecordInvalid)
+          end
+
+          it "raises an error on update" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update!(content: "Brad")
+            }.to raise_error(ActiveRecord::RecordInvalid)
+          end
+
+          it "raises an error on update_columns" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update_columns(content: "Brad")
+            }.not_to raise_error
+          end
+
+          it "raises an error on destroy" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.destroy!
+            }.to raise_error(ActiveRecord::RecordInvalid)
+          end
+        end
+      end
+
+      context "when false" do
+        context "when version cannot be created" do
+          before { PaperTrail.config.always_raise_on_error = false }
+
+          it "raises an error on create" do
+            expect {
+              Comment.create!(content: "Henry")
+            }.to raise_error(ActiveRecord::RecordInvalid)
+          end
+
+          it "does not raise an error on update" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update!(content: "Brad")
+            }.not_to raise_error
+          end
+
+          it "does not raise an error on update_columns" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.update_columns(content: "Brad")
+            }.not_to raise_error
+          end
+
+          it "does not raises an error on destroy" do
+            comment = PaperTrail.request(whodunnit: "Foo") do
+              Comment.create!(content: "Henry")
+            end
+
+            expect {
+              comment.destroy!
+            }.not_to raise_error
+          end
+        end
+      end
+    end
+
     describe ".version_limit", versioning: true do
       after { PaperTrail.config.version_limit = nil }