diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index f4b812ec..7c976673 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -35,7 +35,12 @@ ask for whatever help you need, but it's your job to fix it. ## Development -Install gems with `bundle exec appraisal install`. +```bash +gem install bundler +bundle +bundle exec appraisal install +bundle exec appraisal update # occasionally +``` Testing is a little awkward because the test suite: @@ -43,13 +48,10 @@ Testing is a little awkward because the test suite: 1. Contains a "dummy" rails app with three databases (test, foo, and bar) 1. Supports three different RDBMS': sqlite, mysql, and postgres -### Test sqlite, AR 4 +### Test sqlite, AR 6 ``` -DB=sqlite bundle exec appraisal ar-4.2 rake - -# Run a single test -DB=sqlite bundle exec appraisal ar-4.2 rspec spec/paper_trail_spec.rb +DB=sqlite bundle exec appraisal ar-6.0 rake ``` ### Test sqlite, AR 5 diff --git a/.rubocop.yml b/.rubocop.yml index f24a14ad..eb41466a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,15 +16,12 @@ AllCops: - spec/dummy_app/db/schema.rb # Generated, out of our control # Set to lowest supported version - TargetRubyVersion: 2.3 + TargetRubyVersion: 2.4 Bundler/OrderedGems: Exclude: - gemfiles/* # generated by Appraisal -Layout/AlignParameters: - EnforcedStyle: with_fixed_indentation - Layout/DotPosition: EnforcedStyle: trailing @@ -32,16 +29,25 @@ Layout/DotPosition: Layout/EmptyLineAfterGuardClause: Enabled: false -Layout/IndentHeredoc: +Layout/HeredocIndentation: Exclude: - paper_trail.gemspec +# The Ruby Style Guide recommends to "Limit lines to 80 characters." +# (https://github.com/bbatsov/ruby-style-guide#80-character-limits) +# Please aim for 80, but up to 100 is OK. +Layout/LineLength: + Max: 100 + Layout/MultilineMethodCallIndentation: EnforcedStyle: indented Layout/MultilineOperationIndentation: EnforcedStyle: indented +Layout/ParameterAlignment: + EnforcedStyle: with_fixed_indentation + # Use exactly one space on each side of an operator. Do not align operators # because it makes the code harder to edit, and makes lines unnecessarily long. Layout/SpaceAroundOperators: @@ -61,12 +67,6 @@ Metrics/BlockLength: Metrics/ClassLength: Enabled: false -# The Ruby Style Guide recommends to "Limit lines to 80 characters." -# (https://github.com/bbatsov/ruby-style-guide#80-character-limits) -# Please aim for 80, but up to 100 is OK. -Metrics/LineLength: - Max: 100 - # The number of lines in a method is not a useful metric compared to `AbcSize`. # It's common to have very long methods (> 50 lines) which are quite simple. For # example, a method that returns a long string with only a few interpolations. @@ -92,12 +92,12 @@ Naming/HeredocDelimiterNaming: Enabled: false Naming/PredicateName: - NameWhitelist: has_paper_trail + AllowedMethods: has_paper_trail # Too subtle to lint. # Two-letter param names are OK. Consider `send_email(to:, cc:)`. # Even one-letter names are OK. Consider `draw_point(x, y)`. -Naming/UncommunicativeMethodParamName: +Naming/MethodParameterName: Enabled: false # This cop does not seem to work in rubocop-rspec 1.28.0 @@ -147,6 +147,10 @@ Style/FrozenStringLiteralComment: Style/GuardClause: Enabled: false +# `hash.keys.each` is totally fine. +Style/HashEachMethods: + Enabled: false + # Only use postfix (modifier) conditionals for utterly simple statements. # As a rule of thumb, the entire statement should not exceed 60 chars. # Rubocop used to support this level of configuration, but no longer does. diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b96afd3b..a86e8bc0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -3,7 +3,7 @@ require: rubocop-rspec # Remove these configuration records # one by one as the offenses are removed from the code base. -Layout/AlignArguments: +Layout/ArgumentAlignment: Enabled: false Metrics/AbcSize: @@ -38,3 +38,11 @@ RSpec/NestedGroups: # interface is a public API, so that would be a breaking change. Security/YAMLLoad: Enabled: false + +# We want this, but not until we drop support for ruby 2.4 +Style/HashTransformKeys: + Enabled: false + +# We want this, but not until we drop support for ruby 2.4 +Style/HashTransformValues: + Enabled: false diff --git a/.travis.yml b/.travis.yml index 318c302d..6aa2052f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,9 @@ language: ruby cache: bundler -# For ruby compatibility, we test the highest and lowest minor versions only. -# For example, if our gemspec says `required_ruby_version = ">= 2.3.0"`, and -# ruby 2.5.0 has just been released, then we test 2.3 and 2.5, but not 2.4. +# For ruby, we test the highest and lowest minor versions only. rvm: - - 2.3 + - 2.4 - 2.7 env: @@ -29,13 +27,12 @@ before_install: - gem update bundler gemfile: - - gemfiles/ar_5.1.gemfile - gemfiles/ar_5.2.gemfile - gemfiles/ar_6.0.gemfile matrix: exclude: # rails 6 requires ruby >= 2.5.0 - - rvm: 2.3 + - rvm: 2.4 gemfile: gemfiles/ar_6.0.gemfile fast_finish: true services: diff --git a/Appraisals b/Appraisals index 3c094342..3a584b18 100644 --- a/Appraisals +++ b/Appraisals @@ -9,11 +9,6 @@ # > the version from the appraisal takes precedence. # > https://github.com/thoughtbot/appraisal -appraise "ar-5.1" do - gem "activerecord", [">= 5.1.6.2", "< 5.2"] - gem "rails-controller-testing", "~> 1.0.2" -end - appraise "ar-5.2" do gem "activerecord", [">= 5.2.2.1", "< 5.3"] gem "rails-controller-testing", "~> 1.0.2" diff --git a/CHANGELOG.md b/CHANGELOG.md index 472c93c5..489c2017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,9 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Dependencies -- Drop support for rails 4 (reached EOL on 2019-08-15) +- Drop support for rails <= 5.1 (reached EOL when 6.0 was released, + per https://guides.rubyonrails.org/maintenance_policy.html) +- Drop support for ruby 2.3 (reached EOL on 2019-04-01) ## 10.3.1 (2019-07-31) diff --git a/lib/generators/paper_trail/migration_generator.rb b/lib/generators/paper_trail/migration_generator.rb index ed6938da..e5588ad4 100644 --- a/lib/generators/paper_trail/migration_generator.rb +++ b/lib/generators/paper_trail/migration_generator.rb @@ -28,10 +28,11 @@ module PaperTrail end def migration_version - major = ActiveRecord::VERSION::MAJOR - if major >= 5 - "[#{major}.#{ActiveRecord::VERSION::MINOR}]" - end + format( + "[%d.%d]", + ActiveRecord::VERSION::MAJOR, + ActiveRecord::VERSION::MINOR + ) end end end diff --git a/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb b/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb index 73156b20..283e6192 100644 --- a/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb +++ b/lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb @@ -32,50 +32,18 @@ module PaperTrail end end - if ::ActiveRecord::VERSION::MAJOR >= 5 - # This implementation uses AR 5's `serialize` and `deserialize`. - class CastAttributeSerializer - def serialize(attr, val) - AttributeSerializerFactory.for(@klass, attr).serialize(val) - end - - def deserialize(attr, val) - if defined_enums[attr] && val.is_a?(::String) - # Because PT 4 used to save the string version of enums to `object_changes` - val - else - AttributeSerializerFactory.for(@klass, attr).deserialize(val) - end - end + # Uses AR 5's `serialize` and `deserialize`. + class CastAttributeSerializer + def serialize(attr, val) + AttributeSerializerFactory.for(@klass, attr).serialize(val) end - else - # This implementation uses AR 4.2's `type_cast_for_database`. For - # versions of AR < 4.2 we provide an implementation of - # `type_cast_for_database` in our shim attribute type classes, - # `NoOpAttribute` and `SerializedAttribute`. - class CastAttributeSerializer - def serialize(attr, val) - castable_val = val - if defined_enums[attr] - # `attr` is an enum. Find the number that corresponds to `val`. If `val` is - # a number already, there won't be a corresponding entry, just use `val`. - castable_val = defined_enums[attr][val] || val - end - @klass.type_for_attribute(attr).type_cast_for_database(castable_val) - end - def deserialize(attr, val) - if defined_enums[attr] && val.is_a?(::String) - # Because PT 4 used to save the string version of enums to `object_changes` - val - else - val = @klass.type_for_attribute(attr).type_cast_from_database(val) - if defined_enums[attr] - defined_enums[attr].key(val) - else - val - end - end + def deserialize(attr, val) + if defined_enums[attr] && val.is_a?(::String) + # Because PT 4 used to save the string version of enums to `object_changes` + val + else + AttributeSerializerFactory.for(@klass, attr).deserialize(val) end end end diff --git a/lib/paper_trail/compatibility.rb b/lib/paper_trail/compatibility.rb index 16f1d755..e7bd25e7 100644 --- a/lib/paper_trail/compatibility.rb +++ b/lib/paper_trail/compatibility.rb @@ -17,8 +17,8 @@ module PaperTrail # newer rails versions. Most PT users should avoid incompatible rails # versions. module Compatibility - ACTIVERECORD_GTE = ">= 4.2" - ACTIVERECORD_LT = "< 6.1" + ACTIVERECORD_GTE = ">= 5.2" # enforced in gemspec + ACTIVERECORD_LT = "< 6.1" # not enforced in gemspec E_INCOMPATIBLE_AR = <<-EOS PaperTrail %s is not compatible with ActiveRecord %s. We allow PT diff --git a/lib/paper_trail/frameworks/rails/controller.rb b/lib/paper_trail/frameworks/rails/controller.rb index f388e75f..99881042 100644 --- a/lib/paper_trail/frameworks/rails/controller.rb +++ b/lib/paper_trail/frameworks/rails/controller.rb @@ -25,9 +25,7 @@ module PaperTrail # @api public def user_for_paper_trail return unless defined?(current_user) - ActiveSupport::VERSION::MAJOR >= 4 ? current_user.try!(:id) : current_user.try(:id) - rescue NoMethodError - current_user + current_user.try(:id) || current_user end # Returns any information about the controller or request that you diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index 96e42afe..d18894da 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -128,10 +128,6 @@ module PaperTrail private - def active_record_gem_version - Gem::Version.new(ActiveRecord::VERSION::STRING) - end - # Raises an error if the provided class is an `abstract_class`. # @api private def assert_concrete_activerecord_class(class_name) @@ -141,8 +137,7 @@ module PaperTrail end def cannot_record_after_destroy? - Gem::Version.new(ActiveRecord::VERSION::STRING).release >= Gem::Version.new("5") && - ::ActiveRecord::Base.belongs_to_required_by_default + ::ActiveRecord::Base.belongs_to_required_by_default end # Some options require the presence of the `item_subtype` column. Currently diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 44ce5095..dbdd730d 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -88,9 +88,7 @@ module PaperTrail # # @api private def reify_attribute(k, v, model, version) - enums = model.class.respond_to?(:defined_enums) ? model.class.defined_enums : {} - is_enum_without_type_caster = ::ActiveRecord::VERSION::MAJOR < 5 && enums.key?(k) - if model.has_attribute?(k) && !is_enum_without_type_caster + if model.has_attribute?(k) model[k.to_sym] = v elsif model.respond_to?("#{k}=") model.send("#{k}=", v) diff --git a/paper_trail.gemspec b/paper_trail.gemspec index d6ec68be..acb0f01a 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -26,7 +26,10 @@ has been destroyed. s.require_paths = ["lib"] s.required_rubygems_version = ">= 1.3.6" - s.required_ruby_version = ">= 2.3.0" + + # Ruby 2.4 reaches EoL at the end of March of 2020 + # https://www.ruby-lang.org/en/news/2019/10/02/ruby-2-4-9-released/ + s.required_ruby_version = ">= 2.4.0" # We no longer specify a maximum rails version. # See discussion in paper_trail/compatibility.rb @@ -41,7 +44,7 @@ has been destroyed. s.add_development_dependency "paper_trail-association_tracking", "~> 2.0.0" s.add_development_dependency "rake", "~> 12.3" s.add_development_dependency "rspec-rails", "~> 3.8" - s.add_development_dependency "rubocop", "~> 0.74.0" + s.add_development_dependency "rubocop", "~> 0.80.0" s.add_development_dependency "rubocop-performance", "~> 1.4" s.add_development_dependency "rubocop-rspec", "~> 1.35" diff --git a/spec/controllers/articles_controller_spec.rb b/spec/controllers/articles_controller_spec.rb index c524eee2..a5dac24c 100644 --- a/spec/controllers/articles_controller_spec.rb +++ b/spec/controllers/articles_controller_spec.rb @@ -9,7 +9,7 @@ RSpec.describe ArticlesController, type: :controller do it "returns true" do expect(PaperTrail.enabled?).to eq(true) - post :create, params_wrapper(article: { title: "Doh", content: FFaker::Lorem.sentence }) + post :create, params: { article: { title: "Doh", content: FFaker::Lorem.sentence } } expect(assigns(:article)).not_to be_nil expect(PaperTrail.request.enabled?).to eq(true) expect(assigns(:article).versions.length).to eq(1) @@ -21,7 +21,7 @@ RSpec.describe ArticlesController, type: :controller do context "PaperTrail.enabled? == false" do it "returns false" do expect(PaperTrail.enabled?).to eq(false) - post :create, params_wrapper(article: { title: "Doh", content: FFaker::Lorem.sentence }) + post :create, params: { article: { title: "Doh", content: FFaker::Lorem.sentence } } expect(PaperTrail.request.enabled?).to eq(false) expect(assigns(:article).versions.length).to eq(0) end diff --git a/spec/controllers/widgets_controller_spec.rb b/spec/controllers/widgets_controller_spec.rb index 9768699b..598dca8a 100644 --- a/spec/controllers/widgets_controller_spec.rb +++ b/spec/controllers/widgets_controller_spec.rb @@ -10,7 +10,7 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do describe "#create" do context "PT enabled" do it "stores information like IP address in version" do - post(:create, params_wrapper(widget: { name: "Flugel" })) + post(:create, params: { widget: { name: "Flugel" } }) widget = assigns(:widget) expect(widget.versions.length).to(eq(1)) expect(widget.versions.last.whodunnit.to_i).to(eq(153)) @@ -20,7 +20,7 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do it "controller metadata methods should get evaluated" do request.env["HTTP_USER_AGENT"] = "User-Agent" - post :create, params_wrapper(widget: { name: "Flugel" }) + post :create, params: { widget: { name: "Flugel" } } expect(PaperTrail.request.enabled?).to eq(true) expect(PaperTrail.request.whodunnit).to(eq(153)) expect(PaperTrail.request.controller_info.present?).to(eq(true)) @@ -32,7 +32,7 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do context "PT disabled" do it "does not save a version, and metadata is not set" do request.env["HTTP_USER_AGENT"] = "Disable User-Agent" - post :create, params_wrapper(widget: { name: "Flugel" }) + post :create, params: { widget: { name: "Flugel" } } expect(assigns(:widget).versions.length).to(eq(0)) expect(PaperTrail.request.enabled?).to eq(false) expect(PaperTrail.request.whodunnit).to be_nil @@ -44,17 +44,17 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do describe "#destroy" do it "can be disabled" do request.env["HTTP_USER_AGENT"] = "Disable User-Agent" - post(:create, params_wrapper(widget: { name: "Flugel" })) + post(:create, params: { widget: { name: "Flugel" } }) w = assigns(:widget) expect(w.versions.length).to(eq(0)) - delete(:destroy, params_wrapper(id: w.id)) + delete(:destroy, params: { id: w.id }) expect(PaperTrail::Version.with_item_keys("Widget", w.id).size).to(eq(0)) end it "stores information like IP address in version" do w = Widget.create(name: "Roundel") expect(w.versions.length).to(eq(1)) - delete(:destroy, params_wrapper(id: w.id)) + delete(:destroy, params: { id: w.id }) widget = assigns(:widget) expect(widget.versions.length).to(eq(2)) expect(widget.versions.last.ip).to(eq("127.0.0.1")) @@ -67,7 +67,7 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do it "stores information like IP address in version" do w = Widget.create(name: "Duvel") expect(w.versions.length).to(eq(1)) - put(:update, params_wrapper(id: w.id, widget: { name: "Bugle" })) + put(:update, params: { id: w.id, widget: { name: "Bugle" } }) widget = assigns(:widget) expect(widget.versions.length).to(eq(2)) expect(widget.versions.last.whodunnit.to_i).to(eq(153)) @@ -77,10 +77,10 @@ RSpec.describe WidgetsController, type: :controller, versioning: true do it "can be disabled" do request.env["HTTP_USER_AGENT"] = "Disable User-Agent" - post(:create, params_wrapper(widget: { name: "Flugel" })) + post(:create, params: { widget: { name: "Flugel" } }) w = assigns(:widget) expect(w.versions.length).to(eq(0)) - put(:update, params_wrapper(id: w.id, widget: { name: "Bugle" })) + put(:update, params: { id: w.id, widget: { name: "Bugle" } }) widget = assigns(:widget) expect(widget.versions.length).to(eq(0)) end diff --git a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb index 422cc9d6..ab260a97 100644 --- a/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb +++ b/spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb @@ -6,13 +6,7 @@ # Starting with AR 5.1, we must specify which version of AR we are using. # I tried using `const_get` but I got a `NameError`, then I learned about # `::ActiveRecord::Migration::Current`. -class SetUpTestTables < ( - if ::ActiveRecord::VERSION::MAJOR >= 5 - ::ActiveRecord::Migration::Current - else - ::ActiveRecord::Migration - end -) +class SetUpTestTables < ::ActiveRecord::Migration::Current MYSQL_ADAPTERS = [ "ActiveRecord::ConnectionAdapters::MysqlAdapter", "ActiveRecord::ConnectionAdapters::Mysql2Adapter" diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 4b90ba76..de35250a 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -126,11 +126,7 @@ module PaperTrail before do if column_datatype_override - # In rails < 5, we use truncation, ie. there is no transaction - # around the tests, so we can't use a savepoint. - if active_record_gem_version >= ::Gem::Version.new("5") - ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") - end + ActiveRecord::Base.connection.execute("SAVEPOINT pgtest;") %w[object object_changes].each do |column| ActiveRecord::Base.connection.execute( "ALTER TABLE versions DROP COLUMN #{column};" @@ -147,20 +143,7 @@ module PaperTrail PaperTrail.serializer = PaperTrail::Serializers::YAML if column_datatype_override - # In rails < 5, we use truncation, ie. there is no transaction - # around the tests, so we can't use a savepoint. - if active_record_gem_version >= ::Gem::Version.new("5") - ActiveRecord::Base.connection.execute("ROLLBACK TO SAVEPOINT pgtest;") - else - %w[object object_changes].each do |column| - ActiveRecord::Base.connection.execute( - "ALTER TABLE versions DROP COLUMN #{column};" - ) - ActiveRecord::Base.connection.execute( - "ALTER TABLE versions ADD COLUMN #{column} text;" - ) - end - end + ActiveRecord::Base.connection.execute("ROLLBACK TO SAVEPOINT pgtest;") PaperTrail::Version.reset_column_information end end diff --git a/spec/requests/articles_spec.rb b/spec/requests/articles_spec.rb index f5ef2e0f..2d77ca0f 100644 --- a/spec/requests/articles_spec.rb +++ b/spec/requests/articles_spec.rb @@ -11,7 +11,7 @@ RSpec.describe "Articles management", type: :request, order: :defined do it "does not create a version" do expect(PaperTrail.request).to be_enabled expect { - post articles_path, params_wrapper(valid_params) + post articles_path, params: valid_params }.not_to change(PaperTrail::Version, :count) end end @@ -22,7 +22,7 @@ RSpec.describe "Articles management", type: :request, order: :defined do context "`current_user` method returns a `String`" do it "sets that value as the `whodunnit`" do expect { - post articles_path, params_wrapper(valid_params) + post articles_path, params: valid_params }.to change(PaperTrail::Version, :count).by(1) expect(article.title).to eq("Doh") expect(article.versions.last.whodunnit).to eq("foobar") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dc49df4e..3240e579 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,21 +28,6 @@ RSpec.configure do |config| Kernel.srand config.seed end -def active_record_gem_version - Gem::Version.new(ActiveRecord::VERSION::STRING) -end - -# Wrap args in a hash to support the ActionController::TestCase and -# ActionDispatch::Integration HTTP request method switch to keyword args -# (see https://github.com/rails/rails/blob/master/actionpack/CHANGELOG.md) -def params_wrapper(args) - if defined?(::Rails) && active_record_gem_version >= Gem::Version.new("5.0.0.beta1") - { params: args } - else - args - end -end - require File.expand_path("dummy_app/config/environment", __dir__) require "rspec/rails" require "paper_trail/frameworks/rspec" @@ -54,21 +39,5 @@ require_relative "support/paper_trail_spec_migrator" RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" -end - -# In rails < 5, some tests seem to require DatabaseCleaner-truncation. -# Truncation is about three times slower than transaction rollback, so it'll -# be nice when we can drop support for rails < 5. -if active_record_gem_version < ::Gem::Version.new("5") - require "database_cleaner" - DatabaseCleaner.strategy = :truncation - RSpec.configure do |config| - config.use_transactional_fixtures = false - config.before { DatabaseCleaner.start } - config.after { DatabaseCleaner.clean } - end -else - RSpec.configure do |config| - config.use_transactional_fixtures = true - end + config.use_transactional_fixtures = true end