From 1eff706d76fb722585bcbe3771dcdc20757d066d Mon Sep 17 00:00:00 2001 From: Patrick Talbert Date: Fri, 24 Oct 2025 14:27:33 +0000 Subject: [PATCH 1/4] Update QuickActions DSL definition_by_name to consider target type To allow for quick actions such as /blocks, /blocked_by, and /unlink to also work with merge requests in the EE case, definition_by_name needs to be able to handle a situation where there are multiple registered commands for a given name that operate on different types. Signed-off-by: Patrick Talbert --- .../quick_actions/interpret_service.rb | 6 +-- lib/gitlab/quick_actions/dsl.rb | 12 ++++- spec/lib/gitlab/quick_actions/dsl_spec.rb | 53 +++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index abe4b734ac3701..901badb9e4d126 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -191,7 +191,7 @@ def execution_messages_for(commands) def map_commands(commands, method) commands.flat_map do |name_or_alias, arg| - definition = self.class.definition_by_name(name_or_alias) + definition = self.class.definition_by_name(name_or_alias, self) next unless definition case method @@ -205,7 +205,7 @@ def map_commands(commands, method) def command_names(commands) commands.flatten.map do |name| - definition = self.class.definition_by_name(name) + definition = self.class.definition_by_name(name, self) next unless definition name @@ -214,7 +214,7 @@ def command_names(commands) def extract_updates(commands) commands.each do |name, arg| - definition = self.class.definition_by_name(name) + definition = self.class.definition_by_name(name, self) next unless definition definition.execute(self, arg) diff --git a/lib/gitlab/quick_actions/dsl.rb b/lib/gitlab/quick_actions/dsl.rb index 820dd9077374c0..ff57aef9484ef8 100644 --- a/lib/gitlab/quick_actions/dsl.rb +++ b/lib/gitlab/quick_actions/dsl.rb @@ -187,8 +187,16 @@ def substitution(*substitution_names, &block) define_command(SubstitutionDefinition, *substitution_names, &block) end - def definition_by_name(name) - command_definitions_by_name[name.to_sym] + def definition_by_name(name, context = nil) + if context + target = context.quick_action_target + command_definitions.find do |definition| + definition.all_names.include?(name.to_sym) && + (definition.types.blank? || definition.types.any? { |t| target.is_a?(t) }) + end + else + command_definitions_by_name[name.to_sym] + end end private diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index 22e82c76d35ac2..523b2a4fb15f96 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -177,4 +177,57 @@ expect(has_types.warning).to eq('') end end + + describe '.definition_by_name' do + let(:dummy_class) do + Class.new do + include Gitlab::QuickActions::Dsl + + command :issue_command do + # Issue-specific command + end + types Issue + + command :mr_command do + # MergeRequest-specific command + end + types MergeRequest + + command :any_command do + # No type restriction + end + end + end + + let(:issue) { build(:issue) } + let(:merge_request) { build(:merge_request) } + let(:context_with_issue) { double('context', quick_action_target: issue) } + let(:context_with_mr) { double('context', quick_action_target: merge_request) } + + context 'without context parameter' do + it 'returns command by name from hash' do + definition = dummy_class.definition_by_name(:issue_command) + + expect(definition).to be_a(Gitlab::QuickActions::CommandDefinition) + expect(definition.name).to eq(:issue_command) + end + + it 'returns nil for non-existent command' do + expect(dummy_class.definition_by_name(:nonexistent)).to be_nil + end + end + + context 'with context parameter' do + it 'returns command when target type matches' do + definition = dummy_class.definition_by_name(:issue_command, context_with_issue) + + expect(definition).to be_a(Gitlab::QuickActions::CommandDefinition) + expect(definition.name).to eq(:issue_command) + end + + it 'returns nil for non-existent command' do + expect(dummy_class.definition_by_name(:nonexistent, context_with_issue)).to be_nil + end + end + end end -- GitLab From 1f11cc1100691ccfade3ed1e50e8f3d541d45f76 Mon Sep 17 00:00:00 2001 From: Patrick Talbert Date: Wed, 1 Oct 2025 17:51:46 +0200 Subject: [PATCH 2/4] Enhance the /blocked_by & /blocks quick actions to support MR dependencies The /blocked_by & /blocks quick actions allow for marking an issue or epic as being blocked by or blocking another. Update it to also work in the merge request context to set one MR as being blocked by or blocking another. Dependency relationships between MRs is an EE only feature so the /blocks & /blocked_by quickactions should not be presented to FOSS users. Signed-off-by: Patrick Talbert --- doc/user/project/quick_actions.md | 4 +- .../quick_actions/merge_request_actions.rb | 93 ++++++++++++ .../quick_actions/interpret_service_spec.rb | 135 ++++++++++++++++++ locale/gitlab.pot | 21 +++ 4 files changed, 251 insertions(+), 2 deletions(-) diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 9204f9ae5ef70c..1b1b65744c5718 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -77,8 +77,8 @@ To auto-format this table, use the VS Code Markdown Table formatter: `https://do | `/assign me` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | Assign yourself. | | `/assign_reviewer @user1 @user2` or `/reviewer @user1 @user2` | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | Assign one or more users as reviewers. | | `/assign_reviewer me` or `/reviewer me` | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | Assign yourself as a reviewer. | -| `/blocked_by ` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | Mark the item as blocked by other items. The `` value should be in the format of `#item`, `group/project#item`, or the full URL. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214232) in GitLab 16.0). | -| `/blocks ` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | Mark the item as blocking other items. The `` value should be in the format of `#item`, `group/project#item`, or the full URL. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214232) in GitLab 16.0). | +| `/blocked_by ` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Mark the item as blocked by other items. The `` value should be in the format of `#item` or `!merge_request`, `group/project#item` or `group/project!merge_request`, or the full URL. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214232) in GitLab 16.0 for issues. Support for merge requests [added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/207136) in GitLab 18.6. | +| `/blocks ` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Mark the item as blocking other items. The `` value should be in the format of `#item` or `!merge_request`, `group/project#item` or `group/project!merge_request`, or the full URL. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214232) in GitLab 16.0 for issues. Support for merge requests [added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/207136) in GitLab 18.6. | | `/board_move ~column` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="dotted-circle" >}} No | Move issue to column on the board. The project must have only one issue board. | | `/clear_health_status` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | Clear [health status](issues/managing_issues.md#health-status). | | `/clear_weight` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="dotted-circle" >}} No | Clear weight. | 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 600f82556f0a6d..d2c58d925c40d8 100644 --- a/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb +++ b/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb @@ -23,6 +23,56 @@ module MergeRequestActions command :reassign_reviewer do |reassign_param| @updates[:reviewer_ids] = extract_users(reassign_param).map(&:id) end + + desc { _('Mark this merge request as blocking other merge requests') } + explanation do |merge_requests| + if merge_requests.present? + refs = merge_requests.map { |mr| mr.to_reference(full: true) } + format(_('Marks this merge request as blocking %{references}.'), references: refs.to_sentence) + end + end + execution_message do |merge_requests| + if merge_requests.present? + refs = merge_requests.map { |mr| mr.to_reference(full: true) } + format(_('Marked this merge request as blocking %{references}.'), references: refs.to_sentence) + end + end + params '<#merge_request | group/project!merge_request | merge request URL>' + types MergeRequest + condition do + quick_action_target.persisted? && + quick_action_target.project&.licensed_feature_available?(:blocking_merge_requests) && + current_user.can?(:update_merge_request, quick_action_target) + end + parse_params { |mr_references| extract_references(mr_references, :merge_request) } + command :blocks do |merge_requests| + create_merge_request_blocks(merge_requests, direction: :blocks) + end + + desc { _('Mark this merge request as blocked by other merge requests') } + explanation do |merge_requests| + if merge_requests.present? + refs = merge_requests.map { |mr| mr.to_reference(full: true) } + format(_('Marks this merge request as blocked by %{references}.'), references: refs.to_sentence) + end + end + execution_message do |merge_requests| + if merge_requests.present? + refs = merge_requests.map { |mr| mr.to_reference(full: true) } + format(_('Marked this merge request as blocked by %{references}.'), references: refs.to_sentence) + end + end + params '<#merge_request | group/project!merge_request | merge request URL>' + types MergeRequest + condition do + quick_action_target.persisted? && + quick_action_target.project&.licensed_feature_available?(:blocking_merge_requests) && + current_user.can?(:update_merge_request, quick_action_target) + end + parse_params { |mr_references| extract_references(mr_references, :merge_request) } + command :blocked_by do |merge_requests| + create_merge_request_blocks(merge_requests, direction: :blocked_by) + end end override :process_reviewer_users @@ -49,6 +99,49 @@ def process_reviewer_users_message s_("DuoCodeReview|Your account doesn't have GitLab Duo access. " \ "Please contact your system administrator for access.") end + + private + + def create_merge_request_blocks(merge_requests, direction:) + return [] if merge_requests.empty? + + successful_refs = [] + failed_refs = [] + + merge_requests.uniq.each do |mr| + result = create_single_block(mr, direction) + ref = mr.to_reference(full: true) + + result.success? ? successful_refs << ref : failed_refs << "#{ref}: #{result.message}" + end + + set_error_message(direction, failed_refs) if failed_refs.any? + successful_refs + end + + def set_error_message(direction, failed_refs) + command_name = direction == :blocks ? :blocks : :blocked_by + @execution_message[command_name] = + format(_('Failed to create some dependencies: %{errors}'), errors: failed_refs.join(', ')) + end + + def create_single_block(merge_request, direction) + if direction == :blocks + # /blocks: current MR blocks the referenced MR + ::MergeRequests::CreateBlockService.new( + user: current_user, + merge_request: merge_request, + blocking_merge_request_id: quick_action_target.id + ).execute + else + # /blocked_by: referenced MR blocks the current MR + ::MergeRequests::CreateBlockService.new( + user: current_user, + merge_request: quick_action_target, + blocking_merge_request_id: merge_request.id + ).execute + end + end end end end diff --git a/ee/spec/services/quick_actions/interpret_service_spec.rb b/ee/spec/services/quick_actions/interpret_service_spec.rb index fd1a8e03ed8752..1e827f0c6b977f 100644 --- a/ee/spec/services/quick_actions/interpret_service_spec.rb +++ b/ee/spec/services/quick_actions/interpret_service_spec.rb @@ -2039,6 +2039,141 @@ end end + describe 'merge request dependency commands' do + let_it_be(:guest) { create(:user) } + let_it_be(:restricted_project) { create(:project) } + let_it_be(:merge_request1) { create(:merge_request, source_project: project, source_branch: 'feature-1') } + let_it_be(:merge_request2) { create(:merge_request, source_project: project, source_branch: 'feature-2') } + let_it_be(:merge_request3) { create(:merge_request, source_project: project, source_branch: 'feature-3') } + let_it_be(:mr_ref1) { merge_request1.to_reference(full: true) } + let_it_be(:mr_ref2) { merge_request2.to_reference(full: true) } + let_it_be(:mr_ref3) { merge_request3.to_reference(full: true) } + let(:target_mr) { create(:merge_request, source_project: project, source_branch: 'feature-target') } + + before do + stub_licensed_features(blocking_merge_requests: true) + project.add_developer(current_user) + end + + context 'with /blocks' do + let(:blocks_command) { "/blocks #{mr_ref1} #{mr_ref2}" } + + context 'with sufficient permissions' do + it '/blocks is available' do + _, explanations = service.explain(blocks_command, target_mr) + + expect(explanations).to contain_exactly("Marks this merge request as blocking #{[mr_ref1, mr_ref2].to_sentence}.") + end + + it '/blocks execution creates blocks' do + _, _, message = service.execute(blocks_command, target_mr) + + expect(message).to include("Marked this merge request as blocking") + expect(target_mr.blocked_merge_requests).to contain_exactly(merge_request1, merge_request2) + end + + context 'when providing issue references instead of MR references' do + let_it_be(:issue1) { create(:issue, project: project) } + let_it_be(:issue2) { create(:issue, project: project) } + let(:issue_refs_command) { "/blocks #{issue1.to_reference} #{issue2.to_reference}" } + + it 'ignores non-MR references' do + _, _, _ = service.execute(issue_refs_command, target_mr) + + expect(target_mr.blocked_merge_requests).to be_empty + end + end + + context 'when licensed feature is not available' do + let(:target) { target_mr } + + before do + stub_licensed_features(blocking_merge_requests: false) + end + + it_behaves_like 'quick action is unavailable', :blocks + end + + context 'when target is not a merge request' do + let(:target) { create(:issue, project: project) } + + before do + stub_licensed_features(blocked_issues: false) + end + + it_behaves_like 'quick action is unavailable', :blocks + end + end + + context 'with insufficient permissions' do + let(:current_user) { guest } + let(:target_mr) { create(:merge_request, :skip_diff_creation, source_project: restricted_project, source_branch: 'feature-insufficient-perms-1') } + let(:target) { target_mr } + + it_behaves_like 'quick action is unavailable', :blocks + end + end + + context 'with /blocked_by' do + let(:blocked_by_command) { "/blocked_by #{mr_ref1} #{mr_ref2}" } + + context 'with sufficient permissions' do + it '/blocked_by is available' do + _, explanations = service.explain(blocked_by_command, target_mr) + + expect(explanations).to contain_exactly("Marks this merge request as blocked by #{[mr_ref1, mr_ref2].to_sentence}.") + end + + it '/blocked_by execution creates blocks' do + _, _, message = service.execute(blocked_by_command, target_mr) + + expect(message).to include("Marked this merge request as blocked by") + expect(target_mr.blocking_merge_requests).to contain_exactly(merge_request1, merge_request2) + end + + context 'when providing issue references instead of MR references' do + let_it_be(:issue1) { create(:issue, project: project) } + let_it_be(:issue2) { create(:issue, project: project) } + let(:issue_refs_command) { "/blocked_by #{issue1.to_reference} #{issue2.to_reference}" } + + it 'ignores non-MR references' do + _, _, _ = service.execute(issue_refs_command, target_mr) + + expect(target_mr.blocking_merge_requests).to be_empty + end + end + + context 'when licensed feature is not available' do + let(:target) { target_mr } + + before do + stub_licensed_features(blocking_merge_requests: false) + end + + it_behaves_like 'quick action is unavailable', :blocked_by + end + + context 'when target is not a merge request' do + let(:target) { create(:issue, project: project) } + + before do + stub_licensed_features(blocked_issues: false) + end + + it_behaves_like 'quick action is unavailable', :blocked_by + end + end + + context 'with insufficient permissions' do + let(:current_user) { guest } + let(:target_mr) { create(:merge_request, :skip_diff_creation, source_project: restricted_project, source_branch: 'feature-insufficient-perms-2') } + let(:target) { target_mr } + + it_behaves_like 'quick action is unavailable', :blocked_by + end + end + end + describe 'promote_to command' do let(:content) { '/promote_to objective' } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3cf0fd899af590..47e6d7d0cfbed2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27925,6 +27925,9 @@ msgstr "" msgid "Failed to create security category" msgstr "" +msgid "Failed to create some dependencies: %{errors}" +msgstr "" + msgid "Failed to create wiki" msgstr "" @@ -39560,6 +39563,12 @@ msgstr "" msgid "Mark this issue as a duplicate of another issue" msgstr "" +msgid "Mark this merge request as blocked by other merge requests" +msgstr "" + +msgid "Mark this merge request as blocking other merge requests" +msgstr "" + msgid "Mark to-do item as done" msgstr "" @@ -39698,6 +39707,12 @@ msgstr "" msgid "Marked this %{noun} as ready." msgstr "" +msgid "Marked this merge request as blocked by %{references}." +msgstr "" + +msgid "Marked this merge request as blocking %{references}." +msgstr "" + msgid "Marked to-do item as done." msgstr "" @@ -39707,6 +39722,12 @@ msgstr "" msgid "Marks this %{noun} as ready." msgstr "" +msgid "Marks this merge request as blocked by %{references}." +msgstr "" + +msgid "Marks this merge request as blocking %{references}." +msgstr "" + msgid "Marks to-do item as done." msgstr "" -- GitLab From 9264c34ad04d9d95f86464d5697ecf86727eb9fd Mon Sep 17 00:00:00 2001 From: Patrick Talbert Date: Wed, 22 Oct 2025 13:00:58 +0200 Subject: [PATCH 3/4] Enhance the /unlink quick action to support MRs Add support for removing dependency links between merge requests using the /unlink quick action. Marking MRs as dependencies of each other is an EE-only feature and so the /unlink quickaction should not be listed as an option to FOSS users. Signed-off-by: Patrick Talbert --- doc/user/project/quick_actions.md | 2 +- .../quick_actions/merge_request_actions.rb | 59 ++++++++++++++ .../quick_actions/interpret_service_spec.rb | 77 +++++++++++++++++++ locale/gitlab.pot | 12 +++ 4 files changed, 149 insertions(+), 1 deletion(-) diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 1b1b65744c5718..df2097b914075f 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -149,7 +149,7 @@ To auto-format this table, use the VS Code Markdown Table formatter: `https://do | `/unassign` | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | Remove all assignees. | | `/unlabel ~label1 ~label2` or `/remove_label ~label1 ~label2` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Remove specified labels. | | `/unlabel` or `/remove_label` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Remove all labels. | -| `/unlink ` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="check-circle" >}} Yes | Remove link with to the provided issue. The `` value should be in the format of `#item`, `group/project#item`, or the full URL. ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/414400) in GitLab 16.1). | +| `/unlink ` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Remove link or blocking relationship with the provided issue or merge request. The `` value should be in the format of `#item` or `!merge_request`, `group/project#item` or `group/project!merge_request`, or the full URL. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/414400) for issues in GitLab 16.1. Support for merge requests [added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/207136) in GitLab 18.6. | | `/unlock` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Unlock the discussions. | | `/unsubscribe` | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | {{< icon name="check-circle" >}} Yes | Unsubscribe from notifications. | | `/weight ` | {{< icon name="check-circle" >}} Yes | {{< icon name="dotted-circle" >}} No | {{< icon name="dotted-circle" >}} No | Set weight. Valid values are integers like `0`, `1`, or `2`. | 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 d2c58d925c40d8..b3f98005cd5218 100644 --- a/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb +++ b/ee/lib/ee/gitlab/quick_actions/merge_request_actions.rb @@ -73,6 +73,33 @@ module MergeRequestActions command :blocked_by do |merge_requests| create_merge_request_blocks(merge_requests, direction: :blocked_by) end + + desc { _('Remove blocking relationship with another merge request') } + explanation do |merge_request| + if merge_request + format(_('Removes blocking relationship with %{merge_request_ref}.'), + merge_request_ref: merge_request.to_reference(full: true)) + end + end + execution_message do |merge_request| + if merge_request + format(_('Removed blocking relationship with %{merge_request_ref}.'), + merge_request_ref: merge_request.to_reference(full: true)) + end + end + params '<#merge_request | group/project!merge_request | merge request URL>' + types MergeRequest + condition do + quick_action_target.persisted? && + quick_action_target.project&.licensed_feature_available?(:blocking_merge_requests) && + current_user.can?(:update_merge_request, quick_action_target) + end + parse_params do |mr_param| + extract_references(mr_param, :merge_request).first + end + command :unlink do |merge_request| + destroy_merge_request_block(merge_request) + end end override :process_reviewer_users @@ -142,6 +169,38 @@ def create_single_block(merge_request, direction) ).execute end end + + def destroy_merge_request_block(merge_request) + return unless merge_request + + # Find block in either direction + link = MergeRequestBlock.where( + blocking_merge_request: merge_request, + blocked_merge_request: quick_action_target + ).or( + MergeRequestBlock.where( + blocking_merge_request: quick_action_target, + blocked_merge_request: merge_request + ) + ).first + + if link + blocked_mr = link.blocked_merge_request + + link.destroy! + + # Trigger merge status update and unblocked event + ::GraphqlTriggers.merge_request_merge_status_updated(blocked_mr) + + ::Gitlab::EventStore.publish( + ::MergeRequests::UnblockedStateEvent.new( + data: { current_user_id: current_user.id, merge_request_id: blocked_mr.id } + ) + ) + else + @execution_message[:unlink] = _('No blocking relationship exists between these merge requests.') + end + end end end end diff --git a/ee/spec/services/quick_actions/interpret_service_spec.rb b/ee/spec/services/quick_actions/interpret_service_spec.rb index 1e827f0c6b977f..d133070b1f130d 100644 --- a/ee/spec/services/quick_actions/interpret_service_spec.rb +++ b/ee/spec/services/quick_actions/interpret_service_spec.rb @@ -2172,6 +2172,83 @@ it_behaves_like 'quick action is unavailable', :blocked_by end end + + context 'with /unlink' do + let(:unlink_command) { "/unlink #{mr_ref1}" } + + context 'with sufficient permissions' do + it '/unlink is available' do + create(:merge_request_block, blocking_merge_request: merge_request1, blocked_merge_request: target_mr) + _, explanations = service.explain(unlink_command, target_mr) + + expect(explanations).to contain_exactly("Removes blocking relationship with #{mr_ref1}.") + end + + context 'when current MR blocks referenced MR' do + it '/unlink execution removes the block' do + create(:merge_request_block, blocking_merge_request: target_mr, blocked_merge_request: merge_request1) + _, _, message = service.execute(unlink_command, target_mr) + + expect(message).to include("Removed blocking relationship with #{mr_ref1}") + expect(target_mr.blocked_merge_requests).to be_empty + end + end + + context 'when referenced MR blocks current MR' do + it '/unlink execution removes the block' do + create(:merge_request_block, blocking_merge_request: merge_request1, blocked_merge_request: target_mr) + _, _, message = service.execute(unlink_command, target_mr) + + expect(message).to include("Removed blocking relationship with #{mr_ref1}") + expect(target_mr.blocking_merge_requests).to be_empty + end + end + + context 'when no block exists' do + it 'shows an error message' do + _, _, message = service.execute(unlink_command, target_mr) + + expect(message).to include("No blocking relationship exists between these merge requests") + end + end + + context 'when providing issue references instead of MR references' do + let_it_be(:issue1) { create(:issue, project: project) } + let(:issue_ref_command) { "/unlink #{issue1.to_reference}" } + + it 'ignores non-MR references' do + create(:merge_request_block, blocking_merge_request: merge_request1, blocked_merge_request: target_mr) + _, _, _ = service.execute(issue_ref_command, target_mr) + + expect(target_mr.blocking_merge_requests).to contain_exactly(merge_request1) + end + end + + context 'when licensed feature is not available' do + let(:target) { target_mr } + + before do + stub_licensed_features(blocking_merge_requests: false) + end + + it_behaves_like 'quick action is unavailable', :unlink + end + + context 'when target is not a merge request' do + let(:target) { create(:issue, project: restricted_project) } + + it_behaves_like 'quick action is unavailable', :unlink + end + end + + context 'with insufficient permissions' do + let(:current_user) { guest } + let(:target_mr) { create(:merge_request, :skip_diff_creation, source_project: restricted_project, source_branch: 'feature-insufficient-perms-3') } + let(:target) { target_mr } + + it_behaves_like 'quick action is unavailable', :unlink + end + end end describe 'promote_to command' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 47e6d7d0cfbed2..f723fd3791199d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -43426,6 +43426,9 @@ msgstr "" msgid "No available branches" msgstr "" +msgid "No blocking relationship exists between these merge requests." +msgstr "" + msgid "No branch selected" msgstr "" @@ -54266,6 +54269,9 @@ msgstr "" msgid "Remove avatar" msgstr "" +msgid "Remove blocking relationship with another merge request" +msgstr "" + msgid "Remove bypass" msgstr "" @@ -54446,6 +54452,9 @@ msgstr "" msgid "Removed an issue from an epic." msgstr "" +msgid "Removed blocking relationship with %{merge_request_ref}." +msgstr "" + msgid "Removed linked item %{issue_ref}." msgstr "" @@ -54493,6 +54502,9 @@ msgstr "" msgid "Removes an issue from an epic." msgstr "" +msgid "Removes blocking relationship with %{merge_request_ref}." +msgstr "" + msgid "Removes email participants." msgstr "" -- GitLab From c60a34b3d88e82679f2fd75f5ac62bd786004b43 Mon Sep 17 00:00:00 2001 From: Patrick Talbert Date: Tue, 28 Oct 2025 11:32:04 +0000 Subject: [PATCH 4/4] Update QuickActions CommandDefinion valid_type? method Now there may be multiple defined commands with the same name that work with distinct context types (Issues, MRs, etc). This means the valid_type? method needs to always check if the command type matches the context's quick_action_target or the UI may suggest variants of a command that are not relevant. Signed-off-by: Patrick Talbert --- .../quick_actions/command_definition.rb | 3 +- .../quick_actions/command_definition_spec.rb | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb index 5de1327732c4bc..54dba7f701baad 100644 --- a/lib/gitlab/quick_actions/command_definition.rb +++ b/lib/gitlab/quick_actions/command_definition.rb @@ -162,7 +162,8 @@ def parse_params(arg, context) def valid_type?(context) types.blank? || types.any? do |type| if context.quick_action_target.is_a?(WorkItem) - context.quick_action_target.supported_quick_action_commands.include?(name.to_sym) + context.quick_action_target.is_a?(type) && + context.quick_action_target.supported_quick_action_commands.include?(name.to_sym) else context.quick_action_target.is_a?(type) end diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index 5d3cbfd597cb2d..ce4e1b8e94f271 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -368,4 +368,65 @@ end end end + + describe '#valid_type? with multiple commands having same name but different types' do + let(:issue_blocks_command) { described_class.new(:blocks) } + let(:mr_blocks_command) { described_class.new(:blocks) } + let(:context_issue) { double('context', quick_action_target: build(:issue)) } + let(:context_mr) { double('context', quick_action_target: build(:merge_request)) } + let(:context_work_item) { double('context', quick_action_target: build(:work_item)) } + + before do + issue_blocks_command.types = [Issue] + mr_blocks_command.types = [MergeRequest] + end + + context 'with Issue target' do + it 'allows Issue command' do + expect(issue_blocks_command.send(:valid_type?, context_issue)).to be true + end + + it 'does not allow MergeRequest command' do + expect(mr_blocks_command.send(:valid_type?, context_issue)).to be false + end + end + + context 'with MergeRequest target' do + it 'allows MergeRequest command' do + expect(mr_blocks_command.send(:valid_type?, context_mr)).to be true + end + + it 'does not allow Issue command' do + expect(issue_blocks_command.send(:valid_type?, context_mr)).to be false + end + end + + context 'with WorkItem target' do + before do + allow_next_instance_of(WorkItem) do |work_item| + allow(work_item).to receive(:supported_quick_action_commands).and_return([:blocks]) + end + end + + it 'allows Issue command when in supported_quick_action_commands' do + expect(issue_blocks_command.send(:valid_type?, context_work_item)).to be true + end + + it 'does not allow MergeRequest command even if in supported_quick_action_commands' do + expect(mr_blocks_command.send(:valid_type?, context_work_item)).to be false + end + + context 'when command is not in supported_quick_action_commands' do + before do + allow_next_instance_of(WorkItem) do |work_item| + allow(work_item).to receive(:supported_quick_action_commands).and_return([]) + end + end + + it 'does not allow Issue command' do + expect(issue_blocks_command.send(:valid_type?, context_work_item)).to be false + end + end + end + end end -- GitLab