Merge branch 'ce-issue_8844' into 'master'

[CE Backport] Show commands applied message when promoting issues to epics

See merge request gitlab-org/gitlab-ce!25685

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

app/services/notes/quick_actions_service.rb

 # 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

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

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)

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

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

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

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

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