From 67fca2385099ce571e4ae163ab2d83e226424e9b Mon Sep 17 00:00:00 2001 From: marc_shaw Date: Wed, 3 Dec 2025 12:36:47 +0100 Subject: [PATCH 1/9] Tidy up merge quick action copy --- .../quick_actions/merge_request_actions.rb | 35 +++++-- locale/gitlab.pot | 30 ++++-- .../merge_request_actions_spec.rb | 94 +++++++++++++++++++ .../merge_quick_action_shared_examples.rb | 57 +++++++++-- 4 files changed, 194 insertions(+), 22 deletions(-) create mode 100644 spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index d97d375fc09709..9524274a23c6be 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -19,15 +19,15 @@ module MergeRequestActions # /merge # desc do - if preferred_strategy = preferred_auto_merge_strategy(quick_action_target) - _("Merge automatically (%{strategy})") % { strategy: preferred_strategy.humanize } + if preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :desc) else _("Merge immediately") end end explanation do - if preferred_strategy = preferred_auto_merge_strategy(quick_action_target) - _("Schedules to merge this merge request (%{strategy}).") % { strategy: preferred_strategy.humanize } + if preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :explanation) else _('Merges this merge request immediately.') end @@ -37,8 +37,8 @@ module MergeRequestActions _("The `/merge` quick action requires the SHA of the head of the branch.") elsif params[:merge_request_diff_head_sha] != quick_action_target.diff_head_sha _("Branch has been updated since the merge was requested.") - elsif preferred_strategy = preferred_auto_merge_strategy(quick_action_target) - _("Scheduled to merge this merge request (%{strategy}).") % { strategy: preferred_strategy.humanize } + elsif preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :feedback) else _('Merged this merge request.') end @@ -548,6 +548,29 @@ def reviewers_to_remove?(updates) quick_action_target.reviewers.any? || updates&.dig(:reviewer_ids)&.any? end + def auto_merge_strategy_copy(strategy, type) + case strategy + when 'add_to_merge_train' + case type + when :desc then _('Add to merge train') + when :explanation then _('Adds this merge request to merge train.') + when :feedback then _('Added to merge train.') + end + when 'add_to_merge_train_when_checks_pass' + case type + when :desc then _('Add to merge train when ready') + when :explanation then _('Adds this merge request to merge train when ready.') + when :feedback then _('Set to add to merge train when ready.') + end + when 'merge_when_checks_pass' + case type + when :desc then _('Set to auto-merge') + when :explanation then _('Sets this merge request to auto-merge when ready.') + when :feedback then _('Set to auto-merge.') + end + end + end + def merge_orchestration_service @merge_orchestration_service ||= ::MergeRequests::MergeOrchestrationService.new(project, current_user) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6d9474e9cfbe3e..3862fcd15895fd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4785,6 +4785,9 @@ msgstr "" msgid "Add to merge train when all merge checks pass" msgstr "" +msgid "Add to merge train when ready" +msgstr "" + msgid "Add to review" msgstr "" @@ -4881,6 +4884,9 @@ msgstr "" msgid "Added in this version" msgstr "" +msgid "Added to merge train." +msgstr "" + msgid "Adding new applications is disabled in your GitLab instance. Please contact your GitLab administrator to get the permission." msgstr "" @@ -4940,6 +4946,12 @@ msgstr "" msgid "Adds email participants that don't have a GitLab account." msgstr "" +msgid "Adds this merge request to merge train when ready." +msgstr "" + +msgid "Adds this merge request to merge train." +msgstr "" + msgid "AdherenceReport|%{pendingCount}/%{totalCount} control is pending" msgid_plural "AdherenceReport|%{pendingCount}/%{totalCount} controls are pending" msgstr[0] "" @@ -41699,9 +41711,6 @@ msgstr "" msgid "Merge Requests merged" msgstr "" -msgid "Merge automatically (%{strategy})" -msgstr "" - msgid "Merge blocked: Merge all open dependent merge requests, and remove all closed dependencies." msgstr "" @@ -59023,18 +59032,12 @@ msgstr "" msgid "Scheduled pipelines cannot run more frequently than once per %{limit} minutes. A pipeline configured to run more frequently only starts after %{limit} minutes have elapsed since the last time it ran." msgstr "" -msgid "Scheduled to merge this merge request (%{strategy})." -msgstr "" - msgid "ScheduledJob|delayed manual action (%{remainingTime})" msgstr "" msgid "Schedules" msgstr "" -msgid "Schedules to merge this merge request (%{strategy})." -msgstr "" - msgid "Scope" msgstr "" @@ -63962,9 +63965,15 @@ msgstr "" msgid "Set to 0 to disable timeout." msgstr "" +msgid "Set to add to merge train when ready." +msgstr "" + msgid "Set to auto-merge" msgstr "" +msgid "Set to auto-merge." +msgstr "" + msgid "Set up" msgstr "" @@ -64064,6 +64073,9 @@ msgstr "" msgid "Sets the severity" msgstr "" +msgid "Sets this merge request to auto-merge when ready." +msgstr "" + msgid "Sets time estimate to %{time_estimate}." msgstr "" diff --git a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb new file mode 100644 index 00000000000000..ff5e353d0a2c3e --- /dev/null +++ b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::QuickActions::MergeRequestActions, feature_category: :code_review_workflow do + describe '#auto_merge_strategy_copy' do + let(:quick_action) do + Class.new do + include Gitlab::QuickActions::MergeRequestActions + end.new + end + + describe 'add_to_merge_train strategy' do + let(:strategy) { 'add_to_merge_train' } + + it 'returns correct desc copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) + .to eq('Add to merge train') + end + + it 'returns correct explanation copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) + .to eq('Adds this merge request to merge train.') + end + + it 'returns correct feedback copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) + .to eq('Added to merge train.') + end + end + + describe 'add_to_merge_train_when_checks_pass strategy' do + let(:strategy) { 'add_to_merge_train_when_checks_pass' } + + it 'returns correct desc copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) + .to eq('Add to merge train when ready') + end + + it 'returns correct explanation copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) + .to eq('Adds this merge request to merge train when ready.') + end + + it 'returns correct feedback copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) + .to eq('Set to add to merge train when ready.') + end + end + + describe 'merge_when_checks_pass strategy' do + let(:strategy) { 'merge_when_checks_pass' } + + it 'returns correct desc copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) + .to eq('Set to auto-merge') + end + + it 'returns correct explanation copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) + .to eq('Sets this merge request to auto-merge when ready.') + end + + it 'returns correct feedback copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) + .to eq('Set to auto-merge.') + end + end + + describe 'unknown strategy' do + let(:strategy) { 'unknown_strategy' } + + it 'returns nil for desc' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)).to be_nil + end + + it 'returns nil for explanation' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)).to be_nil + end + + it 'returns nil for feedback' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)).to be_nil + end + end + + describe 'unknown type' do + let(:strategy) { 'add_to_merge_train' } + + it 'returns nil for unknown type' do + expect(quick_action.auto_merge_strategy_copy(strategy, :unknown)).to be_nil + end + end + end +end diff --git a/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb index 2c59d85729c59b..5d09e0224de94a 100644 --- a/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb @@ -40,15 +40,58 @@ stub_licensed_features(merge_request_approvers: true) if Gitlab.ee? end - it 'schedules to merge the MR' do - add_note("/merge") + context 'with merge_when_checks_pass strategy' do + before do + allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| + allow(service).to receive(:preferred_auto_merge_strategy) + .and_return('merge_when_checks_pass') + end + end + + it 'schedules to merge the MR with correct copy' do + add_note("/merge") + + expect(page).to have_content("Set to auto-merge.") + + expect(merge_request.reload).to be_auto_merge_enabled + expect(merge_request.reload).not_to be_merged + end + end + + context 'with add_to_merge_train strategy' do + before do + allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| + allow(service).to receive(:preferred_auto_merge_strategy) + .and_return('add_to_merge_train') + end + end + + it 'schedules to merge the MR with correct copy' do + add_note("/merge") + + expect(page).to have_content("Added to merge train.") + + expect(merge_request.reload).to be_auto_merge_enabled + expect(merge_request.reload).not_to be_merged + end + end + + context 'with add_to_merge_train_when_checks_pass strategy' do + before do + allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| + allow(service).to receive(:preferred_auto_merge_strategy) + .and_return('add_to_merge_train_when_checks_pass') + end + end + + it 'schedules to merge the MR with correct copy' do + add_note("/merge") - expect(page).to( - have_content("Scheduled to merge this merge request (Merge when checks pass).") - ) + expect(page).to have_content("Set to add to merge train when ready.") - expect(merge_request.reload).to be_auto_merge_enabled - expect(merge_request.reload).not_to be_merged + expect(merge_request.reload).to be_auto_merge_enabled + expect(merge_request.reload).not_to be_merged + end end end end -- GitLab From 6fc171d92483d77d13462318dbad36e5de094f86 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Thu, 11 Dec 2025 13:07:21 +0100 Subject: [PATCH 2/9] Memoize preferred_auto_merge_strategy calls --- lib/gitlab/quick_actions/merge_request_actions.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index 9524274a23c6be..fd947c08e6ddcd 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -19,15 +19,15 @@ module MergeRequestActions # /merge # desc do - if preferred_auto_merge_strategy(quick_action_target) - auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :desc) + if strategy = preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(strategy, :desc) else _("Merge immediately") end end explanation do - if preferred_auto_merge_strategy(quick_action_target) - auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :explanation) + if strategy = preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(strategy, :explanation) else _('Merges this merge request immediately.') end @@ -37,8 +37,8 @@ module MergeRequestActions _("The `/merge` quick action requires the SHA of the head of the branch.") elsif params[:merge_request_diff_head_sha] != quick_action_target.diff_head_sha _("Branch has been updated since the merge was requested.") - elsif preferred_auto_merge_strategy(quick_action_target) - auto_merge_strategy_copy(preferred_auto_merge_strategy(quick_action_target), :feedback) + elsif strategy = preferred_auto_merge_strategy(quick_action_target) + auto_merge_strategy_copy(strategy, :feedback) else _('Merged this merge request.') end -- GitLab From be18485bbc8be15720579449f3bbf04a4bf940e0 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Thu, 11 Dec 2025 13:47:05 +0100 Subject: [PATCH 3/9] Fix failing spec for merge automatically command Update the expected message in the shared example to match the new copy. --- spec/services/quick_actions/interpret_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index b81ff4f7580adf..bbc0a7469cfaee 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -606,7 +606,7 @@ expect(updates).to eq(merge: merge_request.diff_head_sha) - expect(message).to eq(_('Scheduled to merge this merge request (Merge when checks pass).')) + expect(message).to eq(_('Set to auto-merge.')) end end -- GitLab From 1f45a51ede8965b5cbaacd2f0ed92a8ea06640ae Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Mon, 15 Dec 2025 14:22:57 +0100 Subject: [PATCH 4/9] Fix failing EE tests for auto-merge quick action copy changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update test expectations to match the new auto-merge copy: - "Scheduled to merge this merge request (Merge when checks pass)." → "Set to auto-merge." --- ee/spec/services/quick_actions/interpret_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/quick_actions/interpret_service_spec.rb b/ee/spec/services/quick_actions/interpret_service_spec.rb index fa5ec702dab749..034f1be8a0f5da 100644 --- a/ee/spec/services/quick_actions/interpret_service_spec.rb +++ b/ee/spec/services/quick_actions/interpret_service_spec.rb @@ -1380,7 +1380,7 @@ expect(updates).to eq(merge: issuable.diff_head_sha) - expect(message).to eq('Scheduled to merge this merge request (Merge when checks pass).') + expect(message).to eq('Set to auto-merge.') end end -- GitLab From e6045a8800b94bc4b07d4c27581fe305131446f2 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Mon, 15 Dec 2025 14:24:14 +0100 Subject: [PATCH 5/9] Fix first failing EE test for auto-merge quick action copy Update the first test expectation to match the new auto-merge copy. --- ee/spec/services/quick_actions/interpret_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/quick_actions/interpret_service_spec.rb b/ee/spec/services/quick_actions/interpret_service_spec.rb index 034f1be8a0f5da..6fe5a9259c9c7d 100644 --- a/ee/spec/services/quick_actions/interpret_service_spec.rb +++ b/ee/spec/services/quick_actions/interpret_service_spec.rb @@ -1354,7 +1354,7 @@ expect(updates).to eq(merge: merge_request.diff_head_sha) - expect(message).to eq('Scheduled to merge this merge request (Merge when checks pass).') + expect(message).to eq('Set to auto-merge.') end end end -- GitLab From 0d07e24cdd6f198ddb564f22f64372c8d1da4501 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Tue, 16 Dec 2025 18:23:42 +0100 Subject: [PATCH 6/9] Address reviewer feedback - Use constants for strategy names instead of strings - DRY up unit specs with shared examples - Reduce feature spec to only test one strategy since there's good unit test coverage --- .../quick_actions/merge_request_actions.rb | 16 ++--- .../merge_request_actions_spec.rb | 67 ++++++------------- .../merge_quick_action_shared_examples.rb | 55 ++------------- 3 files changed, 28 insertions(+), 110 deletions(-) diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index fd947c08e6ddcd..0acc1046c365b5 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -550,24 +550,18 @@ def reviewers_to_remove?(updates) def auto_merge_strategy_copy(strategy, type) case strategy - when 'add_to_merge_train' + when AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS case type - when :desc then _('Add to merge train') - when :explanation then _('Adds this merge request to merge train.') - when :feedback then _('Added to merge train.') + when :desc then _('Set to auto-merge') + when :explanation then _('Sets this merge request to auto-merge when ready.') + when :feedback then _('Set to auto-merge.') end - when 'add_to_merge_train_when_checks_pass' + when AutoMergeService::STRATEGY_ADD_TO_MERGE_TRAIN_WHEN_CHECKS_PASS case type when :desc then _('Add to merge train when ready') when :explanation then _('Adds this merge request to merge train when ready.') when :feedback then _('Set to add to merge train when ready.') end - when 'merge_when_checks_pass' - case type - when :desc then _('Set to auto-merge') - when :explanation then _('Sets this merge request to auto-merge when ready.') - when :feedback then _('Set to auto-merge.') - end end end diff --git a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb index ff5e353d0a2c3e..a005b792bf2b1b 100644 --- a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb +++ b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb @@ -10,81 +10,52 @@ end.new end - describe 'add_to_merge_train strategy' do - let(:strategy) { 'add_to_merge_train' } - + shared_examples 'merge_strategies' do it 'returns correct desc copy' do expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) - .to eq('Add to merge train') + .to eq(expected_description) end it 'returns correct explanation copy' do expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) - .to eq('Adds this merge request to merge train.') + .to eq(expected_explanation) end it 'returns correct feedback copy' do expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) - .to eq('Added to merge train.') + .to eq(expected_feedback) end end describe 'add_to_merge_train_when_checks_pass strategy' do - let(:strategy) { 'add_to_merge_train_when_checks_pass' } - - it 'returns correct desc copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) - .to eq('Add to merge train when ready') - end - - it 'returns correct explanation copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) - .to eq('Adds this merge request to merge train when ready.') - end + let(:strategy) { AutoMergeService::STRATEGY_ADD_TO_MERGE_TRAIN_WHEN_CHECKS_PASS } + let(:expected_description) { 'Add to merge train when ready' } + let(:expected_explanation) { 'Adds this merge request to merge train when ready.' } + let(:expected_feedback) { 'Set to add to merge train when ready.' } - it 'returns correct feedback copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) - .to eq('Set to add to merge train when ready.') - end + it_behaves_like 'merge_strategies' end describe 'merge_when_checks_pass strategy' do - let(:strategy) { 'merge_when_checks_pass' } - - it 'returns correct desc copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) - .to eq('Set to auto-merge') - end + let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS } + let(:expected_description) { 'Set to auto-merge' } + let(:expected_explanation) { 'Sets this merge request to auto-merge when ready.' } + let(:expected_feedback) { 'Set to auto-merge.' } - it 'returns correct explanation copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) - .to eq('Sets this merge request to auto-merge when ready.') - end - - it 'returns correct feedback copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) - .to eq('Set to auto-merge.') - end + it_behaves_like 'merge_strategies' end describe 'unknown strategy' do let(:strategy) { 'unknown_strategy' } + let(:expected_description) { nil } + let(:expected_explanation) { nil } + let(:expected_feedback) { nil } - it 'returns nil for desc' do - expect(quick_action.auto_merge_strategy_copy(strategy, :desc)).to be_nil - end - - it 'returns nil for explanation' do - expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)).to be_nil - end - - it 'returns nil for feedback' do - expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)).to be_nil - end + it_behaves_like 'merge_strategies' end describe 'unknown type' do - let(:strategy) { 'add_to_merge_train' } + let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS } it 'returns nil for unknown type' do expect(quick_action.auto_merge_strategy_copy(strategy, :unknown)).to be_nil diff --git a/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb index 5d09e0224de94a..6a21ee282e7586 100644 --- a/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb @@ -40,58 +40,11 @@ stub_licensed_features(merge_request_approvers: true) if Gitlab.ee? end - context 'with merge_when_checks_pass strategy' do - before do - allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| - allow(service).to receive(:preferred_auto_merge_strategy) - .and_return('merge_when_checks_pass') - end - end - - it 'schedules to merge the MR with correct copy' do - add_note("/merge") - - expect(page).to have_content("Set to auto-merge.") - - expect(merge_request.reload).to be_auto_merge_enabled - expect(merge_request.reload).not_to be_merged - end - end - - context 'with add_to_merge_train strategy' do - before do - allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| - allow(service).to receive(:preferred_auto_merge_strategy) - .and_return('add_to_merge_train') - end - end - - it 'schedules to merge the MR with correct copy' do - add_note("/merge") - - expect(page).to have_content("Added to merge train.") - - expect(merge_request.reload).to be_auto_merge_enabled - expect(merge_request.reload).not_to be_merged - end - end - - context 'with add_to_merge_train_when_checks_pass strategy' do - before do - allow_next_instance_of(MergeRequests::MergeOrchestrationService) do |service| - allow(service).to receive(:preferred_auto_merge_strategy) - .and_return('add_to_merge_train_when_checks_pass') - end - end - - it 'schedules to merge the MR with correct copy' do - add_note("/merge") - - expect(page).to have_content("Set to add to merge train when ready.") + it 'schedules to merge the MR' do + add_note("/merge") - expect(merge_request.reload).to be_auto_merge_enabled - expect(merge_request.reload).not_to be_merged - end + expect(merge_request.reload).to be_auto_merge_enabled + expect(merge_request.reload).not_to be_merged end end end -- GitLab From 073e440ef03cd053271d223a8b6e7c243b866768 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Tue, 16 Dec 2025 18:28:09 +0100 Subject: [PATCH 7/9] Add missing merge_train strategy support Add EE extension to handle the merge_train strategy copy --- .../gitlab/quick_actions/merge_request_actions.rb | 14 ++++++++++++++ .../quick_actions/merge_request_actions_spec.rb | 10 +++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb b/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb index 20370904a0dc75..f082a04548a992 100644 --- a/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb +++ b/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb @@ -48,6 +48,20 @@ def process_reviewer_users_message ::Ai::CodeReviewMessages.manual_error end + + override :auto_merge_strategy_copy + def auto_merge_strategy_copy(strategy, type) + case strategy + when ::EE::AutoMergeService::STRATEGY_MERGE_TRAIN + case type + when :desc then _('Add to merge train') + when :explanation then _('Adds this merge request to merge train.') + when :feedback then _('Added to merge train.') + end + else + super + end + end end end end diff --git a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb index a005b792bf2b1b..fbb091ed43df55 100644 --- a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb +++ b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb @@ -36,11 +36,11 @@ it_behaves_like 'merge_strategies' end - describe 'merge_when_checks_pass strategy' do - let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS } - let(:expected_description) { 'Set to auto-merge' } - let(:expected_explanation) { 'Sets this merge request to auto-merge when ready.' } - let(:expected_feedback) { 'Set to auto-merge.' } + describe 'merge_train strategy', if: Gitlab.ee? do + let(:strategy) { EE::AutoMergeService::STRATEGY_MERGE_TRAIN } + let(:expected_description) { 'Add to merge train' } + let(:expected_explanation) { 'Adds this merge request to merge train.' } + let(:expected_feedback) { 'Added to merge train.' } it_behaves_like 'merge_strategies' end -- GitLab From 951319235d97f6469a2263c22a4c554d2eace5bd Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Tue, 16 Dec 2025 19:37:45 +0100 Subject: [PATCH 8/9] Move merge_train strategy spec to EE folder --- .../merge_request_actions_spec.rb | 39 +++++++++++++++++++ .../merge_request_actions_spec.rb | 20 +++++----- 2 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb diff --git a/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb b/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb new file mode 100644 index 00000000000000..5c75499c2707ae --- /dev/null +++ b/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::QuickActions::MergeRequestActions, feature_category: :code_review_workflow do + describe '#auto_merge_strategy_copy' do + let(:quick_action) do + Class.new do + include Gitlab::QuickActions::MergeRequestActions + end.new + end + + shared_examples 'merge_strategies' do + it 'returns correct desc copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) + .to eq(expected_description) + end + + it 'returns correct explanation copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) + .to eq(expected_explanation) + end + + it 'returns correct feedback copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) + .to eq(expected_feedback) + end + end + + describe 'merge_train strategy' do + let(:strategy) { EE::AutoMergeService::STRATEGY_MERGE_TRAIN } + let(:expected_description) { 'Add to merge train' } + let(:expected_explanation) { 'Adds this merge request to merge train.' } + let(:expected_feedback) { 'Added to merge train.' } + + it_behaves_like 'merge_strategies' + end + end +end \ No newline at end of file diff --git a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb index fbb091ed43df55..b7bc30e573f638 100644 --- a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb +++ b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb @@ -27,6 +27,15 @@ end end + describe 'merge_when_checks_pass strategy' do + let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS } + let(:expected_description) { 'Set to auto-merge' } + let(:expected_explanation) { 'Sets this merge request to auto-merge when ready.' } + let(:expected_feedback) { 'Set to auto-merge.' } + + it_behaves_like 'merge_strategies' + end + describe 'add_to_merge_train_when_checks_pass strategy' do let(:strategy) { AutoMergeService::STRATEGY_ADD_TO_MERGE_TRAIN_WHEN_CHECKS_PASS } let(:expected_description) { 'Add to merge train when ready' } @@ -36,15 +45,6 @@ it_behaves_like 'merge_strategies' end - describe 'merge_train strategy', if: Gitlab.ee? do - let(:strategy) { EE::AutoMergeService::STRATEGY_MERGE_TRAIN } - let(:expected_description) { 'Add to merge train' } - let(:expected_explanation) { 'Adds this merge request to merge train.' } - let(:expected_feedback) { 'Added to merge train.' } - - it_behaves_like 'merge_strategies' - end - describe 'unknown strategy' do let(:strategy) { 'unknown_strategy' } let(:expected_description) { nil } @@ -62,4 +62,4 @@ end end end -end +end \ No newline at end of file -- GitLab From 17c2a493f6895f2a807dc331aabdd21b4c2d8914 Mon Sep 17 00:00:00 2001 From: Marc Shaw Date: Tue, 16 Dec 2025 19:57:26 +0100 Subject: [PATCH 9/9] Extract shared example for auto_merge_strategy_copy specs --- .../merge_request_actions_spec.rb | 19 +-------------- .../merge_request_actions_spec.rb | 23 +++---------------- ...uto_merge_strategy_copy_shared_examples.rb | 18 +++++++++++++++ 3 files changed, 22 insertions(+), 38 deletions(-) create mode 100644 spec/support/shared_examples/quick_actions/auto_merge_strategy_copy_shared_examples.rb diff --git a/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb b/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb index 5c75499c2707ae..6df560fb9da742 100644 --- a/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb +++ b/ee/spec/lib/ee/gitlab/quick_actions/merge_request_actions_spec.rb @@ -10,30 +10,13 @@ end.new end - shared_examples 'merge_strategies' do - it 'returns correct desc copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) - .to eq(expected_description) - end - - it 'returns correct explanation copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) - .to eq(expected_explanation) - end - - it 'returns correct feedback copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) - .to eq(expected_feedback) - end - end - describe 'merge_train strategy' do let(:strategy) { EE::AutoMergeService::STRATEGY_MERGE_TRAIN } let(:expected_description) { 'Add to merge train' } let(:expected_explanation) { 'Adds this merge request to merge train.' } let(:expected_feedback) { 'Added to merge train.' } - it_behaves_like 'merge_strategies' + it_behaves_like 'auto_merge_strategy_copy' end end end \ No newline at end of file diff --git a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb index b7bc30e573f638..f1acf3e8f4717e 100644 --- a/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb +++ b/spec/lib/gitlab/quick_actions/merge_request_actions_spec.rb @@ -10,30 +10,13 @@ end.new end - shared_examples 'merge_strategies' do - it 'returns correct desc copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) - .to eq(expected_description) - end - - it 'returns correct explanation copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) - .to eq(expected_explanation) - end - - it 'returns correct feedback copy' do - expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) - .to eq(expected_feedback) - end - end - describe 'merge_when_checks_pass strategy' do let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS } let(:expected_description) { 'Set to auto-merge' } let(:expected_explanation) { 'Sets this merge request to auto-merge when ready.' } let(:expected_feedback) { 'Set to auto-merge.' } - it_behaves_like 'merge_strategies' + it_behaves_like 'auto_merge_strategy_copy' end describe 'add_to_merge_train_when_checks_pass strategy' do @@ -42,7 +25,7 @@ let(:expected_explanation) { 'Adds this merge request to merge train when ready.' } let(:expected_feedback) { 'Set to add to merge train when ready.' } - it_behaves_like 'merge_strategies' + it_behaves_like 'auto_merge_strategy_copy' end describe 'unknown strategy' do @@ -51,7 +34,7 @@ let(:expected_explanation) { nil } let(:expected_feedback) { nil } - it_behaves_like 'merge_strategies' + it_behaves_like 'auto_merge_strategy_copy' end describe 'unknown type' do diff --git a/spec/support/shared_examples/quick_actions/auto_merge_strategy_copy_shared_examples.rb b/spec/support/shared_examples/quick_actions/auto_merge_strategy_copy_shared_examples.rb new file mode 100644 index 00000000000000..b5a908cd83b267 --- /dev/null +++ b/spec/support/shared_examples/quick_actions/auto_merge_strategy_copy_shared_examples.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'auto_merge_strategy_copy' do + it 'returns correct desc copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :desc)) + .to eq(expected_description) + end + + it 'returns correct explanation copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :explanation)) + .to eq(expected_explanation) + end + + it 'returns correct feedback copy' do + expect(quick_action.auto_merge_strategy_copy(strategy, :feedback)) + .to eq(expected_feedback) + end +end -- GitLab