diff options
6 files changed, 89 insertions, 78 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue index 0fa14615532..bb9bb821de3 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -1,12 +1,9 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; -import createFlash from '~/flash'; import { s__ } from '~/locale'; import noteForm from '../../notes/components/note_form.vue'; -import { getNoteFormData } from '../store/utils'; import autosave from '../../notes/mixins/autosave'; import { DIFF_NOTE_TYPE } from '../constants'; -import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; export default { components: { @@ -39,6 +36,16 @@ export default { }), ...mapGetters('diffs', ['getDiffFileByHash']), ...mapGetters(['isLoggedIn', 'noteableType', 'getNoteableData', 'getNotesDataByProp']), + formData() { + return { + noteableData: this.noteableData, + noteableType: this.noteableType, + noteTargetLine: this.noteTargetLine, + diffViewType: this.diffViewType, + diffFile: this.getDiffFileByHash(this.diffFileHash), + linePosition: this.linePosition, + }; + }, }, mounted() { if (this.isLoggedIn) { @@ -53,8 +60,7 @@ export default { } }, methods: { - ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff']), - ...mapActions(['saveNote', 'refetchDiscussionById']), + ...mapActions('diffs', ['cancelCommentForm', 'assignDiscussionsToDiff', 'saveDiffDiscussion']), handleCancelCommentForm(shouldConfirm, isDirty) { if (shouldConfirm && isDirty) { const msg = s__('Notes|Are you sure you want to cancel creating this comment?'); @@ -73,35 +79,9 @@ export default { }); }, handleSaveNote(note) { - const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash); - const postData = getNoteFormData({ - note, - noteableData: this.noteableData, - noteableType: this.noteableType, - noteTargetLine: this.noteTargetLine, - diffViewType: this.diffViewType, - diffFile: selectedDiffFile, - linePosition: this.linePosition, - }); - - this.saveNote(postData) - .then(result => { - const endpoint = this.getNotesDataByProp('discussionsPath'); - - this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id }) - .then(selectedDiscussion => { - const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]); - this.assignDiscussionsToDiff(lineCodeDiscussions); - - this.handleCancelCommentForm(); - }) - .catch(() => { - createFlash(s__('MergeRequests|Updating discussions failed')); - }); - }) - .catch(() => { - createFlash(s__('MergeRequests|Saving the comment failed')); - }); + return this.saveDiffDiscussion({ note, formData: this.formData }).then(() => + this.handleCancelCommentForm(), + ); }, }, }; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index e60bb9dd7e3..98d8d5943f9 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -1,9 +1,12 @@ import Vue from 'vue'; import axios from '~/lib/utils/axios_utils'; import Cookies from 'js-cookie'; +import createFlash from '~/flash'; +import { s__ } from '~/locale'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; -import { getDiffPositionByLineCode } from './utils'; +import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils'; +import { getDiffPositionByLineCode, getNoteFormData } from './utils'; import * as types from './mutation_types'; import { PARALLEL_DIFF_VIEW_TYPE, @@ -178,5 +181,19 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { }); }; +export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { + const postData = getNoteFormData({ + note, + ...formData, + }); + + return dispatch('saveNote', postData, { root: true }) + .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) + .then(discussion => + dispatch('assignDiscussionsToDiff', reduceDiscussionsToLineCodes([discussion])), + ) + .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 68df63b8539..62b36f300ff 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -44,23 +44,11 @@ export const fetchDiscussions = ({ commit }, path) => commit(types.SET_INITIAL_DISCUSSIONS, discussions); }); -export const refetchDiscussionById = ({ commit, state }, { path, discussionId }) => - new Promise(resolve => { - service - .fetchDiscussions(path) - .then(res => res.json()) - .then(discussions => { - const selectedDiscussion = discussions.find(discussion => discussion.id === discussionId); - if (selectedDiscussion) { - commit(types.UPDATE_DISCUSSION, selectedDiscussion); - // We need to refetch as it is now the transformed one in state - const discussion = utils.findNoteObjectById(state.discussions, discussionId); - - resolve(discussion); - } - }) - .catch(() => {}); - }); +export const updateDiscussion = ({ commit }, discussion) => { + commit(types.UPDATE_DISCUSSION, discussion); + + return discussion; +}; export const deleteNote = ({ commit, dispatch }, note) => service.deleteNote(note.path).then(() => { diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index f1242a0d8be..73e55705f39 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -4,7 +4,8 @@ import * as constants from '../constants'; import { isInMRPage } from '../../lib/utils/common_utils'; export default { - [types.ADD_NEW_NOTE](state, note) { + [types.ADD_NEW_NOTE](state, data) { + const note = data.discussion ? data.discussion.notes[0] : data; const { discussion_id, type } = note; const [exists] = state.discussions.filter(n => n.id === note.discussion_id); const isDiscussion = type === constants.DISCUSSION_NOTE || type === constants.DIFF_NOTE; diff --git a/spec/javascripts/diffs/components/diff_line_note_form_spec.js b/spec/javascripts/diffs/components/diff_line_note_form_spec.js index 6fe5fdaf7f9..f31fc1f0e2b 100644 --- a/spec/javascripts/diffs/components/diff_line_note_form_spec.js +++ b/spec/javascripts/diffs/components/diff_line_note_form_spec.js @@ -69,22 +69,21 @@ describe('DiffLineNoteForm', () => { describe('saveNoteForm', () => { it('should call saveNote action with proper params', done => { - let isPromiseCalled = false; - const formDataSpy = spyOnDependency(DiffLineNoteForm, 'getNoteFormData').and.returnValue({ - postData: 1, - }); - const saveNoteSpy = spyOn(component, 'saveNote').and.returnValue( - new Promise(() => { - isPromiseCalled = true; - done(); - }), + const saveDiffDiscussionSpy = spyOn(component, 'saveDiffDiscussion').and.returnValue( + Promise.resolve(), ); - - component.handleSaveNote('note body'); - - expect(formDataSpy).toHaveBeenCalled(); - expect(saveNoteSpy).toHaveBeenCalled(); - expect(isPromiseCalled).toEqual(true); + spyOnProperty(component, 'formData').and.returnValue('formData'); + + component + .handleSaveNote('note body') + .then(() => { + expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({ + note: 'note body', + formData: 'formData', + }); + }) + .then(done) + .catch(done.fail); }); }); }); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 4c662fac231..5b1a0b2f392 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -21,6 +21,7 @@ import actions, { loadCollapsedDiff, expandAllFiles, toggleFileDiscussions, + saveDiffDiscussion, } from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils'; @@ -245,7 +246,7 @@ describe('DiffsStoreActions', () => { }); describe('startRenderDiffsQueue', () => { - it('should set all files to RENDER_FILE', done => { + it('should set all files to RENDER_FILE', () => { const state = { diffFiles: [ { @@ -268,16 +269,10 @@ describe('DiffsStoreActions', () => { }); }; - startRenderDiffsQueue({ state, commit: pseudoCommit }) - .then(() => { - expect(state.diffFiles[0].renderIt).toBeTruthy(); - expect(state.diffFiles[1].renderIt).toBeTruthy(); + startRenderDiffsQueue({ state, commit: pseudoCommit }); - done(); - }) - .catch(() => { - done.fail(); - }); + expect(state.diffFiles[0].renderIt).toBeTruthy(); + expect(state.diffFiles[1].renderIt).toBeTruthy(); }); }); @@ -582,4 +577,35 @@ describe('DiffsStoreActions', () => { expect(handleLocationHashSpy).toHaveBeenCalledTimes(1); }); }); + + describe('saveDiffDiscussion', () => { + beforeEach(() => { + spyOnDependency(actions, 'getNoteFormData').and.returnValue('testData'); + spyOnDependency(actions, 'reduceDiscussionsToLineCodes').and.returnValue('discussions'); + }); + + it('dispatches actions', done => { + const dispatch = jasmine.createSpy('dispatch').and.callFake(name => { + switch (name) { + case 'saveNote': + return Promise.resolve({ + discussion: 'test', + }); + case 'updateDiscussion': + return Promise.resolve('discussion'); + default: + return Promise.resolve({}); + } + }); + + saveDiffDiscussion({ dispatch }, { note: {}, formData: {} }) + .then(() => { + expect(dispatch.calls.argsFor(0)).toEqual(['saveNote', 'testData', { root: true }]); + expect(dispatch.calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]); + expect(dispatch.calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', 'discussions']); + }) + .then(done) + .catch(done.fail); + }); + }); }); |