diff --git a/CHANGES_NEEDED.md b/CHANGES_NEEDED.md new file mode 100644 index 0000000000000000000000000000000000000000..93a21d5a569ee92669eb6ccc8c4e0a6b7749cea2 --- /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 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 0000000000000000000000000000000000000000..a12218020fa1b3fd89ccc472bf63b9245ac3b063 --- /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