From 05468001af29cdd3a85680b5aa5553c9a6c13299 Mon Sep 17 00:00:00 2001 From: David Butler Date: Fri, 10 May 2013 17:43:56 -0700 Subject: [PATCH 1/2] Fixed #reify so nil values in a given version get set to nil on the model --- lib/paper_trail/version.rb | 8 ++++ test/custom_json_serializer.rb | 13 +++++++ test/unit/serializer_test.rb | 47 +++++++++++++++++++++++- test/unit/serializers/mixin_json_test.rb | 14 +------ 4 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 test/custom_json_serializer.rb diff --git a/lib/paper_trail/version.rb b/lib/paper_trail/version.rb index 722f7331..e58b29f8 100644 --- a/lib/paper_trail/version.rb +++ b/lib/paper_trail/version.rb @@ -80,6 +80,14 @@ class Version < ActiveRecord::Base end model.class.unserialize_attributes_for_paper_trail attrs + + # Look for attributes that exist in the model and not in this version. + # These attributes should be set to nil. + (model.attribute_names - attrs.keys).each do |k| + attrs[k] = nil + end + + # Set all the attributes in this version on the model attrs.each do |k, v| if model.respond_to?("#{k}=") model[k.to_sym] = v diff --git a/test/custom_json_serializer.rb b/test/custom_json_serializer.rb new file mode 100644 index 00000000..f705f9b0 --- /dev/null +++ b/test/custom_json_serializer.rb @@ -0,0 +1,13 @@ +# This custom serializer excludes nil values +module CustomJsonSerializer + extend PaperTrail::Serializers::Json + + def self.load(string) + parsed_value = super(string) + parsed_value.is_a?(Hash) ? parsed_value.reject { |k,v| k.blank? || v.blank? } : parsed_value + end + + def self.dump(object) + object.is_a?(Hash) ? super(object.reject { |k,v| v.nil? }) : super + end +end \ No newline at end of file diff --git a/test/unit/serializer_test.rb b/test/unit/serializer_test.rb index 9f0690b1..bef095a7 100644 --- a/test/unit/serializer_test.rb +++ b/test/unit/serializer_test.rb @@ -1,4 +1,5 @@ require 'test_helper' +require 'custom_json_serializer' class SerializerTest < ActiveSupport::TestCase @@ -29,7 +30,7 @@ class SerializerTest < ActiveSupport::TestCase end end - context 'Custom Serializer' do + context 'JSON Serializer' do setup do PaperTrail.configure do |config| config.serializer = PaperTrail::Serializers::Json @@ -48,7 +49,7 @@ class SerializerTest < ActiveSupport::TestCase PaperTrail.config.serializer = PaperTrail::Serializers::Yaml end - should 'reify with custom serializer' do + should 'reify with JSON serializer' do # Normal behaviour assert_equal 2, @fluxor.versions.length assert_nil @fluxor.versions[0].reify @@ -71,4 +72,46 @@ class SerializerTest < ActiveSupport::TestCase end end + context 'Custom Serializer' do + setup do + PaperTrail.configure do |config| + config.serializer = CustomJsonSerializer + end + + Fluxor.instance_eval <<-END + has_paper_trail + END + + @fluxor = Fluxor.create + @original_fluxor_attributes = @fluxor.send(:item_before_change).attributes.reject { |k,v| v.nil? } # this is exactly what PaperTrail serializes + @fluxor.update_attributes :name => 'Some more text.' + end + + teardown do + PaperTrail.config.serializer = PaperTrail::Serializers::Yaml + end + + should 'reify with custom serializer' do + # Normal behaviour + assert_equal 2, @fluxor.versions.length + assert_nil @fluxor.versions[0].reify + assert_nil @fluxor.versions[1].reify.name + + # Check values are stored as JSON. + assert_equal @original_fluxor_attributes, ActiveSupport::JSON.decode(@fluxor.versions[1].object) + # This test can't consistently pass in Ruby1.8 because hashes do no preserve order, which means the order of the + # attributes in the JSON can't be ensured. + if RUBY_VERSION.to_f >= 1.9 + assert_equal ActiveSupport::JSON.encode(@original_fluxor_attributes), @fluxor.versions[1].object + end + end + + should 'store object_changes' do + initial_changeset = {"id" => [nil, 1]} + second_changeset = {"name"=>[nil, "Some more text."]} + assert_equal initial_changeset, @fluxor.versions[0].changeset + assert_equal second_changeset, @fluxor.versions[1].changeset + end + end + end diff --git a/test/unit/serializers/mixin_json_test.rb b/test/unit/serializers/mixin_json_test.rb index 42af8b50..72055978 100644 --- a/test/unit/serializers/mixin_json_test.rb +++ b/test/unit/serializers/mixin_json_test.rb @@ -1,17 +1,5 @@ require 'test_helper' - -module CustomJsonSerializer - extend PaperTrail::Serializers::Json - - def self.load(string) - parsed_value = super(string) - parsed_value.is_a?(Hash) ? parsed_value.reject { |k,v| k.blank? || v.blank? } : parsed_value - end - - def self.dump(object) - object.is_a?(Hash) ? super(object.reject { |k,v| v.nil? }) : super - end -end +require 'custom_json_serializer' class MixinJsonTest < ActiveSupport::TestCase From e6ae05ea992cc6231d0d1bede35e16e7c9339228 Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Mon, 13 May 2013 10:17:07 -0400 Subject: [PATCH 2/2] Simplifying the nil attribute assignment fix, and making it so that it only happens when the item to be reified is the current version of the model --- lib/paper_trail/version.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/paper_trail/version.rb b/lib/paper_trail/version.rb index e58b29f8..df49599b 100644 --- a/lib/paper_trail/version.rb +++ b/lib/paper_trail/version.rb @@ -72,6 +72,8 @@ class Version < ActiveRecord::Base if item model = item + # Look for attributes that exist in the model and not in this version. These attributes should be set to nil. + (model.attribute_names - attrs.keys).each { |k| attrs[k] = nil } else inheritance_column_name = item_type.constantize.inheritance_column class_name = attrs[inheritance_column_name].blank? ? item_type : attrs[inheritance_column_name] @@ -81,12 +83,6 @@ class Version < ActiveRecord::Base model.class.unserialize_attributes_for_paper_trail attrs - # Look for attributes that exist in the model and not in this version. - # These attributes should be set to nil. - (model.attribute_names - attrs.keys).each do |k| - attrs[k] = nil - end - # Set all the attributes in this version on the model attrs.each do |k, v| if model.respond_to?("#{k}=")