Use returned discussion from API

We now use the returned discussion instead of re-fetching
all of the discussions and filtering out the ones we don't need.
This speeds up the process of creating a diff discussions
by saving us another API request before we can render the discussion
This commit is contained in:
Phil Hughes 2018-09-14 16:51:25 +01:00 committed by André Luís
parent 2036458e15
commit 2497c29ef0
6 changed files with 87 additions and 76 deletions

View File

@ -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(),
);
},
},
};

View File

@ -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 () => {};

View File

@ -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);
export const updateDiscussion = ({ commit }, discussion) => {
commit(types.UPDATE_DISCUSSION, discussion);
resolve(discussion);
}
})
.catch(() => {});
});
return discussion;
};
export const deleteNote = ({ commit, dispatch }, note) =>
service.deleteNote(note.path).then(() => {

View File

@ -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;

View File

@ -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(),
);
spyOnProperty(component, 'formData').and.returnValue('formData');
component.handleSaveNote('note body');
expect(formDataSpy).toHaveBeenCalled();
expect(saveNoteSpy).toHaveBeenCalled();
expect(isPromiseCalled).toEqual(true);
component
.handleSaveNote('note body')
.then(() => {
expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({
note: 'note body',
formData: 'formData',
});
})
.then(done)
.catch(done.fail);
});
});
});

View File

@ -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);
});
});
});