From 1ee868859cadfbde2cbae9c8fd645d74349b2d1c Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Fri, 8 May 2015 12:27:32 -0400 Subject: [PATCH] close #451; Fix VersionConcern#reify in the context of models with a default scope --- CHANGELOG.md | 2 + lib/paper_trail/version_concern.rb | 10 +++- spec/models/boolit_spec.rb | 48 +++++++++++++++++++ test/dummy/app/models/boolit.rb | 4 ++ .../20110208155312_set_up_test_tables.rb | 6 +++ 5 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 spec/models/boolit_spec.rb create mode 100644 test/dummy/app/models/boolit.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index d58b74d26..1142ee342 100644 --- a/CHANGELOG.md +++ b/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.) diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 1f9ec6013..2a51f97f4 100644 --- a/lib/paper_trail/version_concern.rb +++ b/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? diff --git a/spec/models/boolit_spec.rb b/spec/models/boolit_spec.rb new file mode 100644 index 000000000..3e96141d3 --- /dev/null +++ b/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 diff --git a/test/dummy/app/models/boolit.rb b/test/dummy/app/models/boolit.rb new file mode 100644 index 000000000..69c56353d --- /dev/null +++ b/test/dummy/app/models/boolit.rb @@ -0,0 +1,4 @@ +class Boolit < ActiveRecord::Base + default_scope { where(:scoped => true) } + has_paper_trail +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index 1f7f36e0b..29f208111 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/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