diff --git a/README.md b/README.md index df1e7c5f..3691dc5f 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,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 +1009,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 +1070,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 diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb index bbe9225f..e405a3eb 100644 --- a/lib/paper_trail.rb +++ b/lib/paper_trail.rb @@ -121,15 +121,6 @@ module PaperTrail 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 @@ module PaperTrail 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 diff --git a/lib/paper_trail/frameworks/rails/controller.rb b/lib/paper_trail/frameworks/rails/controller.rb index c7854e5f..8450ee97 100644 --- a/lib/paper_trail/frameworks/rails/controller.rb +++ b/lib/paper_trail/frameworks/rails/controller.rb @@ -4,24 +4,12 @@ module PaperTrail # 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 + ) + controller.after_action :warn_about_not_setting_whodunnit end protected diff --git a/lib/paper_trail/version_association_concern.rb b/lib/paper_trail/version_association_concern.rb index e94ac246..5fee7bb0 100644 --- a/lib/paper_trail/version_association_concern.rb +++ b/lib/paper_trail/version_association_concern.rb @@ -8,10 +8,6 @@ module PaperTrail 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 diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 25e45863..3f67d39b 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -21,22 +21,7 @@ module PaperTrail 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 @@ module PaperTrail # 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 @@ module PaperTrail 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 diff --git a/test/dummy/app/controllers/application_controller.rb b/test/dummy/app/controllers/application_controller.rb index 2ab0533a..c5d48b9c 100644 --- a/test/dummy/app/controllers/application_controller.rb +++ b/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 diff --git a/test/dummy/app/controllers/articles_controller.rb b/test/dummy/app/controllers/articles_controller.rb index fb671296..e7a9f5d0 100644 --- a/test/dummy/app/controllers/articles_controller.rb +++ b/test/dummy/app/controllers/articles_controller.rb @@ -11,10 +11,6 @@ class ArticlesController < ApplicationController private def article_params - if PaperTrail.active_record_protected_attributes? - params[:article] - else - params.require(:article).permit! - end + params.require(:article).permit! end end diff --git a/test/dummy/app/controllers/widgets_controller.rb b/test/dummy/app/controllers/widgets_controller.rb index 0b9648e4..1fc58630 100644 --- a/test/dummy/app/controllers/widgets_controller.rb +++ b/test/dummy/app/controllers/widgets_controller.rb @@ -23,10 +23,6 @@ class WidgetsController < ApplicationController private def widget_params - if PaperTrail.active_record_protected_attributes? - params[:widget] - else - params[:widget].permit! - end + params[:widget].permit! end end diff --git a/test/dummy/app/models/animal.rb b/test/dummy/app/models/animal.rb index 8411f2ae..ef2629bc 100644 --- a/test/dummy/app/models/animal.rb +++ b/test/dummy/app/models/animal.rb @@ -1,6 +1,4 @@ class Animal < ActiveRecord::Base has_paper_trail self.inheritance_column = "species" - - attr_accessible :species, :name if ::PaperTrail.active_record_protected_attributes? end diff --git a/test/dummy/app/models/protected_widget.rb b/test/dummy/app/models/protected_widget.rb deleted file mode 100644 index 7a5f6fb1..00000000 --- a/test/dummy/app/models/protected_widget.rb +++ /dev/null @@ -1,3 +0,0 @@ -class ProtectedWidget < Widget - attr_accessible :name, :a_text if ::PaperTrail.active_record_protected_attributes? -end diff --git a/test/dummy/config/application.rb b/test/dummy/config/application.rb index c16ae612..f4511be2 100644 --- a/test/dummy/config/application.rb +++ b/test/dummy/config/application.rb @@ -45,14 +45,6 @@ module Dummy # like if you have constraints or database-specific column types # config.active_record.schema_format = :sql - # Enforce whitelist mode for mass assignment. - # This will create an empty whitelist of attributes available for mass-assignment for all models - # in your app. As such, your models will need to explicitly whitelist or blacklist accessible - # parameters by using an attr_accessible or attr_protected declaration. - if ::PaperTrail.active_record_protected_attributes? - config.active_record.whitelist_attributes = false - end - # `config.assets` is a `NoMethodError` in rails 5. config.assets.enabled = false if config.respond_to?(:assets) diff --git a/test/dummy/config/environments/development.rb b/test/dummy/config/environments/development.rb index 0604cccd..38dafe17 100644 --- a/test/dummy/config/environments/development.rb +++ b/test/dummy/config/environments/development.rb @@ -22,11 +22,6 @@ Dummy::Application.configure do # Only use best-standards-support built into browsers config.action_dispatch.best_standards_support = :builtin - # Raise exception on mass assignment protection for Active Record models - if ::PaperTrail.active_record_protected_attributes? - config.active_record.mass_assignment_sanitizer = :strict - end - # Log the query plan for queries taking more than this (works # with SQLite, MySQL, and PostgreSQL) # config.active_record.auto_explain_threshold_in_seconds = 0.5 diff --git a/test/dummy/config/environments/test.rb b/test/dummy/config/environments/test.rb index dd4d9ae7..eeddff31 100644 --- a/test/dummy/config/environments/test.rb +++ b/test/dummy/config/environments/test.rb @@ -41,11 +41,6 @@ Dummy::Application.configure do # ActionMailer::Base.deliveries array. # config.action_mailer.delivery_method = :test - # Raise exception on mass assignment protection for Active Record models - if ::PaperTrail.active_record_protected_attributes? - config.active_record.mass_assignment_sanitizer = :strict - end - # Print deprecation notices to the stderr config.active_support.deprecation = :stderr end diff --git a/test/dummy/config/initializers/paper_trail.rb b/test/dummy/config/initializers/paper_trail.rb index 815cc748..4af1ebab 100644 --- a/test/dummy/config/initializers/paper_trail.rb +++ b/test/dummy/config/initializers/paper_trail.rb @@ -1,9 +1 @@ PaperTrail.config.track_associations = true - -module PaperTrail - class Version < ActiveRecord::Base - if ::PaperTrail.active_record_protected_attributes? - attr_accessible :answer, :action, :question, :article_id, :ip, :user_agent, :title - end - end -end diff --git a/test/dummy/script/rails b/test/dummy/script/rails index 2d248d55..57c4afb3 100755 --- a/test/dummy/script/rails +++ b/test/dummy/script/rails @@ -1,8 +1,4 @@ #!/usr/bin/env ruby - -# This command will automatically be run when you run "rails" with Rails 3 gems -# installed from the root of your application. - APP_PATH = File.expand_path("../../config/application", __FILE__) require File.expand_path("../../config/boot", __FILE__) require "rails/commands" diff --git a/test/test_helper.rb b/test/test_helper.rb index 897366e8..e5b35826 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -104,16 +104,7 @@ module CleanupCallbacks original_callbacks = nil setup do - if ActiveRecord::VERSION::MAJOR > 3 - original_callbacks = target.send(:get_callbacks, type).deep_dup - else - # While this defeats the purpose of targeted callback - # cleanup, callbacks were incredibly difficult to modify - # prior to Rails 4, and Rails internal callbacks were only - # used for autosaving associations in Rails 3. Our tests - # don't care whether a Fluxor's widget is autosaved. - target.reset_callbacks(type) - end + original_callbacks = target.send(:get_callbacks, type).deep_dup end teardown do diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index 9270c130..ad5348dd 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -678,39 +678,22 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase end end - context "Timestamps" do - setup do - @wotsit = Wotsit.create! name: "wotsit" - end + test "update_attributes! records timestamps" do + wotsit = Wotsit.create! name: "wotsit" + wotsit.update_attributes! name: "changed" + reified = wotsit.versions.last.reify + assert_not_nil reified.created_at + assert_not_nil reified.updated_at + end - should "record timestamps" do - @wotsit.update_attributes! name: "changed" - assert_not_nil @wotsit.versions.last.reify.created_at - assert_not_nil @wotsit.versions.last.reify.updated_at - end - - # Tests that it doesn't try to write created_on as an attribute just because - # a created_on method exists. - # - # - Deprecation warning in Rails 3.2 - # - ActiveModel::MissingAttributeError in Rails 4 - # - # In rails 5, `capture` is deprecated in favor of `capture_io`. - # - should "not generate warning" do - assert_update_raises_nothing = lambda { - assert_nothing_raised { - @wotsit.update_attributes! name: "changed" - } - } - warnings = - if respond_to?(:capture_io) - capture_io { assert_update_raises_nothing.call }.last - else - capture(:stderr) { assert_update_raises_nothing.call } - end - assert_equal "", warnings - end + # Tests that it doesn't try to write created_on as an attribute just because + # a created_on method exists. + # + # - ActiveModel::MissingAttributeError in Rails 4 + # + test "update_attributes! does not raise error" do + wotsit = Wotsit.create! name: "name1" + assert_nothing_raised { wotsit.update_attributes! name: "name2" } end context "A subclass" do diff --git a/test/unit/protected_attrs_test.rb b/test/unit/protected_attrs_test.rb deleted file mode 100644 index bac47712..00000000 --- a/test/unit/protected_attrs_test.rb +++ /dev/null @@ -1,52 +0,0 @@ -require "test_helper" - -class ProtectedAttrsTest < ActiveSupport::TestCase - subject { ProtectedWidget.new } - - # These ActiveModel matchers (provided by shoulda-matchers) only work for - # Rails 3. - if ActiveRecord::VERSION::MAJOR < 4 - accessible_attrs = ProtectedWidget.accessible_attributes.to_a - accessible_attrs.each do |attr_name| - should allow_mass_assignment_of(attr_name.to_sym) - end - inaccessible = ProtectedWidget. - column_names. - reject { |column_name| accessible_attrs.include?(column_name) } - inaccessible.each do |attr_name| - should_not allow_mass_assignment_of(attr_name.to_sym) - end - end - - context "A model with `attr_accessible` created" do - setup do - @widget = ProtectedWidget.create! name: "Henry" - @initial_attributes = @widget.attributes - end - - should "be `nil` in its previous version" do - assert_nil @widget.paper_trail.previous_version - end - - context "which is then updated" do - setup do - @widget.assign_attributes(name: "Jeff", a_text: "Short statement") - @widget.an_integer = 42 - @widget.save! - end - - should "not be `nil` in its previous version" do - assert_not_nil @widget.paper_trail.previous_version - end - - should "the previous version should contain right attributes" do - # For some reason this test seems to be broken in JRuby 1.9 mode in the - # test env even though it works in the console. WTF? - unless ActiveRecord::VERSION::MAJOR >= 4 && defined?(JRUBY_VERSION) - previous_attributes = @widget.paper_trail.previous_version.attributes - assert_attributes_equal previous_attributes, @initial_attributes - end - end - end - end -end