diff options
author | Felipe Artur <fcardozo@gitlab.com> | 2019-03-04 12:21:47 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-03-04 12:21:47 +0300 |
commit | 78dc1b58a64be03e6d3622aab5605e1d5f6a6643 (patch) | |
tree | d0dd8c328bb0c7ce2311c79f525f7f23db9e0ff0 | |
parent | 4b0036b87ee30ce0a3687dd052514b571f4d520f (diff) |
Show commands applied message when promoting issues
Fix 'commands applied' messages not being shown when issue is promoted to epic
using slash commands.
-rw-r--r-- | app/services/notes/create_service.rb | 11 | ||||
-rw-r--r-- | app/services/notes/quick_actions_service.rb | 26 | ||||
-rw-r--r-- | app/services/quick_actions/interpret_service.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/quick_actions/command_definition.rb | 9 | ||||
-rw-r--r-- | spec/features/issues/user_uses_quick_actions_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/quick_actions/command_definition_spec.rb | 7 | ||||
-rw-r--r-- | spec/services/notes/create_service_spec.rb | 13 | ||||
-rw-r--r-- | spec/services/notes/quick_actions_service_spec.rb | 24 | ||||
-rw-r--r-- | spec/services/quick_actions/interpret_service_spec.rb | 10 |
9 files changed, 84 insertions, 25 deletions
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 5a6e7338b42..1b46f6d8a72 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -21,7 +21,7 @@ module Notes if quick_actions_service.supported?(note) options = { merge_request_diff_head_sha: merge_request_diff_head_sha } - content, command_params = quick_actions_service.extract_commands(note, options) + content, update_params = quick_actions_service.execute(note, options) only_commands = content.empty? @@ -43,16 +43,17 @@ module Notes Suggestions::CreateService.new(note).execute end - if command_params.present? - quick_actions_service.execute(command_params, note) + if quick_actions_service.commands_executed_count.to_i > 0 + if update_params.present? + quick_actions_service.apply_updates(update_params, note) + note.commands_changes = update_params + end # We must add the error after we call #save because errors are reset # when #save is called if only_commands note.errors.add(:commands_only, 'Commands applied') end - - note.commands_changes = command_params end note diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index 985a03060bd..0852a708240 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -1,7 +1,18 @@ # frozen_string_literal: true +# QuickActionsService class +# +# Executes quick actions commands extracted from note text +# +# Most commands returns parameters to be applied later +# using QuickActionService#apply_updates +# module Notes class QuickActionsService < BaseService + attr_reader :interpret_service + + delegate :commands_executed_count, to: :interpret_service, allow_nil: true + UPDATE_SERVICES = { 'Issue' => Issues::UpdateService, 'MergeRequest' => MergeRequests::UpdateService, @@ -25,18 +36,21 @@ module Notes self.class.supported?(note) end - def extract_commands(note, options = {}) + def execute(note, options = {}) return [note.note, {}] unless supported?(note) - QuickActions::InterpretService.new(project, current_user, options) - .execute(note.note, note.noteable) + @interpret_service = QuickActions::InterpretService.new(project, current_user, options) + + @interpret_service.execute(note.note, note.noteable) end - def execute(command_params, note) - return if command_params.empty? + # Applies updates extracted to note#noteable + # The update parameters are extracted on self#execute + def apply_updates(update_params, note) + return if update_params.empty? return unless supported?(note) - self.class.noteable_update_service(note).new(note.parent, current_user, command_params).execute(note.noteable) + self.class.noteable_update_service(note).new(note.parent, current_user, update_params).execute(note.noteable) end end end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 5c58caee8cd..131efb8925e 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -7,6 +7,11 @@ module QuickActions attr_reader :issuable + # Counts how many commands have been executed. + # Used to display relevant feedback on UI when a note + # with only commands has been processed. + attr_accessor :commands_executed_count + SHRUG = '¯\\_(ツ)_/¯'.freeze TABLEFLIP = '(╯°□°)╯︵ ┻━┻'.freeze diff --git a/lib/gitlab/quick_actions/command_definition.rb b/lib/gitlab/quick_actions/command_definition.rb index 259345b8a9a..e7bfcb16582 100644 --- a/lib/gitlab/quick_actions/command_definition.rb +++ b/lib/gitlab/quick_actions/command_definition.rb @@ -48,6 +48,8 @@ module Gitlab def execute(context, arg) return if noop? || !available?(context) + count_commands_executed_in(context) + execute_block(action_block, context, arg) end @@ -73,6 +75,13 @@ module Gitlab private + def count_commands_executed_in(context) + return unless context.respond_to?(:commands_executed_count=) + + context.commands_executed_count ||= 0 + context.commands_executed_count += 1 + end + def execute_block(block, context, arg) if arg.present? parsed = parse_params(arg, context) diff --git a/spec/features/issues/user_uses_quick_actions_spec.rb b/spec/features/issues/user_uses_quick_actions_spec.rb index 27cffdc5f8b..b5e7c3954e2 100644 --- a/spec/features/issues/user_uses_quick_actions_spec.rb +++ b/spec/features/issues/user_uses_quick_actions_spec.rb @@ -243,7 +243,9 @@ describe 'Issues > User uses quick actions', :js do it 'does not move the issue' do add_note("/move not/valid") - expect(page).not_to have_content 'Commands applied' + wait_for_requests + + expect(page).to have_content 'Commands applied' expect(issue.reload).to be_open end end diff --git a/spec/lib/gitlab/quick_actions/command_definition_spec.rb b/spec/lib/gitlab/quick_actions/command_definition_spec.rb index 5dae82a63b4..136cfb5bcc5 100644 --- a/spec/lib/gitlab/quick_actions/command_definition_spec.rb +++ b/spec/lib/gitlab/quick_actions/command_definition_spec.rb @@ -72,7 +72,7 @@ describe Gitlab::QuickActions::CommandDefinition do end describe "#execute" do - let(:context) { OpenStruct.new(run: false) } + let(:context) { OpenStruct.new(run: false, commands_executed_count: nil) } context "when the command is a noop" do it "doesn't execute the command" do @@ -80,6 +80,7 @@ describe Gitlab::QuickActions::CommandDefinition do subject.execute(context, nil) + expect(context.commands_executed_count).to be_nil expect(context.run).to be false end end @@ -97,6 +98,7 @@ describe Gitlab::QuickActions::CommandDefinition do it "doesn't execute the command" do subject.execute(context, nil) + expect(context.commands_executed_count).to be_nil expect(context.run).to be false end end @@ -112,6 +114,7 @@ describe Gitlab::QuickActions::CommandDefinition do subject.execute(context, true) expect(context.run).to be true + expect(context.commands_executed_count).to eq(1) end end @@ -120,6 +123,7 @@ describe Gitlab::QuickActions::CommandDefinition do subject.execute(context, nil) expect(context.run).to be true + expect(context.commands_executed_count).to eq(1) end end end @@ -134,6 +138,7 @@ describe Gitlab::QuickActions::CommandDefinition do subject.execute(context, true) expect(context.run).to be true + expect(context.commands_executed_count).to eq(1) end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 1645b67c329..8d8e81173ff 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -220,6 +220,19 @@ describe Notes::CreateService do expect(note.note).to eq "HELLO\nWORLD" end end + + context 'when note only have commands' do + it 'adds commands applied message to note errors' do + note_text = %(/close) + service = double(:service) + allow(Issues::UpdateService).to receive(:new).and_return(service) + expect(service).to receive(:execute) + + note = described_class.new(project, user, opts.merge(note: note_text)).execute + + expect(note.errors[:commands_only]).to be_present + end + end end context 'as a user who cannot update the target' do diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index 14d62763a5b..7d2b6d5b8a7 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -28,8 +28,8 @@ describe Notes::QuickActionsService do end it 'closes noteable, sets labels, assigns, and sets milestone to noteable, and leave no note' do - content, command_params = service.extract_commands(note) - service.execute(command_params, note) + content, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(content).to eq '' expect(note.noteable).to be_closed @@ -47,8 +47,8 @@ describe Notes::QuickActionsService do let(:note_text) { '/reopen' } it 'opens the noteable, and leave no note' do - content, command_params = service.extract_commands(note) - service.execute(command_params, note) + content, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(content).to eq '' expect(note.noteable).to be_open @@ -59,8 +59,8 @@ describe Notes::QuickActionsService do let(:note_text) { '/spend 1h' } it 'updates the spent time on the noteable' do - content, command_params = service.extract_commands(note) - service.execute(command_params, note) + content, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(content).to eq '' expect(note.noteable.time_spent).to eq(3600) @@ -75,8 +75,8 @@ describe Notes::QuickActionsService do end it 'closes noteable, sets labels, assigns, and sets milestone to noteable' do - content, command_params = service.extract_commands(note) - service.execute(command_params, note) + content, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(content).to eq "HELLO\nWORLD" expect(note.noteable).to be_closed @@ -94,8 +94,8 @@ describe Notes::QuickActionsService do let(:note_text) { "HELLO\n/reopen\nWORLD" } it 'opens the noteable' do - content, command_params = service.extract_commands(note) - service.execute(command_params, note) + content, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(content).to eq "HELLO\nWORLD" expect(note.noteable).to be_open @@ -190,8 +190,8 @@ describe Notes::QuickActionsService do end it 'adds only one assignee from the list' do - _, command_params = service.extract_commands(note) - service.execute(command_params, note) + _, update_params = service.execute(note) + service.apply_updates(update_params, note) expect(note.noteable.assignees.count).to eq(1) end diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 938764f40b0..ea33d156c8a 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -1526,5 +1526,15 @@ describe QuickActions::InterpretService do end end end + + context "#commands_executed_count" do + it 'counts commands executed' do + content = "/close and \n/assign me and \n/title new title" + + service.execute(content, issue) + + expect(service.commands_executed_count).to eq(3) + end + end end end |