From d1e046b7b7b88654bfc24c78d85101759d8e4cb6 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Mon, 3 Feb 2025 15:41:33 -0700
Subject: [PATCH 01/10] Add find_or_initialize_project_control_status

This method will find or initialize a control status for a given project
and control. If the status does not exist, it will be created with the
'pending' status. This will allow us to find a control's status for a
given project.

Changelog: added
EE: true
---
 .../compliance_requirements_control.rb        | 15 +++++++
 .../compliance_requirements_control_spec.rb   | 41 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb b/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
index ec3007d0c726e0..60b38b7448e7d4 100644
--- a/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
+++ b/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
@@ -57,6 +57,21 @@ def expression_as_hash(symbolize_names: false)
         nil
       end
 
+      def find_or_initialize_project_control_status(project_id)
+        project_control_compliance_statuses.find_or_initialize_by(
+          compliance_requirements_control_id: id,
+          project_id: project_id
+        ).tap do |project_control_status|
+          if project_control_status.new_record?
+            project_control_status.assign_attributes(
+              status: ProjectControlComplianceStatus::PENDING_STATUS,
+              compliance_requirement_id: compliance_requirement_id,
+              namespace_id: namespace_id
+            )
+          end
+        end
+      end
+
       private
 
       def controls_count_per_requirement
diff --git a/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb b/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
index 2fe9fceec8319e..8ef5a14490961c 100644
--- a/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
+++ b/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
@@ -256,4 +256,45 @@
       end
     end
   end
+
+  describe '#find_or_initialize_project_control_status' do
+    let_it_be(:namespace) { create(:namespace) }
+    let_it_be(:project) { create(:project, namespace: namespace) }
+    let_it_be(:compliance_requirement) { create(:compliance_requirement, namespace: namespace) }
+    let_it_be(:compliance_control) do
+      create(:compliance_requirements_control,
+        namespace: namespace,
+        compliance_requirement: compliance_requirement)
+    end
+
+    subject(:control_status) { compliance_control.find_or_initialize_project_control_status(project.id) }
+
+    context 'when project control status already exists' do
+      let_it_be(:existing_status) do
+        create(:project_control_compliance_status,
+          compliance_requirements_control: compliance_control,
+          project: project,
+          namespace: namespace,
+          status: 'pass')
+      end
+
+      it 'returns the existing status without modification' do
+        expect(control_status).to eq(existing_status)
+        expect(control_status.status).to eq('pass')
+      end
+    end
+
+    context 'when project control status does not exist' do
+      it 'initializes a new status with pending state' do
+        expect(control_status).to be_new_record
+        expect(control_status).to have_attributes(
+          compliance_requirements_control_id: compliance_control.id,
+          project_id: project.id,
+          namespace_id: namespace.id,
+          compliance_requirement_id: compliance_requirement.id,
+          status: 'pending'
+        )
+      end
+    end
+  end
 end
-- 
GitLab


From 6013e0c7c9d6dbc5fdc87f2fff9be45add8c520d Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Mon, 3 Feb 2025 17:49:44 -0700
Subject: [PATCH 02/10] Add pass and fail audit event types

These events are used to track the status of compliance controls.
---
 doc/user/compliance/audit_event_types.md               |  2 ++
 .../types/compliance_control_status_fail.yml           | 10 ++++++++++
 .../types/compliance_control_status_pass.yml           | 10 ++++++++++
 3 files changed, 22 insertions(+)
 create mode 100644 ee/config/audit_events/types/compliance_control_status_fail.yml
 create mode 100644 ee/config/audit_events/types/compliance_control_status_pass.yml

diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md
index 9d7d2a92d104bb..5975d684912528 100644
--- a/doc/user/compliance/audit_event_types.md
+++ b/doc/user/compliance/audit_event_types.md
@@ -148,6 +148,8 @@ Audit event types belong to the following product categories.
 | [`allow_committer_approval_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102256) | Prevent approvals by users who add commits setting is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/373949) | Group |
 | [`allow_overrides_to_approver_list_per_merge_request_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102256) | Prevent editing approval rules in projects and merge requests setting is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.6](https://gitlab.com/gitlab-org/gitlab/-/issues/373949) | Group |
 | [`audit_events_streaming_headers_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | A streaming header for audit events is updated | {{< icon name="check-circle" >}} Yes | GitLab [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | Group |
+| [`compliance_control_status_fail`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107) | A compliance control status is updated to fail | {{< icon name="check-circle" >}} Yes | GitLab [17.10](https://gitlab.com/gitlab-org/gitlab/-/issues/513425) | Project |
+| [`compliance_control_status_pass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107) | A compliance control status is updated to pass | {{< icon name="check-circle" >}} Yes | GitLab [17.10](https://gitlab.com/gitlab-org/gitlab/-/issues/513425) | Project |
 | [`compliance_framework_added`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157893) | A compliance framework is applied to a project | {{< icon name="check-circle" >}} Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/464160) | Project |
 | [`compliance_framework_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65343) | A compliance framework is removed from a project | {{< icon name="check-circle" >}} Yes | GitLab [14.1](https://gitlab.com/gitlab-org/gitlab/-/issues/329362) | Project |
 | [`compliance_framework_id_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/94711) | A compliance framework is updated for a project | {{< icon name="check-circle" >}} Yes | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369310) | Project |
diff --git a/ee/config/audit_events/types/compliance_control_status_fail.yml b/ee/config/audit_events/types/compliance_control_status_fail.yml
new file mode 100644
index 00000000000000..e4a4216dac5fd3
--- /dev/null
+++ b/ee/config/audit_events/types/compliance_control_status_fail.yml
@@ -0,0 +1,10 @@
+---
+name: compliance_control_status_fail
+description: A compliance control status is updated to fail
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/513425
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107
+feature_category: compliance_management
+milestone: '17.10'
+saved_to_database: true
+streamed: true
+scope: [Project]
diff --git a/ee/config/audit_events/types/compliance_control_status_pass.yml b/ee/config/audit_events/types/compliance_control_status_pass.yml
new file mode 100644
index 00000000000000..a3e056b3c08432
--- /dev/null
+++ b/ee/config/audit_events/types/compliance_control_status_pass.yml
@@ -0,0 +1,10 @@
+---
+name: compliance_control_status_pass
+description: A compliance control status is updated to pass
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/513425
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180107
+feature_category: compliance_management
+milestone: '17.10'
+saved_to_database: true
+streamed: true
+scope: [Project]
-- 
GitLab


From 56978540e08a73cf58fd975dd608259bc98e14b7 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Mon, 3 Feb 2025 15:59:18 -0700
Subject: [PATCH 03/10] Add a new
 ComplianceRequirementsControls::UpdateStatusService

This service provides a single place to update the status of a
compliance control for a given project. It will also audit the
status changes.

It assumes that authentication and authorization has already been
performed.
---
 .../update_status_service.rb                  |  81 ++++++++++
 .../update_status_service_spec.rb             | 143 ++++++++++++++++++
 locale/gitlab.pot                             |   3 +
 3 files changed, 227 insertions(+)
 create mode 100644 ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
 create mode 100644 ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb

diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
new file mode 100644
index 00000000000000..afeaa29aed1704
--- /dev/null
+++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+module ComplianceManagement
+  module ComplianceFramework
+    module ComplianceRequirementsControls
+      class UpdateStatusService < BaseService
+        include Gitlab::Utils::StrongMemoize
+
+        VALID_STATUSES = %w[pass fail].freeze
+
+        def initialize(control:, project:, status_value:)
+          @control = control
+          @project = project
+          @status_value = status_value
+        end
+
+        def execute
+          return error('Not permitted to update compliance control status') unless permitted?
+          return error("'#{status_value}' is not a valid status") unless valid_status?
+
+          return error(control_status.errors.full_messages.join(', ')) unless update_control_status
+
+          audit_changes
+          success
+        rescue ArgumentError => e
+          error(e.message)
+        end
+
+        private
+
+        attr_reader :control, :project, :status_value
+
+        def audit_changes
+          control_status.previous_changes.each do |attribute, changes|
+            next if attribute.eql?('updated_at')
+
+            audit_context = {
+              name: "compliance_control_status_#{status_value}",
+              scope: project,
+              target: control_status,
+              message: "Changed compliance control status #{attribute} from '#{changes[0]}' to '#{changes[1]}'",
+              author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(External)')
+            }
+
+            ::Gitlab::Audit::Auditor.audit(audit_context)
+          end
+        end
+
+        def permitted?
+          project.licensed_feature_available?(:custom_compliance_frameworks) && control.external?
+        end
+
+        def update_control_status
+          control_status.update(status: status_value)
+        end
+
+        def success
+          ServiceResponse.success(payload: { status: control_status.status })
+        end
+
+        def valid_status?
+          status_value.in?(VALID_STATUSES)
+        end
+
+        def control_status
+          control.find_or_initialize_project_control_status(project.id)
+        end
+        strong_memoize_attr :control_status
+
+        def error(error_message)
+          ServiceResponse.error(
+            message: format(
+              _("Failed to update compliance control status. Error: %{error_message}"),
+              error_message: error_message
+            )
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
new file mode 100644
index 00000000000000..ba22ef667510fe
--- /dev/null
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ComplianceManagement::ComplianceFramework::ComplianceRequirementsControls::UpdateStatusService,
+  feature_category: :compliance_management do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:requirement) do
+    create(:compliance_requirement, framework: create(:compliance_framework))
+  end
+
+  let_it_be(:control) do
+    create(:compliance_requirements_control,
+      compliance_requirement: requirement,
+      control_type: :external,
+      external_url: 'https://example.com',
+      secret_token: 'foo')
+  end
+
+  let_it_be(:project_control_compliance_status) do
+    create(:project_control_compliance_status,
+      project: project,
+      compliance_requirements_control: control,
+      compliance_requirement: requirement,
+      status: 'pending')
+  end
+
+  subject(:service) { described_class.new(control: control, project: project, status_value: 'pass') }
+
+  context 'when feature is disabled' do
+    before do
+      stub_licensed_features(custom_compliance_frameworks: false)
+      allow(::Gitlab::Audit::Auditor).to receive(:audit)
+    end
+
+    it 'returns an error' do
+      result = service.execute
+
+      expect(result.success?).to be false
+      expect(result.message).to eq(
+        _('Failed to update compliance control status. Error: Not permitted to update compliance control status')
+      )
+    end
+  end
+
+  context 'when feature is enabled' do
+    before do
+      stub_licensed_features(custom_compliance_frameworks: true)
+      allow(::Gitlab::Audit::Auditor).to receive(:audit)
+    end
+
+    context 'when status is valid' do
+      it 'updates the compliance requirement control status' do
+        expect { service.execute }.to change { project_control_compliance_status.reload.status }.to('pass')
+      end
+
+      it 'is successful' do
+        result = service.execute
+
+        expect(result.success?).to be true
+        expect(result.payload[:status]).to eq('pass')
+      end
+
+      it 'audits the changes' do
+        expected_author = instance_of(::Gitlab::Audit::UnauthenticatedAuthor)
+
+        service.execute
+
+        expect(::Gitlab::Audit::Auditor).to have_received(:audit).with(
+          name: 'compliance_control_status_pass',
+          scope: project,
+          target: project_control_compliance_status,
+          message: "Changed compliance control status status from 'pending' to 'pass'",
+          author: expected_author
+        )
+      end
+
+      context 'when project control status does not exist' do
+        before do
+          project_control_compliance_status.destroy!
+        end
+
+        it 'creates a new project control compliance status' do
+          expect { service.execute }.to change {
+            ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus.count
+          }.by(1)
+        end
+
+        it 'sets the correct attributes' do
+          service.execute
+
+          new_status = ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus.last
+          expect(new_status.status).to eq('pass')
+          expect(new_status.project_id).to eq(project.id)
+          expect(new_status.compliance_requirements_control_id).to eq(control.id)
+        end
+      end
+    end
+
+    context 'with invalid params' do
+      shared_examples 'rejects invalid status' do |status|
+        let(:service) { described_class.new(control: control, project: project, status_value: status) }
+
+        it "does not update project control compliance status" do
+          expect { service.execute }.not_to change { project_control_compliance_status.reload.attributes }
+        end
+
+        it "is unsuccessful" do
+          result = service.execute
+
+          expect(result.success?).to be false
+          expect(result.message).to eq(
+            "Failed to update compliance control status. Error: '#{status}' is not a valid status"
+          )
+        end
+
+        it "does not audit the changes" do
+          service.execute
+
+          expect(::Gitlab::Audit::Auditor).not_to have_received(:audit)
+        end
+      end
+
+      it_behaves_like 'rejects invalid status', 'pending'
+      it_behaves_like 'rejects invalid status', 'invalid'
+    end
+
+    context 'when control is not external' do
+      before do
+        control.update!(control_type: :internal)
+      end
+
+      it 'returns an error' do
+        result = service.execute
+
+        expect(result.success?).to be false
+        expect(result.message).to eq(
+          _('Failed to update compliance control status. Error: Not permitted to update compliance control status')
+        )
+      end
+    end
+  end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f89965a0ffa9bd..ed44aab0e62cca 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24604,6 +24604,9 @@ msgstr ""
 msgid "Failed to update branch!"
 msgstr ""
 
+msgid "Failed to update compliance control status. Error: %{error_message}"
+msgstr ""
+
 msgid "Failed to update compliance requirement"
 msgstr ""
 
-- 
GitLab


From 012516c6d64fc6241624624312741af3041c4b7b Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Wed, 12 Feb 2025 12:14:20 -0700
Subject: [PATCH 04/10] Update status service to use
 create_or_find_for_project_and_control

We're introducing this method in another MR, and it's similar enough to
the one in this MR that we can use it here.
---
 .../compliance_requirements_control.rb        | 15 ----
 .../project_control_compliance_status.rb      | 15 ++++
 .../update_status_service.rb                  |  3 +-
 .../compliance_requirements_control_spec.rb   | 41 -----------
 .../project_control_compliance_status_spec.rb | 73 +++++++++++++++++++
 5 files changed, 90 insertions(+), 57 deletions(-)

diff --git a/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb b/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
index 60b38b7448e7d4..ec3007d0c726e0 100644
--- a/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
+++ b/ee/app/models/compliance_management/compliance_framework/compliance_requirements_control.rb
@@ -57,21 +57,6 @@ def expression_as_hash(symbolize_names: false)
         nil
       end
 
-      def find_or_initialize_project_control_status(project_id)
-        project_control_compliance_statuses.find_or_initialize_by(
-          compliance_requirements_control_id: id,
-          project_id: project_id
-        ).tap do |project_control_status|
-          if project_control_status.new_record?
-            project_control_status.assign_attributes(
-              status: ProjectControlComplianceStatus::PENDING_STATUS,
-              compliance_requirement_id: compliance_requirement_id,
-              namespace_id: namespace_id
-            )
-          end
-        end
-      end
-
       private
 
       def controls_count_per_requirement
diff --git a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
index c0b2ea27c5d444..22665deadd1e57 100644
--- a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
+++ b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
@@ -21,6 +21,21 @@ class ProjectControlComplianceStatus < ApplicationRecord
       scope :for_project_and_control, ->(project_id, control_id) {
         where(project_id: project_id, compliance_requirements_control_id: control_id)
       }
+
+      def self.create_or_find_for_project_and_control(project, control)
+        record = for_project_and_control(project.id, control.id).first
+        return record if record.present?
+
+        create!(
+          compliance_requirements_control: control,
+          project: project,
+          compliance_requirement_id: control.compliance_requirement_id,
+          namespace_id: project.namespace_id,
+          status: :pending
+        )
+      rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
+        for_project_and_control(project.id, control.id).first
+      end
     end
   end
 end
diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
index afeaa29aed1704..939af933de15e6 100644
--- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
+++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
@@ -63,7 +63,8 @@ def valid_status?
         end
 
         def control_status
-          control.find_or_initialize_project_control_status(project.id)
+          ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus
+            .create_or_find_for_project_and_control(project, control)
         end
         strong_memoize_attr :control_status
 
diff --git a/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb b/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
index 8ef5a14490961c..2fe9fceec8319e 100644
--- a/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
+++ b/ee/spec/models/compliance_management/compliance_framework/compliance_requirements_control_spec.rb
@@ -256,45 +256,4 @@
       end
     end
   end
-
-  describe '#find_or_initialize_project_control_status' do
-    let_it_be(:namespace) { create(:namespace) }
-    let_it_be(:project) { create(:project, namespace: namespace) }
-    let_it_be(:compliance_requirement) { create(:compliance_requirement, namespace: namespace) }
-    let_it_be(:compliance_control) do
-      create(:compliance_requirements_control,
-        namespace: namespace,
-        compliance_requirement: compliance_requirement)
-    end
-
-    subject(:control_status) { compliance_control.find_or_initialize_project_control_status(project.id) }
-
-    context 'when project control status already exists' do
-      let_it_be(:existing_status) do
-        create(:project_control_compliance_status,
-          compliance_requirements_control: compliance_control,
-          project: project,
-          namespace: namespace,
-          status: 'pass')
-      end
-
-      it 'returns the existing status without modification' do
-        expect(control_status).to eq(existing_status)
-        expect(control_status.status).to eq('pass')
-      end
-    end
-
-    context 'when project control status does not exist' do
-      it 'initializes a new status with pending state' do
-        expect(control_status).to be_new_record
-        expect(control_status).to have_attributes(
-          compliance_requirements_control_id: compliance_control.id,
-          project_id: project.id,
-          namespace_id: namespace.id,
-          compliance_requirement_id: compliance_requirement.id,
-          status: 'pending'
-        )
-      end
-    end
-  end
 end
diff --git a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
index f0ae6bec404e70..5a90b40a4f9075 100644
--- a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
+++ b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
@@ -87,4 +87,77 @@
       expect(result).to contain_exactly(another_project_status)
     end
   end
+
+  describe '.create_or_find_for_project_and_control' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:control) { create(:compliance_requirements_control) }
+
+    context 'when record does not exist' do
+      it 'creates a new record' do
+        expect do
+          described_class.create_or_find_for_project_and_control(project, control)
+        end.to change { described_class.count }.by(1)
+      end
+
+      it 'sets correct attributes', :aggregate_failures do
+        status = described_class.create_or_find_for_project_and_control(project, control)
+
+        expect(status.project_id).to eq(project.id)
+        expect(status.compliance_requirements_control_id).to eq(control.id)
+        expect(status.compliance_requirement_id).to eq(control.compliance_requirement_id)
+        expect(status.namespace_id).to eq(project.namespace_id)
+        expect(status).to be_pending
+      end
+    end
+
+    context 'when record exists' do
+      let_it_be(:existing_status) do
+        create(:project_control_compliance_status,
+          project: project,
+          compliance_requirements_control: control)
+      end
+
+      it 'returns existing record' do
+        status = described_class.create_or_find_for_project_and_control(project, control)
+
+        expect(status).to eq(existing_status)
+      end
+
+      it 'does not create a new record' do
+        expect do
+          described_class.create_or_find_for_project_and_control(project, control)
+        end.not_to change { described_class.count }
+      end
+    end
+
+    context 'when concurrent creation occurs' do
+      [ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique].each do |error|
+        context "when #{error} is raised" do
+          let!(:existing_status) do
+            create(:project_control_compliance_status,
+              project: project,
+              compliance_requirements_control: control)
+          end
+
+          before do
+            empty_relation = described_class.none
+            record_relation = described_class.where(id: existing_status.id)
+
+            allow(described_class).to receive(:for_project_and_control)
+                                        .with(project.id, control.id)
+                                        .and_return(empty_relation, record_relation)
+
+            allow(described_class).to receive(:create!)
+                                        .and_raise(error)
+          end
+
+          it 'handles race condition and returns existing record' do
+            status = described_class.create_or_find_for_project_and_control(project, control)
+
+            expect(status).to eq(existing_status)
+          end
+        end
+      end
+    end
+  end
 end
-- 
GitLab


From a1e937d7c1addbb9de23cddeb4f550dc2245d721 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Thu, 13 Feb 2025 17:35:43 -0700
Subject: [PATCH 05/10] Update UpdateStatusService to work with all control
 types

We don't need to limit UpdateStatusService to work with only external
controls, and can update the service to work with all control types.
---
 .../update_status_service.rb                      |  4 ++--
 .../update_status_service_spec.rb                 | 15 ---------------
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
index 939af933de15e6..354329eb9785d0 100644
--- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
+++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
@@ -39,7 +39,7 @@ def audit_changes
               scope: project,
               target: control_status,
               message: "Changed compliance control status #{attribute} from '#{changes[0]}' to '#{changes[1]}'",
-              author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(External)')
+              author: ::Gitlab::Audit::UnauthenticatedAuthor.new
             }
 
             ::Gitlab::Audit::Auditor.audit(audit_context)
@@ -47,7 +47,7 @@ def audit_changes
         end
 
         def permitted?
-          project.licensed_feature_available?(:custom_compliance_frameworks) && control.external?
+          project.licensed_feature_available?(:custom_compliance_frameworks)
         end
 
         def update_control_status
diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
index ba22ef667510fe..727d8f25f5b575 100644
--- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -124,20 +124,5 @@
       it_behaves_like 'rejects invalid status', 'pending'
       it_behaves_like 'rejects invalid status', 'invalid'
     end
-
-    context 'when control is not external' do
-      before do
-        control.update!(control_type: :internal)
-      end
-
-      it 'returns an error' do
-        result = service.execute
-
-        expect(result.success?).to be false
-        expect(result.message).to eq(
-          _('Failed to update compliance control status. Error: Not permitted to update compliance control status')
-        )
-      end
-    end
   end
 end
