diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index d78e53909dfcb8866f11f9cef2f007f121b0e067..888d4594cf30423373a7a9f0e8fab35b9b6ae99a 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -37,7 +37,7 @@ def execute(note, options = {}) @interpret_service = QuickActions::InterpretService.new( container: note.resource_parent, current_user: current_user, - params: options + params: options.merge(discussion_id: note.discussion_id) ) # NOTE: old_note would be nil if the note hasn't changed or it is a new record @@ -57,11 +57,16 @@ def apply_updates(update_params, note) update_params[:spend_time][:note_id] = note.id end + execute_triggers(note, update_params) execute_update_service(note, update_params) end private + def execute_triggers(note, params) + # This is overridden in EE + end + def execute_update_service(note, params) service_response = noteable_update_service(note, params).execute(note.noteable) diff --git a/ee/app/services/ee/notes/quick_actions_service.rb b/ee/app/services/ee/notes/quick_actions_service.rb index ff56f50c9fde4842e28a9c5f40d6ee0ca0f1e9af..6976744b1bb3be75c30f7b463603222ad32eabd9 100644 --- a/ee/app/services/ee/notes/quick_actions_service.rb +++ b/ee/app/services/ee/notes/quick_actions_service.rb @@ -4,6 +4,7 @@ module EE module Notes module QuickActionsService extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override include ::Gitlab::Utils::StrongMemoize prepended do @@ -25,6 +26,28 @@ def noteable_update_service(note, update_params) Epics::UpdateService.new(group: note.resource_parent, current_user: current_user, params: update_params) end + + override :execute_triggers + def execute_triggers(note, params) + super + + execute_amazon_q_trigger(note, params) + end + + def execute_amazon_q_trigger(note, params) + return unless params[:amazon_q] + + q_params = params.delete(:amazon_q) + + ::Ai::AmazonQ::AmazonQTriggerService.new( + user: current_user, + command: q_params[:command], + input: q_params[:input], + source: q_params[:source], + note: note, + discussion_id: q_params[:discussion_id] + ).execute + end end end end diff --git a/ee/spec/services/ee/notes/quick_actions_service_spec.rb b/ee/spec/services/ee/notes/quick_actions_service_spec.rb index 1f7830a6bca1d7ba5965e4ae400a05e62e784c0e..c3ec25848e4f804b1a7a684e33e8da349c6b1146 100644 --- a/ee/spec/services/ee/notes/quick_actions_service_spec.rb +++ b/ee/spec/services/ee/notes/quick_actions_service_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' RSpec.describe Notes::QuickActionsService, feature_category: :team_planning do + using RSpec::Parameterized::TableSyntax + let_it_be(:group) { create(:group) } let_it_be(:private_group) { create(:group, :private) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -9,6 +11,7 @@ let_it_be(:assignee) { create(:user) } let_it_be(:reviewer) { create(:user) } let_it_be(:issue, reload: true) { create(:issue, project: project) } + let_it_be(:merge_request, reload: true) { create(:merge_request, source_project: project, target_project: project) } let_it_be(:epic, reload: true) { create(:epic, group: group) } let_it_be(:private_epic) { create(:epic, group: private_group) } @@ -104,7 +107,7 @@ def execute(note, include_message: false) end context 'on a merge request' do - let(:note_mr) { create(:note_on_merge_request, project: project, note: note_text) } + let(:note_mr) { create(:note_on_merge_request, project: project, noteable: merge_request, note: note_text) } it 'leaves the note empty' do expect(execute(note_mr)).to be_empty @@ -205,7 +208,7 @@ def execute(note, include_message: false) end context 'on a merge request' do - let(:note_mr) { create(:note_on_merge_request, project: project, note: note_text) } + let(:note_mr) { create(:note_on_merge_request, project: project, noteable: merge_request, note: note_text) } it 'leaves the note empty' do expect(execute(note_mr)).to be_empty @@ -424,14 +427,18 @@ def execute(note, include_message: false) end context 'with a merge request' do - let(:note) { create(:note_on_merge_request, note: note_text, project: project) } + let(:note) { create(:note_on_merge_request, note: note_text, noteable: merge_request, project: project) } it_behaves_like 'assigns one or more reviewers to the merge request', multiline: false do let(:target) { note.noteable } end it_behaves_like 'assigns one or more reviewers to the merge request', multiline: true do - let(:note) { create(:note_on_merge_request, note: multiline_assign_reviewer_text, project: project) } + let(:note) do + create(:note_on_merge_request, note: multiline_assign_reviewer_text, noteable: merge_request, + project: project) + end + let(:target) { note.noteable } end end @@ -474,14 +481,17 @@ def execute(note, include_message: false) end context 'MergeRequest' do - let(:note) { create(:note_on_merge_request, note: note_text, project: project) } + let(:note) { create(:note_on_merge_request, note: note_text, noteable: merge_request, project: project) } it_behaves_like 'assigning an already assigned user', false do let(:target) { note.noteable } end it_behaves_like 'assigning an already assigned user', true do - let(:note) { create(:note_on_merge_request, note: multiline_assign_note_text, project: project) } + let(:note) do + create(:note_on_merge_request, note: multiline_assign_note_text, noteable: merge_request, project: project) + end + let(:target) { note.noteable } end end @@ -509,14 +519,17 @@ def execute(note, include_message: false) end context 'MergeRequest' do - let(:note) { create(:note_on_merge_request, note: note_text, project: project) } + let(:note) { create(:note_on_merge_request, note: note_text, noteable: merge_request, project: project) } it_behaves_like 'unassigning a not assigned user', false do let(:target) { note.noteable } end it_behaves_like 'unassigning a not assigned user', true do - let(:note) { create(:note_on_merge_request, note: multiline_unassign_note_text, project: project) } + let(:note) do + create(:note_on_merge_request, note: multiline_unassign_note_text, noteable: merge_request, project: project) + end + let(:target) { note.noteable } end end @@ -538,14 +551,18 @@ def execute(note, include_message: false) end context 'with a merge request' do - let(:note) { create(:note_on_merge_request, note: note_text, project: project) } + let(:note) { create(:note_on_merge_request, note: note_text, noteable: merge_request, project: project) } it_behaves_like 'unassigning one or more reviewers', multiline: false do let(:target) { note.noteable } end it_behaves_like 'unassigning one or more reviewers', multiline: true do - let(:note) { create(:note_on_merge_request, note: multiline_unassign_reviewer_note_text, project: project) } + let(:note) do + create(:note_on_merge_request, note: multiline_unassign_reviewer_note_text, noteable: merge_request, + project: project) + end + let(:target) { note.noteable } end end @@ -791,6 +808,7 @@ def execute(note, include_message: false) let_it_be(:user) { create(:user) } let(:amazon_q_enabled) { false } let(:note) { create(:note_on_issue, project: project, noteable: issue, note: note_text) } + let(:trigger_service) { instance_double(::Ai::AmazonQ::AmazonQTriggerService) } before do project.add_developer(user) @@ -801,6 +819,8 @@ def execute(note, include_message: false) let(:note_text) { '/q dev' } it 'does not run command' do + expect(::Ai::AmazonQ::AmazonQTriggerService).not_to receive(:new) + result = execute(note, include_message: true) expect(result[0]).to be_empty @@ -810,19 +830,66 @@ def execute(note, include_message: false) context 'when Amazon Q is enabled' do let(:amazon_q_enabled) { true } - let(:note_text) { '/q dev' } + let(:note_text) { "/q #{command}" } + let(:discussion_id) { note.discussion_id } + let(:note_with_quick_action) { note } + + shared_examples 'successful Q command' do + it 'runs the command' do + expect(trigger_service).to receive(:execute) + expect(::Ai::AmazonQ::AmazonQTriggerService).to receive(:new).with( + user: user, + command: command, + input: '', + note: note_with_quick_action, + source: source, + discussion_id: discussion_id + ).and_return(trigger_service) + + result = execute(note_with_quick_action, include_message: true) - it 'runs the command' do - result = execute(note, include_message: true) + expect(result[0]).to be_empty + expect(result[1]).to eq('Q got your message!') + end + end - expect(result[0]).to be_empty - expect(result[1]).to eq('Q got your message!') + context 'with issue' do + let(:source) { issue } + + where(command: ::Ai::AmazonQ::Commands::ISSUE_SUBCOMMANDS) + + with_them do + it_behaves_like 'successful Q command' + end + end + + context 'with merge_request' do + let(:note) { create(:note_on_merge_request, project: project, noteable: merge_request, note: note_text) } + let(:source) { merge_request } + + where(command: ::Ai::AmazonQ::Commands::MERGE_REQUEST_SUBCOMMANDS) + + with_them do + it_behaves_like 'successful Q command' + end + + context 'with a note on an existing discussion' do + let(:note_with_quick_action) do + build(:note, noteable: merge_request, project: project, discussion_id: note.discussion_id, note: "/q dev") + end + + let(:command) { 'dev' } + + it_behaves_like 'successful Q command' + end end context 'when using unsupported sub-command for issue' do let_it_be(:note_text) { '/q unknown' } it 'returns an error' do + expect(::Ai::AmazonQ::AmazonQTriggerService).not_to receive(:new) + content, update_params, message, _ = service.execute(note) expect(content).to be_blank @@ -836,6 +903,8 @@ def execute(note, include_message: false) let_it_be(:note) { create(:note_on_epic, project: project, note: note_text) } it 'returns an error' do + expect(::Ai::AmazonQ::AmazonQTriggerService).not_to receive(:new) + content, update_params, message, _ = service.execute(note) expect(content).to be_blank @@ -846,9 +915,11 @@ def execute(note, include_message: false) context 'when using unsupported sub-command transform for merge request' do let_it_be(:note_text) { '/q transform' } - let_it_be(:note) { create(:note_on_merge_request, project: project, note: note_text) } + let_it_be(:note) { create(:note_on_merge_request, project: project, noteable: merge_request, note: note_text) } it 'returns an error' do + expect(::Ai::AmazonQ::AmazonQTriggerService).not_to receive(:new) + content, update_params, message, _ = service.execute(note) expect(content).to be_blank