Commit a13abd67 authored by Patrick Derichs's avatar Patrick Derichs Committed by Kamil Trzciński

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
parent e5e6a5fb
......@@ -19,6 +19,7 @@ const httpStatusCodes = {
UNAUTHORIZED: 401,
FORBIDDEN: 403,
NOT_FOUND: 404,
GONE: 410,
UNPROCESSABLE_ENTITY: 422,
};
......
......@@ -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,7 +192,12 @@ export default {
this.updateSuccess();
callback();
})
.catch(() => {
.catch(response => {
if (response.status === httpStatusCodes.GONE) {
this.removeNote(this.note);
this.updateSuccess();
callback();
} else {
this.isRequesting = false;
this.isEditing = true;
this.$nextTick(() => {
......@@ -194,6 +206,7 @@ export default {
this.recoverNoteContent(noteText);
callback();
});
}
});
},
formCancelHandler(shouldConfirm, isDirty) {
......
......@@ -61,8 +61,7 @@ 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(() => {
export const removeNote = ({ commit, dispatch, state }, note) => {
const discussion = state.discussions.find(({ id }) => id === note.discussion_id);
commit(types.DELETE_NOTE, note);
......@@ -73,6 +72,11 @@ export const deleteNote = ({ commit, dispatch, state }, note) =>
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 }) =>
......
......@@ -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|
......
......@@ -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]
......
......@@ -8,13 +8,56 @@ module Notes
old_mentioned_users = note.mentioned_users.to_a
note.update(params.merge(updated_by: current_user))
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)
if note.previous_changes.include?('note')
TodoService.new.update_note(note, current_user, old_mentioned_users)
update_todos(note, old_mentioned_users)
update_suggestions(note)
end
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
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?
if note.supports_suggestion?
Suggestion.transaction do
note.suggestions.delete_all
Suggestions::CreateService.new(note).execute
......@@ -25,7 +68,10 @@ module Notes
note.reset
end
note
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
---
title: Apply quickactions when modifying comments
merge_request: 31136
author:
type: added
......@@ -2977,6 +2977,9 @@ msgstr ""
msgid "Commands applied"
msgstr ""
msgid "Commands did not apply"
msgstr ""
msgid "Comment"
msgstr ""
......
......@@ -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 = {
......
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment