Merge pull request #809 from md5/log-errors

Log errors when versions can't be created
paper-trail-gem · May 18, 2016 · 7963d63 · 7963d63
2 parents a184c37 + 0bd1321
commit 7963d63
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 16 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -8,7 +8,7 @@ Metrics/CyclomaticComplexity:
   Max: 13 # Goal: 6
 
 Metrics/ModuleLength:
-  Max: 299
+  Max: 311
 
 Metrics/PerceivedComplexity:
   Max: 16 # Goal: 7
diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb
@@ -375,9 +375,7 @@ def record_create
           if pt_record_object_changes? && changed_notably?
             data[:object_changes] = pt_recordable_object_changes
           end
-          if self.class.paper_trail_version_class.column_names.include?("transaction_id")
-            data[:transaction_id] = PaperTrail.transaction_id
-          end
+          add_transaction_id_to(data)
           version = send(self.class.versions_association_name).create! merge_metadata(data)
           update_transaction_id(version)
           save_associations(version)
@@ -397,12 +395,14 @@ def record_update(force = nil)
           if pt_record_object_changes?
             data[:object_changes] = pt_recordable_object_changes
           end
-          if self.class.paper_trail_version_class.column_names.include?("transaction_id")
-            data[:transaction_id] = PaperTrail.transaction_id
-          end
+          add_transaction_id_to(data)
           version = send(self.class.versions_association_name).create merge_metadata(data)
-          update_transaction_id(version)
-          save_associations(version)
+          if version.errors.any?
+            log_version_errors(version, :update)
+          else
+            update_transaction_id(version)
+            save_associations(version)
+          end
         end
       end
 
@@ -478,14 +478,16 @@ def record_destroy
             object: pt_recordable_object,
             whodunnit: PaperTrail.whodunnit
           }
-          if self.class.paper_trail_version_class.column_names.include?("transaction_id")
-            data[:transaction_id] = PaperTrail.transaction_id
-          end
+          add_transaction_id_to(data)
           version = self.class.paper_trail_version_class.create(merge_metadata(data))
-          send("#{self.class.version_association_name}=", version)
-          send(self.class.versions_association_name).send :load_target
-          update_transaction_id(version)
-          save_associations(version)
+          if version.errors.any?
+            log_version_errors(version, :destroy)
+          else
+            send("#{self.class.version_association_name}=", version)
+            send(self.class.versions_association_name).send :load_target
+            update_transaction_id(version)
+            save_associations(version)
+          end
         end
       end
 
@@ -638,6 +640,11 @@ def save_version?
         (if_condition.blank? || if_condition.call(self)) && !unless_condition.try(:call, self)
       end
 
+      def add_transaction_id_to(data)
+        return unless self.class.paper_trail_version_class.column_names.include?("transaction_id")
+        data[:transaction_id] = PaperTrail.transaction_id
+      end
+
       # @api private
       def update_transaction_id(version)
         return unless self.class.paper_trail_version_class.column_names.include?("transaction_id")
@@ -647,6 +654,13 @@ def update_transaction_id(version)
           version.save
         end
       end
+
+      def log_version_errors(version, action)
+        version.logger.warn(
+          "Unable to create version for #{action} of #{self.class.name}##{id}: " +
+          version.errors.full_messages.join(", ")
+        )
+      end
     end
   end
 end