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 Papertrail initializer option to switch PaperTrail::Reifiers::HasOne::FoundMoreThanOne for a warning instead #1090

Closed
westonganger opened this issue May 18, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@westonganger
Copy link
Contributor

commented May 18, 2018

After trying to upgrade from v8.1.2 to v9+. I started recieving this error PaperTrail::Reifiers::HasOne::FoundMoreThanOne. I believe the old paper trail just kinda said "I don't care about being totally correct, just carry on". So now I cannot upgrade because of this exception. Can we add an option to the initializer to instead just warn and continue.

raise FoundMoreThanOne.new(base_class_name, versions.length)

We could change this to something like

if PaperTrail.config.associations_just_warn_enabled
  # do warning
  versions.first
else
  # do raise
end

@westonganger westonganger changed the title Add Papertrail initializer option to switch PaperTrail::Reifiers::HasOne::FoundMoreThanOne for a warning instaed Add Papertrail initializer option to switch PaperTrail::Reifiers::HasOne::FoundMoreThanOne for a warning instead May 18, 2018

@jaredbeck

This comment has been minimized.

Copy link
Member

commented May 18, 2018

I'm hesitant to add any more complexity related to association tracking. I'd like to see it have a dedicated maintainer and/or be extracted into a separate gem first. But, you're going to help with that, right? 🙏

That said, I don't want to stop anyone from upgrading, so I guess we should add this configuration option. I'd call it association_reify_error_behavior and values can be :warn or :error. I think error should remain the default.

This should also be added to the list of known issues in the readme.

@westonganger

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2018

Let me know if I should make a PR for this or if you are going to.

@jaredbeck

This comment has been minimized.

Copy link
Member

commented May 18, 2018

Let me know if I should make a PR for this or if you are going to.

Please go ahead. I don't use association tracking, so I don't have much motivation to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.