Merge branch 'mc/fix-unviolated-rules-violations' into 'master'

Fix policy violations not being deleted properly

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/168373



Merged-by: Sashi Kumar Kumaresan <skumar@gitlab.com>
Approved-by: Sashi Kumar Kumaresan <skumar@gitlab.com>
Co-authored-by: Martin Čavoj <mcavoj@gitlab.com>

ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb

@@ -47,7 +47,7 @@ def update_required_approvals(violated_rules, unviolated_rules)
         ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any?
 
         log_violated_rules(violated_rules)
-        violations.add(violated_rules.map(&:scan_result_policy_read), unviolated_rules)
+        violations.add(violated_rules.map(&:scan_result_policy_read), unviolated_rules.map(&:scan_result_policy_read))
         violations.execute
       end
 

ee/app/services/security/scan_result_policies/update_license_approvals_service.rb

@@ -57,7 +57,7 @@ def update_approvals(license_approval_rules)
         merge_request.reset_required_approvals(violated_rules)
         ApprovalMergeRequestRule.remove_required_approved(unviolated_rules)
 
-        violations.add(violated_rules.map(&:scan_result_policy_read), unviolated_rules)
+        violations.add(violated_rules.map(&:scan_result_policy_read), unviolated_rules.map(&:scan_result_policy_read))
         violations.execute
 
         violated_rules.each do |approval_rule|

ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb

@@ -143,7 +143,7 @@
         it_behaves_like 'sets approvals_required to 0'
         it_behaves_like 'triggers policy bot comment', :scan_finding, false
         it_behaves_like 'does not log violations'
-        it_behaves_like 'merge request without scan result violations', previous_violation: false
+        it_behaves_like 'merge request without scan result violations'
 
         context 'when there are other scan_finding violations' do
           let_it_be_with_reload(:scan_result_policy_read_other_scan_finding) do

ee/spec/services/security/scan_result_policies/update_license_approvals_service_spec.rb

@@ -31,10 +31,14 @@
 
   let_it_be(:preexisting_states) { false }
 
+  let(:scan_result_policy_read) do
+    create(:scan_result_policy_read, project: project, license_states: ['newly_detected'])
+  end
+
   let!(:license_finding_rule) do
     create(:report_approver_rule, :license_scanning,
       merge_request: merge_request,
-      scan_result_policy_read: create(:scan_result_policy_read, project: project, license_states: ['newly_detected']),
+      scan_result_policy_read: scan_result_policy_read,
       approvals_required: 1
     )
   end
@@ -96,7 +100,7 @@
     end
   end
 
-  context 'for prexisting states' do
+  context 'for preexisting states' do
     let_it_be(:preexisting_states) { true }
     let_it_be(:pipeline) { nil }
 
@@ -142,6 +146,7 @@
 
       it_behaves_like 'does not require approvals'
       it_behaves_like 'triggers policy bot comment', :license_scanning, false
+      it_behaves_like 'merge request without scan result violations'
 
       it 'does not call logger' do
         expect(Gitlab::AppJsonLogger).not_to receive(:info)