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

Add always_raise_on_error config option #1450

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

modosc
Copy link

@modosc modosc commented Nov 16, 2023

NOTE: all the specs pass locally but rubocop is failing on a complexity check - i'm unsure exactly how to simplify this code to satisfy rubocop without making the intent less clear. happy to hear suggestions? for now i've disabled the check around the offending method

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)

This pr adds a always_raise_on_error global config option. When set, all of the above failures will raise an error.

Fixes #1449

Thank you for your contribution!

Check the following boxes:

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

@modosc modosc changed the title Add always_raise_on_error config option Add always_raise_on_error config option (fixes #1449) Nov 16, 2023
@modosc modosc changed the title Add always_raise_on_error config option (fixes #1449) Add always_raise_on_error config option Nov 16, 2023
@@ -52,7 +52,7 @@ has been destroyed.
# PT supports request_store versions for 3 years.
s.add_dependency "request_store", "~> 1.4"

s.add_development_dependency "appraisal", "~> 2.4.1"
s.add_development_dependency "appraisal", "~> 2.5.0"
Copy link
Author

@modosc modosc Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was necessary because i couldn't run appraisal locally with ruby-3.2.2 - i think this fix is what got it working again

Copy link

This PR has been automatically marked as stale due to inactivity.
The resources of our volunteers are limited.
If this is something you are committed to continue working on, please address any concerns raised by review and/or ping us again.
Thank you for all your contributions.

@github-actions github-actions bot added the Stale label Feb 15, 2024
@modosc
Copy link
Author

modosc commented Feb 22, 2024

is there any chance of getting this looked at? silently not saving version data is a pretty serious issue imho.

Copy link

This PR has been automatically marked as stale due to inactivity.
The resources of our volunteers are limited.
If this is something you are committed to continue working on, please address any concerns raised by review and/or ping us again.
Thank you for all your contributions.

@github-actions github-actions bot added the Stale label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

errors are not raised when update, update_columns, or destroy versions fail to create
1 participant