From 11465756f560cf7ee8109ecf92bd83b3e36f26d0 Mon Sep 17 00:00:00 2001 From: Andy Schoenen Date: Tue, 21 Oct 2025 16:39:49 +0200 Subject: [PATCH 1/2] Add tests for preserve_open_policy_dismissals! default branch check This adds comprehensive tests for the policy dismissal preservation functionality to ensure it only preserves dismissals when merge requests are merged into the default branch. Test coverage includes: - Preserving dismissals when merged into default branch - Not preserving when merged into non-default branch - Not preserving when MR is not merged - Handling edge cases like no dismissals - Testing the merged_into_default_branch? helper method Changelog: added EE: true --- .../merge_request_preserve_dismissals_spec.rb | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 ee/spec/models/ee/merge_request_preserve_dismissals_spec.rb diff --git a/ee/spec/models/ee/merge_request_preserve_dismissals_spec.rb b/ee/spec/models/ee/merge_request_preserve_dismissals_spec.rb new file mode 100644 index 00000000000000..a12218020fa1b3 --- /dev/null +++ b/ee/spec/models/ee/merge_request_preserve_dismissals_spec.rb @@ -0,0 +1,158 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'EE::MergeRequest#preserve_open_policy_dismissals!', feature_category: :security_policy_management do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let_it_be(:security_policy) { create(:security_policy) } + + before_all do + project.add_maintainer(user) + end + + describe '#preserve_open_policy_dismissals!' do + context 'when merge request is merged into default branch' do + let!(:merge_request) do + create(:merge_request, :merged, + source_project: project, + target_project: project, + target_branch: project.default_branch) + end + + let!(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + user: user, + status: 'open') + end + + it 'preserves open policy dismissals' do + expect { merge_request.preserve_open_policy_dismissals! } + .to change { policy_dismissal.reload.status }.from('open').to('preserved') + end + + context 'when dismissal is already preserved' do + let!(:preserved_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + user: user, + status: 'preserved') + end + + it 'does not change already preserved dismissals' do + expect { merge_request.preserve_open_policy_dismissals! } + .not_to change { preserved_dismissal.reload.status } + end + end + end + + context 'when merge request is merged into non-default branch' do + let!(:merge_request) do + create(:merge_request, :merged, + source_project: project, + target_project: project, + target_branch: 'feature-branch') + end + + let!(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + user: user, + status: 'open') + end + + it 'does not preserve policy dismissals' do + expect { merge_request.preserve_open_policy_dismissals! } + .not_to change { policy_dismissal.reload.status } + end + end + + context 'when merge request is not merged' do + let!(:merge_request) do + create(:merge_request, + source_project: project, + target_project: project, + target_branch: project.default_branch) + end + + let!(:policy_dismissal) do + create(:policy_dismissal, + project: project, + merge_request: merge_request, + security_policy: security_policy, + user: user, + status: 'open') + end + + it 'does not preserve policy dismissals' do + expect { merge_request.preserve_open_policy_dismissals! } + .not_to change { policy_dismissal.reload.status } + end + end + + context 'when there are no policy dismissals' do + let!(:merge_request) do + create(:merge_request, :merged, + source_project: project, + target_project: project, + target_branch: project.default_branch) + end + + it 'does not raise an error' do + expect { merge_request.preserve_open_policy_dismissals! }.not_to raise_error + end + end + end + + describe '#merged_into_default_branch?' do + context 'when target branch is the default branch' do + let(:merge_request) do + create(:merge_request, + source_project: project, + target_project: project, + target_branch: project.default_branch) + end + + it 'returns true' do + expect(merge_request.send(:merged_into_default_branch?)).to be true + end + end + + context 'when target branch is not the default branch' do + let(:merge_request) do + create(:merge_request, + source_project: project, + target_project: project, + target_branch: 'feature-branch') + end + + it 'returns false' do + expect(merge_request.send(:merged_into_default_branch?)).to be false + end + end + + context 'when project default branch changes' do + let(:merge_request) do + create(:merge_request, + source_project: project, + target_project: project, + target_branch: 'main') + end + + before do + project.update!(default_branch: 'main') + end + + it 'reflects the current default branch' do + expect(merge_request.send(:merged_into_default_branch?)).to be true + end + end + end +end -- GitLab From bea5d1b1cc21285aa05da8091d7f7c485cec66bb Mon Sep 17 00:00:00 2001 From: Andy Schoenen Date: Tue, 21 Oct 2025 16:46:15 +0200 Subject: [PATCH 2/2] Document required changes for default branch check This documents the exact changes needed to modify the preserve_open_policy_dismissals! method to only preserve dismissals when merged into the default branch. The changes need to be applied manually to: ee/app/models/ee/merge_request.rb Changelog: added EE: true --- CHANGES_NEEDED.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 CHANGES_NEEDED.md diff --git a/CHANGES_NEEDED.md b/CHANGES_NEEDED.md new file mode 100644 index 00000000000000..93a21d5a569ee9 --- /dev/null +++ b/CHANGES_NEEDED.md @@ -0,0 +1,22 @@ +# This patch modifies the preserve_open_policy_dismissals! method +# to only preserve dismissals when merged into the default branch + +# Original method (around line 753 in ee/app/models/ee/merge_request.rb): +# def preserve_open_policy_dismissals! +# return unless merged? +# +# policy_dismissals.with_status(:open).find_each(&:preserve!) +# end + +# Should be changed to: +# def preserve_open_policy_dismissals! +# return unless merged? +# return unless merged_into_default_branch? +# +# policy_dismissals.with_status(:open).find_each(&:preserve!) +# end + +# And add this helper method in the private section: +# def merged_into_default_branch? +# target_branch == target_project.default_branch +# end -- GitLab