Merge pull request #731 from cbarton/object_changes_w_enum

Maps enums to database values before storing in `object_changes`

lib/paper_trail/attributes_serialization.rb

@@ -25,13 +25,53 @@ def type_cast_from_database(data)
       end
     end
 
-    SERIALIZE, DESERIALIZE =
-      if ::ActiveRecord::VERSION::MAJOR >= 5
-        [:serialize, :deserialize]
-      else
-        [:type_cast_for_database, :type_cast_from_database]
+    class AbstractSerializer
+      def initialize(klass)
+        @klass = klass
       end
 
+      private
+
+      def apply_serialization(method, attr, val)
+        @klass.type_for_attribute(attr).send(method, val)
+      end
+    end
+
+    if ::ActiveRecord::VERSION::MAJOR >= 5
+      class CastedAttributeSerializer < AbstractSerializer
+        def serialize(attr, val)
+          apply_serialization(:serialize, attr, val)
+        end
+
+        def deserialize(attr, val)
+          apply_serialization(:deserialize, attr, val)
+        end
+      end
+    else
+      class CastedAttributeSerializer < AbstractSerializer
+        def serialize(attr, val)
+          val = defined_enums[attr][val] if defined_enums[attr]
+          apply_serialization(:type_cast_for_database, attr, val)
+        end
+
+        def deserialize(attr, val)
+          val = apply_serialization(:type_cast_from_database, attr, val)
+
+          if defined_enums[attr]
+            defined_enums[attr].key(val)
+          else
+            val
+          end
+        end
+
+        private
+
+        def defined_enums
+          @defined_enums ||= (@klass.respond_to?(:defined_enums) ? @klass.defined_enums : {})
+        end
+      end
+    end
+
     if ::ActiveRecord::VERSION::STRING < "4.2"
       # Backport Rails 4.2 and later's `type_for_attribute` to build
       # on a common interface.
@@ -49,40 +89,43 @@ def serialized_attribute_types
 
     # Used for `Version#object` attribute.
     def serialize_attributes_for_paper_trail!(attributes)
-      alter_attributes_for_paper_trail!(SERIALIZE, attributes)
+      alter_attributes_for_paper_trail!(:serialize, attributes)
     end
 
     def unserialize_attributes_for_paper_trail!(attributes)
-      alter_attributes_for_paper_trail!(DESERIALIZE, attributes)
+      alter_attributes_for_paper_trail!(:deserialize, attributes)
     end
 
-    def alter_attributes_for_paper_trail!(serializer, attributes)
+    def alter_attributes_for_paper_trail!(serialization_method, attributes)
       # Don't serialize before values before inserting into columns of type
       # `JSON` on `PostgreSQL` databases.
       return attributes if paper_trail_version_class.object_col_is_json?
 
+      serializer = CastedAttributeSerializer.new(self)
       attributes.each do |key, value|
-        attributes[key] = type_for_attribute(key).send(serializer, value)
+        attributes[key] = serializer.send(serialization_method, key, value)
       end
     end
 
     # Used for Version#object_changes attribute.
     def serialize_attribute_changes_for_paper_trail!(changes)
-      alter_attribute_changes_for_paper_trail!(SERIALIZE, changes)
+      alter_attribute_changes_for_paper_trail!(:serialize, changes)
     end
 
     def unserialize_attribute_changes_for_paper_trail!(changes)
-      alter_attribute_changes_for_paper_trail!(DESERIALIZE, changes)
+      alter_attribute_changes_for_paper_trail!(:deserialize, changes)
     end
 
-    def alter_attribute_changes_for_paper_trail!(serializer, changes)
+    def alter_attribute_changes_for_paper_trail!(serialization_method, changes)
       # Don't serialize before values before inserting into columns of type
       # `JSON` on `PostgreSQL` databases.
       return changes if paper_trail_version_class.object_changes_col_is_json?
 
+      serializer = CastedAttributeSerializer.new(self)
       changes.clone.each do |key, change|
