Merge branch '351577-add-severity-level-to-compliance-violations-table' into 'master'

Add severity level to compliance violations table

See merge request gitlab-org/gitlab!79523

db/migrate/20220131135725_add_severity_level_to_merge_requests_compliance_violations.rb

+# frozen_string_literal: true
+
+class AddSeverityLevelToMergeRequestsComplianceViolations < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :merge_requests_compliance_violations, :severity_level, :integer, limit: 2, null: false, default: 0
+  end
+end

db/schema_migrations/20220131135725

+a0e9bb92512b9ddb3fd718e086b0fd116ec307be6acc5789a872b1d3004fdebd
\ No newline at end of file

db/structure.sql

@@ -16489,7 +16489,8 @@ CREATE TABLE merge_requests_compliance_violations (
     id bigint NOT NULL,
     violating_user_id bigint NOT NULL,
     merge_request_id bigint NOT NULL,
-    reason smallint NOT NULL
+    reason smallint NOT NULL,
+    severity_level smallint DEFAULT 0 NOT NULL
 );
 
 CREATE SEQUENCE merge_requests_compliance_violations_id_seq

ee/app/models/concerns/enums/merge_requests/compliance_violation.rb

+# frozen_string_literal: true
+
+module Enums
+  module MergeRequests
+    module ComplianceViolation
+      # Reasons are defined by GitLab in our public documentation.
+      # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties
+      def self.reasons
+        {
+          ::Gitlab::ComplianceManagement::Violations::ApprovedByMergeRequestAuthor::REASON => 0,
+          ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON => 1,
+          ::Gitlab::ComplianceManagement::Violations::ApprovedByInsufficientUsers::REASON => 2
+        }.freeze
+      end
+
+      def self.severity_levels
+        {
+          info: 0,
+          low: 1,
+          medium: 2,
+          high: 3,
+          critical: 4
+        }.freeze
+      end
+    end
+  end
+end

ee/app/models/merge_requests/compliance_violation.rb

@@ -6,13 +6,8 @@ module MergeRequests
 
     self.table_name = 'merge_requests_compliance_violations'
 
-    # Reasons are defined by GitLab in our public documentation.
-    # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties
-    enum reason: {
-      ::Gitlab::ComplianceManagement::Violations::ApprovedByMergeRequestAuthor::REASON => 0,
-      ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON => 1,
-      ::Gitlab::ComplianceManagement::Violations::ApprovedByInsufficientUsers::REASON => 2
-    }
+    enum reason: ::Enums::MergeRequests::ComplianceViolation.reasons
+    enum severity_level: ::Enums::MergeRequests::ComplianceViolation.severity_levels
 
     scope :approved_by_committer, -> { where(reason: ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON) }
 
@@ -27,6 +22,7 @@ module MergeRequests
                 message: -> (_object, _data) { _('compliance violation has already been recorded') }
               }
     validates :reason, presence: true
