From 7dafd009716bfcef760e26bfa45f6274a2caf0bb Mon Sep 17 00:00:00 2001 From: Weston Ganger Date: Tue, 13 Mar 2018 16:18:30 -0700 Subject: [PATCH 1/3] integrate versioning into AR touch method --- lib/paper_trail/model_config.rb | 10 ++++++++++ spec/models/widget_spec.rb | 8 ++++++++ spec/paper_trail/model_spec.rb | 6 +++--- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index f84b9559a..f5453db2c 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -116,6 +116,14 @@ def on_update @model_class.paper_trail_options[:on] << :update end + # Adds a callback that records a version after a "touch" event. + # @api public + def on_touch + @model_class.after_touch { |r| + r.paper_trail.record_update(force: true, in_after_callback: true) + } + end + # Set up `@model_class` for PaperTrail. Installs callbacks, associations, # "class attributes", instance methods, and more. # @api private @@ -196,6 +204,8 @@ def setup_callbacks_from_options(options_on = []) options_on.each do |event| public_send("on_#{event}") end + + public_send("on_touch") end def setup_habtm_change_callbacks(assoc) diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 753e0403e..64f2bff22 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -250,6 +250,14 @@ end end + describe "touch", versioning: true do + it "creates a version" do + expect { widget.touch }.to change { + widget.versions.count + }.by(+1) + end + end + describe ".paper_trail.update_columns", versioning: true do it "creates a version record" do widget = Widget.create diff --git a/spec/paper_trail/model_spec.rb b/spec/paper_trail/model_spec.rb index ee2bd9ba0..a9a1b9a67 100644 --- a/spec/paper_trail/model_spec.rb +++ b/spec/paper_trail/model_spec.rb @@ -56,15 +56,15 @@ context "and then updated without any changes" do before { @widget.touch } - it "not have a new version" do - expect(@widget.versions.length).to(eq(1)) + it "to have two previous versions" do + expect(@widget.versions.length).to(eq(2)) end end context "and then updated with changes" do before { @widget.update_attributes(name: "Harry") } - it "have two previous versions" do + it "have three previous versions" do expect(@widget.versions.length).to(eq(2)) end From 1510095f4dff3d2fc099fd72b06b4ae09629e2a8 Mon Sep 17 00:00:00 2001 From: Weston Ganger Date: Wed, 14 Mar 2018 13:50:04 -0700 Subject: [PATCH 2/3] add touch to list of :on events, deprecate touch_with_version --- CHANGELOG.md | 2 ++ README.md | 21 ++++++++++++++++----- lib/paper_trail/model_config.rb | 4 +--- lib/paper_trail/record_trail.rb | 16 +++++++++++----- spec/models/article_spec.rb | 3 +++ spec/models/callback_modifier_spec.rb | 2 +- spec/models/not_on_update_spec.rb | 4 ++++ spec/models/on/empty_array_spec.rb | 2 ++ spec/models/on/update_spec.rb | 11 +++++++++-- spec/models/post_with_status_spec.rb | 2 ++ spec/models/widget_spec.rb | 12 +++++++++++- spec/paper_trail/model_spec.rb | 2 ++ 12 files changed, 64 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f552b85..cf756a279 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,12 +25,14 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Deprecated +- [#1063](https://github.com/airblade/paper_trail/pull/1063) - `touch_with_version` is deprecated in favor of using the original AR `touch` method - [#1033](https://github.com/airblade/paper_trail/pull/1033) - Request variables are now set using eg. `PaperTrail.request.whodunnit=` and the old way, `PaperTrail.whodunnit=` is deprecated. ### Added +- [#1063](https://github.com/airblade/paper_trail/pull/1063) - AR `touch` method now creates a version if the `:on` option includes `touch`. By default it also includes the `touch` event now. - [#1033](https://github.com/airblade/paper_trail/pull/1033) - Set request variables temporarily using a block, eg. `PaperTrail.request(whodunnit: 'Jared') do .. end` diff --git a/README.md b/README.md index cfe5d8c52..50fc614b5 100644 --- a/README.md +++ b/README.md @@ -207,10 +207,6 @@ widget.paper_trail.previous_version # Returns the widget (not a version) as it became next. widget.paper_trail.next_version -# Generates a version for a `touch` event (`widget.touch` does NOT generate a -# version) -widget.paper_trail.touch_with_version - # Enable/disable PaperTrail, for Widget, for the current request (not all threads) PaperTrail.request.disable_model(Widget) PaperTrail.request.enable_model(Widget) @@ -292,7 +288,7 @@ ignore `create` events: ```ruby class Article < ActiveRecord::Base - has_paper_trail on: [:update, :destroy] + has_paper_trail on: [:update, :destroy, :touch] end ``` @@ -337,6 +333,7 @@ class Article < ActiveRecord::Base paper_trail.on_destroy # add destroy callback paper_trail.on_update # etc. paper_trail.on_create + paper_trail.on_touch end ``` @@ -706,6 +703,20 @@ sql> delete from versions where created_at < 2010-06-01; PaperTrail::Version.delete_all ['created_at < ?', 1.week.ago] ``` +### 3.e. Trigger Version creation + +At some point you may want to trigger a new version to be created. To do this we utilize the AR `touch` method. + +```ruby +widget.touch +``` + +Note if you are using the `:on` option in your model you must specify the `touch` event also. For example: + +```ruby +has_paper_trail on: [:update, :destroy, :touch] +``` + ## 4. Saving More Information About Versions ### 4.a. Finding Out Who Was Responsible For A Change diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index f5453db2c..c6cd16c8d 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -128,7 +128,7 @@ def on_touch # "class attributes", instance methods, and more. # @api private def setup(options = {}) - options[:on] ||= %i[create update destroy] + options[:on] ||= %i[create update destroy touch] options[:on] = Array(options[:on]) # Support single symbol @model_class.send :include, ::PaperTrail::Model::InstanceMethods setup_options(options) @@ -204,8 +204,6 @@ def setup_callbacks_from_options(options_on = []) options_on.each do |event| public_send("on_#{event}") end - - public_send("on_touch") end def setup_habtm_change_callbacks(assoc) diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index e0b9016d5..77bf41f00 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -7,6 +7,12 @@ class RecordTrail my_model_instance.paper_trail.whodunnit('John') is deprecated, please use PaperTrail.request(whodunnit: 'John') STR + + DPR_TOUCH_WITH_VERSION = <<-STR.squish.freeze + my_model_instance.paper_trail.touch_with_version is deprecated, + please use my_model_instance.touch + STR + RAILS_GTE_5_1 = ::ActiveRecord.gem_version >= ::Gem::Version.new("5.1.0.beta1") def initialize(record) @@ -461,8 +467,9 @@ def source_version # version records are inserted. It's unclear under which specific # circumstances this technique should be adopted. # - # @api public + # @deprecated def touch_with_version(name = nil) + ::ActiveSupport::Deprecation.warn(DPR_TOUCH_WITH_VERSION, caller(1)) unless @record.persisted? raise ::ActiveRecord::ActiveRecordError, "can not touch on a new record object" end @@ -563,7 +570,7 @@ def attribute_changed_in_latest_version?(attr_name) # Rails 5.1 changed the API of `ActiveRecord::Dirty`. See # https://github.com/airblade/paper_trail/pull/899 # - # Event can be any of the three (create, update, destroy). + # Event can be any of the following (create, update, destroy, touch). # # @api private def attribute_in_previous_version(attr_name) @@ -571,9 +578,8 @@ def attribute_in_previous_version(attr_name) if @in_after_callback @record.attribute_before_last_save(attr_name.to_s) else - # Either we are doing a `touch_with_version` or `record_destroy`. - # Other events, like `record_create`, can only be done in an - # after-callback. + # We are performing a `record_destroy`. Other events, + # like `record_create`, can only be done in an after-callback. @record.attribute_in_database(attr_name.to_s) end else diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 3bc67fb57..73e412fbd 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -189,9 +189,12 @@ describe "#touch_with_version" do it "creates a version, ignoring the :only option" do article = described_class.create + + allow(::ActiveSupport::Deprecation).to receive(:warn) expect { article.paper_trail.touch_with_version }.to change { ::PaperTrail::Version.count }.by(+1) + expect(::ActiveSupport::Deprecation).to have_received(:warn).once end end end diff --git a/spec/models/callback_modifier_spec.rb b/spec/models/callback_modifier_spec.rb index f7683e33a..e4344e181 100644 --- a/spec/models/callback_modifier_spec.rb +++ b/spec/models/callback_modifier_spec.rb @@ -67,7 +67,7 @@ context "when no callback-method used" do it "sets paper_trail_options[:on] to [:create, :update, :destroy]" do modifier = DefaultModifier.create!(some_content: FFaker::Lorem.sentence) - expect(modifier.paper_trail_options[:on]).to eq %i[create update destroy] + expect(modifier.paper_trail_options[:on]).to eq %i[create update destroy touch] end it "tracks destroy" do diff --git a/spec/models/not_on_update_spec.rb b/spec/models/not_on_update_spec.rb index 23c62422c..ebf64b5ce 100644 --- a/spec/models/not_on_update_spec.rb +++ b/spec/models/not_on_update_spec.rb @@ -7,17 +7,21 @@ let!(:record) { described_class.create! } it "creates a version, regardless" do + allow(::ActiveSupport::Deprecation).to receive(:warn) expect { record.paper_trail.touch_with_version }.to change { PaperTrail::Version.count }.by(+1) + expect(::ActiveSupport::Deprecation).to have_received(:warn).once end it "increments the `:updated_at` timestamp" do before = record.updated_at + allow(::ActiveSupport::Deprecation).to receive(:warn) # Travel 1 second because MySQL lacks sub-second resolution Timecop.travel(1) do record.paper_trail.touch_with_version end + expect(::ActiveSupport::Deprecation).to have_received(:warn).once expect(record.updated_at).to be > before end end diff --git a/spec/models/on/empty_array_spec.rb b/spec/models/on/empty_array_spec.rb index acf61f6fe..ac772fe05 100644 --- a/spec/models/on/empty_array_spec.rb +++ b/spec/models/on/empty_array_spec.rb @@ -15,7 +15,9 @@ module On describe "#touch_with_version" do it "creates a version record" do record = described_class.create(name: "Alice") + allow(::ActiveSupport::Deprecation).to receive(:warn) record.paper_trail.touch_with_version + expect(::ActiveSupport::Deprecation).to have_received(:warn).once expect(record.versions.length).to(eq(1)) v = record.versions.first expect(v.event).to(eq("update")) diff --git a/spec/models/on/update_spec.rb b/spec/models/on/update_spec.rb index fb3fda99a..0fc4c0962 100644 --- a/spec/models/on/update_spec.rb +++ b/spec/models/on/update_spec.rb @@ -5,9 +5,10 @@ module On RSpec.describe Update, type: :model, versioning: true do + let(:record) { described_class.create(name: "Alice") } + describe "#versions" do it "only creates one version record, for the update event" do - record = described_class.create(name: "Alice") record.update_attributes(name: "blah") record.destroy expect(record.versions.length).to(eq(1)) @@ -17,7 +18,6 @@ module On context "#paper_trail_event" do it "rembembers the custom event name" do - record = described_class.create(name: "Alice") record.paper_trail_event = "banana" record.update_attributes(name: "blah") record.destroy @@ -25,5 +25,12 @@ module On expect(record.versions.last.event).to(eq("banana")) end end + + describe "#touch" do + it "does not create a version for the touch event" do + count = record.versions.count + expect(count).to eq(count) + end + end end end diff --git a/spec/models/post_with_status_spec.rb b/spec/models/post_with_status_spec.rb index fc4433591..df0477361 100644 --- a/spec/models/post_with_status_spec.rb +++ b/spec/models/post_with_status_spec.rb @@ -37,7 +37,9 @@ expect(post.versions.count).to eq(1) expect(post.status).to eq("draft") Timecop.travel 1.second.since # because MySQL lacks fractional seconds precision + allow(::ActiveSupport::Deprecation).to receive(:warn) post.paper_trail.touch_with_version + expect(::ActiveSupport::Deprecation).to have_received(:warn).once expect(post.versions.count).to eq(2) expect(post.versions.last[:object]).to include("status: 0") expect(post.paper_trail.previous_version.status).to eq("draft") diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 64f2bff22..83cdd7fda 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -232,21 +232,24 @@ describe "#touch_with_version", versioning: true do it "creates a version" do + allow(::ActiveSupport::Deprecation).to receive(:warn) count = widget.versions.size - # Travel 1 second because MySQL lacks sub-second resolution Timecop.travel(1) do widget.paper_trail.touch_with_version end expect(widget.versions.size).to eq(count + 1) + expect(::ActiveSupport::Deprecation).to have_received(:warn).once end it "increments the `:updated_at` timestamp" do + allow(::ActiveSupport::Deprecation).to receive(:warn) time_was = widget.updated_at # Travel 1 second because MySQL lacks sub-second resolution Timecop.travel(1) do widget.paper_trail.touch_with_version end expect(widget.updated_at).to be > time_was + expect(::ActiveSupport::Deprecation).to have_received(:warn).once end end @@ -256,6 +259,13 @@ widget.versions.count }.by(+1) end + + it "does not create a version using without_versioning" do + count = widget.versions.count + widget.paper_trail.without_versioning do + expect(count).to eq(count) + end + end end describe ".paper_trail.update_columns", versioning: true do diff --git a/spec/paper_trail/model_spec.rb b/spec/paper_trail/model_spec.rb index a9a1b9a67..68aca8fd6 100644 --- a/spec/paper_trail/model_spec.rb +++ b/spec/paper_trail/model_spec.rb @@ -381,7 +381,9 @@ context "given a symbol, specifying a method name" do it "does not create a new version" do + allow(::ActiveSupport::Deprecation).to receive(:warn) @widget.paper_trail.without_versioning(:touch_with_version) + expect(::ActiveSupport::Deprecation).to have_received(:warn).once expect(@widget.versions.length).to(eq(@count)) expect(::PaperTrail.request.enabled_for_model?(Widget)).to eq(true) end From 5c8cdcdcea8174930c32e6b8d1218e01cab53e42 Mon Sep 17 00:00:00 2001 From: Weston Ganger Date: Wed, 14 Mar 2018 16:27:10 -0700 Subject: [PATCH 3/3] integrate versioning into AR touch method --- README.md | 4 +++- lib/paper_trail/record_trail.rb | 2 +- spec/models/on/update_spec.rb | 5 +++-- spec/models/widget_spec.rb | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 50fc614b5..fa15e8d11 100644 --- a/README.md +++ b/README.md @@ -711,7 +711,9 @@ At some point you may want to trigger a new version to be created. To do this we widget.touch ``` -Note if you are using the `:on` option in your model you must specify the `touch` event also. For example: +The new versions event will be saved as `update`. + +If you are using the `:on` option in your model you must specify the `touch` event also. For example: ```ruby has_paper_trail on: [:update, :destroy, :touch] diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index 77bf41f00..d38969d7a 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -570,7 +570,7 @@ def attribute_changed_in_latest_version?(attr_name) # Rails 5.1 changed the API of `ActiveRecord::Dirty`. See # https://github.com/airblade/paper_trail/pull/899 # - # Event can be any of the following (create, update, destroy, touch). + # Event can be any of the three (create, update, destroy). # # @api private def attribute_in_previous_version(attr_name) diff --git a/spec/models/on/update_spec.rb b/spec/models/on/update_spec.rb index 0fc4c0962..d97f8165d 100644 --- a/spec/models/on/update_spec.rb +++ b/spec/models/on/update_spec.rb @@ -5,10 +5,9 @@ module On RSpec.describe Update, type: :model, versioning: true do - let(:record) { described_class.create(name: "Alice") } - describe "#versions" do it "only creates one version record, for the update event" do + record = described_class.create(name: "Alice") record.update_attributes(name: "blah") record.destroy expect(record.versions.length).to(eq(1)) @@ -18,6 +17,7 @@ module On context "#paper_trail_event" do it "rembembers the custom event name" do + record = described_class.create(name: "Alice") record.paper_trail_event = "banana" record.update_attributes(name: "blah") record.destroy @@ -28,6 +28,7 @@ module On describe "#touch" do it "does not create a version for the touch event" do + record = described_class.create(name: "Alice") count = record.versions.count expect(count).to eq(count) end diff --git a/spec/models/widget_spec.rb b/spec/models/widget_spec.rb index 83cdd7fda..d2514f85d 100644 --- a/spec/models/widget_spec.rb +++ b/spec/models/widget_spec.rb @@ -263,8 +263,9 @@ it "does not create a version using without_versioning" do count = widget.versions.count widget.paper_trail.without_versioning do - expect(count).to eq(count) + widget.touch end + expect(count).to eq(count) end end