From e1f94d45978f3f2720807662bf01721c655208fa Mon Sep 17 00:00:00 2001 From: Chris Barton Date: Fri, 11 Mar 2016 01:18:40 -0800 Subject: [PATCH] Maps enums to database values before storing in `object_changes` Keep consistency between versions with regard to `changes` and `object_changes` and how enum columns store their values. Before, `changes` would map the changed attributes enum columns to the database values (integer values). This allows reifying that version to maintain the integrity of the enum. It did not do so for `object_changes` and thus, `0` for non-json columns, and the enum value for json columns would be stored instead. For the non-json columns, it mapped any non-integer enum value to `0` because during serialization that column is an `integer`. Now this is fixed, so that `object_changes` stores the enum mapped value. Here is an example: ```ruby class PostWithStatus < ActiveRecord::Base has_paper_trail enum status: { draft: 0, published: 1, archived: 2 } end post = PostWithStatus.new(status: :draft) post.published! version = post.versions.last # Before version.changeset #> { 'status' => ['draft', 'draft'] } (stored as [0, 0]) # After version.changeset #> { 'status' => ['draft', 'published'] } (stored as [0, 1]) ``` --- lib/paper_trail/attributes_serialization.rb | 71 +++++++++++++++++---- lib/paper_trail/has_paper_trail.rb | 19 ++---- lib/paper_trail/reifier.rb | 39 +++++++---- spec/models/post_with_status_spec.rb | 10 +++ 4 files changed, 98 insertions(+), 41 deletions(-) diff --git a/lib/paper_trail/attributes_serialization.rb b/lib/paper_trail/attributes_serialization.rb index bd5ca3f6..fa52dd51 100644 --- a/lib/paper_trail/attributes_serialization.rb +++ b/lib/paper_trail/attributes_serialization.rb @@ -25,13 +25,53 @@ module PaperTrail 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 @@ module PaperTrail # 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 diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index b9ac8b8d..80fd4d2e 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -364,11 +364,10 @@ module PaperTrail # `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 @@ module PaperTrail 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 diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 0d1e794d..685aaf76 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -57,20 +57,7 @@ module PaperTrail 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 @@ module PaperTrail 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`. # diff --git a/spec/models/post_with_status_spec.rb b/spec/models/post_with_status_spec.rb index 683e5860..094629da 100644 --- a/spec/models/post_with_status_spec.rb +++ b/spec/models/post_with_status_spec.rb @@ -12,6 +12,16 @@ describe PostWithStatus, type: :model do 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