Prepare for v11 (#1235)

* Drop support for ruby 2.3

* Docs: appraisal

* Drop support for rails <= 5.1

* Upgrade rubocop to 0.80

* Remove defunct code for EoL rails versions

* Rails 5+ style controller test params keyword

* Squash me

Co-Authored-By: Todd Lynam <TLynam@gmail.com>

Co-authored-by: Todd Lynam <TLynam@gmail.com>
This commit is contained in:
Jared Beck 2020-03-02 21:58:33 -05:00 committed by GitHub
parent 64edf2317f
commit f92037c048
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 82 additions and 165 deletions

View File

@ -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

View File

@ -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.

View File

@ -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

View File

@ -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:

View File

@ -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"

View File

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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

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

View File

@ -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"

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -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

View File

@ -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")

View File

@ -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