From a37045c032012bb4ecf2e2124dfb59b5d35dc106 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Thu, 15 Jun 2017 16:02:18 -0700 Subject: [PATCH] reduce repeated/unecessary code --- CHANGELOG.md | 5 ++- README.md | 2 +- lib/audited/auditor.rb | 76 ++++++++++++++--------------------- lib/audited/rspec_matchers.rb | 11 ++--- spec/audited/auditor_spec.rb | 35 +++++++++------- 5 files changed, 61 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85e51f1e3..01fcbcc7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ Breaking changes -- None +- remove `non_audited_columns` in favor of editing `audited_options[:except]` +- `audited_options` keys `:only` and `:except` are converted to array of strings Added @@ -12,7 +13,7 @@ Added Changed -- None +- creation and deletion will follow same only/except rules updates do Fixed diff --git a/README.md b/README.md index 7905c46e6..11a0d7ce8 100644 --- a/README.md +++ b/README.md @@ -249,7 +249,7 @@ end To disable auditing on a column: ```ruby -User.non_audited_columns = [:first_name, :last_name] +User.audited_options[:except] = ["first_name", "last_name"] ``` To disable auditing on an entire model: diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index 58aeadef7..4d1dfdf56 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -38,6 +38,11 @@ def audited(options = {}) # don't allow multiple calls return if included_modules.include?(Audited::Auditor::AuditedInstanceMethods) + options = options.dup + [:only, :except].each do |column_list| + options[column_list] = Array.wrap(options[column_list]).map(&:to_s) if options[column_list] + end + class_attribute :audit_associated_with, instance_writer: false class_attribute :audited_options, instance_writer: false @@ -127,21 +132,8 @@ def revision_at(date_or_time) revision_with Audited.audit_class.reconstruct_attributes(audits) unless audits.empty? end - # List of attributes that are audited. - def audited_attributes - attributes.except(*non_audited_columns.map(&:to_s)) - end - - def non_audited_columns - self.class.non_audited_columns - end - protected - def non_audited_columns - self.class.non_audited_columns - end - def revision_with(attributes) dup.tap do |revision| revision.id = id @@ -174,18 +166,24 @@ def rails_below?(rails_version) private + # Hash of audited attributes and their current value. + def audited_attributes + select_audited_columns(attributes) + end + + # Hash of audited attributes and their [was, is] values. def audited_changes - collection = - if audited_options[:only] - audited_columns = self.class.audited_columns.map(&:name) - changed_attributes.slice(*audited_columns) - else - changed_attributes.except(*non_audited_columns) - end + select_audited_columns(changes) + end - collection.inject({}) do |changes, (attr, old_value)| - changes[attr] = [old_value, self[attr]] - changes + def select_audited_columns(hash) + options = self.class.audited_options + if options[:only] + hash.slice(*options[:only]) + else + except = self.class.default_ignored_attributes + Audited.ignored_attributes + except |= options[:except] if options[:except] + hash.except(*except) end end @@ -249,28 +247,6 @@ def auditing_enabled=(val) end # InstanceMethods module AuditedClassMethods - # Returns an array of columns that are audited. See non_audited_columns - def audited_columns - columns.reject { |c| non_audited_columns.map(&:to_s).include?(c.name) } - end - - def non_audited_columns - @non_audited_columns ||= begin - options = audited_options - if options[:only] - except = column_names - Array.wrap(options[:only]).flatten.map(&:to_s) - else - except = default_ignored_attributes + Audited.ignored_attributes - except |= Array(options[:except]).collect(&:to_s) if options[:except] - end - except - end - end - - def non_audited_columns=(columns) - @non_audited_columns = columns - end - # Executes the block with auditing disabled. # # Foo.without_auditing do @@ -308,6 +284,16 @@ def auditing_enabled def auditing_enabled=(val) Audited.store["#{table_name}_auditing_enabled"] = val end + + # remove after 2018-01-01 + def non_audited_columns + raise NotImplementedError, "Use .audited_options[:only] / [:except]" + end + + # remove after 2018-01-01 + def non_audited_columns= + raise NotImplementedError, "Use .audited_options[:only]= / [:except]=" + end end end end diff --git a/lib/audited/rspec_matchers.rb b/lib/audited/rspec_matchers.rb index 40e0b7fe8..033e0c593 100644 --- a/lib/audited/rspec_matchers.rb +++ b/lib/audited/rspec_matchers.rb @@ -111,14 +111,15 @@ def associated_with_model? def records_changes_to_specified_fields? if @options[:only] || @options[:except] if @options[:only] - except = model_class.column_names - @options[:only].map(&:to_s) + expected = @options[:only].map(&:to_s) else - except = model_class.default_ignored_attributes + Audited.ignored_attributes - except |= @options[:except].collect(&:to_s) if @options[:except] + expected = model_class.column_names - model_class.default_ignored_attributes - Audited.ignored_attributes + expected -= @options[:except].map(&:to_s) if @options[:except] end - expects "non audited columns (#{model_class.non_audited_columns.inspect}) to match (#{expect})" - model_class.non_audited_columns =~ except + actual = model_class.new.send(:audited_attributes).keys + expects "audited columns (#{actual.inspect}) to equal (#{expected})" + actual == expected else true end diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index ea360c0a6..759bb2569 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -13,32 +13,37 @@ ['created_at', 'updated_at', 'created_on', 'updated_on', 'lock_version', 'id', 'password'].each do |column| it "should not audit #{column}" do - expect(Models::ActiveRecord::User.non_audited_columns).to include(column) + expect(Models::ActiveRecord::User.new.send(:audited_attributes).keys).to_not include(column) end end it "should be configurable which attributes are not audited via ignored_attributes" do - Audited.ignored_attributes = ['delta', 'top_secret', 'created_at'] - class Secret < ::ActiveRecord::Base - audited + begin + old = Audited.ignored_attributes + Audited.ignored_attributes = ['activated', 'username', 'created_at'] + expect(Models::ActiveRecord::User.new.send(:audited_attributes).keys).to_not include('activated', 'username', 'created_at') + ensure + Audited.ignored_attributes = old end - - expect(Secret.non_audited_columns).to include('delta', 'top_secret', 'created_at') end - it "should be configurable which attributes are not audited via non_audited_columns=" do - class Secret2 < ::ActiveRecord::Base - audited - self.non_audited_columns = ['delta', 'top_secret', 'created_at'] + it "should be configurable which attributes are not audited" do + class User2 < ::ActiveRecord::Base + self.table_name = "users" + audited except: [:activated, :username, :created_at] end - expect(Secret2.non_audited_columns).to include('delta', 'top_secret', 'created_at') + expect(User2.new.send(:audited_attributes).keys).to eq(["name", "password", "suspended_at", "logins", "favourite_device"]) end it "should not save non-audited columns" do - Models::ActiveRecord::User.non_audited_columns = (Models::ActiveRecord::User.non_audited_columns << :favourite_device) + begin + Models::ActiveRecord::User.audited_options[:except] << "favourite_device" - expect(create_user.audits.first.audited_changes.keys.any? { |col| ['favourite_device', 'created_at', 'updated_at', 'password'].include?( col ) }).to eq(false) + expect(create_user.audits.first.send(:audited_changes).keys).to eq(["name", "username", "activated", "suspended_at", "logins"]) + ensure + Models::ActiveRecord::User.audited_options[:except].pop + end end it "should not save other columns than specified in 'only' option" do @@ -135,7 +140,7 @@ def non_column_attr=(val) end it "should store all the audited attributes" do - expect(user.audits.first.audited_changes).to eq(user.audited_attributes) + expect(user.audits.first.audited_changes).to eq(user.send(:audited_attributes)) end it "should store comment" do @@ -238,7 +243,7 @@ def non_column_attr=(val) it "should store all of the audited attributes" do @user.destroy - expect(@user.audits.last.audited_changes).to eq(@user.audited_attributes) + expect(@user.audits.last.audited_changes).to eq(@user.send(:audited_attributes)) end it "should be able to reconstruct a destroyed record without history" do