diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index 6c80a18c..a74b1a90 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -266,8 +266,9 @@ module PaperTrail PaperTrail.whodunnit = current_whodunnit end - # Mimicks behavior of `touch` method from `ActiveRecord::Persistence`, - # but generates a version + # Mimics the `touch` method from `ActiveRecord::Persistence`, but also + # creates a version. A version is created regardless of options such as + # `:on`, `:if`, or `:unless`. # # TODO: look into leveraging the `after_touch` callback from # `ActiveRecord` to allow the regular `touch` method go generate a version @@ -281,13 +282,20 @@ module PaperTrail current_time = current_time_from_proper_timezone attributes.each { |column| write_attribute(column, current_time) } - # ensure a version is written even if the `:on` collection is empty - record_update(true) if paper_trail_options[:on] == [] + + record_update(true) unless will_record_after_update? save!(:validate => false) end private + # Returns true if `save` will cause `record_update` + # to be called via the `after_update` callback. + def will_record_after_update? + on = paper_trail_options[:on] + on.nil? || on.include?(:update) + end + def source_version send self.class.version_association_name end diff --git a/spec/models/not_on_update_spec.rb b/spec/models/not_on_update_spec.rb new file mode 100644 index 00000000..7b57c0de --- /dev/null +++ b/spec/models/not_on_update_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +describe NotOnUpdate, :type => :model do + describe "#touch_with_version", :versioning => true do + let!(:record) { described_class.create! } + + it "should create a version, regardless" do + expect { record.touch_with_version }.to change { + PaperTrail::Version.count + }.by(+1) + end + + it "increments the `:updated_at` timestamp" do + before = record.updated_at + record.touch_with_version + expect(record.updated_at).to be > before + end + end +end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index ddbbd93a..c0226301 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -252,13 +252,13 @@ describe Widget, :type => :model do describe '#touch_with_version' do it { is_expected.to respond_to(:touch_with_version) } - it "should generate a version" do + it "creates a version" do count = widget.versions.size widget.touch_with_version expect(widget.versions.size).to eq(count + 1) end - it "should increment the `:updated_at` timestamp" do + it "increments the `:updated_at` timestamp" do time_was = widget.updated_at widget.touch_with_version expect(widget.updated_at).to be > time_was diff --git a/test/dummy/app/models/not_on_update.rb b/test/dummy/app/models/not_on_update.rb new file mode 100644 index 00000000..1ceff816 --- /dev/null +++ b/test/dummy/app/models/not_on_update.rb @@ -0,0 +1,4 @@ +# This model does not record versions when updated. +class NotOnUpdate < ActiveRecord::Base + has_paper_trail :on => [:create, :destroy] +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index afb9b750..f4b09805 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -73,6 +73,10 @@ class SetUpTestTables < ActiveRecord::Migration add_index :json_versions, [:item_type, :item_id] end + create_table :not_on_updates, :force => true do |t| + t.timestamps :null => true + end + create_table :bananas, :force => true do |t| t.timestamps :null => true end @@ -205,6 +209,7 @@ class SetUpTestTables < ActiveRecord::Migration def self.down drop_table :animals + drop_table :not_on_updates drop_table :posts drop_table :songs drop_table :editors