Skip to content
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

Closed
modosc opened this issue Apr 8, 2024 · 4 comments

Comments

@modosc
Copy link
Contributor

modosc commented Apr 8, 2024

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 when Version records cannot be created. in some cases (create) an error is raised, in others (update, update_columns, and destroy) 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:

However, when updating, using update_columns, or destroying a versioned record no error is raised if the Version cannot be created:

version = versions_assoc.create(data)

version = @record.class.paper_trail.version_class.create(data)

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:

  1. an opt-in configuration parameter is added to the current major version which causes update and destroy failures to raise an error.
  2. in the next major version bump this becomes the default behavior.
Copy link

github-actions bot commented Jul 8, 2024

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.

@github-actions github-actions bot added the Stale label Jul 8, 2024
@modosc
Copy link
Contributor Author

modosc commented Jul 9, 2024

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.

Copy link

github-actions bot commented Oct 8, 2024

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.

@github-actions github-actions bot added the Stale label Oct 8, 2024
@modosc
Copy link
Contributor Author

modosc commented Oct 8, 2024

@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.

@github-actions github-actions bot removed the Stale label Oct 9, 2024
@modosc modosc closed this as completed Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant