paper-trail-gem / paper_trail

Track changes to your rails models
MIT License
6.76k stars 895 forks source link

Empty "update" versions when using active storage #1465

Open mochetts opened 6 months ago

mochetts commented 6 months ago

Thank you for your contribution!

Due to limited volunteers, issues that do not follow these instructions will be closed without comment.

Check the following boxes:

Due to limited volunteers, we cannot answer usage questions. Please ask such questions on StackOverflow.

I couldn't use the script template as I needs Rails to reproduce the bug. But I'm attaching a demo repo in which the bug reproduces.

1) Checkout this repository: https://github.com/mochetts/paper-trail-active-storage-bug

2) Run rails db:migrate to run migrations.

3) Run rake to run minitests.

4) See that the test fails:

Screenshot 2024-03-13 at 7 34 08 AM

Some analysis I've been doing:

It seems that Rails touches records when storing a has_one_attached relationship as per this bug report and its consequent fix.

This means that every time a new record is created with an associated attachment, it touches the model multiple times, generating multiple "update" versions for it with nil "object_changes". This seems to happen because the touch is occurring on creation, when there's no "updated_at" to touch.

Screenshot 2024-03-12 at 6 49 20 PM

Now the real question is if this is a Rails bug or a Papertrail bug, as it only happens under this scenario. Worthy to mention that rails forbids you from performing a touch on a new record, but under these circumstances it seems to be doing by itself.

Screenshot 2024-03-13 at 7 28 02 AM

I would suggest skipping the insertion of "update" versions when object_changes = nil, as these become noise in the versions history of the model.

I can send a PR for this, but I'd need confirmation that this is the right path.

mochetts commented 6 months ago

Note that this issue doesn't happen if you turn off version creation on touch events.

As in:

class User < ApplicationRecord
  has_paper_trail on: %i[create update destroy]
end
hasannadeem commented 4 months ago

I'm facing the same issue

yfxie commented 4 months ago

@mochetts typo should be destroy

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale due to inactivity. The resources of our volunteers are limited. Bug reports must provide a script that reproduces the bug, using our template. Feature suggestions must include a promise to build the feature yourself. Thank you for all your contributions.

aejohnmartin commented 1 month ago

I am also seeing this issue when using Action Text, i.e. a model with has_rich_text :foobar and has_paper_trail