Merge pull request #1007 from airblade/update_dev_deps_2017-10-27

Various: 2017-10-27

.byebug_history

@@ -0,0 +1,3 @@
+continue
+$done_debugging = true
+next

.rubocop_todo.yml

@@ -12,6 +12,8 @@ Metrics/CyclomaticComplexity:
 Metrics/PerceivedComplexity:
   Max: 9 # Goal: 7
 
+# Switching to frozen strings internally would be fine, but if any of those
+# frozen strings cross our public API, that would be a breaking change.
 Style/FrozenStringLiteralComment:
   Enabled: false
 
@@ -28,13 +30,13 @@ RSpec/InstanceVariable:
     - spec/paper_trail/associations_spec.rb
     - spec/paper_trail/model_spec.rb
 
-RSpec/NamedSubject:
-  Enabled: false
-
 RSpec/NestedGroups:
   Exclude:
     - spec/paper_trail/associations_spec.rb
     - spec/paper_trail/model_spec.rb
 
+# It may be possible for us to use safe_load, but we'd have to pass the
+# safelists, like `whitelist_classes` into our serializer, and the serializer
+# interface is a public API, so that would be a breaking change.
 Security/YAMLLoad:
   Enabled: false

CHANGELOG.md

@@ -3,6 +3,20 @@
 This project follows [semver 2.0.0](http://semver.org/spec/v2.0.0.html) and the
 recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
+## Next Major Version: 9
+
+In PT 8, the following are deprecated, and will be removed in 9:
+
+- [#479](https://github.com/airblade/paper_trail/issues/479) - Deprecated
+  `originator` method, use `paper_trail_originator`
+- Using paper_trail.on_destroy(:after) with ActiveRecord's
+  belongs_to_required_by_default will produce an error instead of a warning.
+- Failing to set PaperTrail.config.track_associations will no longer produce
+  a warning. The default (false) will remain the same.
+- warn_about_not_setting_whodunnit controller method
+- [#997](https://github.com/airblade/paper_trail/pull/997) -
+  where_object_changes reading YAML from a text column
+
 ## Unreleased
 
 ### Breaking Changes

lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb

@@ -6,8 +6,7 @@ module PaperTrail
     # to an attribute of type `ActiveRecord::Type::Integer`.
     #
     # This implementation depends on the `type_for_attribute` method, which was
-    # introduced in rails 4.2. In older versions of rails, we shim this method
-    # with `LegacyActiveRecordShim`.
+    # introduced in rails 4.2. As of PT 8, we no longer support rails < 4.2.
     class CastAttributeSerializer
       def initialize(klass)
         @klass = klass

lib/paper_trail/attribute_serializers/legacy_active_record_shim.rb

@@ -1,48 +0,0 @@
-module PaperTrail
-  module AttributeSerializers
-    # Included into model if AR version is < 4.2. Backport Rails 4.2 and later's
-    # `type_for_attribute` so we can build on a common interface.
-    module LegacyActiveRecordShim
-      # An attribute which needs no processing. It is part of our backport (shim)
-      # of rails 4.2's attribute API. See `type_for_attribute` below.
-      class NoOpAttribute
-        def type_cast_for_database(value)
-          value
-        end
-
-        def type_cast_from_database(data)
-          data
-        end
-      end
-      NO_OP_ATTRIBUTE = NoOpAttribute.new
-
-      # An attribute which requires manual (de)serialization to/from what we get
-      # from the database. It is part of our backport (shim) of rails 4.2's
-      # attribute API. See `type_for_attribute` below.
-      class SerializedAttribute
-        def initialize(coder)
-          @coder = coder.respond_to?(:dump) ? coder : PaperTrail.serializer
-        end
-
-        def type_cast_for_database(value)
-          @coder.dump(value)
-        end
-
-        def type_cast_from_database(data)
-          @coder.load(data)
-        end
-      end
-
-      def type_for_attribute(attr_name)
-        serialized_attribute_types[attr_name.to_s] || NO_OP_ATTRIBUTE
-      end
-
-      def serialized_attribute_types
-        @attribute_types ||= Hash[serialized_attributes.map do |attr_name, coder|
-          [attr_name, SerializedAttribute.new(coder)]
-        end]
-      end
-      private :serialized_attribute_types
-    end
-  end
-end

lib/paper_trail/has_paper_trail.rb

@@ -1,5 +1,4 @@
 require "active_support/core_ext/object" # provides the `try` method
-require "paper_trail/attribute_serializers/legacy_active_record_shim"
 require "paper_trail/attribute_serializers/object_attribute"
 require "paper_trail/attribute_serializers/object_changes_attribute"
 require "paper_trail/model_config"

lib/paper_trail/model_config.rb

@@ -79,13 +79,6 @@ module PaperTrail
       options[:on] ||= %i[create update destroy]
       options[:on] = Array(options[:on]) # Support single symbol
       @model_class.send :include, ::PaperTrail::Model::InstanceMethods
-      if ::ActiveRecord::VERSION::STRING < "4.2"
-        ::ActiveSupport::Deprecation.warn(
-          "Your version of ActiveRecord (< 4.2) has reached EOL. PaperTrail " \
-          "will soon drop support. Please upgrade ActiveRecord ASAP."
-        )
-        @model_class.send :extend, AttributeSerializers::LegacyActiveRecordShim
-      end
       setup_options(options)
       setup_associations(options)
       setup_transaction_callbacks

paper_trail.gemspec

@@ -30,17 +30,17 @@ has been destroyed.
   s.add_dependency "request_store", "~> 1.1"
 
   s.add_development_dependency "appraisal", "~> 2.2"
+  s.add_development_dependency "byebug", "~> 9.1"
   s.add_development_dependency "database_cleaner", "~> 1.6"
   s.add_development_dependency "ffaker", "~> 2.7"
   s.add_development_dependency "generator_spec", "~> 0.9.4"
   s.add_development_dependency "mysql2", "~> 0.4.9"
   s.add_development_dependency "pg", "~> 0.21.0"
-  s.add_development_dependency "pry-byebug", "~> 3.5"
   s.add_development_dependency "rack-test", "~> 0.6.3"
   s.add_development_dependency "rake", "~> 12.1"
   s.add_development_dependency "rspec-rails", "~> 3.7.1"
-  s.add_development_dependency "rubocop", "0.50.0"
-  s.add_development_dependency "rubocop-rspec", "~> 1.18.0"
+  s.add_development_dependency "rubocop", "0.51.0"
+  s.add_development_dependency "rubocop-rspec", "~> 1.19.0"
   s.add_development_dependency "sqlite3", "~> 1.3"
   s.add_development_dependency "timecop", "~> 0.9.1"
 end

spec/models/boolit_spec.rb

@@ -2,39 +2,39 @@ require "spec_helper"
 require "support/custom_json_serializer"
 
 RSpec.describe Boolit, type: :model, versioning: true do
-  subject { Boolit.create! }
+  let(:boolit) { Boolit.create! }
 
-  before { subject.update_attributes!(name: FFaker::Name.name) }
+  before { boolit.update_attributes!(name: FFaker::Name.name) }
 
   it "has two versions" do
-    expect(subject.versions.size).to eq(2)
+    expect(boolit.versions.size).to eq(2)
   end
 
   it "can be reified and persisted" do
-    expect { subject.versions.last.reify.save! }.not_to raise_error
+    expect { boolit.versions.last.reify.save! }.not_to raise_error
   end
 
   context "Instance falls out of default scope" do
-    before { subject.update_attributes!(scoped: false) }
+    before { boolit.update_attributes!(scoped: false) }
 
     it "is NOT scoped" do
       expect(Boolit.first).to be_nil
     end
 
     it "still can be reified and persisted" do
-      expect { subject.paper_trail.previous_version.save! }.not_to raise_error
+      expect { boolit.paper_trail.previous_version.save! }.not_to raise_error
     end
 
     context "with `nil` attributes on the live instance" do
       before do
         PaperTrail.serializer = CustomJsonSerializer
-        subject.update_attributes!(name: nil)
-        subject.update_attributes!(name: FFaker::Name.name)
+        boolit.update_attributes!(name: nil)
+        boolit.update_attributes!(name: FFaker::Name.name)
       end
       after { PaperTrail.serializer = PaperTrail::Serializers::YAML }
 
       it "does not overwrite that attribute during the reification process" do
-        expect(subject.paper_trail.previous_version.name).to be_nil
+        expect(boolit.paper_trail.previous_version.name).to be_nil
       end
     end
   end

spec/models/gadget_spec.rb

@@ -32,31 +32,31 @@ RSpec.describe Gadget, type: :model do
 
     context "persisted record without update timestamps" do
       it "only acknowledges non-ignored attrs" do
-        subject = Gadget.create!(created_at: Time.now)
-        subject.name = "Wrench"
-        expect(subject.paper_trail.changed_notably?).to be true
+        gadget = Gadget.create!(created_at: Time.now)
+        gadget.name = "Wrench"
+        expect(gadget.paper_trail.changed_notably?).to be true
       end
 
       it "does not acknowledge ignored attr (brand)" do
-        subject = Gadget.create!(created_at: Time.now)
-        subject.brand = "Acme"
-        expect(subject.paper_trail.changed_notably?).to be false
+        gadget = Gadget.create!(created_at: Time.now)
+        gadget.brand = "Acme"
+        expect(gadget.paper_trail.changed_notably?).to be false
       end
     end
 
     context "persisted record with update timestamps" do
       it "only acknowledges non-ignored attrs" do
-        subject = Gadget.create!(created_at: Time.now)
-        subject.name = "Wrench"
-        subject.updated_at = Time.now
-        expect(subject.paper_trail.changed_notably?).to be true
+        gadget = Gadget.create!(created_at: Time.now)
+        gadget.name = "Wrench"
+        gadget.updated_at = Time.now
+        expect(gadget.paper_trail.changed_notably?).to be true
       end
 
       it "does not acknowledge ignored attrs and timestamps only" do
-        subject = Gadget.create!(created_at: Time.now)
-        subject.brand = "Acme"
-        subject.updated_at = Time.now
-        expect(subject.paper_trail.changed_notably?).to be false
+        gadget = Gadget.create!(created_at: Time.now)
+        gadget.brand = "Acme"
+        gadget.updated_at = Time.now
+        expect(gadget.paper_trail.changed_notably?).to be false
       end
     end
   end

spec/models/post_with_status_spec.rb

@@ -21,12 +21,11 @@ RSpec.describe PostWithStatus, type: :model do
     end
 
     context "storing enum object_changes" do
-      subject { post.versions.last }
-
       it "saves the enum value properly in versions object_changes" do
         post.published!
         post.archived!
-        expect(subject.changeset["status"]).to eql %w[published archived]
+        post_version = post.versions.last
+        expect(post_version.changeset["status"]).to eql(%w[published archived])
       end
     end
 

spec/models/version_spec.rb

@@ -62,12 +62,12 @@ module PaperTrail
     end
 
     describe "#originator" do
-      it "sets the invoke `paper_trail_originator`" do
+      it "delegates to paper_trail_originator" do
         allow(ActiveSupport::Deprecation).to receive(:warn)
-        subject = PaperTrail::Version.new
-        allow(subject).to receive(:paper_trail_originator)
-        subject.originator
-        expect(subject).to have_received(:paper_trail_originator)
+        version = PaperTrail::Version.new
+        allow(version).to receive(:paper_trail_originator)
+        version.originator
+        expect(version).to have_received(:paper_trail_originator)
       end
 
       it "displays a deprecation warning" do
@@ -81,19 +81,19 @@ module PaperTrail
     describe "#terminator" do
       it "is an alias for the `whodunnit` attribute" do
         attributes = { whodunnit: FFaker::Name.first_name }
-        subject = PaperTrail::Version.new(attributes)
-        expect(subject.terminator).to eq(attributes[:whodunnit])
+        version = PaperTrail::Version.new(attributes)
+        expect(version.terminator).to eq(attributes[:whodunnit])
       end
     end
 
     describe "#version_author" do
       it "is an alias for the `terminator` method" do
-        subject = PaperTrail::Version.new
-        expect(subject.method(:version_author)).to eq(subject.method(:terminator))
+        version = PaperTrail::Version.new
+        expect(version.method(:version_author)).to eq(version.method(:terminator))
       end
     end
 
-    describe "Methods" do
+    context "changing the data type of database columns on the fly" do
       # TODO: Changing the data type of these database columns in the middle
       # of the test suite adds a fair amount of complication. Is there a better
       # way? We already have a `json_versions` table in our tests, maybe we

spec/models/widget_spec.rb

@@ -52,14 +52,12 @@ RSpec.describe Widget, type: :model do
 
   describe "Callbacks", versioning: true do
     describe "before_save" do
-      before { widget.update_attributes!(name: "Foobar") }
-
-      subject { widget.versions.last.reify }
-
       it "resets value for timestamp attrs for update so that value gets updated properly" do
+        widget.update_attributes!(name: "Foobar")
+        w = widget.versions.last.reify
         # Travel 1 second because MySQL lacks sub-second resolution
         Timecop.travel(1) do
-          expect { subject.save! }.to change(subject, :updated_at)
+          expect { w.save! }.to change(w, :updated_at)
         end
       end
     end
@@ -73,15 +71,15 @@ RSpec.describe Widget, type: :model do
     end
 
     describe "after_update" do
-      before { widget.update_attributes!(name: "Foobar", updated_at: Time.now + 1.week) }
-
-      subject { widget.versions.last.reify }
-
-      it { expect(subject.paper_trail).not_to be_live }
+      before do
+        widget.update_attributes!(name: "Foobar", updated_at: Time.now + 1.week)
+      end
 
       it "clears the `versions_association_name` virtual attribute" do
-        subject.save!
-        expect(subject.paper_trail).to be_live
+        w = widget.versions.last.reify
+        expect(w.paper_trail).not_to be_live
+        w.save!
+        expect(w.paper_trail).to be_live
       end
 
       it "corresponding version uses the widget updated_at" do

spec/paper_trail/model_spec.rb

@@ -215,7 +215,7 @@ RSpec.describe(::PaperTrail, versioning: true) do
 
   context "a record's papertrail" do
     before do
-      @date_time = DateTime.now.utc
+      @date_time = Time.now
       @time = Time.now
       @date = Date.new(2009, 5, 29)
       @widget = Widget.create(

spec/paper_trail/version_limit_spec.rb

@@ -18,8 +18,7 @@ module PaperTrail
     end
 
     it "deletes oldest versions, when the database returns them in a different order" do
-      epoch = DateTime.new(2017, 1, 1)
-
+      epoch = Date.new(2017, 1, 1)
       widget = Timecop.freeze(epoch) { Widget.create }
 
       # Sometimes a database will returns records in a different order than

spec/paper_trail_spec.rb

@@ -2,26 +2,24 @@ require "spec_helper"
 
 RSpec.describe PaperTrail do
   describe "#config", versioning: true do
-    it { is_expected.to respond_to(:config) }
-
     it "allows for config values to be set" do
-      expect(subject.config.enabled).to eq(true)
-      subject.config.enabled = false
-      expect(subject.config.enabled).to eq(false)
+      expect(described_class.config.enabled).to eq(true)
+      described_class.config.enabled = false
+      expect(described_class.config.enabled).to eq(false)
     end
 
     it "accepts blocks and yield the config instance" do
-      expect(subject.config.enabled).to eq(true)
-      subject.config { |c| c.enabled = false }
-      expect(subject.config.enabled).to eq(false)
+      expect(described_class.config.enabled).to eq(true)
+      described_class.config { |c| c.enabled = false }
+      expect(described_class.config.enabled).to eq(false)
     end
   end
 
   describe "#configure" do
-    it { is_expected.to respond_to(:configure) }
-
     it "is an alias for the `config` method" do
-      expect(subject.method(:configure)).to eq(subject.method(:config))
+      expect(described_class.method(:configure)).to eq(
+        described_class.method(:config)
+      )
     end
   end
 

spec/spec_helper.rb

@@ -1,9 +1,8 @@
-require "pry"
-
-# This file is copied to spec/ when you run 'rails generate rspec:install'
 ENV["RAILS_ENV"] ||= "test"
 ENV["DB"] ||= "sqlite"
 
+require "byebug"
+
 unless File.exist?(File.expand_path("dummy_app/config/database.yml", __dir__))
   warn "WARNING: No database.yml detected for the dummy app, please run `rake prepare` first"
 end