-- 
GitLab


From e6dba64ccc87106a67ebdfe2221591e28f495fec Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Tue, 18 Feb 2025 10:24:49 -0700
Subject: [PATCH 06/10] Add handling for different ActiveRecord::RecordInvalid
 errors

We were rescuing all ActiveRecord::RecordInvalid errors, but we should be
only rescuing the ones that are caused by the project_id uniqueness
constraint.
---
 .../project_control_compliance_status.rb      |  11 +-
 .../project_control_compliance_status_spec.rb | 105 +++++++++++++-----
 2 files changed, 89 insertions(+), 27 deletions(-)

diff --git a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
index 22665deadd1e57..ee60ebeeb512a8 100644
--- a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
+++ b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
@@ -33,8 +33,15 @@ def self.create_or_find_for_project_and_control(project, control)
           namespace_id: project.namespace_id,
           status: :pending
         )
-      rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
-        for_project_and_control(project.id, control.id).first
+      rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e
+        if e.is_a?(ActiveRecord::RecordNotUnique) ||
+            (e.is_a?(ActiveRecord::RecordInvalid) && e.record&.errors&.details&.[](:project_id)&.any? do |error|
+              error[:error] == :taken
+            end)
+          for_project_and_control(project.id, control.id).first
+        else
+          raise e
+        end
       end
     end
   end
diff --git a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
index 5a90b40a4f9075..8b39a9a56cd2f4 100644
--- a/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
+++ b/ee/spec/models/compliance_management/compliance_framework/project_control_compliance_status_spec.rb
@@ -131,32 +131,87 @@
     end
 
     context 'when concurrent creation occurs' do
-      [ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique].each do |error|
-        context "when #{error} is raised" do
-          let!(:existing_status) do
-            create(:project_control_compliance_status,
-              project: project,
-              compliance_requirements_control: control)
-          end
-
-          before do
-            empty_relation = described_class.none
-            record_relation = described_class.where(id: existing_status.id)
-
-            allow(described_class).to receive(:for_project_and_control)
-                                        .with(project.id, control.id)
-                                        .and_return(empty_relation, record_relation)
-
-            allow(described_class).to receive(:create!)
-                                        .and_raise(error)
-          end
-
-          it 'handles race condition and returns existing record' do
-            status = described_class.create_or_find_for_project_and_control(project, control)
-
-            expect(status).to eq(existing_status)
-          end
+      context "when ActiveRecord::RecordNotUnique is raised" do
+        let!(:existing_status) do
+          create(:project_control_compliance_status,
+            project: project,
+            compliance_requirements_control: control)
         end
+
+        before do
+          empty_relation = described_class.none
+          record_relation = described_class.where(id: existing_status.id)
+
+          allow(described_class).to receive(:for_project_and_control)
+                                      .with(project.id, control.id)
+                                      .and_return(empty_relation, record_relation)
+
+          allow(described_class).to receive(:create!)
+                                      .and_raise(ActiveRecord::RecordNotUnique)
+        end
+
+        it 'handles race condition and returns existing record' do
+          status = described_class.create_or_find_for_project_and_control(project, control)
+
+          expect(status).to eq(existing_status)
+        end
+      end
+
+      context "when ActiveRecord::RecordInvalid is raised" do
+        let!(:existing_status) do
+          create(:project_control_compliance_status,
+            project: project,
+            compliance_requirements_control: control)
+        end
+
+        before do
+          empty_relation = described_class.none
+          record_relation = described_class.where(id: existing_status.id)
+
+          allow(described_class).to receive(:for_project_and_control)
+                                      .with(project.id, control.id)
+                                      .and_return(empty_relation, record_relation)
+
+          allow(described_class).to receive(:create!)
+                                      .and_raise(
+                                        ActiveRecord::RecordInvalid.new(
+                                          existing_status.tap do |status|
+                                            status.errors.add(:project_id, :taken, message: "has already been taken")
+                                          end
+                                        )
+                                      )
+        end
+
+        it 'handles race condition and returns existing record' do
+          status = described_class.create_or_find_for_project_and_control(project, control)
+
+          expect(status).to eq(existing_status)
+        end
+      end
+    end
+
+    context "when ActiveRecord::RecordInvalid isn't cause by project_id" do
+      let!(:existing_status) do
+        create(:project_control_compliance_status,
+          project: project,
+          compliance_requirements_control: control)
+      end
+
+      before do
+        empty_relation = described_class.none
+        record_relation = described_class.where(id: existing_status.id)
+
+        allow(described_class).to receive(:for_project_and_control)
+                                    .with(project.id, control.id)
+                                    .and_return(empty_relation, record_relation)
+
+        allow(described_class).to receive(:create!).and_raise(ActiveRecord::RecordInvalid)
+      end
+
+      it 'raises the error' do
+        expect do
+          described_class.create_or_find_for_project_and_control(project, control)
+        end.to raise_error(ActiveRecord::RecordInvalid)
       end
     end
   end
