diff --git a/.travis.yml b/.travis.yml index a52a081f..a65f6c89 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,8 +30,6 @@ gemfile: matrix: fast_finish: true - allow_failures: - - gemfile: gemfiles/ar5.gemfile exclude: - gemfile: gemfiles/ar5.gemfile rvm: 1.9.3 diff --git a/Appraisals b/Appraisals index 8acc3080..116da6da 100644 --- a/Appraisals +++ b/Appraisals @@ -35,4 +35,7 @@ appraise "ar5" do gem "rspec-expectations", github: "rspec/rspec-expectations" gem "rspec-mocks", github: "rspec/rspec-mocks" gem "rspec-support", github: "rspec/rspec-support" + gem 'rails-controller-testing' + # Sinatra stable conflicts with AR5's rack dependency + gem 'sinatra', github: 'sinatra/sinatra' end diff --git a/CHANGELOG.md b/CHANGELOG.md index 14295e8b..198d2e84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ ### Added -None +- [#689](https://github.com/airblade/paper_trail/pull/689) - + Rails 5 compatibility ### Fixed diff --git a/gemfiles/ar3.gemfile b/gemfiles/ar3.gemfile index c3b2bb82..88ccd0cb 100644 --- a/gemfiles/ar3.gemfile +++ b/gemfiles/ar3.gemfile @@ -2,24 +2,16 @@ source "https://rubygems.org" -gem "activerecord", "~> 3.2" +gem "activerecord", "~> 3.2.22" gem "i18n", "~> 0.6.11" gem "request_store", "~> 1.1.0" group :development, :test do - gem "railties", "~> 3.0" - gem "test-unit", "~> 3.0" + gem "railties", "~> 3.2.22" + gem "test-unit", "~> 3.1.5" platforms :ruby do gem "mysql2", "~> 0.3.20" - gem "pg", "~> 0.17.1" - end - - platforms :jruby do - gem "activerecord-jdbcsqlite3-adapter", "~> 1.3" - gem "activerecord-jdbcpostgresql-adapter", "~> 1.3" - gem "activerecord-jdbcmysql-adapter", "~> 1.3" - gem "activerecord-jdbc-adapter", "1.3.15" end end diff --git a/gemfiles/ar5.gemfile b/gemfiles/ar5.gemfile index 9b2fed95..3662ce1c 100644 --- a/gemfiles/ar5.gemfile +++ b/gemfiles/ar5.gemfile @@ -11,5 +11,7 @@ gem "rspec-core", :github => "rspec/rspec-core" gem "rspec-expectations", :github => "rspec/rspec-expectations" gem "rspec-mocks", :github => "rspec/rspec-mocks" gem "rspec-support", :github => "rspec/rspec-support" +gem "rails-controller-testing" +gem "sinatra", :github => "sinatra/sinatra" gemspec :path => "../" diff --git a/lib/paper_trail/record_history.rb b/lib/paper_trail/record_history.rb index c9b953b6..c3e6a225 100644 --- a/lib/paper_trail/record_history.rb +++ b/lib/paper_trail/record_history.rb @@ -16,7 +16,7 @@ module PaperTrail # Returns ordinal position of `version` in `sequence`. # @api private def index(version) - sequence.index(version) + sequence.to_a.index(version) end private diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 137d9f68..15dbe057 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -154,7 +154,9 @@ module PaperTrail else child = version.reify(options.merge(:has_many => false, :has_one => false)) model.appear_as_new_record do - model.send "#{assoc.name}=", child + without_persisting(child) do + model.send "#{assoc.name}=", child + end end end end @@ -265,6 +267,18 @@ module PaperTrail where("id IN (#{version_id_subquery})"). inject({}) { |acc, v| acc.merge!(v.item_id => v) } end + + # Temporarily suppress #save so we can reassociate with the reified + # master of a has_one relationship. Since ActiveRecord 5 the related + # object is saved when it is assigned to the association. ActiveRecord + # 5 also happens to be the first version that provides #suppress. + def without_persisting(record) + if record.class.respond_to? :suppress + record.class.suppress { yield } + else + yield + end + end end end end diff --git a/spec/models/gadget_spec.rb b/spec/models/gadget_spec.rb index 7782e9ba..ab243cc7 100644 --- a/spec/models/gadget_spec.rb +++ b/spec/models/gadget_spec.rb @@ -15,7 +15,8 @@ describe Gadget, :type => :model do end it "should still generate a version when only the `updated_at` attribute is updated" do - expect { gadget.update_attribute(:updated_at, Time.now) }.to change{gadget.versions.size}.by(1) + # Plus 1 second because MySQL lacks sub-second resolution + expect { gadget.update_attribute(:updated_at, Time.now + 1) }.to change{gadget.versions.size}.by(1) end end diff --git a/spec/models/not_on_update_spec.rb b/spec/models/not_on_update_spec.rb index 7b57c0de..7aa4eb19 100644 --- a/spec/models/not_on_update_spec.rb +++ b/spec/models/not_on_update_spec.rb @@ -12,7 +12,10 @@ describe NotOnUpdate, :type => :model do it "increments the `:updated_at` timestamp" do before = record.updated_at - record.touch_with_version + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + record.touch_with_version + end expect(record.updated_at).to be > before end end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 453df9f9..65dbacaf 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -43,7 +43,10 @@ describe Widget, :type => :model do subject { widget.versions.last.reify } it "should reset the value for the timestamp attrs for update so that value gets updated properly" do - expect { subject.save! }.to change(subject, :updated_at) + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + expect { subject.save! }.to change(subject, :updated_at) + end end end end @@ -253,13 +256,19 @@ describe Widget, :type => :model do it "creates a version" do count = widget.versions.size - widget.touch_with_version + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + widget.touch_with_version + end expect(widget.versions.size).to eq(count + 1) end it "increments the `:updated_at` timestamp" do time_was = widget.updated_at - widget.touch_with_version + # Travel 1 second because MySQL lacks sub-second resolution + Timecop.travel(1) do + widget.touch_with_version + end expect(widget.updated_at).to be > time_was end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index f44d4cc3..358e0f3e 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -12,6 +12,7 @@ require 'rspec/rails' require 'paper_trail/frameworks/rspec' require 'shoulda/matchers' require 'ffaker' +require 'timecop' # prevent Test::Unit's AutoRunner from executing during RSpec's rake task Test::Unit.run = true if defined?(Test::Unit) && Test::Unit.respond_to?(:run=) diff --git a/spec/requests/articles_spec.rb b/spec/requests/articles_spec.rb index 70b61eaa..a39fc847 100644 --- a/spec/requests/articles_spec.rb +++ b/spec/requests/articles_spec.rb @@ -8,7 +8,7 @@ describe "Articles management", :type => :request, :order => :defined do it "should not create a version" do expect(PaperTrail).to be_enabled_for_controller - expect { post(articles_path, valid_params) }.to_not change(PaperTrail::Version, :count) + expect { post articles_path, params_wrapper(valid_params) }.to_not change(PaperTrail::Version, :count) end it "should not leak the state of the `PaperTrail.enabled_for_controller?` into the next test" do @@ -21,7 +21,7 @@ describe "Articles management", :type => :request, :order => :defined do context "`current_user` method returns a `String`" do it "should set that value as the `whodunnit`" do - expect { post articles_path, valid_params }.to change(PaperTrail::Version, :count).by(1) + expect { post articles_path, params_wrapper(valid_params) }.to change(PaperTrail::Version, :count).by(1) expect(article.title).to eq('Doh') expect(article.versions.last.whodunnit).to eq('foobar') end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0dca3a47..36b63407 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -102,3 +102,14 @@ RSpec.configure do |config| Kernel.srand config.seed =end 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) && Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1') + { params: args } + else + args + end +end \ No newline at end of file diff --git a/test/dummy/config/application.rb b/test/dummy/config/application.rb index 0b38150a..20973f84 100644 --- a/test/dummy/config/application.rb +++ b/test/dummy/config/application.rb @@ -69,6 +69,9 @@ module Dummy if v >= Gem::Version.new("4.2") && v < Gem::Version.new("5.0.0.beta1") config.active_record.raise_in_transactional_callbacks = true end + if v >= Gem::Version.new("5.0.0.beta1") + config.active_record.time_zone_aware_types = [:datetime] + end end # Set test order for Test::Unit if possible diff --git a/test/functional/controller_test.rb b/test/functional/controller_test.rb index 8eead732..800114e2 100644 --- a/test/functional/controller_test.rb +++ b/test/functional/controller_test.rb @@ -15,32 +15,32 @@ class ControllerTest < ActionController::TestCase test 'disable on create' do @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent' - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) assert_equal 0, assigns(:widget).versions.length end test 'disable on update' do @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent' - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) w = assigns(:widget) assert_equal 0, w.versions.length - put :update, :id => w.id, :widget => { :name => 'Bugle' } + put :update, params_wrapper({ id: w.id, widget: { name: 'Bugle' } }) widget = assigns(:widget) assert_equal 0, widget.versions.length end test 'disable on destroy' do @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent' - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) w = assigns(:widget) assert_equal 0, w.versions.length - delete :destroy, :id => w.id + delete :destroy, params_wrapper({ id: w.id }) widget = assigns(:widget) assert_equal 0, PaperTrail::Version.with_item_keys('Widget', w.id).size end test 'create' do - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) widget = assigns(:widget) assert_equal 1, widget.versions.length assert_equal 153, widget.versions.last.whodunnit.to_i @@ -51,7 +51,7 @@ class ControllerTest < ActionController::TestCase test 'update' do w = Widget.create :name => 'Duvel' assert_equal 1, w.versions.length - put :update, :id => w.id, :widget => { :name => 'Bugle' } + put :update, params_wrapper({ id: w.id, widget: { name: 'Bugle' } }) widget = assigns(:widget) assert_equal 2, widget.versions.length assert_equal 153, widget.versions.last.whodunnit.to_i @@ -62,7 +62,7 @@ class ControllerTest < ActionController::TestCase test 'destroy' do w = Widget.create :name => 'Roundel' assert_equal 1, w.versions.length - delete :destroy, :id => w.id + delete :destroy, params_wrapper({ id: w.id }) widget = assigns(:widget) assert_equal 2, widget.versions.length assert_equal '127.0.0.1', widget.versions.last.ip @@ -72,7 +72,7 @@ class ControllerTest < ActionController::TestCase test "controller metadata methods should get evaluated if paper trail is enabled for controller" do @request.env['HTTP_USER_AGENT'] = 'User-Agent' - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) assert PaperTrail.enabled_for_controller? assert_equal 153, PaperTrail.whodunnit assert PaperTrail.controller_info.present? @@ -82,7 +82,7 @@ class ControllerTest < ActionController::TestCase test "controller metadata methods should not get evaluated if paper trail is disabled for controller" do @request.env['HTTP_USER_AGENT'] = 'Disable User-Agent' - post :create, :widget => { :name => 'Flugel' } + post :create, params_wrapper({ widget: { name: 'Flugel' } }) assert_equal 0, assigns(:widget).versions.length assert !PaperTrail.enabled_for_controller? assert PaperTrail.whodunnit.nil? diff --git a/test/functional/enabled_for_controller_test.rb b/test/functional/enabled_for_controller_test.rb index e8a6eb2a..928002b9 100644 --- a/test/functional/enabled_for_controller_test.rb +++ b/test/functional/enabled_for_controller_test.rb @@ -6,7 +6,7 @@ class EnabledForControllerTest < ActionController::TestCase context "`PaperTrail.enabled? == true`" do should 'enabled_for_controller?.should == true' do assert PaperTrail.enabled? - post :create, :article => { :title => 'Doh', :content => FFaker::Lorem.sentence } + post :create, params_wrapper({ article: { title: 'Doh', content: FFaker::Lorem.sentence } }) assert_not_nil assigns(:article) assert PaperTrail.enabled_for_controller? assert_equal 1, assigns(:article).versions.length @@ -18,7 +18,7 @@ class EnabledForControllerTest < ActionController::TestCase should 'enabled_for_controller?.should == false' do assert !PaperTrail.enabled? - post :create, :article => { :title => 'Doh', :content => FFaker::Lorem.sentence } + post :create, params_wrapper({ article: { title: 'Doh', content: FFaker::Lorem.sentence } }) assert !PaperTrail.enabled_for_controller? assert_equal 0, assigns(:article).versions.length end diff --git a/test/test_helper.rb b/test/test_helper.rb index 193ccf71..8b405b41 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -21,6 +21,11 @@ require 'shoulda' require 'ffaker' require 'database_cleaner' +if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1') + # See https://github.com/rails/rails-controller-testing/issues/5 + ActionController::TestCase.send(:include, Rails::Controller::Testing::TestProcess) +end + Rails.backtrace_cleaner.remove_silencers! # Run any available migration @@ -35,7 +40,11 @@ DatabaseCleaner.strategy = :truncation # global setup block resetting Thread.current class ActiveSupport::TestCase if using_mysql? - self.use_transactional_fixtures = false + if respond_to? :use_transactional_tests= + self.use_transactional_tests = false + else + self.use_transactional_fixtures = false + end setup { DatabaseCleaner.start } end @@ -89,6 +98,22 @@ def change_schema reset_version_class_column_info! 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) && Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1') + { params: args } + else + args + end +end + +def reset_version_class_column_info! + PaperTrail::Version.connection.schema_cache.clear! + PaperTrail::Version.reset_column_information +end + def restore_schema ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define do @@ -97,9 +122,4 @@ def restore_schema end ActiveRecord::Migration.verbose = true reset_version_class_column_info! -end - -def reset_version_class_column_info! - PaperTrail::Version.connection.schema_cache.clear! - PaperTrail::Version.reset_column_information -end +end \ No newline at end of file diff --git a/test/unit/associations_test.rb b/test/unit/associations_test.rb index ea1c4ead..5fb2e73a 100644 --- a/test/unit/associations_test.rb +++ b/test/unit/associations_test.rb @@ -19,7 +19,11 @@ class AssociationsTest < ActiveSupport::TestCase # These would have been done in test_helper.rb if using_mysql? is true unless using_mysql? - self.use_transactional_fixtures = false + if self.respond_to? :use_transactional_tests= + self.use_transactional_tests = false + else + self.use_transactional_fixtures = false + end setup { DatabaseCleaner.start } end