-
Notifications
You must be signed in to change notification settings - Fork 674
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
Optionally pass audited action to conditionals #754
base: main
Are you sure you want to change the base?
Optionally pass audited action to conditionals #754
Conversation
@@ -33,7 +33,7 @@ module ClassMethods | |||
# audited except: :password | |||
# end | |||
# | |||
# * +require_comment+ - Ensures that audit_comment is supplied before | |||
# * +comment_required+ - Ensures that audit_comment is supplied before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of related documentation fix: The private method is called require_comment
, the option is called comment_required
.
return true if condition.blank? | ||
return condition.call(self) == matching if condition.respond_to?(:call) | ||
return send(condition) == matching if respond_to?(condition.to_sym, true) | ||
result = case condition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewritten in the style of evaluate_max_audits
above
return condition.call(self) == matching if condition.respond_to?(:call) | ||
return send(condition) == matching if respond_to?(condition.to_sym, true) | ||
result = case condition | ||
when Proc then condition.call(self, action) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about passing action
as Symbol but Audit.action
is also a String so it's more consistent this way.
It would be easy to also incorporate #697 into this PR, but I'm not sure it's worth it. Using |
I only want to record destructions under some circumstances. The proposed implementation allows users to optionally accept an
action
parameter in their condition method or Proc. I've implemented tests and updated the documentation. Looking forward to your feedback!