Merge pull request #734 from airblade/refactor_reify

Extract a few private methods in Reifier

.rubocop.yml

@@ -39,6 +39,12 @@ Style/DotPosition:
 Style/DoubleNegation:
   Enabled: false
 
+# The decision of when to use a guard clause to improve readability is subtle,
+# and it's not clear that it can be linted. Certainly, the default
+# `MinBodyLength` of 1 can actually hurt readability.
+Style/GuardClause:
+  MinBodyLength: 3
+
 # The Ruby Style Guide says:
 #
 # > Use \ instead of + or << to concatenate two string literals at line end.

lib/paper_trail/reifier.rb

@@ -31,7 +31,6 @@ module PaperTrail
         # table inheritance and the `item_type` will be the base class, not the
         # actual subclass. If `type` is present but empty, the class is the base
         # class.
-
         if options[:dup] != true && version.item
           model = version.item
           # Look for attributes that exist in the model and not in this
@@ -40,11 +39,7 @@ module PaperTrail
             (model.attribute_names - attrs.keys).each { |k| attrs[k] = nil }
           end
         else
-          inheritance_column_name = version.item_type.constantize.inheritance_column
-          class_name = attrs[inheritance_column_name].blank? ?
-            version.item_type :
-            attrs[inheritance_column_name]
-          klass = class_name.constantize
+          klass = version_reification_class(version, attrs)
           # The `dup` option always returns a new object, otherwise we should
           # attempt to look for the item outside of default scope(s).
           if options[:dup] || (item_found = klass.unscoped.find_by_id(version.item_id)).nil?
@@ -58,22 +53,22 @@ module PaperTrail
         end
 
         reify_attributes(model, version, attrs)
-
         model.send "#{model.class.version_association_name}=", version
-
-        unless options[:has_one] == false
-          reify_has_ones version.transaction_id, model, options
-        end
-
-        unless options[:has_many] == false
-          reify_has_manys version.transaction_id, model, options
-        end
-
+        reify_associations(model, options, version)
         model
       end
 
       private
 
+      def reify_associations(model, options, version)
+        if options[:has_one]
+          reify_has_ones version.transaction_id, model, options
+        end
+        if options[:has_many]
+          reify_has_manys version.transaction_id, model, options
+        end
+      end
+
       # Set all the attributes in this version on the model.
       def reify_attributes(model, version, attrs)
         enums = model.class.respond_to?(:defined_enums) ? model.class.defined_enums : {}
@@ -259,6 +254,18 @@ module PaperTrail
         end
       end
 
+      # Given a `version`, return the class to reify. This method supports
+      # Single Table Inheritance (STI). For example, given a `version` whose
+      # `item_type` is "Banana", where `Banana` is an STI model in the `fruits`
+      # table, this method would return the constant `Fruit`.
+      def version_reification_class(version, attrs)
+        inheritance_column_name = version.item_type.constantize.inheritance_column
+        class_name = attrs[inheritance_column_name].blank? ?
+          version.item_type :
+          attrs[inheritance_column_name]
+        class_name.constantize
+      end
+
       # Given a SQL fragment that identifies the IDs of version records,
       # returns a `Hash` mapping those IDs to `Version`s.
       #