Merge pull request #903 from airblade/drop_rails3_features

Drop rails3 features

.github/CONTRIBUTING.md

@@ -89,9 +89,9 @@ DB=postgres bundle exec appraisal ar-5.0 rake
 1. In the changelog,
   - Replace "Unreleased" with the date in iso-8601 format
   - Add a new "Unreleased" section
-1. In the readme,
-  - Update any other references to version number, including
-  - Update version number(s) in the documentation links table
+1. In the readme, update references to version number, including
+  - documentation links table
+  - compatability table
 1. Commit
 1. git tag -a -m "v5.0.0" "v5.0.0" # or whatever number
 1. git push --tags origin 5-stable # or whatever branch

CHANGELOG.md

@@ -17,6 +17,21 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 - None
 
+## 6.0.1 (2016-12-04)
+
+### Breaking Changes
+
+- None
+
+### Added
+
+- None
+
+### Fixed
+
+- Remove rails 3 features that are no longer supported, most notably,
+  `protected_attributes`.
+
 ## 6.0.0 (2016-12-03)
 
 Now with rails 5.1 support, and less model pollution! About 40 methods that were

README.md

@@ -10,7 +10,8 @@ has been destroyed.
 
 | Version        | Documentation |
 | -------------- | ------------- |
-| 6.0.0          | https://github.com/airblade/paper_trail/blob/master/README.md |
+| Unreleased     | https://github.com/airblade/paper_trail/blob/master/README.md |
+| 6.0.1          | https://github.com/airblade/paper_trail/blob/v6.0.1/README.md |
 | 5.2.3          | https://github.com/airblade/paper_trail/blob/v5.2.3/README.md |
 | 4.2.0          | https://github.com/airblade/paper_trail/blob/v4.2.0/README.md |
 | 3.0.9          | https://github.com/airblade/paper_trail/blob/v3.0.9/README.md |
