Merge branch '246532-on-design-discussion-comment-is-added-twice' into 'master'

Resolve "On design discussion, comment is added twice"

See merge request gitlab-org/gitlab!41687

app/assets/javascripts/design_management/components/design_notes/design_discussion.vue

@@ -2,18 +2,19 @@
 import { ApolloMutation } from 'vue-apollo';
 import { GlTooltipDirective, GlIcon, GlLoadingIcon, GlLink } from '@gitlab/ui';
 import { s__ } from '~/locale';
+import createFlash from '~/flash';
 import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import allVersionsMixin from '../../mixins/all_versions';
 import createNoteMutation from '../../graphql/mutations/create_note.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 DesignNote from './design_note.vue';
 import DesignReplyForm from './design_reply_form.vue';
-import { updateStoreAfterAddDiscussionComment } from '../../utils/cache_update';
 import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
 import ToggleRepliesWidget from './toggle_replies_widget.vue';
+import { hasErrors } from '../../utils/cache_update';
+import { ADD_DISCUSSION_COMMENT_ERROR } from '../../utils/error_messages';
 
 export default {
   components: {
@@ -136,21 +137,10 @@ export default {
     },
   },
   methods: {
-    addDiscussionComment(
-      store,
-      {
-        data: { createNote },
-      },
-    ) {
-      updateStoreAfterAddDiscussionComment(
-        store,
-        createNote,
-        getDesignQuery,
-        this.designVariables,
-        this.discussion.id,
-      );
-    },
-    onDone() {
+    onDone({ data: { createNote } }) {
+      if (hasErrors(createNote)) {
+        createFlash({ message: ADD_DISCUSSION_COMMENT_ERROR });
+      }
       this.discussionComment = '';
       this.hideForm();
       if (this.shouldChangeResolvedStatus) {
@@ -278,7 +268,6 @@ export default {
           :variables="{
             input: mutationPayload,
           }"
-          :update="addDiscussionComment"
           @done="onDone"
           @error="onCreateNoteError"
         >

app/assets/javascripts/design_management/utils/cache_update.js

@@ -7,15 +7,11 @@ import { extractCurrentDiscussion, extractDesign, extractDesigns } from './desig
 import {
   ADD_IMAGE_DIFF_NOTE_ERROR,
   UPDATE_IMAGE_DIFF_NOTE_ERROR,
-  ADD_DISCUSSION_COMMENT_ERROR,
   designDeletionError,
 } from './error_messages';
 
 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 sourceData = store.readQuery(query);
 
@@ -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 sourceData = store.readQuery({
     query,
@@ -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) => {
   if (hasErrors(data)) {
     onError(data, ADD_IMAGE_DIFF_NOTE_ERROR);

changelogs/unreleased/246532-on-design-discussion-comment-is-added-twice.yml

+---
+title: Resolve design discussion bug where a comment is added twice
+merge_request: 41687
+author:
+type: fixed

spec/frontend/design_management/components/design_notes/design_discussion_spec.js

@@ -32,7 +32,6 @@ describe('Design discussions component', () => {
 
   const mutationVariables = {
     mutation: createNoteMutation,
-    update: expect.anything(),
     variables: {
       input: {
         noteableId: 'noteable-id',
@@ -41,7 +40,7 @@ describe('Design discussions component', () => {
       },
     },
   };
-  const mutate = jest.fn(() => Promise.resolve());
+  const mutate = jest.fn().mockResolvedValue({ data: { createNote: { errors: [] } } });
   const $apollo = {
     mutate,
   };
@@ -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(
       { discussionWithOpenForm: defaultMockDiscussion.id },
       { discussionComment: 'test', isFormRendered: true },
@@ -236,13 +235,10 @@ describe('Design discussions component', () => {
     findReplyForm().vm.$emit('submitForm');
     expect(mutate).toHaveBeenCalledWith(mutationVariables);
 
-    return mutate()
-      .then(() => {
-        return wrapper.vm.$nextTick();
-      })
-      .then(() => {
-        expect(findReplyForm().exists()).toBe(false);
-      });
+    await mutate();
+    await wrapper.vm.$nextTick();
+
+    expect(findReplyForm().exists()).toBe(false);
   });
 
   it('clears the discussion comment on closing comment form', () => {

spec/frontend/design_management/utils/cache_update_spec.js

 import { InMemoryCache } from 'apollo-cache-inmemory';
 import {
   updateStoreAfterDesignsDelete,
-  updateStoreAfterAddDiscussionComment,
   updateStoreAfterAddImageDiffNote,
   updateStoreAfterUploadDesign,
   updateStoreAfterUpdateImageDiffNote,
 } from '~/design_management/utils/cache_update';
 import {
   designDeletionError,
-  ADD_DISCUSSION_COMMENT_ERROR,
   ADD_IMAGE_DIFF_NOTE_ERROR,
   UPDATE_IMAGE_DIFF_NOTE_ERROR,
 } from '~/design_management/utils/error_messages';
@@ -28,12 +26,11 @@ describe('Design Management cache update', () => {
 
   describe('error handling', () => {
     it.each`
-      fnName                                    | subject                                 | errorMessage                               | extraArgs
-      ${'updateStoreAfterDesignsDelete'}        | ${updateStoreAfterDesignsDelete}        | ${designDeletionError({ singular: true })} | ${[[design]]}
-      ${'updateStoreAfterAddDiscussionComment'} | ${updateStoreAfterAddDiscussionComment} | ${ADD_DISCUSSION_COMMENT_ERROR}            | ${[]}
-      ${'updateStoreAfterAddImageDiffNote'}     | ${updateStoreAfterAddImageDiffNote}     | ${ADD_IMAGE_DIFF_NOTE_ERROR}               | ${[]}
-      ${'updateStoreAfterUploadDesign'}         | ${updateStoreAfterUploadDesign}         | ${mockErrors[0]}                           | ${[]}
-      ${'updateStoreAfterUpdateImageDiffNote'}  | ${updateStoreAfterUpdateImageDiffNote}  | ${UPDATE_IMAGE_DIFF_NOTE_ERROR}            | ${[]}
+      fnName                                   | subject                                | errorMessage                               | extraArgs
+      ${'updateStoreAfterDesignsDelete'}       | ${updateStoreAfterDesignsDelete}       | ${designDeletionError({ singular: true })} | ${[[design]]}
+      ${'updateStoreAfterAddImageDiffNote'}    | ${updateStoreAfterAddImageDiffNote}    | ${ADD_IMAGE_DIFF_NOTE_ERROR}               | ${[]}
+      ${'updateStoreAfterUploadDesign'}        | ${updateStoreAfterUploadDesign}        | ${mockErrors[0]}                           | ${[]}
+      ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR}            | ${[]}
     `('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => {
       expect(createFlash).not.toHaveBeenCalled();
       expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow();