close #479; Rename PaperTrail::Model::InstanceMethods#originator and VersionConcern#originator to #paper_trail_originator to avoid name collisions

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

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'
 ```
 

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

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.

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
 

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) }
 

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