-
Notifications
You must be signed in to change notification settings - Fork 899
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
errors are not raised when update, update_columns, or destroy versions fail to create #1466
Comments
This issue has been automatically marked as stale due to inactivity. |
this still isn't stale but if it's auto-closed (again) i'm giving up. i've been trying for 8 months to and i honestly don't care anymore. |
This issue has been automatically marked as stale due to inactivity. |
@jaredbeck can we get a new release since #1450 has been merged? that would close this issue out. i'm happy to help if there's anything i can do on my end. |
NOTE: this was #1449 but it was automatically closed. i've provided a bug report as well as a fix. can i please get feedback on this?
paper_trail
is inconsistent whenVersion
records cannot be created. in some cases (create
) an error is raised, in others (update
,update_columns
, anddestroy
) an error is logged but not raised. this can result in history silently being lost which is certainly unexpected behavior.Currently when creating a new versioned record an error is raised if the
Version
cannot be created - this is because#save!
is used:paper_trail/lib/paper_trail/record_trail.rb
Line 62 in 47dbc22
However, when updating, using
update_columns
, or destroying a versioned record no error is raised if theVersion
cannot be created:paper_trail/lib/paper_trail/record_trail.rb
Line 111 in 47dbc22
paper_trail/lib/paper_trail/record_trail.rb
Line 301 in 47dbc22
paper_trail/lib/paper_trail/record_trail.rb
Line 84 in 47dbc22
proposal
i think the behavior should be consistent - all failures should either raise errors or not. personally i don't see any advantage to not raising an error in these cases - it likely means that
whodunnit
isn't defined which i would expect to be an oversight rather than intentional. i realize this is not backwards compatible so i propose a two step process:update
anddestroy
failures to raise an error.The text was updated successfully, but these errors were encountered: