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 support for audit_attributes #732

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

Conversation

nazamoresco
Copy link

In the current project I'm working on, we introduced a new column to the audit class but struggled to find an elegant way to set it dynamically. A feature similar to audit_comment but more flexible would provide a more convenient solution.

This PR introduces the audit_attributes feature to the gem, offering a flexible approach to include additional custom attributes during the auditing process. The following changes have been made:

  • Added audit_attributes to track custom attributes during auditing.
  • Implemented validation to ensure audit_attributes is a hash containing only valid keys from the audit class.
  • Enhanced the audit creation, update, and destroy methods to include audit_attributes in audit records.
  • Updated specs to test the inclusion of audit_attributes in audit records and validate its correctness.
  • Modified the schema to include a new custom_attribute column in the audits table.
  • Ran the linter to ensure code quality.

@danielmorrison
Copy link
Member

This is interesting. Before merging, it would need:

  • Updates to the README
  • A new migration

At first glance, I don't like the use of **safe_audit_attributes and might prefer it to be a explicit key.

@nazamoresco
Copy link
Author

Hey Daniel,

Thanks for taking a look! I think there might be a slight misinterpretation of what audit_attributes is intended to do. It’s not about introducing a new column to the audits table within the gem itself, but rather providing a flexible way for users to set any custom columns they’ve already added to the audit table in their own applications.

No migration is needed from the gem side because the custom columns are expected to be added by the users based on their specific requirements.

Regarding the use of an explicit key in write_audit, it wouldn’t be feasible with the current setup since we can’t predict which custom columns the users might create in their own implementations.

I’ve updated the README.md to include a section explaining the intended usage of audit_attributes. I hope this helps clarify the feature! Let me know what you think!

@partbotdev
Copy link

Having the same issue in our project, this seems like a no-brainer. Our use-case is the need to store an array of associated record ids (rather than using the associated_with, as that tracks too many audits).

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.

3 participants