-
Notifications
You must be signed in to change notification settings - Fork 896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable control over the order of AR callbacks #614
Changes from all commits
9008d67
297f5fc
baca9b0
02b1c09
d63eb4e
0f40d74
8fe6ada
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -330,6 +330,27 @@ a.versions.size # 3 | |
a.versions.last.event # 'update' | ||
``` | ||
|
||
### Controlling the Order of AR Callbacks | ||
|
||
You can also use the corresponding callback-methods seperately instead of using | ||
the :on option. If you choose to use the callback-methods, PaperTrail will only | ||
track the according events - so `paper_trail_on_create` is basically the same as | ||
`has_paper_trail :on => :create`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd put a heading like "Controlling the Order of AR Callbacks" above this paragraph, as (for me) that's the point of this new feature. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. Since I am not an english native-speaker, every suggestion to my readme addition is much appreciated. |
||
|
||
```ruby | ||
class Article < ActiveRecord::Base | ||
has_paper_trail :on => [] | ||
paper_trail_on_destroy | ||
paper_trail_on_update | ||
paper_trail_on_create | ||
end | ||
``` | ||
|
||
The `paper_trail_on_destroy` method can be configured to be called `:before` or `:after` the | ||
destroy event. This can be usefull if you are using a third party tool that alters the | ||
destroy method (for example paranoia). If you do not pass an argument, it will default | ||
to after_destroy. | ||
|
||
## Choosing When To Save New Versions | ||
|
||
You can choose the conditions when to add new versions with the `if` and | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,18 @@ module ClassMethods | |
# column if it exists. Default is true | ||
# | ||
def has_paper_trail(options = {}) | ||
options[:on] ||= [:create, :update, :destroy] | ||
|
||
# Wrap the :on option in an array if necessary. This allows a single | ||
# symbol to be passed in. | ||
options[:on] = Array(options[:on]) | ||
|
||
setup_model_for_paper_trail(options) | ||
|
||
setup_callbacks_from_options options[:on] | ||
end | ||
|
||
def setup_model_for_paper_trail(options = {}) | ||
# Lazily include the instance methods so we don't clutter up | ||
# any more ActiveRecord models than we have to. | ||
send :include, InstanceMethods | ||
|
@@ -60,6 +72,7 @@ def has_paper_trail(options = {}) | |
self.version_class_name = options[:class_name] || 'PaperTrail::Version' | ||
|
||
class_attribute :paper_trail_options | ||
|
||
self.paper_trail_options = options.dup | ||
|
||
[:ignore, :skip, :only].each do |k| | ||
|
@@ -87,26 +100,53 @@ def has_paper_trail(options = {}) | |
:order => self.paper_trail_version_class.timestamp_sort_order | ||
end | ||
|
||
options[:on] ||= [:create, :update, :destroy] | ||
|
||
# Wrap the :on option in an array if necessary. This allows a single | ||
# symbol to be passed in. | ||
options_on = Array(options[:on]) | ||
|
||
after_create :record_create, :if => :save_version? if options_on.include?(:create) | ||
if options_on.include?(:update) | ||
before_save :reset_timestamp_attrs_for_update_if_needed!, :on => :update | ||
after_update :record_update, :if => :save_version? | ||
after_update :clear_version_instance! | ||
end | ||
after_destroy :record_destroy, :if => :save_version? if options_on.include?(:destroy) | ||
|
||
# Reset the transaction id when the transaction is closed. | ||
after_commit :reset_transaction_id | ||
after_rollback :reset_transaction_id | ||
after_rollback :clear_rolled_back_versions | ||
end | ||
|
||
def setup_callbacks_from_options(options_on = []) | ||
options_on.each do |option| | ||
send "paper_trail_on_#{option}" | ||
end | ||
end | ||
|
||
# Record version before or after "destroy" event | ||
def paper_trail_on_destroy(recording_order = 'after') | ||
unless %w[after before].include?(recording_order.to_s) | ||
fail ArgumentError, 'recording order can only be "after" or "before"' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of |
||
end | ||
|
||
send "#{recording_order}_destroy", | ||
:record_destroy, | ||
:if => :save_version? | ||
|
||
return if paper_trail_options[:on].include?(:destroy) | ||
paper_trail_options[:on] << :destroy | ||
end | ||
|
||
# Record version after "update" event | ||
def paper_trail_on_update | ||
before_save :reset_timestamp_attrs_for_update_if_needed!, | ||
:on => :update | ||
after_update :record_update, | ||
:if => :save_version? | ||
after_update :clear_version_instance! | ||
|
||
return if paper_trail_options[:on].include?(:update) | ||
paper_trail_options[:on] << :update | ||
end | ||
|
||
# Record version after "create" event | ||
def paper_trail_on_create | ||
after_create :record_create, | ||
:if => :save_version? | ||
|
||
return if paper_trail_options[:on].include?(:create) | ||
paper_trail_options[:on] << :create | ||
end | ||
|
||
# Switches PaperTrail off for this class. | ||
def paper_trail_off! | ||
PaperTrail.enabled_for_model(self, false) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
require 'rails_helper' | ||
require 'support/callback_modifier' | ||
|
||
describe CallbackModifier, :type => :model do | ||
with_versioning do | ||
describe 'callback-methods', :versioning => true do | ||
describe 'paper_trail_on_destroy' do | ||
it 'should add :destroy to paper_trail_options[:on]' do | ||
modifier = NoArgDestroyModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.paper_trail_options[:on]).to eq [:destroy] | ||
end | ||
|
||
context 'when :before' do | ||
it 'should create the version before destroy' do | ||
modifier = BeforeDestroyModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.test_destroy | ||
expect(modifier.versions.last.reify).not_to be_flagged_deleted | ||
end | ||
end | ||
|
||
context 'when :after' do | ||
it 'should create the version after destroy' do | ||
modifier = AfterDestroyModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.test_destroy | ||
expect(modifier.versions.last.reify).to be_flagged_deleted | ||
end | ||
end | ||
|
||
context 'when no argument' do | ||
it 'should default to after destroy' do | ||
modifier = NoArgDestroyModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.test_destroy | ||
expect(modifier.versions.last.reify).to be_flagged_deleted | ||
end | ||
end | ||
end | ||
|
||
describe 'paper_trail_on_update' do | ||
it 'should add :update to paper_trail_options[:on]' do | ||
modifier = UpdateModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.paper_trail_options[:on]).to eq [:update] | ||
end | ||
|
||
it 'should create a version' do | ||
modifier = UpdateModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.update_attributes! :some_content => 'modified' | ||
expect(modifier.versions.last.event).to eq 'update' | ||
end | ||
end | ||
|
||
describe 'paper_trail_on_create' do | ||
it 'should add :create to paper_trail_options[:on]' do | ||
modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.paper_trail_options[:on]).to eq [:create] | ||
end | ||
|
||
it 'should create a version' do | ||
modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.versions.last.event).to eq 'create' | ||
end | ||
end | ||
|
||
context 'when no callback-method used' do | ||
it 'should set paper_trail_options[:on] to [:create, :update, :destroy]' do | ||
modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.paper_trail_options[:on]).to eq [:create, :update, :destroy] | ||
end | ||
|
||
it 'should default to track destroy' do | ||
modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.destroy | ||
expect(modifier.versions.last.event).to eq 'destroy' | ||
end | ||
|
||
it 'should default to track update' do | ||
modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.update_attributes! :some_content => 'modified' | ||
expect(modifier.versions.last.event).to eq 'update' | ||
end | ||
|
||
it 'should default to track create' do | ||
modifier = DefaultModifier.create!(:some_content => Faker::Lorem.sentence) | ||
expect(modifier.versions.last.event).to eq 'create' | ||
end | ||
end | ||
|
||
context 'when only one callback-method' do | ||
it 'does only track the corresponding event' do | ||
modifier = CreateModifier.create!(:some_content => Faker::Lorem.sentence) | ||
modifier.update_attributes!(:some_content => 'modified') | ||
modifier.test_destroy | ||
expect(modifier.versions.collect(&:event)).to eq ['create'] | ||
end | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
class BeforeDestroyModifier < CallbackModifier | ||
has_paper_trail :on => [] | ||
paper_trail_on_destroy :before | ||
end | ||
|
||
class AfterDestroyModifier < CallbackModifier | ||
has_paper_trail :on => [] | ||
paper_trail_on_destroy :after | ||
end | ||
|
||
class NoArgDestroyModifier < CallbackModifier | ||
has_paper_trail :on => [] | ||
paper_trail_on_destroy | ||
end | ||
|
||
class UpdateModifier < CallbackModifier | ||
has_paper_trail :on => [] | ||
paper_trail_on_update | ||
end | ||
|
||
class CreateModifier < CallbackModifier | ||
has_paper_trail :on => [] | ||
paper_trail_on_create | ||
end | ||
|
||
class DefaultModifier < CallbackModifier | ||
has_paper_trail | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you choose to put these classes in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because for me they are more like helper-classes wrapped around the model, it didn't feel right to put them there with all the "normal" activerecord models. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point, but I'd still rather keep all the test models in one place. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
class CallbackModifier < ActiveRecord::Base | ||
has_paper_trail :on => [] | ||
|
||
def test_destroy | ||
transaction do | ||
run_callbacks(:destroy) do | ||
self.deleted = true | ||
save! | ||
end | ||
end | ||
end | ||
|
||
def flagged_deleted? | ||
deleted? | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -212,6 +212,11 @@ def self.up | |
t.boolean :scoped, :default => true | ||
end | ||
|
||
create_table :callback_modifiers, :force => true do |t| | ||
t.string :some_content | ||
t.boolean :deleted, :default => false | ||
end | ||
|
||
create_table :chapters, :force => true do |t| | ||
t.string :name | ||
end | ||
|
@@ -277,5 +282,7 @@ def self.down | |
remove_index :version_associations, :column => [:version_id] | ||
remove_index :version_associations, :name => 'index_version_associations_on_foreign_key' | ||
drop_table :version_associations | ||
drop_table :filter_modifier | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this line is probably an oversight. We can remove it, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohhh.. Yeah that one is very old. No one will miss that line ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't forget to delete this line. |
||
drop_table :callback_modifiers | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paper_trail_on_update
.