Merge branch '301124-move-compliance-auditing-to-new-class' into 'master'

Move compliance framework auditor to a new class

See merge request gitlab-org/gitlab!82589

ee/lib/ee/audit/compliance_framework_changes_auditor.rb

+# frozen_string_literal: true
+module EE
+  module Audit
+    class ComplianceFrameworkChangesAuditor < BaseChangesAuditor
+      def initialize(current_user, compliance_framework_setting, project)
+        @project = project
+
+        super(current_user, compliance_framework_setting)
+      end
+
+      def execute
+        return if model.blank?
+
+        if model.destroyed?
+          audit_context = {
+            author: @current_user,
+            scope: @project,
+            target: @project,
+            message: 'Unassigned project compliance framework'
+          }
+
+          ::Gitlab::Audit::Auditor.audit(audit_context)
+        else
+          audit_changes(:framework_id, as: 'compliance framework', model: model, entity: @project)
+        end
+      end
+
+      def framework_changes
+        model.previous_changes["framework_id"]
+      end
+
+      def old_framework_name
+        ComplianceManagement::Framework.find_by_id(framework_changes.first)&.name || "None"
+      end
+
+      def new_framework_name
+        ComplianceManagement::Framework.find_by_id(framework_changes.last)&.name || "None"
+      end
+
+      def attributes_from_auditable_model(column)
+        {
+          from: old_framework_name,
+          to: new_framework_name,
+          target_details: @project.full_path
+        }
+      end
+    end
+  end
+end

ee/lib/ee/audit/project_changes_auditor.rb

@@ -23,39 +23,12 @@ module EE
 
       private
 
-      def audit_compliance_framework_changes
-        setting = model.compliance_framework_setting
-
-        return if setting.blank?
-
-        if setting.destroyed?
-          audit_context = {
-            author: @current_user,
-            scope: model,
-            target: model,
-            message: "Unassigned project compliance framework"
-          }
-
-          ::Gitlab::Audit::Auditor.audit(audit_context)
-        else
-          audit_changes(:framework_id, as: 'compliance framework', model: model.compliance_framework_setting, entity: model)
-        end
-      end
-
       def audit_project_feature_changes
         ::EE::Audit::ProjectFeatureChangesAuditor.new(@current_user, model.project_feature, model).execute
       end
 
-      def framework_changes
-        model.previous_changes["framework_id"]
-      end
-
-      def old_framework_name
-        ComplianceManagement::Framework.find_by_id(framework_changes.first)&.name || "None"
-      end
-
-      def new_framework_name
-        ComplianceManagement::Framework.find_by_id(framework_changes.last)&.name || "None"
+      def audit_compliance_framework_changes
+        ::EE::Audit::ComplianceFrameworkChangesAuditor.new(@current_user, model.compliance_framework_setting, model).execute
       end
 
       def attributes_from_auditable_model(column)
@@ -72,7 +45,7 @@ module EE
           }
         when :visibility_level
           {
-            from:  ::Gitlab::VisibilityLevel.level_name(model.previous_changes[column].first),
+            from: ::Gitlab::VisibilityLevel.level_name(model.previous_changes[column].first),
             to: ::Gitlab::VisibilityLevel.level_name(model.previous_changes[column].last)
           }
         when :namespace_id
@@ -85,11 +58,6 @@ module EE
             from: !model.previous_changes[column].first,
             to: !model.previous_changes[column].last
           }
-        when :framework_id
-          {
-            from: old_framework_name,
-            to: new_framework_name
-          }
         else
           {
             from: model.previous_changes[column].first,

ee/spec/lib/ee/audit/compliance_framework_changes_auditor_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::Audit::ComplianceFrameworkChangesAuditor do
+  describe 'auditing compliance framework changes' do
+    let_it_be(:user) { create(:user) }
+
+    let(:project) { create(:project) }
+
+    before do
+      project.reload
+      stub_licensed_features(extended_audit_events: true)
+    end
+
+    let(:subject) { described_class.new(user, project.compliance_framework_setting, project) }
+
+    context 'when a project has no compliance framework' do
+      context 'when the framework is added' do
+        let_it_be(:framework) { create(:compliance_framework) }
+
+        before do
+          project.update!(compliance_management_framework: framework)
+        end
+
+        it 'adds an audit event' do
+          expect { subject.execute }.to change { AuditEvent.count }.by(1)
+          expect(AuditEvent.last.details).to include({
+                                                       change: 'compliance framework',
+                                                       from: 'None',
+                                                       to: 'GDPR'
+                                                     })
+        end
+      end
+    end
+
+    context 'when a project has a compliance framework' do
+      let_it_be(:framework) { create(:compliance_framework) }
+
+      before do
+        project.update!(compliance_management_framework: framework)
+      end
+      context 'when the framework is removed' do
+        before do
+          project.update!(compliance_management_framework: nil)
+        end
+        it 'adds an audit event' do
+          expect { subject.execute }.to change { AuditEvent.count }.by(1)
+          expect(AuditEvent.last.details).to include({
+                                                       custom_message: "Unassigned project compliance framework"
+                                                     })
+        end
+      end
+
+      context 'when the framework is changed' do
+        before do
+          project.update!(compliance_management_framework: create(:compliance_framework, namespace: project.namespace, name: 'SOX'))
+        end
+
+        it 'adds an audit event' do
+          expect { subject.execute }.to change { AuditEvent.count }.by(1)
+          expect(AuditEvent.last.details).to include({
+                                                       change: 'compliance framework',
+                                                       from: 'GDPR',
+                                                       to: 'SOX'
+                                                     })
+        end
+      end
+    end
+
+    context 'when the framework is not changed' do
+      before do
+        project.update!(description: 'This is a description of a project')
+      end
+
+      it 'does not add an audit event' do
+        expect { subject.execute }.not_to change { AuditEvent.count }
+      end
+    end
+  end
+end

ee/spec/lib/ee/audit/project_changes_auditor_spec.rb

@@ -37,65 +37,6 @@ RSpec.describe EE::Audit::ProjectChangesAuditor do
       end
     end
 
-    describe 'auditing compliance framework changes' do
-      context 'when a project has no compliance framework' do
-        context 'when the framework is changed' do
-          let_it_be(:framework) { create(:compliance_framework) }
-
-          before do
-            project.update!(compliance_management_framework: framework)
-          end
-
-          it 'adds an audit event' do
-            expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
-            expect(AuditEvent.last.details).to include({
-              change: 'compliance framework',
-              from: 'None',
-              to: 'GDPR'
-            })
-          end
-
-          context 'when the framework is removed' do
-            before do
-              project.update!(compliance_management_framework: nil)
-            end
-
-            it 'adds an audit event' do
-              expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
-              expect(AuditEvent.last.details).to include({
-                custom_message: "Unassigned project compliance framework"
-              })
-            end
-          end
-
-          context 'when the framework is changed again' do
-            before do
-              project.update!(compliance_management_framework: create(:compliance_framework, namespace: project.namespace, name: 'SOX'))
-            end
-
-            it 'adds an audit event' do
-              expect { foo_instance.execute }.to change { AuditEvent.count }.by(1)
-              expect(AuditEvent.last.details).to include({
-                change: 'compliance framework',
-                from: 'GDPR',
-                to: 'SOX'
-              })
-            end
-          end
-        end
-
-        context 'when the framework is not changed' do
-          before do
-            project.update!(description: 'This is a description of a project')
-          end
-
-          it 'does not add an audit event' do
-            expect { foo_instance.execute }.not_to change { AuditEvent.count }
-          end
-        end
-      end
-    end
-
     describe 'audit changes' do
       it 'creates an event when the visibility change' do
         project.update!(visibility_level: 20)