-        type = type_for_attribute(key)
-        changes[key] = Array(change).map { |value| type.send(serializer, value) }
+        changes[key] = Array(change).map do |value|
+          serializer.send(serialization_method, key, value)
+        end
       end
     end
   end

lib/paper_trail/has_paper_trail.rb

@@ -364,11 +364,10 @@ def pt_record_object_changes?
       # `PaperTrail.serializer`.
       # @api private
       def pt_recordable_object
-        object_attrs = object_attrs_for_paper_trail(attributes_before_change)
         if self.class.paper_trail_version_class.object_col_is_json?
-          object_attrs
+          object_attrs_for_paper_trail
         else
-          PaperTrail.serializer.dump(object_attrs)
+          PaperTrail.serializer.dump(object_attrs_for_paper_trail)
         end
       end
 
@@ -494,20 +493,14 @@ def merge_metadata(data)
       end
 
       def attributes_before_change
-        attributes.tap do |prev|
-          enums = respond_to?(:defined_enums) ? defined_enums : {}
-          attrs = changed_attributes.select { |k, _v| self.class.column_names.include?(k) }
-          attrs.each do |attr, before|
-            before = enums[attr][before] if enums[attr]
-            prev[attr] = before
-          end
-        end
+        changed = changed_attributes.select { |k, _v| self.class.column_names.include?(k) }
+        attributes.merge(changed)
       end
 
       # Returns hash of attributes (with appropriate attributes serialized),
       # ommitting attributes to be skipped.
-      def object_attrs_for_paper_trail(attributes_hash)
-        attrs = attributes_hash.except(*paper_trail_options[:skip])
+      def object_attrs_for_paper_trail
+        attrs = attributes_before_change.except(*paper_trail_options[:skip])
         self.class.serialize_attributes_for_paper_trail!(attrs)
         attrs
       end

lib/paper_trail/reifier.rb

@@ -57,20 +57,7 @@ def reify(version, options)
           end
         end
 
-        model.class.unserialize_attributes_for_paper_trail! attrs
-
-        # Set all the attributes in this version on the model.
-        attrs.each do |k, v|
-          if model.has_attribute?(k)
-            model[k.to_sym] = v
-          elsif model.respond_to?("#{k}=")
-            model.send("#{k}=", v)
-          else
-            version.logger.warn(
-              "Attribute #{k} does not exist on #{version.item_type} (Version id: #{version.id})."
-            )
-          end
-        end
+        reify_attributes(model, version, attrs)
 
         model.send "#{model.class.version_association_name}=", version
 
@@ -87,6 +74,30 @@ def reify(version, options)
 
       private
 
+      # 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 : {}
+        model.class.unserialize_attributes_for_paper_trail! attrs
+
+        attrs.each do |k, v|
+          # `unserialize_attributes_for_paper_trail!` will return the mapped enum value
+          # and in Rails < 5, the []= uses the integer type caster from the column
+          # definition (in general) and thus will turn a (usually) string to 0 instead
+          # of the correct value
+          is_enum_without_type_caster = ::ActiveRecord::VERSION::MAJOR < 5 && enums[k]
+
+          if model.has_attribute?(k) && !is_enum_without_type_caster
+            model[k.to_sym] = v
+          elsif model.respond_to?("#{k}=")
+            model.send("#{k}=", v)
+          else
+            version.logger.warn(
+              "Attribute #{k} does not exist on #{version.item_type} (Version id: #{version.id})."
+            )
+          end
+        end
+      end
+
       # Replaces each record in `array` with its reified version, if present
       # in `versions`.
       #

spec/models/post_with_status_spec.rb

@@ -12,6 +12,16 @@
         post.archived!
         expect(post.previous_version.published?).to be true
       end
+
+      context "storing enum object_changes" do
+        subject { post.versions.last }
+
+        it "should stash the enum value properly in versions object_changes" do
+          post.published!
+          post.archived!
+          expect(subject.changeset["status"]).to eql %w(published archived)
+        end
+      end
     end
   end
 end