Skip to content

Commit

Permalink
Update PaperTrail and test suite for ActiveRecord 5 compatibility
Browse files Browse the repository at this point in the history
  • Loading branch information
owenr committed Jan 16, 2016
1 parent a80c23f commit 85733fe
Show file tree
Hide file tree
Showing 17 changed files with 91 additions and 30 deletions.
1 change: 1 addition & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Style/AccessorMethodName:
Style/Alias:
Exclude:
- 'lib/paper_trail/has_paper_trail.rb'
- 'lib/paper_trail/reifier.rb'

# Offense count: 2
# Cop supports --auto-correct.
Expand Down
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ gemfile:

matrix:
fast_finish: true
allow_failures:
- gemfile: gemfiles/ar5.gemfile
exclude:
- gemfile: gemfiles/ar5.gemfile
rvm: 1.9.3
Expand Down
3 changes: 3 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -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', branch: '2.2.0-alpha'
end
2 changes: 2 additions & 0 deletions gemfiles/ar5.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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", :branch => "2.2.0-alpha"

gemspec :path => "../"
2 changes: 1 addition & 1 deletion lib/paper_trail/record_history.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def initialize(versions, version_class)
# Returns ordinal position of `version` in `sequence`.
# @api private
def index(version)
sequence.index(version)
sequence.to_a.index(version)
end

private
Expand Down
16 changes: 15 additions & 1 deletion lib/paper_trail/reifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ def reify_has_ones(transaction_id, model, options = {})
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
Expand Down Expand Up @@ -265,6 +267,18 @@ def versions_by_id(klass, version_id_subquery)
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
2 changes: 1 addition & 1 deletion spec/models/gadget_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
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)
expect { gadget.update_attribute(:updated_at, Time.now + 1) }.to change{gadget.versions.size}.by(1)
end
end

Expand Down
4 changes: 3 additions & 1 deletion spec/models/not_on_update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

it "increments the `:updated_at` timestamp" do
before = record.updated_at
record.touch_with_version
Timecop.travel(1) do
record.touch_with_version
end
expect(record.updated_at).to be > before
end
end
Expand Down
12 changes: 9 additions & 3 deletions spec/models/widget_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
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)
Timecop.travel(5) do
expect { subject.save! }.to change(subject, :updated_at)
end
end
end
end
Expand Down Expand Up @@ -253,13 +255,17 @@

it "creates a version" do
count = widget.versions.size
widget.touch_with_version
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
Timecop.travel(1) do
widget.touch_with_version
end
expect(widget.updated_at).to be > time_was
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
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=)
Expand Down
4 changes: 2 additions & 2 deletions spec/requests/articles_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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
Expand All @@ -21,7 +21,7 @@

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
Expand Down
8 changes: 8 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,11 @@
Kernel.srand config.seed
=end
end

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
3 changes: 3 additions & 0 deletions test/dummy/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ class Application < Rails::Application
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
Expand Down
20 changes: 10 additions & 10 deletions test/functional/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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?
Expand All @@ -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?
Expand Down
4 changes: 2 additions & 2 deletions test/functional/enabled_for_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 25 additions & 6 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ def using_mysql?
end

require File.expand_path("../dummy/config/environment.rb", __FILE__)
if Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new('5.0.0.beta1')
ActiveSupport.on_load(:action_controller) do
ActionController::TestCase.send(:include, Rails::Controller::Testing::TestProcess)
ActionController::TestCase.send(:include, Rails::Controller::Testing::TemplateAssertions)

ActionDispatch::IntegrationTest.send(:include, Rails::Controller::Testing::TemplateAssertions)
ActionDispatch::IntegrationTest.send(:include, Rails::Controller::Testing::Integration)

ActionView::TestCase.send(:include, Rails::Controller::Testing::TemplateAssertions)
end
end
require "rails/test_help"
require 'shoulda'
require 'ffaker'
Expand Down Expand Up @@ -89,6 +100,19 @@ def change_schema
reset_version_class_column_info!
end

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
Expand All @@ -97,9 +121,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
6 changes: 5 additions & 1 deletion test/unit/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 85733fe

Please sign in to comment.