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

Proper single-table inheritance support for Audit#revision #368

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

noiges
Copy link

@noiges noiges commented Aug 9, 2017

Details

This PR fixes an issue where the subclass of a record from a single-inheritance table can't be recovered anymore with Audit#revision after the record was deleted.

The reason for this is that the inheritance_column is not audited by default, which means the subclass is impossible to retrieve after a record has been deleted.

My naive solution for this problem is to just track changes to the inheritance_column, and make sure the audited class is instantiated properly when a revision is created.

Concerns

  1. I am not fully aware why the inheritance_column has been ignored so far — this fix is based on the assumption that this was just an oversight. Are there any issues with this? All tests are passing on my machine though.

  2. If the inheritance column or the class name have ever been refactored without a migration of the existing audits’ inheritance column, an ActiveRecord::SubclassNotFound will be raised if an audit's revision is retrieved (because we rely on this). Should this be handled more gracefully, with a fall back to the parent class?

@noiges
Copy link
Author

noiges commented Aug 11, 2017

Looks like all Rails 4 builds are failing. I'm gonna close this for now until I've figured out a fix.

@noiges noiges closed this Aug 11, 2017
If a Rails 4 application is using the protected_attributes gem, the
inheritance_column is blacklisted by default and therefore can't be
mass-assigned, which means Audit#revision will restored the parent
class, instead of the correct subclass. This is consistent with
ActiveRecord's default behaviour.
@noiges noiges changed the title Proper single-table inheritance support for revision Proper single-table inheritance support for Audit#revision Aug 16, 2017
@noiges
Copy link
Author

noiges commented Aug 16, 2017

Turns out the specs were failing for Rails 4 applications that are using protected_attributes, since the inheritance column is blacklisted by default for mass assignment, which results in the parent class being instantiated, instead of the right subclass. I think this is fine, since it's consistent with ActiveRecord's default behaviour.

I've modified my specs to take this into account, for cases where protected_attributes are used.

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

Successfully merging this pull request may close these issues.

None yet

1 participant