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.

Sign up for GitHub

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

Jump to bottom

Allow custom Audit classes #319

Merged
merged 1 commit into from
Mar 28, 2017
Merged

Allow custom Audit classes #319

merged 1 commit into from
Mar 28, 2017

Conversation

nickrivadeneira
Copy link
Contributor

Classes that inherit from Audited::Audit can be used instead of the
default by assigning Audited.audit_class in an initializer.

Resolves #314

@nickrivadeneira
Copy link
Contributor Author

One potential problem with this implementation is that it makes use of the .audit_class accessor that was previously deprecated. It fit so perfectly, so I used it, but I wonder if it'd break for people who were using it previously to assign adapters. Maybe not since those adapters have been removed and would result in a const missing exception that would need to be addressed anyway.

@nickrivadeneira nickrivadeneira mentioned this pull request Dec 22, 2016
Closed
domcleal
domcleal requested changes Mar 27, 2017
View reviewed changes
Copy link
Collaborator

@domcleal domcleal left a comment

Choose a reason for hiding this comment

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

I agree that .audit_class is a good fit and don't have a problem with this functionality, and it looks like @danielmorrison is also in favour of the general direction from #314 (comment).

Could you rebase the PR please?

spec/audited/audit_spec.rb Outdated
expect(audit).to be_a CustomAudit
expect(audit.custom_method).to eq "I'm custom!"

Audited.config { |config| config.audit_class = nil }
Copy link
Collaborator

Choose a reason for hiding this comment

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

These cleanup sections should be in an after block, rather than the main spec body. (If possible, also move the new classes and setup to a before too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@domcleal I've moved this into an around block so that the audit class can be reset to its original value. Let me know if you'd prefer the before/after split and having config.audit_class reset nil as it is here.

@nickrivadeneira
Allow custom Audit classes
2900181 
Classes that inherit from `Audited::Audit` can be used instead of the
default by assigning `Audited.audit_class` in an initializer.

Resolves collectiveidea#314
@nickrivadeneira
Copy link
Contributor Author

👍 Rebased. I made an additional change to lib/audited/audit.rb on lines 19 and 27 to ensure these changes take into account the addition of ce9bd0b.

@domcleal domcleal merged commit bea190e into collectiveidea:master Mar 28, 2017
@domcleal
Copy link
Collaborator

Thanks for the update @nickrivadeneira, merged!

@nickrivadeneira nickrivadeneira deleted the custom-audit-class branch March 28, 2017 13:22
@VasudevaSharmaM
Copy link

Hi ,

I am using audited in building an application , I am facing the issue with this gem which says - "method_missing': undefined method audited_class_names' for #Class:0x00007fffcef14138 (NoMethodError)", I am trying to work around and found that this behavior is expected from the previous conversation. Could anybody please assist me a solution for this ?

It would be very helpful

Thank you !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domcleal domcleal domcleal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Why we don't have Audits model in rails ?
3 participants
@nickrivadeneira @domcleal @VasudevaSharmaM