+    validates :severity_level, presence: true
 
     # The below violations need to either ignore or handle their errors to help prevent the merge process failing
     VIOLATIONS = [

ee/lib/gitlab/compliance_management/violations/approved_by_committer.rb

@@ -5,6 +5,7 @@ module Gitlab
     module Violations
       class ApprovedByCommitter
         REASON = :approved_by_committer
+        SEVERITY_LEVEL = :high
 
         def initialize(merge_request)
           @merge_request = merge_request
@@ -24,7 +25,8 @@ module Gitlab
           violating_user_ids.map do |user_id|
             @merge_request.compliance_violations.new(
               violating_user_id: user_id,
-              reason: REASON
+              reason: REASON,
+              severity_level: SEVERITY_LEVEL
             )
           end
         end

ee/lib/gitlab/compliance_management/violations/approved_by_insufficient_users.rb

@@ -5,6 +5,7 @@ module Gitlab
     module Violations
       class ApprovedByInsufficientUsers
         REASON = :approved_by_insufficient_users
+        SEVERITY_LEVEL = :high
 
         # The minimum number of approvers is defined by GitLab in our public documentation.
         # https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties
@@ -18,7 +19,8 @@ module Gitlab
           if violation?
             @merge_request.compliance_violations.create(
               violating_user: @merge_request.metrics&.merged_by || @merge_request.merge_user,
-              reason: REASON
+              reason: REASON,
+              severity_level: SEVERITY_LEVEL
             )
           end
         end

ee/lib/gitlab/compliance_management/violations/approved_by_merge_request_author.rb

@@ -5,6 +5,7 @@ module Gitlab
     module Violations
       class ApprovedByMergeRequestAuthor
         REASON = :approved_by_merge_request_author
+        SEVERITY_LEVEL = :high
 
         def initialize(merge_request)
           @merge_request = merge_request
@@ -14,7 +15,8 @@ module Gitlab
           if violation?
             @merge_request.compliance_violations.create(
               violating_user: @merge_request.author,
-              reason: REASON
+              reason: REASON,
+              severity_level: SEVERITY_LEVEL
             )
           end
         end

ee/spec/factories/compliance_violations.rb

@@ -7,14 +7,17 @@ FactoryBot.define do
 
     trait :approved_by_merge_request_author do
       reason { :approved_by_merge_request_author }
+      severity_level { :high }
     end
 
     trait :approved_by_committer do
       reason { :approved_by_committer }
+      severity_level { :high }
     end
 
     trait :approved_by_insufficient_users do
       reason { :approved_by_insufficient_users }
+      severity_level { :high }
     end
   end
 end

ee/spec/lib/gitlab/compliance_management/violations/approved_by_committer_spec.rb

@@ -38,6 +38,7 @@ RSpec.describe Gitlab::ComplianceManagement::Violations::ApprovedByCommitter do
         violations = merge_request.compliance_violations.where(reason: described_class::REASON)
 
         expect(violations.map(&:violating_user)).to contain_exactly(user, user2)
+        expect(violations.map(&:severity_level)).to contain_exactly('high', 'high')
       end
 
       context 'when called more than once with the same violations' do
@@ -54,6 +55,7 @@ RSpec.describe Gitlab::ComplianceManagement::Violations::ApprovedByCommitter do
           violations = merge_request.compliance_violations.where(reason: described_class::REASON)
 
           expect(violations.map(&:violating_user)).to contain_exactly(user, user2, user3)
+          expect(violations.map(&:severity_level)).to contain_exactly('high', 'high', 'high')
         end
       end
     end

ee/spec/lib/gitlab/compliance_management/violations/approved_by_insufficient_users_spec.rb

@@ -33,6 +33,7 @@ RSpec.describe Gitlab::ComplianceManagement::Violations::ApprovedByInsufficientU
         violations = merge_request.compliance_violations.where(reason: described_class::REASON)
 
         expect(violations.map(&:violating_user)).to contain_exactly(user)
+        expect(violations.map(&:severity_level)).to contain_exactly('high')
       end
 
       context 'when the merge requests merge user is within metrics' do
@@ -44,6 +45,7 @@ RSpec.describe Gitlab::ComplianceManagement::Violations::ApprovedByInsufficientU
           violations = merge_request.compliance_violations.where(reason: described_class::REASON)
 
           expect(violations.map(&:violating_user)).to contain_exactly(user)
+          expect(violations.map(&:severity_level)).to contain_exactly('high')
         end
       end
 

ee/spec/lib/gitlab/compliance_management/violations/approved_by_merge_request_author_spec.rb

@@ -16,6 +16,7 @@ RSpec.describe Gitlab::ComplianceManagement::Violations::ApprovedByMergeRequestA
         violations = merge_request.compliance_violations.where(reason: described_class::REASON)
 
         expect(violations.map(&:violating_user)).to contain_exactly(author)
+        expect(violations.map(&:severity_level)).to contain_exactly('high')
       end
     end
 

ee/spec/models/merge_requests/compliance_violation_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe MergeRequests::ComplianceViolation, type: :model do
     it { is_expected.to validate_presence_of(:violating_user) }
     it { is_expected.to validate_presence_of(:merge_request) }
     it { is_expected.to validate_presence_of(:reason) }
+    it { is_expected.to validate_presence_of(:severity_level) }
 
     context "when a violation exists with the same reason and user for a merge request" do
       let_it_be(:user) { create(:user) }
@@ -35,15 +36,9 @@ RSpec.describe MergeRequests::ComplianceViolation, type: :model do
   end
 
   describe "Enums" do
-    it {
-      is_expected.to define_enum_for(:reason).with_values(
-        [
-          ::Gitlab::ComplianceManagement::Violations::ApprovedByMergeRequestAuthor::REASON,
-          ::Gitlab::ComplianceManagement::Violations::ApprovedByCommitter::REASON,
-          ::Gitlab::ComplianceManagement::Violations::ApprovedByInsufficientUsers::REASON
-        ]
-      )
-    }
+    it { is_expected.to define_enum_for(:reason).with_values(::Enums::MergeRequests::ComplianceViolation.reasons) }
+
+    it { is_expected.to define_enum_for(:severity_level).with_values(::Enums::MergeRequests::ComplianceViolation.severity_levels) }
   end
 
   describe '#approved_by_committer' do