From 221e1b448817f840605891ee03592caba9a5fb32 Mon Sep 17 00:00:00 2001
From: Sashi Kumar Kumaresan <skumar@gitlab.com>
Date: Thu, 22 Jun 2023 12:02:23 +0000
Subject: [PATCH] Fix approval notification for MRs without scan result
policies
This change fixes a bug in approval notification
where the note was created if MR does not have
scan result policy applied.
EE: true
Changelog: fixed
---
.../scan_result_policies/update_approvals_service.rb | 12 ++++++------
.../update_approvals_service_spec.rb | 10 ++++++++++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/ee/app/services/security/scan_result_policies/update_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
index 618f58a3b1d05..84f3342e987ce 100644
--- a/ee/app/services/security/scan_result_policies/update_approvals_service.rb
+++ b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
@@ -13,19 +13,19 @@ def initialize(merge_request:, pipeline:)
end
def execute
- return if scan_removed? && Feature.disabled?(:security_policy_approval_notification, pipeline.project)
+ return if Feature.disabled?(:security_policy_approval_notification, pipeline.project) && scan_removed?
- violated_rules, unviolated_rules = merge_request.approval_rules.scan_finding.partition do |approval_rule|
+ approval_rules = merge_request.approval_rules.scan_finding
+ return if approval_rules.empty?
+
+ violated_rules, unviolated_rules = approval_rules.partition do |approval_rule|
approval_rule = approval_rule.source_rule if approval_rule.source_rule
violates_approval_rule?(approval_rule)
end
+ ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any? && !scan_removed?
generate_policy_bot_comment(violated_rules.any? || scan_removed?)
-
- return if scan_removed?
-
- ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any?
end
private
diff --git a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
index 8293b2d7d6010..3051304af1f13 100644
--- a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
+++ b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
@@ -106,6 +106,16 @@
end
end
+ context 'when approval rules are empty' do
+ let!(:report_approver_rule) { nil }
+
+ it 'does not enqueue Security::GeneratePolicyViolationCommentWorker' do
+ expect(Security::GeneratePolicyViolationCommentWorker).not_to receive(:perform_async)
+
+ service
+ end
+ end
+
context 'when security scan is removed in current pipeline' do
let_it_be(:pipeline) { create(:ee_ci_pipeline, project: project, ref: merge_request.source_branch) }
--
GitLab