-- 
GitLab


From 8b1b3d52ee04bfb967dece7a6aad671e3ca5a9f1 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Tue, 18 Feb 2025 10:43:57 -0700
Subject: [PATCH 07/10] Only audit the status field when auditing Control
 Status

The other fields shouldn't be changing here.
---
 .../project_control_compliance_status.rb      |  4 +--
 .../update_status_service.rb                  | 25 +++++++++----------
 .../update_status_service_spec.rb             |  2 +-
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
index ee60ebeeb512a8..4a3a5468f1732f 100644
--- a/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
+++ b/ee/app/models/compliance_management/compliance_framework/project_control_compliance_status.rb
@@ -35,9 +35,7 @@ def self.create_or_find_for_project_and_control(project, control)
         )
       rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e
         if e.is_a?(ActiveRecord::RecordNotUnique) ||
-            (e.is_a?(ActiveRecord::RecordInvalid) && e.record&.errors&.details&.[](:project_id)&.any? do |error|
-              error[:error] == :taken
-            end)
+            (e.is_a?(ActiveRecord::RecordInvalid) && e.record&.errors&.of_kind?(:project_id, :taken))
           for_project_and_control(project.id, control.id).first
         else
           raise e
diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
index 354329eb9785d0..f61ce7f512d29c 100644
--- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
+++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
@@ -31,19 +31,18 @@ def execute
         attr_reader :control, :project, :status_value
 
         def audit_changes
-          control_status.previous_changes.each do |attribute, changes|
-            next if attribute.eql?('updated_at')
-
-            audit_context = {
-              name: "compliance_control_status_#{status_value}",
-              scope: project,
-              target: control_status,
-              message: "Changed compliance control status #{attribute} from '#{changes[0]}' to '#{changes[1]}'",
-              author: ::Gitlab::Audit::UnauthenticatedAuthor.new
-            }
-
-            ::Gitlab::Audit::Auditor.audit(audit_context)
-          end
+          old_status = control_status.status_before_last_save
+          new_status = control_status.status
+
+          audit_context = {
+            name: "compliance_control_status_#{new_status}",
+            scope: project,
+            target: control_status,
+            message: "Changed compliance control status from '#{old_status}' to '#{new_status}'",
+            author: ::Gitlab::Audit::UnauthenticatedAuthor.new
+          }
+
+          ::Gitlab::Audit::Auditor.audit(audit_context)
         end
 
         def permitted?
diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
index 727d8f25f5b575..9bf2bb554412a8 100644
--- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -70,7 +70,7 @@
           name: 'compliance_control_status_pass',
           scope: project,
           target: project_control_compliance_status,
-          message: "Changed compliance control status status from 'pending' to 'pass'",
+          message: "Changed compliance control status from 'pending' to 'pass'",
           author: expected_author
         )
       end
-- 
GitLab


From 23907ccdfac19beb36a5c106461e90df59dc7ef8 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Wed, 19 Feb 2025 11:36:52 -0700
Subject: [PATCH 08/10] Add test coverage

---
 .../update_status_service_spec.rb             | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
index 9bf2bb554412a8..964b9e4700d995 100644
--- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -124,5 +124,47 @@
       it_behaves_like 'rejects invalid status', 'pending'
       it_behaves_like 'rejects invalid status', 'invalid'
     end
