From dfa7d9bf584ea029c003d8be402e021dd6552dec Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Fri, 18 Jul 2025 10:02:41 +0200 Subject: [PATCH 1/2] Use the approved_approvers data for any_approver post merge MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/198252 --- .../models/approval_wrapped_any_approver_rule.rb | 7 +++++++ .../approval_wrapped_any_approver_rule_spec.rb | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/ee/app/models/approval_wrapped_any_approver_rule.rb b/ee/app/models/approval_wrapped_any_approver_rule.rb index 543b4bef90df52..84160cbabe5594 100644 --- a/ee/app/models/approval_wrapped_any_approver_rule.rb +++ b/ee/app/models/approval_wrapped_any_approver_rule.rb @@ -7,6 +7,13 @@ def name end def approved_approvers + if merge_request.merged? && + approval_rule.is_a?(ApprovalMergeRequestRule) && + approval_rule.approved_approvers.any? + + return filter_approvers(approval_rule.approved_approvers) + end + filter_approvers(merge_request.approved_by_users) end diff --git a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb index 72b846dd755cd4..50bd791791d1ef 100644 --- a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb +++ b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb @@ -42,6 +42,21 @@ expect(subject.approved_approvers).to contain_exactly(approver1, approver2) end end + + context 'when merged' do + let(:merge_request) { create(:merge_request, source_branch: 'test') } + + before do + rule.approved_approvers << approver2 + create(:approval, merge_request: merge_request, user: merge_request.author) + + merge_request.mark_as_merged! + end + + it 'returns approved approvers from database' do + expect(subject.approved_approvers).to contain_exactly(approver2) + end + end end it '#approved?' do -- GitLab From a21a37aad7fe20973082a1c7f0afdec2d47e1722 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Thu, 24 Jul 2025 12:18:44 +0200 Subject: [PATCH 2/2] Add test for filtering approvers --- ee/app/models/approval_wrapped_any_approver_rule.rb | 2 +- ee/spec/models/approval_wrapped_any_approver_rule_spec.rb | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ee/app/models/approval_wrapped_any_approver_rule.rb b/ee/app/models/approval_wrapped_any_approver_rule.rb index 84160cbabe5594..0a7ddeb5f87211 100644 --- a/ee/app/models/approval_wrapped_any_approver_rule.rb +++ b/ee/app/models/approval_wrapped_any_approver_rule.rb @@ -11,7 +11,7 @@ def approved_approvers approval_rule.is_a?(ApprovalMergeRequestRule) && approval_rule.approved_approvers.any? - return filter_approvers(approval_rule.approved_approvers) + return approval_rule.approved_approvers end filter_approvers(merge_request.approved_by_users) diff --git a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb index 50bd791791d1ef..b0bb8362f28b77 100644 --- a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb +++ b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb @@ -47,9 +47,15 @@ let(:merge_request) { create(:merge_request, source_branch: 'test') } before do - rule.approved_approvers << approver2 + merge_request.project.update!( + merge_requests_author_approval: false, + merge_requests_disable_committers_approval: true + ) + create(:approval, merge_request: merge_request, user: merge_request.author) + rule.approved_approvers << approver2 + merge_request.mark_as_merged! end -- GitLab