diff --git a/.travis.yml b/.travis.yml index 6dbe886b..3c9e6e33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ gemfile: - gemfiles/3.0.gemfile matrix: + fast_finish: true allow_failures: - rvm: jruby-18mode gemfile: Gemfile diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f6df0d3..f7244c5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,20 @@ ## 3.0.1 (Unreleased) + - [#340](https://github.com/airblade/paper_trail/issues/340) - Prevent potential error encountered when using the `InstallGenerator` + with Rails `4.1.0.rc1`. + - [#334](https://github.com/airblade/paper_trail/pull/334) - Add small-scope `whodunnit` method to `PaperTrail::Model::InstanceMethods`. - [#329](https://github.com/airblade/paper_trail/issues/329) - Add `touch_with_version` method to `PaperTrail::Model::InstanceMethods`, to allow for generating a version `touch`ing a model. - - [#328](https://github.com/airblade/paper_trail/pull/328) / [#326](https://github.com/airblade/paper_trail/issues/326)/ + - [#328](https://github.com/airblade/paper_trail/pull/328) / [#326](https://github.com/airblade/paper_trail/issues/326) / [#307](https://github.com/airblade/paper_trail/issues/307) - `Model.paper_trail_enabled_for_model?` and `model_instance.without_versioning` is now thread-safe. - [#316](https://github.com/airblade/paper_trail/issues/316) - `user_for_paper_trail` should default to `current_user.try(:id)` instead of `current_user` (if `current_user` is defined). - [#313](https://github.com/airblade/paper_trail/pull/313) - Make the `Rails::Controller` helper compatible with `ActionController::API` for compatibility with the [`rails-api`](https://github.com/rails-api/rails-api) gem. + - [#312](https://github.com/airblade/paper_trail/issues/312) - Fix RSpec `with_versioning` class level helper method. + - `model_instance.without_versioning` now yields the `model_instance`, enabling syntax like this: + `model_instance.without_versioning { |obj| obj.update_attributes(:name => 'value') }`. - Deprecated `Model.paper_trail_on` and `Model.paper_trail_off` in favor of bang versions of the methods. Deprecation warning informs users that the non-bang versions of the methods will be removed in version `3.1.0`. diff --git a/README.md b/README.md index 50aaddf7..6338adad 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ The Rails 2.3 code is on the [`rails2`](https://github.com/airblade/paper_trail/ ### Sinatra In order to configure `PaperTrail` for usage with [Sinatra](http://www.sinatrarb.com), -your `Sinatra` app must be using `ActiveRecord` 3 or `ActiveRecord` 4. It is also recommended to use the +your `Sinatra` app must be using `ActiveRecord` 3 or `ActiveRecord` 4. It is also recommended to use the [Sinatra ActiveRecord Extension](https://github.com/janko-m/sinatra-activerecord) or something similar for managing your applications `ActiveRecord` connection in a manner similar to the way `Rails` does. If using the aforementioned `Sinatra ActiveRecord Extension`, steps for setting up your app with `PaperTrail` will look something like this: @@ -480,15 +480,28 @@ In a console session you can manually set who is responsible like this: You can avoid having to do this manually by setting your initializer to pick up the username of the current user from the OS, like this: ```ruby -class PaperTrail::Version < ActiveRecord::Base - if defined?(Rails::Console) - PaperTrail.whodunnit = "#{`whoami`.strip}: console" - elsif File.basename($0) == "rake" - PaperTrail.whodunnit = "#{`whoami`.strip}: rake #{ARGV.join ' '}" - end +# config/initializers/paper_trail.rb +if defined?(::Rails::Console) + PaperTrail.whodunnit = "#{`whoami`.strip}: console" +elsif File.basename($0) == "rake" + PaperTrail.whodunnit = "#{`whoami`.strip}: rake #{ARGV.join ' '}" end ``` +Sometimes you want to define who is responsible for a change in a small scope without overwriting value of `PaperTrail.whodunnit`. It is possible to define the `whodunnit` value for an operation inside a block like this: + +```ruby +>> PaperTrail.whodunnit = 'Andy Stewart' +>> widget.whodunnit('Lucas Souza') do +>> widget.update_attributes :name => 'Wibble' +>> end +>> widget.versions.last.whodunnit # Lucas Souza +>> widget.update_attributes :name => 'Clair' +>> widget.versions.last.whodunnit # Andy Stewart +>> widget.whodunnit('Ben Atkins') { |w| w.update_attributes :name => 'Beth' } # this syntax also works +>> widget.versions.last.whodunnit # Ben Atkins +``` + A version's `whodunnit` records who changed the object causing the `version` to be stored. Because a version stores the object as it looked before the change (see the table above), `whodunnit` returns who stopped the object looking like this -- not who made it look like this. Hence `whodunnit` is aliased as `terminator`. To find out who made a version's object look that way, use `version.originator`. And to find out who made a "live" object look like it does, use `originator` on the object. @@ -540,7 +553,7 @@ Alternatively you could store certain metadata for one type of version, and othe If you only use custom version classes and don't use PaperTrail's built-in one, on Rails `>= 3.2` you must: -- either declare PaperTrail's version class abstract like this (in `config/initializers/paper_trail_patch.rb`): +- either declare the `PaperTrail::Version` class to be abstract like this (in an initializer): ```ruby PaperTrail::Version.module_eval do @@ -580,8 +593,14 @@ If you can think of a good way to achieve this, please let me know. PaperTrail can restore `:has_one` associations as they were at (actually, 3 seconds before) the time. ```ruby +class Location < ActiveRecord::Base + belongs_to :treasure + has_paper_trail +end + class Treasure < ActiveRecord::Base has_one :location + has_paper_trail end >> treasure.amount # 100 @@ -696,7 +715,7 @@ PaperTrail will call your proc with the current article and store the result in N.B. You must also: * Add your metadata columns to the `versions` table. -* Declare your metadata columns using `attr_accessible`. (If you are using `Rails 3`, or `Rails 4` with the [ProtectedAttributes](https://github.com/rails/protected_attributes) gem) +* Declare your metadata columns using `attr_accessible`. (If you are using `ActiveRecord 3`, or `ActiveRecord 4` with the [ProtectedAttributes](https://github.com/rails/protected_attributes) gem) For example: @@ -1062,6 +1081,7 @@ Many thanks to: * [Vlad Bokov](https://github.com/razum2um) * [Sean Marcia](https://github.com/SeanMarcia) * [Chulki Lee](https://github.com/chulkilee) +* [Lucas Souza](https://github.com/lucasas) ## Inspirations diff --git a/gemfiles/3.0.gemfile b/gemfiles/3.0.gemfile index 6f2f0811..4f33a833 100644 --- a/gemfiles/3.0.gemfile +++ b/gemfiles/3.0.gemfile @@ -19,6 +19,7 @@ group :development, :test do # RSpec testing gem 'rspec-rails', '~> 2.14' + gem 'generator_spec' platforms :jruby, :ruby_18 do # shoulda-matchers > 2.0 is not compatible with Ruby18. diff --git a/lib/generators/paper_trail/install_generator.rb b/lib/generators/paper_trail/install_generator.rb index 67cde954..7ab5da02 100644 --- a/lib/generators/paper_trail/install_generator.rb +++ b/lib/generators/paper_trail/install_generator.rb @@ -1,5 +1,4 @@ require 'rails/generators' -require 'rails/generators/migration' require 'rails/generators/active_record' module PaperTrail @@ -13,7 +12,7 @@ module PaperTrail def create_migration_file add_paper_trail_migration('create_versions') - add_paper_trail_migration('add_object_changes_column_to_versions') if options.with_changes? + add_paper_trail_migration('add_object_changes_to_versions') if options.with_changes? add_paper_trail_migration('create_version_associations') add_paper_trail_migration('add_transaction_id_column_to_versions') end diff --git a/lib/generators/paper_trail/templates/add_object_changes_column_to_versions.rb b/lib/generators/paper_trail/templates/add_object_changes_column_to_versions.rb deleted file mode 100644 index f4674c48..00000000 --- a/lib/generators/paper_trail/templates/add_object_changes_column_to_versions.rb +++ /dev/null @@ -1,9 +0,0 @@ -class AddObjectChangesColumnToVersions < ActiveRecord::Migration - def self.up - add_column :versions, :object_changes, :text - end - - def self.down - remove_column :versions, :object_changes - end -end diff --git a/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb b/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb new file mode 100644 index 00000000..2d723e06 --- /dev/null +++ b/lib/generators/paper_trail/templates/add_object_changes_to_versions.rb @@ -0,0 +1,5 @@ +class AddObjectChangesToVersions < ActiveRecord::Migration + def change + add_column :versions, :object_changes, :text + end +end diff --git a/lib/generators/paper_trail/templates/create_versions.rb b/lib/generators/paper_trail/templates/create_versions.rb index 701f7ea7..23be970c 100644 --- a/lib/generators/paper_trail/templates/create_versions.rb +++ b/lib/generators/paper_trail/templates/create_versions.rb @@ -1,5 +1,5 @@ class CreateVersions < ActiveRecord::Migration - def self.up + def change create_table :versions do |t| t.string :item_type, :null => false t.integer :item_id, :null => false @@ -10,9 +10,4 @@ class CreateVersions < ActiveRecord::Migration end add_index :versions, [:item_type, :item_id] end - - def self.down - remove_index :versions, [:item_type, :item_id] - drop_table :versions - end end diff --git a/lib/paper_trail.rb b/lib/paper_trail.rb index 49be73b3..50d619f6 100644 --- a/lib/paper_trail.rb +++ b/lib/paper_trail.rb @@ -34,12 +34,12 @@ module PaperTrail # Sets whether PaperTrail is enabled or disabled for this model in the current request. def self.enabled_for_model(model, value) - paper_trail_store[:"request_enabled_for_#{model}"] = value + paper_trail_store[:"enabled_for_#{model}"] = value end # Returns `true` if PaperTrail is enabled for this model in the current request, `false` otherwise. def self.enabled_for_model?(model) - !!paper_trail_store.fetch(:"request_enabled_for_#{model}", true) + !!paper_trail_store.fetch(:"enabled_for_#{model}", true) end # Set the field which records when a version was created. diff --git a/lib/paper_trail/frameworks/rspec.rb b/lib/paper_trail/frameworks/rspec.rb index 174a9992..564208c6 100644 --- a/lib/paper_trail/frameworks/rspec.rb +++ b/lib/paper_trail/frameworks/rspec.rb @@ -1,9 +1,10 @@ require 'rspec/core' require 'rspec/matchers' -require File.expand_path('../rspec/extensions', __FILE__) +require 'paper_trail/frameworks/rspec/helpers' RSpec.configure do |config| - config.include ::PaperTrail::RSpec::Extensions + config.include ::PaperTrail::RSpec::Helpers::InstanceMethods + config.extend ::PaperTrail::RSpec::Helpers::ClassMethods config.before(:each) do ::PaperTrail.enabled = false diff --git a/lib/paper_trail/frameworks/rspec/extensions.rb b/lib/paper_trail/frameworks/rspec/extensions.rb deleted file mode 100644 index c2149918..00000000 --- a/lib/paper_trail/frameworks/rspec/extensions.rb +++ /dev/null @@ -1,20 +0,0 @@ -module PaperTrail - module RSpec - module Extensions - # :call-seq: - # with_versioning - # - # enable versioning for specific blocks - - def with_versioning - was_enabled = ::PaperTrail.enabled? - ::PaperTrail.enabled = true - begin - yield - ensure - ::PaperTrail.enabled = was_enabled - end - end - end - end -end diff --git a/lib/paper_trail/frameworks/rspec/helpers.rb b/lib/paper_trail/frameworks/rspec/helpers.rb new file mode 100644 index 00000000..63eab98f --- /dev/null +++ b/lib/paper_trail/frameworks/rspec/helpers.rb @@ -0,0 +1,25 @@ +module PaperTrail + module RSpec + module Helpers + module InstanceMethods + # enable versioning for specific blocks (at instance-level) + def with_versioning + was_enabled = ::PaperTrail.enabled? + ::PaperTrail.enabled = true + yield + ensure + ::PaperTrail.enabled = was_enabled + end + end + + module ClassMethods + # enable versioning for specific blocks (at class-level) + def with_versioning(&block) + context 'with versioning', :versioning => true do + class_exec(&block) + end + end + end + end + end +end diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index c2607974..44767ae4 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -215,7 +215,7 @@ module PaperTrail def without_versioning(method = nil) paper_trail_was_enabled = self.paper_trail_enabled_for_model? self.class.paper_trail_off! - method ? method.to_proc.call(self) : yield + method ? method.to_proc.call(self) : yield(self) ensure self.class.paper_trail_on! if paper_trail_was_enabled end @@ -230,6 +230,16 @@ module PaperTrail instance_eval { alias :new_record? :old_new_record? } end + # Temporarily overwrites the value of whodunnit and then executes the provided block. + def whodunnit(value) + raise ArgumentError, 'expected to receive a block' unless block_given? + current_whodunnit = PaperTrail.whodunnit + PaperTrail.whodunnit = value + yield self + ensure + PaperTrail.whodunnit = current_whodunnit + end + # Mimicks behavior of `touch` method from `ActiveRecord::Persistence`, but generates a version # # TODO: lookinto leveraging the `after_touch` callback from `ActiveRecord` to allow the diff --git a/paper_trail.gemspec b/paper_trail.gemspec index 9025243c..6418db03 100644 --- a/paper_trail.gemspec +++ b/paper_trail.gemspec @@ -30,6 +30,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'sinatra', '~> 1.0' s.add_development_dependency 'rack-test', '>= 0.6' s.add_development_dependency 'rspec-rails', '~> 2.14' + s.add_development_dependency 'generator_spec' # JRuby support for the test ENV unless defined?(JRUBY_VERSION) diff --git a/spec/generators/install_generator_spec.rb b/spec/generators/install_generator_spec.rb new file mode 100644 index 00000000..298217ed --- /dev/null +++ b/spec/generators/install_generator_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' +require 'generator_spec/test_case' +require File.expand_path('../../../lib/generators/paper_trail/install_generator', __FILE__) + +describe PaperTrail::InstallGenerator, :type => :generator do + include GeneratorSpec::TestCase + destination File.expand_path('../tmp', __FILE__) + + after(:all) { prepare_destination } # cleanup the tmp directory + + describe "no options" do + before(:all) do + prepare_destination + run_generator + end + + it "generates a migration for creating the 'versions' table" do + destination_root.should have_structure { + directory 'db' do + directory 'migrate' do + migration 'create_versions' do + contains 'class CreateVersions' + contains 'def change' + contains 'create_table :versions do |t|' + end + end + end + } + end + end + + describe "`--with-changes` option set to `true`" do + before(:all) do + prepare_destination + run_generator %w(--with-changes) + end + + it "generates a migration for creating the 'versions' table" do + destination_root.should have_structure { + directory 'db' do + directory 'migrate' do + migration 'create_versions' do + contains 'class CreateVersions' + contains 'def change' + contains 'create_table :versions do |t|' + end + end + end + } + end + + it "generates a migration for adding the 'object_changes' column to the 'versions' table" do + destination_root.should have_structure { + directory 'db' do + directory 'migrate' do + migration 'add_object_changes_to_versions' do + contains 'class AddObjectChangesToVersions' + contains 'def change' + contains 'add_column :versions, :object_changes, :text' + end + end + end + } + end + end + +end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 0d951ba7..4184724b 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -23,6 +23,46 @@ describe Widget do describe "Methods" do describe "Instance", :versioning => true do + describe :whodunnit do + it { should respond_to(:whodunnit) } + + context "no block given" do + it "should raise an error" do + expect { widget.whodunnit('Ben') }.to raise_error(ArgumentError, 'expected to receive a block') + end + end + + context "block given" do + let(:orig_name) { Faker::Name.name } + let(:new_name) { Faker::Name.name } + + before do + PaperTrail.whodunnit = orig_name + widget.versions.last.whodunnit.should == orig_name # persist `widget` + end + + it "should modify value of `PaperTrail.whodunnit` while executing the block" do + widget.whodunnit(new_name) do + PaperTrail.whodunnit.should == new_name + widget.update_attributes(:name => 'Elizabeth') + end + widget.versions.last.whodunnit.should == new_name + end + + it "should revert the value of `PaperTrail.whodunnit` to it's previous value after executing the block" do + widget.whodunnit(new_name) { |w| w.update_attributes(:name => 'Elizabeth') } + PaperTrail.whodunnit.should == orig_name + end + + context "error within block" do + it "should ensure that the whodunnit value still reverts to it's previous value" do + expect { widget.whodunnit(new_name) { raise } }.to raise_error + PaperTrail.whodunnit.should == orig_name + end + end + end + end + describe :touch_with_version do it { should respond_to(:touch_with_version) } diff --git a/spec/paper_trail_spec.rb b/spec/paper_trail_spec.rb index e238fddb..97f34d84 100644 --- a/spec/paper_trail_spec.rb +++ b/spec/paper_trail_spec.rb @@ -5,13 +5,21 @@ describe "PaperTrail RSpec Helper" do it 'should have versioning off by default' do ::PaperTrail.should_not be_enabled end - it 'should turn versioning on in a with_versioning block' do + it 'should turn versioning on in a `with_versioning` block' do ::PaperTrail.should_not be_enabled with_versioning do ::PaperTrail.should be_enabled end ::PaperTrail.should_not be_enabled end + + context "error within `with_versioning` block" do + it "should revert the value of `PaperTrail.enabled?` to it's previous state" do + ::PaperTrail.should_not be_enabled + expect { with_versioning { raise } }.to raise_error + ::PaperTrail.should_not be_enabled + end + end end context '`versioning: true`', :versioning => true do @@ -27,6 +35,19 @@ describe "PaperTrail RSpec Helper" do end end + context '`with_versioning` block at class level' do + it { ::PaperTrail.should_not be_enabled } + + with_versioning do + it 'should have versioning on by default' do + ::PaperTrail.should be_enabled + end + end + it 'should not leak the `enabled?` state into successive tests' do + ::PaperTrail.should_not be_enabled + end + end + describe :whodunnit do before(:all) { ::PaperTrail.whodunnit = 'foobar' } diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index 27983f47..81a97ec0 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -515,10 +515,24 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase @widget.without_versioning do @widget.update_attributes :name => 'Ford' end + # The model instance should yield itself for convenience purposes + @widget.without_versioning { |w| w.update_attributes :name => 'Nixon' } end should 'not create new version' do - assert_equal 1, @widget.versions.length + assert_equal @count, @widget.versions.length + end + + should 'enable paper trail after call' do + assert Widget.paper_trail_enabled_for_model? + end + end + + context 'when receiving a method name as an argument' do + setup { @widget.without_versioning(:touch_with_version) } + + should 'not create new version' do + assert_equal @count, @widget.versions.length end should 'enable paper trail after call' do @@ -771,7 +785,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'store dynamic meta data based on a method of the item' do assert_equal @article.action_data_provider_method, @article.versions.last.action end - + should 'store dynamic meta data based on an attribute of the item prior to creation' do assert_equal nil, @article.versions.last.title end @@ -793,7 +807,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'store dynamic meta data which depends on the item' do assert_equal @article.id, @article.versions.last.article_id end - + should 'store dynamic meta data based on an attribute of the item prior to the update' do assert_equal @initial_title, @article.versions.last.title end @@ -814,7 +828,7 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'store dynamic meta data which depends on the item' do assert_equal @article.id, @article.versions.last.article_id end - + should 'store dynamic meta data based on an attribute of the item prior to the destruction' do assert_equal @initial_title, @article.versions.last.title end @@ -837,7 +851,6 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase should 'return its previous self' do assert_equal @widget.versions[-2].reify, @widget.previous_version end - end