Papertrail does not delete properly when combined with Paranoia

[NupMK]

Hello,
my software is using Paranoia at the moment for a soft-deletes.
I have now added PaperTrail for versioning so I can track updates in certain tables.
While testing I found that deleting something will write a version data set, but only AFTER paranoia has done its update and set the "deleted" flag.

That means that code like this:

data = Example.first
data.destroy
data_new = Example.with_deleted.first
data_new.previous_version

Will "restore" the data set to its deleted state, which is of course not what I want it to do.

I dug around in the code a little bit and found that changing some code in has_paper_trail.rb
seems to solve my problem.
The code I changed is the following (starting in line 97)

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)

And I changed it to:

if options_on.include?(:update)
      before_save   :reset_timestamp_attrs_for_update_if_needed!, :on => :update
      before_update  :record_update, :if => :save_version?
      after_update  :clear_version_instance!
end
before_destroy :record_destroy, :if => :save_version? if options_on.include?(:destroy)

To solve my problem changing after_destroy to before_destroy would have been sufficient, but it seemed sensible to change update too.

I am unsure wether I created new problems with my changes or maybe destroyed functions that I am simply not using (yet), so I'd be very grateful for somebody with more experience taking a look at my problem and/or solution.

Thank you for your time, and many thanks in advance.

[batter]

I think it should work fine, although I will say that it seems sort of redundant to use Paranoia and PaperTrail in conjunction while recording destroy events. We use an after_destroy callback due to the fact that we want to ensure the destroy actually executed and wasn't aborted (since it is possible for a DB transaction to be aborted in ActiveRecord if there is an exception raised by a callback prior to it executing).

If you want to continue using both plugins in conjunction with each other, it may benefit you to just have PaperTrail skip creating a version for destroy events entirely? See the README for more information about the on option.

[Ninigi]

Hi, just throwing in some more information on this topic:
We built an API using Paranioa (or rather its predecessor) and are trying to transition into an API Version 2 (using the same database) without breaking Version 1. So there is a reason we are using both redundantly.
I see where your reason to use the after callbacks is comming from, but it might be usefull to add an option to save the version before and (to be extra paranoid) after destroy - because the issue of other hooks kicking in before yours still stands...

[jaredbeck]

it might be usefull to add an option to save the version before and (to be extra paranoid) after destroy

Instead of making PaperTrail more complicated by adding configuration options, I'd prefer to make PaperTrail less complicated by allowing users to add the callbacks themselves. This is what libraries like CanCanCan and Pundit do. I suggested something similar back in June: #556

I would be happy to review a PR that gives users control over the order of the destroy callback, without making PaperTrail more complicated by adding a configuration option.

I'm going to close this issue, because I feel that Ben has answered the original question.