Normalize options on initialization and minor cleanup

Normalizing options passed on audited initialization simplifies code in
further steps. Also cleaned up the code a bit.

CHANGELOG.md

@@ -27,6 +27,8 @@ Changed
   [#391](https://github.com/collectiveidea/audited/pull/391)
 - Simplify `audited_changes` calculation
   [#389](https://github.com/collectiveidea/audited/pull/389)
+- Normalize options passed to `audited` method
+  [#397](https://github.com/collectiveidea/audited/pull/397)
 
 Fixed
 

lib/audited/auditor.rb

@@ -38,26 +38,29 @@ def audited(options = {})
         # don't allow multiple calls
         return if included_modules.include?(Audited::Auditor::AuditedInstanceMethods)
 
+        extend Audited::Auditor::AuditedClassMethods
+        include Audited::Auditor::AuditedInstanceMethods
+
         class_attribute :audit_associated_with,   instance_writer: false
         class_attribute :audited_options,       instance_writer: false
+        attr_accessor :version, :audit_comment
 
         self.audited_options = options
-        self.audit_associated_with = options[:associated_with]
+        normalize_audited_options
 
-        if options[:comment_required]
+        self.audit_associated_with = audited_options[:associated_with]
+
+        if audited_options[:comment_required]
           validates_presence_of :audit_comment, if: :auditing_enabled
           before_destroy :require_comment
         end
 
-        attr_accessor :audit_comment
-
         has_many :audits, -> { order(version: :asc) }, as: :auditable, class_name: Audited.audit_class.name
         Audited.audit_class.audited_class_names << to_s
 
-        on = Array(options[:on])
-        after_create :audit_create    if on.empty? || on.include?(:create)
-        before_update :audit_update   if on.empty? || on.include?(:update)
-        before_destroy :audit_destroy if on.empty? || on.include?(:destroy)
+        after_create :audit_create    if audited_options[:on].include?(:create)
+        before_update :audit_update   if audited_options[:on].include?(:update)
+        before_destroy :audit_destroy if audited_options[:on].include?(:destroy)
 
         # Define and set after_audit and around_audit callbacks. This might be useful if you want
         # to notify a party after the audit has been created or if you want to access the newly-created
@@ -66,21 +69,12 @@ def audited(options = {})
         set_callback :audit, :after, :after_audit, if: lambda { respond_to?(:after_audit, true) }
         set_callback :audit, :around, :around_audit, if: lambda { respond_to?(:around_audit, true) }
 
-        attr_accessor :version
-
-        extend Audited::Auditor::AuditedClassMethods
-        include Audited::Auditor::AuditedInstanceMethods
-
-        self.auditing_enabled = true
+        enable_auditing
       end
 
       def has_associated_audits
         has_many :associated_audits, as: :associated, class_name: Audited.audit_class.name
       end
-
-      def default_ignored_attributes
-        [primary_key, inheritance_column]
-      end
     end
 
     module AuditedInstanceMethods
@@ -109,11 +103,7 @@ def without_auditing(&block)
       def revisions(from_version = 1)
         audits = self.audits.from_version(from_version)
         return [] if audits.empty?
-        revisions = []
-        audits.each do |audit|
-          revisions << audit.revision
-        end
-        revisions
+        audits.map(&:revision)
       end
 
       # Get a specific revision specified by the version number, or +:previous+
@@ -129,7 +119,7 @@ def revision_at(date_or_time)
 
       # List of attributes that are audited.
       def audited_attributes
-        attributes.except(*non_audited_columns.map(&:to_s))
+        attributes.except(*non_audited_columns)
       end
 
       protected
@@ -176,7 +166,7 @@ def rails_below?(rails_version)
 
       def audited_changes
         all_changes = respond_to?(:changes_to_save) ? changes_to_save : changes
-        if audited_options[:only]
+        if audited_options[:only].present?
           all_changes.slice(*audited_columns)
         else
           all_changes.except(*non_audited_columns)
@@ -230,9 +220,6 @@ def require_comment
         alias_method "#{attr_name}_callback".to_sym, attr_name
       end
 
-      def empty_callback #:nodoc:
-      end
-
       def auditing_enabled
         self.class.auditing_enabled
       end
@@ -248,21 +235,16 @@ def audited_columns
         @audited_columns ||= column_names - non_audited_columns
       end
 
+      # We have to calculate this here since column_names may not be available when `audited` is called
       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
+        @non_audited_columns ||= audited_options[:only].present? ?
+                                 column_names - audited_options[:only] :
+                                 default_ignored_attributes | audited_options[:except]
       end
 
       def non_audited_columns=(columns)
-        @non_audited_columns = columns
+        @audited_columns = nil # reset cached audited columns on assignment
+        @non_audited_columns = columns.map(&:to_s)
       end
 
       # Executes the block with auditing disabled.
@@ -302,6 +284,18 @@ def auditing_enabled
       def auditing_enabled=(val)
         Audited.store["#{table_name}_auditing_enabled"] = val
       end
+
+      protected
+      def default_ignored_attributes
+        [primary_key, inheritance_column] + Audited.ignored_attributes
+      end
+
+      def normalize_audited_options
+        audited_options[:on] = Array.wrap(audited_options[:on])
+        audited_options[:on] = [:create, :update, :destroy] if audited_options[:on].empty?
+        audited_options[:only] = Array.wrap(audited_options[:only]).map(&:to_s)
+        audited_options[:except] = Array.wrap(audited_options[:except]).map(&:to_s)
+      end
     end
   end
 end

spec/audited/auditor_spec.rb

@@ -60,6 +60,25 @@ def non_column_attr=(val)
       expect(user.audits.last.audited_changes.keys).to eq(%w{password})
     end
 
+    it "should save attributes not specified in 'except' option" do
+      user = Models::ActiveRecord::User.create
+      user.instance_eval do
+        def non_column_attr
+          @non_column_attr
+        end
+
+        def non_column_attr=(val)
+          attribute_will_change!("non_column_attr")
+          @non_column_attr = val
+        end
+      end
+
+      user.password = "password"
+      user.non_column_attr = "some value"
+      user.save!
+      expect(user.audits.last.audited_changes.keys).to eq(%w{non_column_attr})
+    end
+
     if ActiveRecord::Base.connection.adapter_name == 'PostgreSQL' && Rails.version >= "4.2.0.0" # Postgres json and jsonb support was added in Rails 4.2
       describe "'json' and 'jsonb' audited_changes column type" do
         let(:migrations_path) { SPEC_ROOT.join("support/active_record/postgres") }