Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,8 @@ class User < ActiveRecord::Base
end
```

Optionally your callback method can take an `action` parameter whose value is either `create`, `update` or `destroy`.

Just like in ActiveModel, you can use an inline Proc in your conditions:

```ruby
Expand All @@ -338,7 +340,7 @@ class User < ActiveRecord::Base
end
```

In the above case, the user will only be audited when `User#ninja` is `false`.
In the above case, the user will only be audited when `User#ninja` is `false`. Similar to the method the Proc can accept the audited action as second parameter.

### Disabling auditing

Expand Down
37 changes: 24 additions & 13 deletions lib/audited/auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Author

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.

# any create, update or destroy operation.
# * +max_audits+ - Limits the number of stored audits.

Expand Down Expand Up @@ -375,7 +375,7 @@ def audit_destroy
def write_audit(attrs)
self.audit_comment = nil

if auditing_enabled
if auditing_enabled(attrs[:action])
attrs[:associated] = send(audit_associated_with) unless audit_associated_with.nil?

run_callbacks(:audit) {
Expand All @@ -393,10 +393,16 @@ def presence_of_audit_comment
end

def comment_required_state?
auditing_enabled &&
action = if audited_options[:on].include?(:create) && new_record?
"create"
elsif audited_options[:on].include?(:update) && persisted? && changed?
"update"
else
nil
end
auditing_enabled(action) &&
audited_changes.present? &&
((audited_options[:on].include?(:create) && new_record?) ||
(audited_options[:on].include?(:update) && persisted? && changed?))
(action == "create" || action == "update")
end

def combine_audits_if_needed
Expand All @@ -420,7 +426,9 @@ def evaluate_max_audits
end

def require_comment
if auditing_enabled && audit_comment.blank?
# this method is only called as before_destroy callback
# therefore it's safe to statically pass "destroy"
if auditing_enabled("destroy") && audit_comment.blank?
errors.add(:audit_comment, :blank)
throw(:abort)
end
Expand All @@ -430,18 +438,21 @@ def require_comment
alias_method "#{attr_name}_callback".to_sym, attr_name
end

def auditing_enabled
run_conditional_check(audited_options[:if]) &&
run_conditional_check(audited_options[:unless], matching: false) &&
def auditing_enabled(action = nil)
run_conditional_check(audited_options[:if], action: action) &&
run_conditional_check(audited_options[:unless], action: action, matching: false) &&
self.class.auditing_enabled
end

def run_conditional_check(condition, matching: true)
def run_conditional_check(condition, action:, matching: true)
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
Copy link
Author

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

when Proc then condition.call(self, action)
Copy link
Author

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.

when Symbol then method(condition.to_sym).arity == 0 ? send(condition) : send(condition, action)
else return true
end

true
result == matching
end

def reconstruct_attributes(audits)
Expand Down
183 changes: 153 additions & 30 deletions spec/audited/auditor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,29 @@ def public?
end
end

class ConditionalParameterCompany < ::ActiveRecord::Base
self.table_name = "companies"

audited if: :public?

def public?(action)
end
end

class ExclusiveCompany < ::ActiveRecord::Base
self.table_name = "companies"
audited if: proc { false }
end

class ExclusiveParameterCompany < ::ActiveRecord::Base
self.table_name = "companies"
attr_accessor :recorded_action
audited if: proc { |c, action|
c.recorded_action = action
false
}
end

class ExclusionaryCompany < ::ActiveRecord::Base
self.table_name = "companies"

Expand All @@ -39,25 +57,57 @@ def non_profit?
end
end

class ExclusionaryParameterCompany < ::ActiveRecord::Base
self.table_name = "companies"

audited unless: :non_profit?

def non_profit?(action)
end
end

class ExclusionaryCompany2 < ::ActiveRecord::Base
self.table_name = "companies"
audited unless: proc { |c| c.exclusive? }
audited unless: proc { |c| true }
end

def exclusive?
class ExclusionaryParameterCompany2 < ::ActiveRecord::Base
self.table_name = "companies"
attr_accessor :recorded_action
audited unless: proc { |c, action|
c.recorded_action = action
true
end
}
end

class InclusiveCompany < ::ActiveRecord::Base
self.table_name = "companies"
audited if: proc { true }
end

class InclusiveParameterCompany < ::ActiveRecord::Base
self.table_name = "companies"
attr_accessor :recorded_action
audited if: proc { |c, action|
c.recorded_action = action
true
}
end

class InclusiveCompany2 < ::ActiveRecord::Base
self.table_name = "companies"
audited unless: proc { false }
end

class InclusiveParameterCompany2 < ::ActiveRecord::Base
self.table_name = "companies"
attr_accessor :recorded_action
audited unless: proc { |c, action|
c.recorded_action = action
false
}
end

class Secret < ::ActiveRecord::Base
audited
end
Expand Down Expand Up @@ -92,56 +142,129 @@ class Secret2 < ::ActiveRecord::Base
it { is_expected.to be_truthy }
end

context "when passing a method name" do
context "when conditions are true" do
before { allow_any_instance_of(ConditionalCompany).to receive(:public?).and_return(true) }
it { is_expected.to be_truthy }
context "when using a method name" do
context "when method takes no parameters" do
context "when conditions are true" do
before { allow_any_instance_of(ConditionalCompany).to receive(:public?).and_return(true) }
it { is_expected.to be_truthy }
end

context "when conditions are false" do
before { allow_any_instance_of(ConditionalCompany).to receive(:public?).and_return(false) }
it { is_expected.to be_falsey }
end
end

context "when conditions are false" do
before { allow_any_instance_of(ConditionalCompany).to receive(:public?).and_return(false) }
it { is_expected.to be_falsey }
context "when method takes action parameter" do
subject { ConditionalParameterCompany.new.send(:auditing_enabled, "create") }

context "when conditions are true" do
before { expect_any_instance_of(ConditionalParameterCompany).to receive(:public?).with("create").and_return(true) }
it { is_expected.to be_truthy }
end

context "when conditions are false" do
before { expect_any_instance_of(ConditionalParameterCompany).to receive(:public?).with("create").and_return(false) }
it { is_expected.to be_falsey }
end
end

end

context "when passing a Proc" do
context "when conditions are true" do
subject { InclusiveCompany.new.send(:auditing_enabled) }
context "when using a Proc" do
context "when Proc takes no parameters" do
context "when conditions are true" do
subject { InclusiveCompany.new.send(:auditing_enabled) }

it { is_expected.to be_truthy }
end

it { is_expected.to be_truthy }
context "when conditions are false" do
subject { ExclusiveCompany.new.send(:auditing_enabled) }
it { is_expected.to be_falsey }
end
end

context "when conditions are false" do
subject { ExclusiveCompany.new.send(:auditing_enabled) }
it { is_expected.to be_falsey }
context "when Proc takes parameters" do
context "when conditions are true" do
subject { InclusiveParameterCompany.new }
it {
subject.send(:auditing_enabled, "create")
expect(subject.recorded_action).to eq("create")
}
end

context "when conditions are false" do
subject { ExclusiveParameterCompany.new }
it {
subject.send(:auditing_enabled, "create")
expect(subject.recorded_action).to eq("create")
}
end
end
end
end

context "should be configurable which conditions aren't audited" do
context "when using a method name" do
subject { ExclusionaryCompany.new.send(:auditing_enabled) }
context "when method takes no parameters" do
subject { ExclusionaryCompany.new.send(:auditing_enabled) }

context "when conditions are true" do
before { allow_any_instance_of(ExclusionaryCompany).to receive(:non_profit?).and_return(true) }
it { is_expected.to be_falsey }
context "when conditions are true" do
before { allow_any_instance_of(ExclusionaryCompany).to receive(:non_profit?).and_return(true) }
it { is_expected.to be_falsey }
end

context "when conditions are false" do
before { allow_any_instance_of(ExclusionaryCompany).to receive(:non_profit?).and_return(false) }
it { is_expected.to be_truthy }
end
end

context "when conditions are false" do
before { allow_any_instance_of(ExclusionaryCompany).to receive(:non_profit?).and_return(false) }
it { is_expected.to be_truthy }
context "when method takes action parameter" do
subject { ExclusionaryParameterCompany.new.send(:auditing_enabled, "create") }

context "when conditions are true" do
before { expect_any_instance_of(ExclusionaryParameterCompany).to receive(:non_profit?).with("create").and_return(true) }
it { is_expected.to be_falsey }
end

context "when conditions are false" do
before { expect_any_instance_of(ExclusionaryParameterCompany).to receive(:non_profit?).and_return(false) }
it { is_expected.to be_truthy }
end
end
end

context "when using a proc" do
context "when conditions are true" do
subject { ExclusionaryCompany2.new.send(:auditing_enabled) }
it { is_expected.to be_falsey }
context "when Proc takes no parameters" do
context "when conditions are true" do
subject { ExclusionaryCompany2.new.send(:auditing_enabled) }
it { is_expected.to be_falsey }
end

context "when conditions are false" do
subject { InclusiveCompany2.new.send(:auditing_enabled) }
it { is_expected.to be_truthy }
end
end

context "when conditions are false" do
subject { InclusiveCompany2.new.send(:auditing_enabled) }
it { is_expected.to be_truthy }
context "when Proc takes parameters" do
context "when conditions are true" do
subject { ExclusionaryParameterCompany2.new }
it {
subject.send(:auditing_enabled, "create")
expect(subject.recorded_action).to eq("create")
}
end

context "when conditions are false" do
subject { InclusiveParameterCompany2.new }
it {
subject.send(:auditing_enabled, "create")
expect(subject.recorded_action).to eq("create")
}
end
end
end
end
Expand Down