close #451; Fix VersionConcern#reify in the context of models with a …

…default scope

CHANGELOG.md

@@ -24,6 +24,8 @@ If you depend on the `RSpec` or `Cucumber` helpers, you will need to [manually l
     [PostgreSQL's `JSONB` Type](http://www.postgresql.org/docs/9.4/static/datatype-json.html) for storing `object` and `object_changes`.
   - [#458](https://github.com/airblade/paper_trail/pull/458) - For `create` events, metadata pointing at attributes should attempt
     to grab the current value instead of looking at the value prior to the change (which would always be `nil`)
+  - [#451](https://github.com/airblade/paper_trail/issues/451) - Fix `reify` method in context of model where the base class
+    has a default scope, and the live instance is not scoped within that default scope
   - [#440](https://github.com/airblade/paper_trail/pull/440) - `versions` association should clear/reload after a transaction rollback.
   - [#439](https://github.com/airblade/paper_trail/pull/439) / [#12](https://github.com/airblade/paper_trail/issues/12) -
     Support for versioning of associations (Has Many, Has One, HABTM, etc.)

lib/paper_trail/version_concern.rb

@@ -196,7 +196,15 @@ def reify(options = {})
           inheritance_column_name = item_type.constantize.inheritance_column
           class_name = attrs[inheritance_column_name].blank? ? item_type : attrs[inheritance_column_name]
           klass = class_name.constantize
-          model = klass.new
+          # 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 = klass.unscoped.find_by_id(item_id)).nil?
+            model = klass.new
+          else
+            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 }
+          end
         end
 
         if PaperTrail.serialized_attributes?

spec/models/boolit_spec.rb

@@ -0,0 +1,48 @@
+require 'rails_helper'
+require Rails.root.join('..', 'custom_json_serializer')
+
+describe Boolit, :type => :model do
+  it { is_expected.to be_versioned }
+
+  it "has a default scope" do
+    expect(subject.default_scopes).to_not be_empty
+  end
+
+  describe "Versioning", :versioning => true do
+    subject { Boolit.create! }
+    before { subject.update_attributes!(:name => Faker::Name.name) }
+
+    it "should have versions" do
+      expect(subject.versions.size).to eq(2)
+    end
+
+    it "should be able to be reified and persisted" do
+      expect { subject.versions.last.reify.save! }.to_not raise_error
+    end
+
+    context "Instance falls out of default scope" do
+      before { subject.update_attributes!(:scoped => false) }
+
+      it "is NOT scoped" do
+        expect(Boolit.first).to be_nil
+      end
+
+      it "should still be able to be reified and persisted" do
+        expect { subject.previous_version.save! }.to_not raise_error
+      end
+
+      context "with `nil` attributes on the live instance" do
+        before do
+          PaperTrail.serializer = CustomJsonSerializer
+          subject.update_attributes!(:name => nil)
+          subject.update_attributes!(:name => Faker::Name.name)
+        end
+        after { PaperTrail.serializer = PaperTrail::Serializers::YAML }
+
+        it "should not overwrite that attribute during the reification process" do
+          expect(subject.previous_version.name).to be_nil
+        end
+      end
+    end
+  end
+end

test/dummy/app/models/boolit.rb

@@ -0,0 +1,4 @@
+class Boolit < ActiveRecord::Base
+  default_scope { where(:scoped => true) }
+  has_paper_trail
+end

test/dummy/db/migrate/20110208155312_set_up_test_tables.rb

@@ -182,6 +182,11 @@ def self.up
       t.string :name
       t.string :color
     end
+
+    create_table :boolits, :force => true do |t|
+      t.string :name
+      t.boolean :scoped, :default => true
+    end
   end
 
   def self.down
@@ -215,6 +220,7 @@ def self.down
     drop_table :orders
     drop_table :line_items
     drop_table :fruits
+    drop_table :boolits
     remove_index :version_associations, :column => [:version_id]
     remove_index :version_associations, :name => 'index_version_associations_on_foreign_key'
     drop_table :version_associations