From f31e4f052b29c647b82bdaca781a1408166b05e1 Mon Sep 17 00:00:00 2001 From: Ben Atkins Date: Fri, 8 May 2015 13:28:47 -0400 Subject: [PATCH] close #479; Rename PaperTrail::Model::InstanceMethods#originator and VersionConcern#originator to #paper_trail_originator to avoid name collisions --- CHANGELOG.md | 5 ++- README.md | 14 ++++----- lib/paper_trail/has_paper_trail.rb | 9 +++++- lib/paper_trail/version_concern.rb | 7 ++++- spec/models/version_spec.rb | 49 +++++++++++++++++++++++++++++- spec/models/widget_spec.rb | 33 ++++++++++++++++---- test/unit/model_test.rb | 14 ++++----- 7 files changed, 107 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1142ee34..1c3a94d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,10 @@ If you depend on the `RSpec` or `Cucumber` helpers, you will need to [manually l - [#494](https://github.com/airblade/paper_trail/issues/494) - The install generator will warn the user if the migration they are attempting to generate already exists. - [#484](https://github.com/airblade/paper_trail/pull/484) - Support for - [PostgreSQL's `JSONB` Type](http://www.postgresql.org/docs/9.4/static/datatype-json.html) for storing `object` and `object_changes`. + [PostgreSQL's `JSONB` Type](http://www.postgresql.org/docs/9.4/static/datatype-json.html) for storing `object` + and `object_changes`. + - [#479](https://github.com/airblade/paper_trail/issues/479) - Deprecated `originator` method in favor of `paper_trail_originator` + Deprecation warning informs users that the `originator` of the methods will be removed in version `4.0` - [#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 diff --git a/README.md b/README.md index 52d871d5..b5b12e12 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ widget.version widget.live? # Returns who put the widget into its current state. -widget.originator +widget.paper_trail_originator # Returns the widget (not a version) as it looked at the given timestamp. widget.version_at(timestamp) @@ -152,7 +152,7 @@ version.reify(options = {}) version.reify(dup: true) # Returns who put the item into the state stored in this version. -version.originator +version.paper_trail_originator # Returns who changed the item from the state it had in this version. version.terminator @@ -529,22 +529,22 @@ Sometimes you want to define who is responsible for a change in a small scope wi A version's `whodunnit` records who changed the object causing the `version` to be stored. Because a version stores the object as it looked before the change (see the table above), `whodunnit` returns who stopped the object looking like this -- not who made it look like this. Hence `whodunnit` is aliased as `terminator`. -To find out who made a version's object look that way, use `version.originator`. And to find out who made a "live" object look like it does, use `originator` on the object. +To find out who made a version's object look that way, use `version.paper_trail_originator`. And to find out who made a "live" object look like it does, call `paper_trail_originator` on the object. ```ruby >> widget = Widget.find 153 # assume widget has 0 versions >> PaperTrail.whodunnit = 'Alice' >> widget.update_attributes :name => 'Yankee' ->> widget.originator # 'Alice' +>> widget..paper_trail_originator # 'Alice' >> PaperTrail.whodunnit = 'Bob' >> widget.update_attributes :name => 'Zulu' ->> widget.originator # 'Bob' +>> widget.paper_trail_originator # 'Bob' >> first_version, last_version = widget.versions.first, widget.versions.last >> first_version.whodunnit # 'Alice' ->> first_version.originator # nil +>> first_version.paper_trail_originator # nil >> first_version.terminator # 'Alice' >> last_version.whodunnit # 'Bob' ->> last_version.originator # 'Alice' +>> last_version.paper_trail_originator # 'Alice' >> last_version.terminator # 'Bob' ``` diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index ebb65679..31fbaf06 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object' # provides the `try` method + module PaperTrail module Model @@ -185,10 +187,15 @@ module PaperTrail end # Returns who put the object into its current state. - def originator + def paper_trail_originator (source_version || send(self.class.versions_association_name).last).try(:whodunnit) end + def originator + warn "DEPRECATED: use `paper_trail_originator` instead of `originator`. Support for `originator` will be removed in PaperTrail 4.0" + self.paper_trail_originator + end + # Invoked after rollbacks to ensure versions records are not created # for changes that never actually took place def clear_rolled_back_versions diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 2a51f97f..a0e38151 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -252,8 +252,13 @@ module PaperTrail end # Returns who put the item into the state stored in this version. + def paper_trail_originator + @paper_trail_originator ||= previous.whodunnit rescue nil + end + def originator - @originator ||= previous.whodunnit rescue nil + warn "DEPRECATED: use `paper_trail_originator` instead of `originator`. Support for `originator` will be removed in PaperTrail 4.0" + self.paper_trail_originator end # Returns who changed the item from the state it had in this version. diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 26e815dd..005c50a9 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -45,13 +45,60 @@ describe PaperTrail::Version, :type => :model do describe "Instance" do subject { PaperTrail::Version.new(attributes) rescue PaperTrail::Version.new } + describe '#paper_trail_originator' do + it { is_expected.to respond_to(:paper_trail_originator) } + + context "No previous versions" do + specify { expect(subject.previous).to be_nil } + + it "should return nil" do + expect(subject.paper_trail_originator).to be_nil + end + end + + context "Has previous version", :versioning => true do + let(:name) { Faker::Name.name } + let(:widget) { Widget.create!(name: Faker::Name.name) } + before do + widget.versions.first.update_attributes!(:whodunnit => name) + widget.update_attributes!(name: Faker::Name.first_name) + end + subject { widget.versions.last } + + specify { expect(subject.previous).to be_instance_of(PaperTrail::Version) } + + it "should return nil" do + expect(subject.paper_trail_originator).to eq(name) + end + end + end + + describe "#originator" do + it { is_expected.to respond_to(:originator) } + let(:warning_msg) do + "DEPRECATED: use `paper_trail_originator` instead of `originator`." + + " Support for `originator` will be removed in PaperTrail 4.0" + end + + it 'should set the invoke `paper_trail_originator`' do + is_expected.to receive(:warn) + is_expected.to receive(:paper_trail_originator) + subject.originator + end + + it 'should display a deprecation warning' do + is_expected.to receive(:warn).with(warning_msg) + subject.originator + end + end + describe '#terminator' do it { is_expected.to respond_to(:terminator) } let(:attributes) { {:whodunnit => Faker::Name.first_name} } it "is an alias for the `whodunnit` attribute" do - expect(subject.whodunnit).to eq(attributes[:whodunnit]) + expect(subject.terminator).to eq(attributes[:whodunnit]) end end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index d621fd3a..ddbbd93a 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -121,8 +121,8 @@ describe Widget, :type => :model do describe "Methods" do describe "Instance", :versioning => true do - describe '#originator' do - it { is_expected.to respond_to(:originator) } + describe '#paper_trail_originator' do + it { is_expected.to respond_to(:paper_trail_originator) } describe "return value" do let(:orig_name) { Faker::Name.name } @@ -133,9 +133,9 @@ describe Widget, :type => :model do specify { expect(widget).to be_live } it "should return the originator for the model at a given state" do - expect(widget.originator).to eq(orig_name) + expect(widget.paper_trail_originator).to eq(orig_name) widget.whodunnit(new_name) { |w| w.update_attributes(:name => 'Elizabeth') } - expect(widget.originator).to eq(new_name) + expect(widget.paper_trail_originator).to eq(new_name) end end @@ -150,7 +150,7 @@ describe Widget, :type => :model do let(:reified_widget) { widget.versions[1].reify } it "should return the appropriate originator" do - expect(reified_widget.originator).to eq(orig_name) + expect(reified_widget.paper_trail_originator).to eq(orig_name) end it "should not create a new model instance" do @@ -162,7 +162,7 @@ describe Widget, :type => :model do let(:reified_widget) { widget.versions[1].reify(:dup => true) } it "should return the appropriate originator" do - expect(reified_widget.originator).to eq(orig_name) + expect(reified_widget.paper_trail_originator).to eq(orig_name) end it "should not create a new model instance" do @@ -173,6 +173,27 @@ describe Widget, :type => :model do end end + describe "#originator" do + subject { widget } + + it { is_expected.to respond_to(:originator) } + let(:warning_msg) do + "DEPRECATED: use `paper_trail_originator` instead of `originator`." + + " Support for `originator` will be removed in PaperTrail 4.0" + end + + it 'should set the invoke `paper_trail_originator`' do + is_expected.to receive(:warn) + is_expected.to receive(:paper_trail_originator) + subject.originator + end + + it 'should display a deprecation warning' do + is_expected.to receive(:warn).with(warning_msg) + subject.originator + end + end + describe '#version_at' do it { is_expected.to respond_to(:version_at) } diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index af5bf6cd..fe799c69 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -596,9 +596,9 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'track who made the change' do assert_equal 'Alice', @version.whodunnit - assert_nil @version.originator + assert_nil @version.paper_trail_originator assert_equal 'Alice', @version.terminator - assert_equal 'Alice', @widget.originator + assert_equal 'Alice', @widget.paper_trail_originator end context 'when a record is updated' do @@ -610,9 +610,9 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'track who made the change' do assert_equal 'Bob', @version.whodunnit - assert_equal 'Alice', @version.originator + assert_equal 'Alice', @version.paper_trail_originator assert_equal 'Bob', @version.terminator - assert_equal 'Bob', @widget.originator + assert_equal 'Bob', @widget.paper_trail_originator end context 'when a record is destroyed' do @@ -624,9 +624,9 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'track who made the change' do assert_equal 'Charlie', @version.whodunnit - assert_equal 'Bob', @version.originator + assert_equal 'Bob', @version.paper_trail_originator assert_equal 'Charlie', @version.terminator - assert_equal 'Charlie', @widget.originator + assert_equal 'Charlie', @widget.paper_trail_originator end end end @@ -675,7 +675,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'should return the correct originator' do PaperTrail.whodunnit = 'Ben' @foo.update_attribute(:name, 'Geoffrey') - assert_equal PaperTrail.whodunnit, @foo.originator + assert_equal PaperTrail.whodunnit, @foo.paper_trail_originator end context 'when destroyed' do