Commit 27df17ad authored by Natalia Tepluhina's avatar Natalia Tepluhina Committed by Nicolò Maria Mezzopera

Resolve "On design discussion, comment is added twice"

parent 9714640d
...@@ -2,18 +2,19 @@ ...@@ -2,18 +2,19 @@
import { ApolloMutation } from 'vue-apollo'; import { ApolloMutation } from 'vue-apollo';
import { GlTooltipDirective, GlIcon, GlLoadingIcon, GlLink } from '@gitlab/ui'; import { GlTooltipDirective, GlIcon, GlLoadingIcon, GlLink } from '@gitlab/ui';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import createFlash from '~/flash';
import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import allVersionsMixin from '../../mixins/all_versions'; import allVersionsMixin from '../../mixins/all_versions';
import createNoteMutation from '../../graphql/mutations/create_note.mutation.graphql'; import createNoteMutation from '../../graphql/mutations/create_note.mutation.graphql';
import toggleResolveDiscussionMutation from '../../graphql/mutations/toggle_resolve_discussion.mutation.graphql'; import toggleResolveDiscussionMutation from '../../graphql/mutations/toggle_resolve_discussion.mutation.graphql';
import getDesignQuery from '../../graphql/queries/get_design.query.graphql';
import activeDiscussionQuery from '../../graphql/queries/active_discussion.query.graphql'; import activeDiscussionQuery from '../../graphql/queries/active_discussion.query.graphql';
import DesignNote from './design_note.vue'; import DesignNote from './design_note.vue';
import DesignReplyForm from './design_reply_form.vue'; import DesignReplyForm from './design_reply_form.vue';
import { updateStoreAfterAddDiscussionComment } from '../../utils/cache_update';
import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants'; import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
import ToggleRepliesWidget from './toggle_replies_widget.vue'; import ToggleRepliesWidget from './toggle_replies_widget.vue';
import { hasErrors } from '../../utils/cache_update';
import { ADD_DISCUSSION_COMMENT_ERROR } from '../../utils/error_messages';
export default { export default {
components: { components: {
...@@ -136,21 +137,10 @@ export default { ...@@ -136,21 +137,10 @@ export default {
}, },
}, },
methods: { methods: {
addDiscussionComment( onDone({ data: { createNote } }) {
store, if (hasErrors(createNote)) {
{ createFlash({ message: ADD_DISCUSSION_COMMENT_ERROR });
data: { createNote }, }
},
) {
updateStoreAfterAddDiscussionComment(
store,
createNote,
getDesignQuery,
this.designVariables,
this.discussion.id,
);
},
onDone() {
this.discussionComment = ''; this.discussionComment = '';
this.hideForm(); this.hideForm();
if (this.shouldChangeResolvedStatus) { if (this.shouldChangeResolvedStatus) {
...@@ -278,7 +268,6 @@ export default { ...@@ -278,7 +268,6 @@ export default {
:variables="{ :variables="{
input: mutationPayload, input: mutationPayload,
}" }"
:update="addDiscussionComment"
@done="onDone" @done="onDone"
@error="onCreateNoteError" @error="onCreateNoteError"
> >
......
...@@ -7,15 +7,11 @@ import { extractCurrentDiscussion, extractDesign, extractDesigns } from './desig ...@@ -7,15 +7,11 @@ import { extractCurrentDiscussion, extractDesign, extractDesigns } from './desig
import { import {
ADD_IMAGE_DIFF_NOTE_ERROR, ADD_IMAGE_DIFF_NOTE_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR, UPDATE_IMAGE_DIFF_NOTE_ERROR,
ADD_DISCUSSION_COMMENT_ERROR,
designDeletionError, designDeletionError,
} from './error_messages'; } from './error_messages';
const designsOf = data => data.project.issue.designCollection.designs; const designsOf = data => data.project.issue.designCollection.designs;
const isParticipating = (design, username) =>
design.issue.participants.nodes.some(participant => participant.username === username);
const deleteDesignsFromStore = (store, query, selectedDesigns) => { const deleteDesignsFromStore = (store, query, selectedDesigns) => {
const sourceData = store.readQuery(query); const sourceData = store.readQuery(query);
...@@ -57,36 +53,6 @@ const addNewVersionToStore = (store, query, version) => { ...@@ -57,36 +53,6 @@ const addNewVersionToStore = (store, query, version) => {
}); });
}; };
const addDiscussionCommentToStore = (store, createNote, query, queryVariables, discussionId) => {
const sourceData = store.readQuery({
query,
variables: queryVariables,
});
const newParticipant = {
__typename: 'User',
...createNote.note.author,
};
const data = produce(sourceData, draftData => {
const design = extractDesign(draftData);
const currentDiscussion = extractCurrentDiscussion(design.discussions, discussionId);
currentDiscussion.notes.nodes = [...currentDiscussion.notes.nodes, createNote.note];
if (!isParticipating(design, createNote.note.author.username)) {
design.issue.participants.nodes = [...design.issue.participants.nodes, newParticipant];
}
design.notesCount += 1;
});
store.writeQuery({
query,
variables: queryVariables,
data,
});
};
const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) => { const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) => {
const sourceData = store.readQuery({ const sourceData = store.readQuery({
query, query,
...@@ -246,20 +212,6 @@ export const updateStoreAfterDesignsDelete = (store, data, query, designs) => { ...@@ -246,20 +212,6 @@ export const updateStoreAfterDesignsDelete = (store, data, query, designs) => {
} }
}; };
export const updateStoreAfterAddDiscussionComment = (
store,
data,
query,
queryVariables,
discussionId,
) => {
if (hasErrors(data)) {
onError(data, ADD_DISCUSSION_COMMENT_ERROR);
} else {
addDiscussionCommentToStore(store, data, query, queryVariables, discussionId);
}
};
export const updateStoreAfterAddImageDiffNote = (store, data, query, queryVariables) => { export const updateStoreAfterAddImageDiffNote = (store, data, query, queryVariables) => {
if (hasErrors(data)) { if (hasErrors(data)) {
onError(data, ADD_IMAGE_DIFF_NOTE_ERROR); onError(data, ADD_IMAGE_DIFF_NOTE_ERROR);
......
---
title: Resolve design discussion bug where a comment is added twice
merge_request: 41687
author:
type: fixed
...@@ -32,7 +32,6 @@ describe('Design discussions component', () => { ...@@ -32,7 +32,6 @@ describe('Design discussions component', () => {
const mutationVariables = { const mutationVariables = {
mutation: createNoteMutation, mutation: createNoteMutation,
update: expect.anything(),
variables: { variables: {
input: { input: {
noteableId: 'noteable-id', noteableId: 'noteable-id',
...@@ -41,7 +40,7 @@ describe('Design discussions component', () => { ...@@ -41,7 +40,7 @@ describe('Design discussions component', () => {
}, },
}, },
}; };
const mutate = jest.fn(() => Promise.resolve()); const mutate = jest.fn().mockResolvedValue({ data: { createNote: { errors: [] } } });
const $apollo = { const $apollo = {
mutate, mutate,
}; };
...@@ -227,7 +226,7 @@ describe('Design discussions component', () => { ...@@ -227,7 +226,7 @@ describe('Design discussions component', () => {
}); });
}); });
it('calls mutation on submitting form and closes the form', () => { it('calls mutation on submitting form and closes the form', async () => {
createComponent( createComponent(
{ discussionWithOpenForm: defaultMockDiscussion.id }, { discussionWithOpenForm: defaultMockDiscussion.id },
{ discussionComment: 'test', isFormRendered: true }, { discussionComment: 'test', isFormRendered: true },
...@@ -236,13 +235,10 @@ describe('Design discussions component', () => { ...@@ -236,13 +235,10 @@ describe('Design discussions component', () => {
findReplyForm().vm.$emit('submitForm'); findReplyForm().vm.$emit('submitForm');
expect(mutate).toHaveBeenCalledWith(mutationVariables); expect(mutate).toHaveBeenCalledWith(mutationVariables);
return mutate() await mutate();
.then(() => { await wrapper.vm.$nextTick();
return wrapper.vm.$nextTick();
}) expect(findReplyForm().exists()).toBe(false);
.then(() => {
expect(findReplyForm().exists()).toBe(false);
});
}); });
it('clears the discussion comment on closing comment form', () => { it('clears the discussion comment on closing comment form', () => {
......
import { InMemoryCache } from 'apollo-cache-inmemory'; import { InMemoryCache } from 'apollo-cache-inmemory';
import { import {
updateStoreAfterDesignsDelete, updateStoreAfterDesignsDelete,
updateStoreAfterAddDiscussionComment,
updateStoreAfterAddImageDiffNote, updateStoreAfterAddImageDiffNote,
updateStoreAfterUploadDesign, updateStoreAfterUploadDesign,
updateStoreAfterUpdateImageDiffNote, updateStoreAfterUpdateImageDiffNote,
} from '~/design_management/utils/cache_update'; } from '~/design_management/utils/cache_update';
import { import {
designDeletionError, designDeletionError,
ADD_DISCUSSION_COMMENT_ERROR,
ADD_IMAGE_DIFF_NOTE_ERROR, ADD_IMAGE_DIFF_NOTE_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR, UPDATE_IMAGE_DIFF_NOTE_ERROR,
} from '~/design_management/utils/error_messages'; } from '~/design_management/utils/error_messages';
...@@ -28,12 +26,11 @@ describe('Design Management cache update', () => { ...@@ -28,12 +26,11 @@ describe('Design Management cache update', () => {
describe('error handling', () => { describe('error handling', () => {
it.each` it.each`
fnName | subject | errorMessage | extraArgs fnName | subject | errorMessage | extraArgs
${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]}
${'updateStoreAfterAddDiscussionComment'} | ${updateStoreAfterAddDiscussionComment} | ${ADD_DISCUSSION_COMMENT_ERROR} | ${[]} ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]}
${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]}
${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]}
${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]}
`('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => { `('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => {
expect(createFlash).not.toHaveBeenCalled(); expect(createFlash).not.toHaveBeenCalled();
expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow(); expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow();
......
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