Add edit_note and spec for editing quick actions

Call QuickActionsService on Note update

Add support for notes which just contain
commands after editing

Return http status gone (410) if note was deleted

Temporary frontend addition so it is not
failing when a note is deleted

Move specs to shared examples

Fix rubocop style issue

Deleting note on frontend when status is 410

Use guard clause for note which got deleted

Simplified condition for nil note

This method should no longer be called
with nil note

Refactoring of execute method to reduce
complexity

Move errors update to delete_note method

Note is now deleted visually when it only
contains commands after update

Add expectation

Fix style issues

Changing action to fix tests

Add tests for removeNote and update
deleteNote expectations

10 files changed, 197 insertions, 32 deletions

diff --git a/app/assets/javascripts/lib/utils/http_status.js b/app/assets/javascripts/lib/utils/http_status.js
index 37ad1676f7a..5e5d10883a3 100644
--- a/app/assets/javascripts/lib/utils/http_status.js
+++ b/app/assets/javascripts/lib/utils/http_status.js
@@ -19,6 +19,7 @@ const httpStatusCodes = {
   UNAUTHORIZED: 401,
   FORBIDDEN: 403,
   NOT_FOUND: 404,
+  GONE: 410,
   UNPROCESSABLE_ENTITY: 422,
 };
 
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index 2f201839d45..9019f0542b6 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/app/assets/javascripts/notes/components/noteable_note.vue
@@ -14,6 +14,7 @@ import NoteBody from './note_body.vue';
 import eventHub from '../event_hub';
 import noteable from '../mixins/noteable';
 import resolvable from '../mixins/resolvable';
+import httpStatusCodes from '~/lib/utils/http_status';
 
 export default {
   name: 'NoteableNote',
@@ -122,7 +123,13 @@ export default {
   },
 
   methods: {
-    ...mapActions(['deleteNote', 'updateNote', 'toggleResolveNote', 'scrollToNoteIfNeeded']),
+    ...mapActions([
+      'deleteNote',
+      'removeNote',
+      'updateNote',
+      'toggleResolveNote',
+      'scrollToNoteIfNeeded',
+    ]),
     editHandler() {
       this.isEditing = true;
       this.$emit('handleEdit');
@@ -185,15 +192,21 @@ export default {
           this.updateSuccess();
           callback();
         })
-        .catch(() => {
-          this.isRequesting = false;
-          this.isEditing = true;
-          this.$nextTick(() => {
-            const msg = __('Something went wrong while editing your comment. Please try again.');
-            Flash(msg, 'alert', this.$el);
-            this.recoverNoteContent(noteText);
+        .catch(response => {
+          if (response.status === httpStatusCodes.GONE) {
+            this.removeNote(this.note);
+            this.updateSuccess();
             callback();
-          });
+          } else {
+            this.isRequesting = false;
+            this.isEditing = true;
+            this.$nextTick(() => {
+              const msg = __('Something went wrong while editing your comment. Please try again.');
+              Flash(msg, 'alert', this.$el);
+              this.recoverNoteContent(noteText);
+              callback();
+            });
+          }
         });
     },
     formCancelHandler(shouldConfirm, isDirty) {
diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js
index b7857997d42..411bd585672 100644
--- a/app/assets/javascripts/notes/stores/actions.js
+++ b/app/assets/javascripts/notes/stores/actions.js
@@ -61,18 +61,22 @@ export const updateDiscussion = ({ commit, state }, discussion) => {
   return utils.findNoteObjectById(state.discussions, discussion.id);
 };
 
-export const deleteNote = ({ commit, dispatch, state }, note) =>
-  axios.delete(note.path).then(() => {
-    const discussion = state.discussions.find(({ id }) => id === note.discussion_id);
+export const removeNote = ({ commit, dispatch, state }, note) => {
+  const discussion = state.discussions.find(({ id }) => id === note.discussion_id);
 
-    commit(types.DELETE_NOTE, note);
+  commit(types.DELETE_NOTE, note);
 
-    dispatch('updateMergeRequestWidget');
-    dispatch('updateResolvableDiscussionsCounts');
+  dispatch('updateMergeRequestWidget');
+  dispatch('updateResolvableDiscussionsCounts');
 
-    if (isInMRPage()) {
-      dispatch('diffs/removeDiscussionsFromDiff', discussion);
-    }
+  if (isInMRPage()) {
+    dispatch('diffs/removeDiscussionsFromDiff', discussion);
+  }
+};
+
+export const deleteNote = ({ dispatch }, note) =>
+  axios.delete(note.path).then(() => {
+    dispatch('removeNote', note);
   });
 
 export const updateNote = ({ commit, dispatch }, { endpoint, note }) =>
diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb
index d2a961efff7..4b7899d469b 100644
--- a/app/controllers/concerns/notes_actions.rb
+++ b/app/controllers/concerns/notes_actions.rb
@@ -73,6 +73,11 @@ module NotesActions
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def update
     @note = Notes::UpdateService.new(project, current_user, update_note_params).execute(note)
+    unless @note
+      head :gone
+      return
+    end
+
     prepare_notes_for_rendering([@note])
 
     respond_to do |format|
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 65d9b074eee..13e8453ed00 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -6,7 +6,7 @@ class Projects::NotesController < Projects::ApplicationController
   include NotesHelper
   include ToggleAwardEmoji
 
-  before_action :whitelist_query_limiting, only: [:create]
+  before_action :whitelist_query_limiting, only: [:create, :update]
   before_action :authorize_read_note!
   before_action :authorize_create_note!, only: [:create]
   before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb
index 384d1dd2e50..853faed9d85 100644
--- a/app/services/notes/update_service.rb
+++ b/app/services/notes/update_service.rb
@@ -8,24 +8,70 @@ module Notes
       old_mentioned_users = note.mentioned_users.to_a
 
       note.update(params.merge(updated_by: current_user))
-      note.create_new_cross_references!(current_user)
 
-      if note.previous_changes.include?('note')
-        TodoService.new.update_note(note, current_user, old_mentioned_users)
+      only_commands = false
+
+      quick_actions_service = QuickActionsService.new(project, current_user)
+      if quick_actions_service.supported?(note)
+        content, update_params, message = quick_actions_service.execute(note, {})
+
+        only_commands = content.empty?
+
+        note.note = content
+      end
+
+      unless only_commands
+        note.create_new_cross_references!(current_user)
+
+        update_todos(note, old_mentioned_users)
+
+        update_suggestions(note)
       end
 
-      if note.supports_suggestion?
-        Suggestion.transaction do
-          note.suggestions.delete_all
-          Suggestions::CreateService.new(note).execute
+      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 need to refresh the previous suggestions call cache
-        # in order to get the new records.
-        note.reset
+        if only_commands
+          delete_note(note, message)
+          note = nil
+        else
+          note.save
+        end
       end
 
       note
     end
+
+    private
+
+    def delete_note(note, message)
+      # We must add the error after we call #save because errors are reset
+      # when #save is called
+      note.errors.add(:commands_only, message.presence || _('Commands did not apply'))
+
+      Notes::DestroyService.new(project, current_user).execute(note)
+    end
+
+    def update_suggestions(note)
+      return unless note.supports_suggestion?
+
+      Suggestion.transaction do
+        note.suggestions.delete_all
+        Suggestions::CreateService.new(note).execute
+      end
+
+      # We need to refresh the previous suggestions call cache
+      # in order to get the new records.
+      note.reset
+    end
+
+    def update_todos(note, old_mentioned_users)
+      return unless note.previous_changes.include?('note')
+
+      TodoService.new.update_note(note, current_user, old_mentioned_users)
+    end
   end
 end
diff --git a/changelogs/unreleased/21505-quickactions-update-pd.yml b/changelogs/unreleased/21505-quickactions-update-pd.yml
new file mode 100644
index 00000000000..243f8eda4e3
--- /dev/null
+++ b/changelogs/unreleased/21505-quickactions-update-pd.yml
@@ -0,0 +1,5 @@
+---
+title: Apply quickactions when modifying comments
+merge_request: 31136
+author:
+type: added
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 61642fbbd59..8c8574d0a48 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2977,6 +2977,9 @@ msgstr ""
 msgid "Commands applied"
 msgstr ""
 
+msgid "Commands did not apply"
+msgstr ""
+
 msgid "Comment"
 msgstr ""
 
diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js
index e55aa0e965a..1fd4a9a7612 100644
--- a/spec/javascripts/notes/stores/actions_spec.js
+++ b/spec/javascripts/notes/stores/actions_spec.js
@@ -336,7 +336,7 @@ describe('Actions Notes Store', () => {
     });
   });
 
-  describe('deleteNote', () => {
+  describe('removeNote', () => {
     const endpoint = `${TEST_HOST}/note`;
     let axiosMock;
 
@@ -357,7 +357,7 @@ describe('Actions Notes Store', () => {
       const note = { path: endpoint, id: 1 };
 
       testAction(
-        actions.deleteNote,
+        actions.removeNote,
         note,
         store.state,
         [
@@ -384,7 +384,7 @@ describe('Actions Notes Store', () => {
       $('body').attr('data-page', 'projects:merge_requests:show');
 
       testAction(
-        actions.deleteNote,
+        actions.removeNote,
         note,
         store.state,
         [
@@ -409,6 +409,45 @@ describe('Actions Notes Store', () => {
     });
   });
 
+  describe('deleteNote', () => {
+    const endpoint = `${TEST_HOST}/note`;
+    let axiosMock;
+
+    beforeEach(() => {
+      axiosMock = new AxiosMockAdapter(axios);
+      axiosMock.onDelete(endpoint).replyOnce(200, {});
+
+      $('body').attr('data-page', '');
+    });
+
+    afterEach(() => {
+      axiosMock.restore();
+
+      $('body').attr('data-page', '');
+    });
+
+    it('dispatches removeNote', done => {
+      const note = { path: endpoint, id: 1 };
+
+      testAction(
+        actions.deleteNote,
+        note,
+        {},
+        [],
+        [
+          {
+            type: 'removeNote',
+            payload: {
+              id: 1,
+              path: 'http://test.host/note',
+            },
+          },
+        ],
+        done,
+      );
+    });
+  });
+
   describe('createNewNote', () => {
     describe('success', () => {
       const res = {
diff --git a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb
index a37b2392d52..bebc8509d53 100644
--- a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb
+++ b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb
@@ -89,5 +89,54 @@ shared_examples 'move quick action' do
         it_behaves_like 'applies the commands to issues in both projects, target and source'
       end
     end
+
+    context 'when editing comments' do
+      let(:target_project) { create(:project, :public) }
+
+      before do
+        target_project.add_maintainer(user)
+
+        sign_in(user)
+        visit project_issue_path(project, issue)
+        wait_for_all_requests
+      end
+
+      it 'moves the issue after quickcommand note was updated' do
+        # misspelled quick action
+        add_note("test note.\n/mvoe #{target_project.full_path}")
+
+        expect(issue.reload).not_to be_closed
+
+        edit_note("/mvoe #{target_project.full_path}", "test note.\n/move #{target_project.full_path}")
+        wait_for_all_requests
+
+        expect(page).to have_content 'test note.'
+        expect(issue.reload).to be_closed
+
+        visit project_issue_path(target_project, issue)
+        wait_for_all_requests
+
+        expect(page).to have_content 'Issues 1'
+      end
+
+      it 'deletes the note if it was updated to just contain a command' do
+        # missspelled quick action
+        add_note("test note.\n/mvoe #{target_project.full_path}")
+
+        expect(page).not_to have_content 'Commands applied'
+        expect(issue.reload).not_to be_closed
+
+        edit_note("/mvoe #{target_project.full_path}", "/move #{target_project.full_path}")
+        wait_for_all_requests
+
+        expect(page).not_to have_content "/move #{target_project.full_path}"
+        expect(issue.reload).to be_closed
+
+        visit project_issue_path(target_project, issue)
+        wait_for_all_requests
+
+        expect(page).to have_content 'Issues 1'
+      end
+    end
   end
 end