Merge pull request #556 from airblade/set_paper_trail_whodunnit

Control the order of `set_paper_trail_whodunnit` callback

CHANGELOG.md

@@ -1,3 +1,20 @@
+## 5.0.0 (Unreleased)
+
+### Changed
+
+- If you are tracking who is responsible for changes with `whodunnit`, be aware
+  that PaperTrail no longer adds the `set_paper_trail_whodunnit` before_filter
+  for you. Please add this before_filter to your ApplicationController to
+  continue recording whodunnit. See the readme for an example.
+
+### Added
+
+None
+
+### Fixed
+
+None
+
 ## 4.1.0 (Unreleased)
 
 ### Changed
@@ -6,7 +23,6 @@
   [#588](https://github.com/airblade/paper_trail/issues/588) -
   `PaperTrail.timestamp_field` should be assigned to match the `updated_at`
   value when a version is generated for new versions.
-
 
 ### Added
 

README.md

@@ -729,28 +729,35 @@ PaperTrail::Version.delete_all ["created_at < ?", 1.week.ago]
 
 ## Finding Out Who Was Responsible For A Change
 
-If your `ApplicationController` has a `current_user` method, PaperTrail will
-attempt to store the value returned by `current_user.id` in the version's
+Set `PaperTrail.whodunnit=`, and that value will be stored in the version's
 `whodunnit` column.
 
-You may want PaperTrail to call a different method to find out who is
-responsible.  To do so, override the `user_for_paper_trail` method in your
-controller like this:
+```ruby
+PaperTrail.whodunnit = 'Andy Stewart'
+widget.update_attributes :name => 'Wibble'
+widget.versions.last.whodunnit              # Andy Stewart
+```
+
+If your controller has a `current_user` method, PaperTrail provides a
+`before_filter` that will assign `current_user.id` to `PaperTrail.whodunnit`.
+You can add this `before_filter` to your `ApplicationController`.
 
 ```ruby
 class ApplicationController
-  def user_for_paper_trail
-    logged_in? ? current_member.id : 'Public user'  # or whatever
-  end
+  before_filter :set_paper_trail_whodunnit
 end
 ```
 
-In a console session you can manually set who is responsible like this:
+You may want `set_paper_trail_whodunnit` to call a different method to find out
+who is responsible. To do so, override the `user_for_paper_trail` method in
+your controller like this:
 
 ```ruby
-PaperTrail.whodunnit = 'Andy Stewart'
-widget.update_attributes :name => 'Wibble'
-widget.versions.last.whodunnit              # Andy Stewart
+class ApplicationController
+  def user_for_paper_trail
+    logged_in? ? current_member.id : 'Public user'  # or whatever
+  end
+end
 ```
 
 See also: [Setting whodunnit in the rails console][33]

lib/paper_trail/frameworks/rails/controller.rb

@@ -4,7 +4,8 @@ module Controller
 
       def self.included(base)
         base.before_filter :set_paper_trail_enabled_for_controller
-        base.before_filter :set_paper_trail_whodunnit, :set_paper_trail_controller_info
+        base.before_filter :set_paper_trail_controller_info
+        base.after_filter :warn_about_not_setting_whodunnit
       end
 
       protected
@@ -71,6 +72,20 @@ def set_paper_trail_controller_info
         ::PaperTrail.controller_info = info_for_paper_trail if ::PaperTrail.enabled_for_controller?
       end
 
+      def warn_about_not_setting_whodunnit
+        enabled = ::PaperTrail.enabled_for_controller?
+        user_present = user_for_paper_trail.present?
+        whodunnit_blank = ::PaperTrail.whodunnit.blank?
+        if enabled && user_present && whodunnit_blank
+          warn <<-EOS.strip_heredoc
+            user_for_paper_trail is present, but whodunnit has not been set.
+            PaperTrail no longer adds the set_paper_trail_whodunnit
+            before_filter for you. Please add this before_filter to your
+            ApplicationController to continue recording whodunnit. See the
+            PaperTrail readme for an example.
+          EOS
+        end
+      end
     end
   end
 

test/dummy/app/controllers/application_controller.rb

@@ -1,20 +1,36 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
 
+  # Some applications and libraries modify `current_user`. Their changes need
+  # to be reflected in `whodunnit`, so the `set_paper_trail_whodunnit` below
+  # must happen after this.
+  before_filter :modify_current_user
+
+  # Going forward, we'll no longer add this `before_filter`, requiring people
+  # to do so themselves, allowing them to control the order in which this filter
+  # happens.
+  before_filter :set_paper_trail_whodunnit
+
   def rescue_action(e)
     raise e
   end
 
   # Returns id of hypothetical current user
   def current_user
-    @current_user ||= OpenStruct.new(:id => 153).tap do |obj|
-      # Invoking `id` returns the `object_id` value in Ruby18 unless specifically overwritten
-      def obj.id; 153; end if RUBY_VERSION < '1.9'
-    end
+    @current_user
   end
 
   def info_for_paper_trail
     {:ip => request.remote_ip, :user_agent => request.user_agent}
   end
-
+
+  private
+
+  def modify_current_user
+    @current_user = OpenStruct.new(:id => 153).tap do |obj|
+      # Support ruby 1.8, in which `id` returns the `object_id`
+      # unless specifically overwritten.
+      def obj.id; 153; end if RUBY_VERSION < '1.9'
+    end
+  end
 end

test/dummy/app/controllers/articles_controller.rb

@@ -10,7 +10,7 @@ def create
 
   def current_user
     'foobar'.tap do |string|
-      # Invoking `id` returns the `object_id` value in Ruby18 by default
+      # Support ruby 1.8, in which `String` responds to `id`.
       string.class_eval { undef_method(:id) } if RUBY_VERSION < '1.9'
     end
   end