Merge branch 'nil_attribute_fix'

CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 2.7.2 (Unreleased)
 
   - [#187](https://github.com/airblade/paper_trail/pull/187) - Confirmed JRuby support.
+  - [#219](https://github.com/airblade/paper_trail/pull/219) - Fixed issue where attributes stored with `nil` value might not get
+  reified properly depending on the way the serializer worked.
 
 ## 2.7.1
 

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]
@@ -80,6 +82,8 @@ class Version < ActiveRecord::Base
         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.respond_to?("#{k}=")
             model[k.to_sym] = v

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

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

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