audited icon indicating copy to clipboard operation
audited copied to clipboard

Avoid orphaned audits

Open ledermann opened this issue 7 years ago • 4 comments

Leaving orphans is harmful. With MySQL it's a disaster, because record id's can be recycled (please see #247). But it's never a good idea to have foreign key references pointing to the nowhere.

This PR cleans up existing audits if an audited record gets destroyed by nullifying the references. This ensures referential integrity of the database.

ledermann avatar Sep 20 '16 11:09 ledermann

One downside seems to be that you could no longer restore a record to its original state (with original id). That isn't a design goal of Audited, but I do think it is something people use it for.

danielmorrison avatar Sep 20 '16 18:09 danielmorrison

@danielmorrison You are right, with my proposal the following scenario will NOT be possible anymore:

  1. Create a auditable (audit 1)
  2. Change some, but not all attributes (audit 2)
  3. Delete the auditable (audit 3)
  4. Restore the auditable from audit 2. This does not work because audit 1 can not be found anymore

Fun fact: There is no spec for such a feature in the test suite :) My PR does not brake the tests. Ok, I have made some small adjustments, because record.audits now returns a blank list if the record is deleted.

IMHO, by all means issue #247 should be fixed (please see my additional comment). Otherwise, using the gem with MySQL can not be recommended.

If you don't like my fix, maybe we can build another one. Without further investigations, an additional field in the audits table could help. On destroy, this field gets a unique value (some kind of UUID or a timestamp) to link the audits together. But this would be a more complex change.

ledermann avatar Sep 21 '16 13:09 ledermann

@danielmorrison Seems you are not convinced. What about about making the after_destroy callback optional via options[:strict] or something?

Please note, that restoring a record to its original state (with original id) will fail if the id is reused by another record.

ledermann avatar Sep 24 '16 07:09 ledermann

Don't take my slow response as anything other than a slow response. ;) I probably won't have time to dig in until Monday or so.

danielmorrison avatar Sep 24 '16 17:09 danielmorrison