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

Rails 5: belongs_to_required_by_default prevents versioning of destroy events #682

Closed
owenr opened this issue Dec 31, 2015 · 3 comments
Closed

Comments

@owenr
Copy link
Contributor

owenr commented Dec 31, 2015

When using the ActiveRecord configuration belongs_to_required_by_default (set to true for new Rails 5 projects), PaperTrail::Version tries to validate presence of the versioned item. Since versioning is done after destroy (by default), no matching item exists and the version creation silently fails.

See https://gist.github.com/owenr/60b34a2cdc13edf89412 for a test using the bug report template.

Proposed fix: explicitly make the item relationship optional.

@jaredbeck
Copy link
Member

paper_trail_on_destroy already supports a before_destroy instead of the default after_destroy. Can we just change the default?

- def paper_trail_on_destroy(recording_order = 'after')
+ def paper_trail_on_destroy(recording_order = 'before')

If there are no drawbacks to changing the default, I'd prefer to keep the belongs_to :item relationship required.

@owenr
Copy link
Contributor Author

owenr commented Jan 2, 2016

The only drawback is that a reified model cannot be interrogated to see if it was destroyed. The version event still records the action. Since callback order is undefined in the current release, switching to :before seems to be a sensible approach.

@jaredbeck
Copy link
Member

.. switching to :before seems to be a sensible approach.

Great! Our test suite now supports ActiveRecord 5 (support added by #684) but is not passing on AR 5 yet. Once our test suite is passing on AR 5, we can write a failing test for this issue.

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

2 participants