Merge branch 'collectiveidea:main' into conditional-require-railtie

CHANGELOG.md

@@ -1,7 +1,25 @@
 # Audited ChangeLog
 
-## Unreleased
+## 5.3.2 (2023-02-22)
 
+- Touch audit bug fixes - @mcyoung
+  [#662](https://github.com/collectiveidea/audited/pull/662)
+
+## 5.3.1 (2023-02-21)
+
+- Ensure touch support doesn't cause double audits - @mcyoung
+  [#660](https://github.com/collectiveidea/audited/pull/660)
+- Testing Improvements - @vlad-psh
+  [#628](https://github.com/collectiveidea/audited/pull/628)
+- Testing Improvements - @mcyoung
+  [#658](https://github.com/collectiveidea/audited/pull/658)
+
+## 5.3.0 (2023-02-14)
+
+- Audit touch calls - @mcyoung
+  [#657](https://github.com/collectiveidea/audited/pull/657)
+- Allow using with Padrino and other non-Rails projects - @nicduke38degrees
+  [#655](https://github.com/collectiveidea/audited/pull/655)
 - Testing updates - @jdufresne
   [#652](https://github.com/collectiveidea/audited/pull/652)
   [#653](https://github.com/collectiveidea/audited/pull/653)

lib/audited/auditor.rb

@@ -84,6 +84,7 @@ def audited(options = {})
 
         after_create :audit_create if audited_options[:on].include?(:create)
         before_update :audit_update if audited_options[:on].include?(:update)
+        after_touch :audit_touch if audited_options[:on].include?(:touch) && ::ActiveRecord::VERSION::MAJOR >= 6
         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
@@ -230,15 +231,31 @@ def revision_with(attributes)
 
       private
 
-      def audited_changes
-        all_changes = respond_to?(:changes_to_save) ? changes_to_save : changes
+      def audited_changes(for_touch: false)
+        all_changes = if for_touch
+          previous_changes
+        elsif respond_to?(:changes_to_save)
+          changes_to_save
+        else
+          changes
+        end
+
         filtered_changes = \
           if audited_options[:only].present?
             all_changes.slice(*self.class.audited_columns)
           else
             all_changes.except(*self.class.non_audited_columns)
           end
 
+        if for_touch
+          filtered_changes.reject! do |k, v|
+            next unless audits.present?
+
+            audits.last.audited_changes[k].to_json == v.to_json ||
+              audits.last.audited_changes[k].to_json == v[1].to_json
+          end
+        end
+
         filtered_changes = redact_values(filtered_changes)
         filtered_changes = filter_encrypted_attrs(filtered_changes)
         filtered_changes = normalize_enum_changes(filtered_changes)
@@ -324,6 +341,13 @@ def audit_update
         end
       end
 
+      def audit_touch
+        unless (changes = audited_changes(for_touch: true)).empty?
+          write_audit(action: "update", audited_changes: changes,
+            comment: audit_comment)
+        end
+      end
+
       def audit_destroy
         unless new_record?
           write_audit(action: "destroy", audited_changes: audited_attributes,
@@ -474,7 +498,7 @@ def default_ignored_attributes
 
       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[:on] = [:create, :update, :touch, :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)
         max_audits = audited_options[:max_audits] || Audited.max_audits

lib/audited/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Audited
-  VERSION = "5.2.0"
+  VERSION = "5.3.2"
 end

spec/audited/auditor_spec.rb

@@ -1,6 +1,9 @@
 require "spec_helper"
 
-SingleCov.covered! uncovered: 9 # not testing proxy_respond_to? hack / 2 methods / deprecation of `version`
+# not testing proxy_respond_to? hack / 2 methods / deprecation of `version`
+# also, an additional 6 around `after_touch` for Versions before 6.
+uncovered = (ActiveRecord::VERSION::MAJOR < 6) ? 15 : 9
+SingleCov.covered! uncovered: uncovered
 
 class ConditionalPrivateCompany < ::ActiveRecord::Base
   self.table_name = "companies"
@@ -143,7 +146,7 @@ class Secret2 < ::ActiveRecord::Base
     end
 
     it "should be configurable which attributes are not audited via ignored_attributes" do
-      Audited.ignored_attributes = ["delta", "top_secret", "created_at"]
+      Audited.ignored_attributes = ["delta", "top_secret", "created_at", "updated_at"]
 
       expect(Secret.non_audited_columns).to include("delta", "top_secret", "created_at")
     end
@@ -215,17 +218,25 @@ def non_column_attr=(val)
       redacted = Audited::Auditor::AuditedInstanceMethods::REDACTED
       user =
         Models::ActiveRecord::UserMultipleRedactedAttributes.create(
-          password: "password",
-          ssn: 123456789
+          password: "password"
         )
       user.save!
       expect(user.audits.last.audited_changes["password"]).to eq(redacted)
+      # Saving '[REDACTED]' value for 'ssn' even if value wasn't set explicitly when record was created
       expect(user.audits.last.audited_changes["ssn"]).to eq(redacted)
+
       user.password = "new_password"
       user.ssn = 987654321
       user.save!
       expect(user.audits.last.audited_changes["password"]).to eq([redacted, redacted])
       expect(user.audits.last.audited_changes["ssn"]).to eq([redacted, redacted])
+
+      # If we haven't changed any attrs from 'redacted' list, audit should not contain these keys
+      user.name = "new name"
+      user.save!
+      expect(user.audits.last.audited_changes).to have_key("name")
+      expect(user.audits.last.audited_changes).not_to have_key("password")
+      expect(user.audits.last.audited_changes).not_to have_key("ssn")
     end
 
     it "should redact columns in 'redacted' column with custom option" do
@@ -414,6 +425,99 @@ def non_column_attr=(val)
     end
   end
 
+  if ::ActiveRecord::VERSION::MAJOR >= 6
+    describe "on touch" do
+      before do
+        @user = create_user(name: "Brandon", status: :active)
+      end
+
+      it "should save an audit" do
+        expect { @user.touch(:suspended_at) }.to change(Audited::Audit, :count).by(1)
+      end
+
+      it "should set the action to 'update'" do
+        @user.touch(:suspended_at)
+        expect(@user.audits.last.action).to eq("update")
+        expect(Audited::Audit.updates.order(:id).last).to eq(@user.audits.last)
+        expect(@user.audits.updates.last).to eq(@user.audits.last)
+      end
+
+      it "should store the changed attributes" do
+        @user.touch(:suspended_at)
+        expect(@user.audits.last.audited_changes["suspended_at"][0]).to be_nil
+        expect(Time.parse(@user.audits.last.audited_changes["suspended_at"][1].to_s)).to be_within(2.seconds).of(Time.current)
+      end
+
+      it "should store audit comment" do
+        @user.audit_comment = "Here exists a touch comment"
+        @user.touch(:suspended_at)
+        expect(@user.audits.last.action).to eq("update")
+        expect(@user.audits.last.comment).to eq("Here exists a touch comment")
+      end
+
+      it "should not save an audit if only specified on create/destroy" do
+        on_create_destroy = Models::ActiveRecord::OnCreateDestroyUser.create(name: "Bart")
+        expect {
+          on_create_destroy.touch(:suspended_at)
+        }.to_not change(Audited::Audit, :count)
+      end
+
+      it "should store an audit if touch is the only audit" do
+        on_touch = Models::ActiveRecord::OnTouchOnly.create(name: "Bart")
+        expect {
+          on_touch.update(name: "NotBart")
+        }.to_not change(Audited::Audit, :count)
+        expect {
+          on_touch.touch(:suspended_at)
+        }.to change(on_touch.audits, :count).from(0).to(1)
+
+        @user.audits.destroy_all
+        expect(@user.audits).to be_empty
+        expect {
+          @user.touch(:suspended_at)
+        }.to change(@user.audits, :count).from(0).to(1)
+      end
+
+      context "don't double audit" do
+        let(:user) { Models::ActiveRecord::Owner.create(name: "OwnerUser", suspended_at: 1.month.ago, companies_attributes: [{name: "OwnedCompany"}]) }
+        let(:company) { user.companies.first }
+
+        it "should only create 1 (create) audit for object" do
+          expect(user.audits.count).to eq(1)
+          expect(user.audits.first.action).to eq("create")
+        end
+
+        it "should only create 1 (create) audit for nested resource" do
+          expect(company.audits.count).to eq(1)
+          expect(company.audits.first.action).to eq("create")
+        end
+
+        context "after creating" do
+          it "updating / touching nested resource shouldn't save touch audit on parent object" do
+            expect { company.touch(:type) }.not_to change(user.audits, :count)
+            expect { company.update(type: "test") }.not_to change(user.audits, :count)
+          end
+
+          it "updating / touching parent object shouldn't save previous data" do
+            expect { user.touch(:suspended_at) }.to change(user.audits, :count).from(1).to(2)
+            expect(user.audits.last.action).to eq("update")
+            expect(user.audits.last.audited_changes.keys).to eq(%w[suspended_at])
+          end
+        end
+
+        context "after updating" do
+          it "changing nested resource shouldn't audit owner" do
+            expect { user.update(username: "test") }.to change(user.audits, :count).from(1).to(2)
+            expect { company.update(type: "test") }.not_to change(user.audits, :count)
+
+            expect { user.touch(:suspended_at) }.to change(user.audits, :count).from(2).to(3)
+            expect { company.update(type: "another_test") }.not_to change(user.audits, :count)
+          end
+        end
+      end
+    end
+  end
+
   describe "on destroy" do
     before do
       @user = create_user(status: :active)

spec/support/active_record/models.rb

@@ -124,11 +124,12 @@ class Owner < ::ActiveRecord::Base
       audited
       has_associated_audits
       has_many :companies, class_name: "OwnedCompany", dependent: :destroy
+      accepts_nested_attributes_for :companies
     end
 
     class OwnedCompany < ::ActiveRecord::Base
       self.table_name = "companies"
-      belongs_to :owner, class_name: "Owner"
+      belongs_to :owner, class_name: "Owner", touch: true
       attr_accessible :name, :owner if respond_to?(:attr_accessible) # declare attr_accessible before calling aaa
       audited associated_with: :owner
     end
@@ -146,6 +147,11 @@ class OnCreateDestroy < ::ActiveRecord::Base
       audited on: [:create, :destroy]
     end
 
+    class OnCreateDestroyUser < ::ActiveRecord::Base
+      self.table_name = "users"
+      audited on: [:create, :destroy]
+    end
+
     class OnCreateDestroyExceptName < ::ActiveRecord::Base
       self.table_name = "companies"
       audited except: :name, on: [:create, :destroy]
@@ -155,5 +161,10 @@ class OnCreateUpdate < ::ActiveRecord::Base
       self.table_name = "companies"
       audited on: [:create, :update]
     end
+
+    class OnTouchOnly < ::ActiveRecord::Base
+      self.table_name = "users"
+      audited on: [:touch]
+    end
   end
 end