@@ -45,6 +46,7 @@ has been destroyed.
   - [5.a. Single Table Inheritance](#5a-single-table-inheritance-sti)
   - [5.b. Configuring the `versions` Association](#5b-configuring-the-versions-association)
   - [5.c. Generators](#5c-generators)
+  - [5.d. Protected Attributes](#5d-protected-attributes)
 - [6. Extensibility](#6-extensibility)
   - [6.a. Custom Version Classes](#6a-custom-version-classes)
   - [6.b. Custom Serializer](#6b-custom-serializer)
@@ -1008,30 +1010,12 @@ class ApplicationController
 end
 ```
 
-#### Protected Attributes and Metadata
-
-If you are using rails 3 or the [protected_attributes][17] gem you must declare
-your metadata columns to be `attr_accessible`.
-
-```ruby
-# app/models/paper_trail/version.rb
-module PaperTrail
-  class Version < ActiveRecord::Base
-    include PaperTrail::VersionConcern
-    attr_accessible :author_id, :word_count, :answer
-  end
-end
-```
-
-If you're using [strong_parameters][18] instead of [protected_attributes][17]
-then there is no need to use `attr_accessible`.
-
 ## 5. ActiveRecord
 
 ### 5.a. Single Table Inheritance (STI)
 
 PaperTrail supports [Single Table Inheritance][39], and even supports an
-un-versioned base model, as of 23ffbdc7e1.
+un-versioned base model, as of `23ffbdc7e1`.
 
 ```ruby
 class Fruit < ActiveRecord::Base
@@ -1087,6 +1071,12 @@ Runtime options:
 Generates (but does not run) a migration to add a versions table.  Also generates an initializer file for configuring PaperTrail
 ```
 
+### 5.d. Protected Attributes
+
+As of version 6, PT no longer supports rails 3 or the [protected_attributes][17]
+gem. If you are still using them, you may use PT 5 or lower. We recommend
+upgrading to [strong_parameters][18] as soon as possible.
+
 ## 6. Extensibility
 
 ### 6.a. Custom Version Classes

lib/paper_trail.rb

@@ -121,15 +121,6 @@ def serializer
       PaperTrail.config.serializer
     end
 
-    # Returns a boolean indicating whether "protected attibutes" should be
-    # configured, e.g. attr_accessible, mass_assignment_sanitizer,
-    # whitelist_attributes, etc.
-    # @api public
-    def active_record_protected_attributes?
-      @active_record_protected_attributes ||= ::ActiveRecord::VERSION::MAJOR < 4 ||
-        !!defined?(ProtectedAttributes)
-    end
-
     # @api public
     def transaction?
       ::ActiveRecord::Base.connection.open_transactions > 0
@@ -167,17 +158,6 @@ def version
   end
 end
 
-# If available, ensure that the `protected_attributes` gem is loaded
-# before the `Version` class.
-unless PaperTrail.active_record_protected_attributes?
-  PaperTrail.send(:remove_instance_variable, :@active_record_protected_attributes)
-  begin
-    require "protected_attributes"
-  rescue LoadError # rubocop:disable Lint/HandleExceptions
-    # In case `protected_attributes` gem is not available.
-  end
-end
-
 ActiveSupport.on_load(:active_record) do
   include PaperTrail::Model
 end

lib/paper_trail/frameworks/rails/controller.rb

@@ -4,24 +4,11 @@ module Rails
     # information to the model layer, with `controller_info` and `whodunnit`.
     # Also includes a convenient on/off switch, `enabled_for_controller`.
     module Controller
-      def self.included(base)
-        before = [
+      def self.included(controller)
+        controller.before_action(
           :set_paper_trail_enabled_for_controller,
           :set_paper_trail_controller_info
-        ]
-        after = [
-          :warn_about_not_setting_whodunnit
-        ]
-
-        if base.respond_to? :before_action
-          # Rails 4+
-          before.map { |sym| base.before_action sym }
-          after.map  { |sym| base.after_action  sym }
-        else
-          # Rails 3.
-          before.map { |sym| base.before_filter sym }
-          after.map  { |sym| base.after_filter  sym }
-        end
+        )
       end
 
       protected
@@ -89,24 +76,14 @@ def set_paper_trail_controller_info
         ::PaperTrail.controller_info = info_for_paper_trail if ::PaperTrail.enabled_for_controller?
       end
 
+      # We have removed this warning. We no longer add it as a callback.
+      # However, some people use `skip_after_action :warn_about_not_setting_whodunnit`,
+      # so removing this method would be a breaking change. We can remove it
+      # in the next major version.
       def warn_about_not_setting_whodunnit
-        return unless ::PaperTrail.enabled_for_controller?
-
-        user_present = user_for_paper_trail.present?
-        whodunnit_blank = ::PaperTrail.whodunnit.blank?
-        if user_present && whodunnit_blank && !@set_paper_trail_whodunnit_called
-          source_file_location = self.class.instance_methods(false).map { |m|
-            self.class.instance_method(m).source_location.first
-          }.uniq.first
-          ::Kernel.warn <<-EOS.strip_heredoc
-            #{source_file_location}:
-            user_for_paper_trail is present, but whodunnit has not been set.
-            PaperTrail no longer adds the set_paper_trail_whodunnit callback for
-            you. To continue recording whodunnit, please add this before_action
-            callback to your ApplicationController. For more information,
-            please see https://git.io/vrTsk
-          EOS
-        end
+        ::ActiveSupport::Deprecation.warn(
+          "warn_about_not_setting_whodunnit is a no-op and is deprecated."
+        )
       end
     end
   end

lib/paper_trail/version_association_concern.rb

@@ -8,10 +8,6 @@ module VersionAssociationConcern
 
     included do
       belongs_to :version
-
-      if PaperTrail.active_record_protected_attributes?
-        attr_accessible :version_id, :foreign_key_name, :foreign_key_id
-      end
     end
   end
 end

lib/paper_trail/version_concern.rb

@@ -21,22 +21,7 @@ module VersionConcern
       end
 
       validates_presence_of :event
-
-      if PaperTrail.active_record_protected_attributes?
-        attr_accessible(
-          :item_type,
-          :item_id,
-          :event,
-          :whodunnit,
-          :object,
-          :object_changes,
-          :transaction_id,
-          :created_at
-        )
-      end
-
       after_create :enforce_version_limit!
-
       scope :within_transaction, ->(id) { where transaction_id: id }
     end
 
@@ -225,9 +210,7 @@ def object_deserialized
     #
     def reify(options = {})
       return nil if object.nil?
-      without_identity_map do
-        ::PaperTrail::Reifier.reify(self, options)
-      end
+      ::PaperTrail::Reifier.reify(self, options)
     end
 
     # Returns what changed in this version of the item.
@@ -323,17 +306,6 @@ def object_changes_deserialized
       end
     end
 
-    # In Rails 3.1+, calling reify on a previous version confuses the
-    # IdentityMap, if enabled. This prevents insertion into the map.
-    # @api private
-    def without_identity_map(&block)
-      if defined?(::ActiveRecord::IdentityMap) && ::ActiveRecord::IdentityMap.respond_to?(:without)
-        ::ActiveRecord::IdentityMap.without(&block)
-      else
-        yield
-      end
-    end
-
     # Checks that a value has been set for the `version_limit` config
     # option, and if so enforces it.
     # @api private

lib/paper_trail/version_number.rb

@@ -3,7 +3,7 @@ module PaperTrail
   module VERSION
     MAJOR = 6
     MINOR = 0
-    TINY = 0
+    TINY = 1
     PRE = nil
 
     STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".").freeze

spec/models/post_with_status_spec.rb

@@ -1,49 +1,45 @@
 require "rails_helper"
 
-# This model tests ActiveRecord::Enum, which was added in AR 4.1
-# http://edgeguides.rubyonrails.org/4_1_release_notes.html#active-record-enums
 describe PostWithStatus, type: :model do
-  if defined?(ActiveRecord::Enum)
-    with_versioning do
-      let(:post) { described_class.create!(status: "draft") }
+  with_versioning do
+    let(:post) { described_class.create!(status: "draft") }
 
-      it "should stash the enum value properly in versions" do
-        post.published!
-        post.archived!
-        expect(post.paper_trail.previous_version.published?).to be true
-      end
+    it "should stash the enum value properly in versions" do
+      post.published!
+      post.archived!
+      expect(post.paper_trail.previous_version.published?).to be true
+    end
 
-      it "can read enums in version records written by PT 4" do
-        post = described_class.create(status: "draft")
-        post.published!
-        version = post.versions.last
-        # Simulate behavior PT 4, which used to save the string version of
-        # enums to `object_changes`
-        version.update(object_changes: "---\nid:\n- \n- 1\nstatus:\n- draft\n- published\n")
-        assert_equal %w(draft published), version.changeset["status"]
-      end
+    it "can read enums in version records written by PT 4" do
+      post = described_class.create(status: "draft")
+      post.published!
+      version = post.versions.last
+      # Simulate behavior PT 4, which used to save the string version of
+      # enums to `object_changes`
+      version.update(object_changes: "---\nid:\n- \n- 1\nstatus:\n- draft\n- published\n")
+      assert_equal %w(draft published), version.changeset["status"]
+    end
 
-      context "storing enum object_changes" do
-        subject { post.versions.last }
+    context "storing enum object_changes" do
+      subject { post.versions.last }
 
-        it "should stash the enum value properly in versions object_changes" do
-          post.published!
-          post.archived!
-          expect(subject.changeset["status"]).to eql %w(published archived)
-        end
+      it "should stash the enum value properly in versions object_changes" do
+        post.published!
+        post.archived!
+        expect(subject.changeset["status"]).to eql %w(published archived)
       end
+    end
 
-      describe "#touch_with_version" do
-        it "preserves the enum value (and all other attributes)" do
-          post = described_class.create(status: :draft)
-          expect(post.versions.count).to eq(1)
-          expect(post.status).to eq("draft")
-          Timecop.travel 1.second.since # because MySQL lacks fractional seconds precision
-          post.paper_trail.touch_with_version
-          expect(post.versions.count).to eq(2)
-          expect(post.versions.last[:object]).to include("status: 0")
-          expect(post.paper_trail.previous_version.status).to eq("draft")
-        end
+    describe "#touch_with_version" do
+      it "preserves the enum value (and all other attributes)" do
+        post = described_class.create(status: :draft)
+        expect(post.versions.count).to eq(1)
+        expect(post.status).to eq("draft")
+        Timecop.travel 1.second.since # because MySQL lacks fractional seconds precision
+        post.paper_trail.touch_with_version
+        expect(post.versions.count).to eq(2)
+        expect(post.versions.last[:object]).to include("status: 0")
+        expect(post.paper_trail.previous_version.status).to eq("draft")
       end
     end
   end

test/dummy/app/controllers/application_controller.rb

@@ -1,18 +1,15 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
 
-  # Rails 5 deprecates `before_filter`
-  name_of_before_callback = respond_to?(:before_action) ? :before_action : :before_filter
-
   # 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.
-  send(name_of_before_callback, :modify_current_user)
+  before_action :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.
-  send(name_of_before_callback, :set_paper_trail_whodunnit)
+  # PT used to add this callback automatically. Now people are required to add
+  # it themsevles, like this, allowing them to control the order of callbacks.
+  # The `modify_current_user` callback above shows why this control is useful.
+  before_action :set_paper_trail_whodunnit
 
   def rescue_action(e)
     raise e

test/dummy/app/controllers/articles_controller.rb

@@ -11,10 +11,6 @@ def current_user
   private
 
   def article_params
-    if PaperTrail.active_record_protected_attributes?
-      params[:article]
-    else
-      params.require(:article).permit!
-    end
+    params.require(:article).permit!
   end
 end

test/dummy/app/controllers/widgets_controller.rb

@@ -23,10 +23,6 @@ def destroy
   private
 
   def widget_params
-    if PaperTrail.active_record_protected_attributes?
-      params[:widget]
-    else
-      params[:widget].permit!
-    end
+    params[:widget].permit!
   end
 end