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 618f58a3b1d056f2a902f52171024676e0c468ca..84f3342e987ce8c4d23ad8e0d84a278fc95bf7a3 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 8293b2d7d60107ff9fa14c5432d6f3ca368ca079..3051304af1f13fa096f08f3d717e0b634a2b8761 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) }