+
+    context 'when status update fails' do
+      before do
+        # Stub the existing status record to fail on update
+        allow(project_control_compliance_status).to receive_messages(update: false,
+          errors: instance_double(ActiveModel::Errors, full_messages: ['Some validation error']))
+
+        # Ensure create_or_find_for_project_and_control returns our stubbed instance
+        allow(ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus)
+          .to receive(:create_or_find_for_project_and_control)
+          .with(project, control)
+          .and_return(project_control_compliance_status)
+      end
+
+      it 'returns an error with validation messages' do
+        result = service.execute
+
+        expect(result.success?).to be false
+        expect(result.message).to eq(
+          'Failed to update compliance control status. Error: Some validation error'
+        )
+      end
+
+      it 'does not audit the changes' do
+        service.execute
+
+        expect(::Gitlab::Audit::Auditor).not_to have_received(:audit)
+      end
+    end
+
+    context 'when an ArgumentError is raised' do
+      before do
+        allow(service).to receive(:update_control_status).and_raise(ArgumentError, 'test error message')
+      end
+
+      it 'returns an error response with the error message' do
+        result = service.execute
+
+        expect(result).to be_error
+        expect(result.message).to eq('Failed to update compliance control status. Error: test error message')
+      end
+    end
   end
 end
-- 
GitLab


From 9294a36aa6ebffee926494e569bdbf79f83b10bd Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Fri, 21 Feb 2025 16:05:43 -0700
Subject: [PATCH 09/10] Add current_user as a parameter to the
 update_status_service

This will make the service more flexible and easier to use for different
use cases.
---
 .../update_status_service.rb                           |  5 +++--
 .../update_status_service_spec.rb                      | 10 ++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
index f61ce7f512d29c..23cdb02d2db570 100644
--- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
+++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service.rb
@@ -8,7 +8,8 @@ class UpdateStatusService < BaseService
 
         VALID_STATUSES = %w[pass fail].freeze
 
-        def initialize(control:, project:, status_value:)
+        def initialize(current_user:, control:, project:, status_value:)
+          @current_user = current_user
           @control = control
           @project = project
           @status_value = status_value
@@ -39,7 +40,7 @@ def audit_changes
             scope: project,
             target: control_status,
             message: "Changed compliance control status from '#{old_status}' to '#{new_status}'",
-            author: ::Gitlab::Audit::UnauthenticatedAuthor.new
+            author: current_user
           }
 
           ::Gitlab::Audit::Auditor.audit(audit_context)
diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
index 964b9e4700d995..41262f5a3f5db1 100644
--- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -25,7 +25,11 @@
       status: 'pending')
   end
 
-  subject(:service) { described_class.new(control: control, project: project, status_value: 'pass') }
+  let_it_be(:user) { ::Gitlab::Audit::UnauthenticatedAuthor.new }
+
+  subject(:service) do
+    described_class.new(current_user: user, control: control, project: project, status_value: 'pass')
+  end
 
   context 'when feature is disabled' do
     before do
@@ -99,7 +103,9 @@
 
     context 'with invalid params' do
       shared_examples 'rejects invalid status' do |status|
-        let(:service) { described_class.new(control: control, project: project, status_value: status) }
+        let(:service) do
+          described_class.new(current_user: user, control: control, project: project, status_value: status)
+        end
 
         it "does not update project control compliance status" do
           expect { service.execute }.not_to change { project_control_compliance_status.reload.attributes }
-- 
GitLab


From c0d9d93cf5ecf6d1291cb07353937f431f7bdd65 Mon Sep 17 00:00:00 2001
From: dakotadux <ddux@gitlab.com>
Date: Mon, 24 Feb 2025 15:48:46 -0700
Subject: [PATCH 10/10] Replace unauthenticated author with user in tests

This will allow us to audit the specific user who made the changes
instead of an instance of an unauthenticated author.
---
 .../update_status_service_spec.rb                           | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
index 41262f5a3f5db1..a8c8c0ea4b7b29 100644
--- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
+++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements_controls/update_status_service_spec.rb
@@ -25,7 +25,7 @@
       status: 'pending')
   end
 
-  let_it_be(:user) { ::Gitlab::Audit::UnauthenticatedAuthor.new }
+  let_it_be(:user) { create(:user) }
 
   subject(:service) do
     described_class.new(current_user: user, control: control, project: project, status_value: 'pass')
@@ -66,8 +66,6 @@
       end
 
       it 'audits the changes' do
-        expected_author = instance_of(::Gitlab::Audit::UnauthenticatedAuthor)
-
         service.execute
 
         expect(::Gitlab::Audit::Auditor).to have_received(:audit).with(
@@ -75,7 +73,7 @@
           scope: project,
           target: project_control_compliance_status,
           message: "Changed compliance control status from 'pending' to 'pass'",
-          author: expected_author
+          author: user
         )
       end
 
-- 
GitLab