Skip to content

Commit

Permalink
Change paper_trail_on_destroy default to 'before'
Browse files Browse the repository at this point in the history
Also warn if paper_trail_on_destroy(:after) is combined with
ActiveRecord belongs_to_required_by_default
  • Loading branch information
owenr committed Jan 15, 2016
1 parent eef918b commit 18d35ef
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
that PaperTrail no longer adds the `set_paper_trail_whodunnit` before_filter
for you. Please add this before_filter to your ApplicationController to
continue recording whodunnit. See the readme for an example.
- [#683](https://github.com/airblade/paper_trail/pull/683) /
[#682](https://github.com/airblade/paper_trail/issues/682) -
Destroy callback default changed to :before to accommodate ActiveRecord 5
option `belongs_to_required_by_default` and new Rails 5 default.

### Added

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ end
```

The `paper_trail_on_destroy` method can be further configured to happen
`:before` or `:after` the destroy event. By default, it will happen after.
`:before` or `:after` the destroy event. By default, it will happen before.

## Choosing When To Save New Versions

Expand Down
13 changes: 12 additions & 1 deletion lib/paper_trail/has_paper_trail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,22 @@ def setup_callbacks_from_options(options_on = [])
end

# Record version before or after "destroy" event
def paper_trail_on_destroy(recording_order = 'after')
def paper_trail_on_destroy(recording_order = 'before')
unless %w[after before].include?(recording_order.to_s)
fail ArgumentError, 'recording order can only be "after" or "before"'
end

if recording_order == 'after' and
Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("5")
if ::ActiveRecord::Base.belongs_to_required_by_default
::ActiveSupport::Deprecation.warn(
"paper_trail_on_destroy(:after) is incompatible with ActiveRecord " +
"belongs_to_required_by_default and has no effect. Please use :before " +
"or disable belongs_to_required_by_default."
)
end
end

send "#{recording_order}_destroy", :record_destroy, :if => :save_version?

return if paper_trail_options[:on].include?(:destroy)
Expand Down
4 changes: 2 additions & 2 deletions spec/models/callback_modifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
end

context 'when no argument' do
it 'should default to after destroy' do
it 'should default to before destroy' do
modifier = NoArgDestroyModifier.create!(:some_content => FFaker::Lorem.sentence)
modifier.test_destroy
expect(modifier.versions.last.reify).to be_flagged_deleted
expect(modifier.versions.last.reify).not_to be_flagged_deleted
end
end
end
Expand Down

0 comments on commit 18d35ef

Please sign in to comment.