Merge branch '228743-use-graphql-to-save-a-history-note' into 'master'

Save a comment through graphql

See merge request gitlab-org/gitlab!68995

app/assets/javascripts/graphql_shared/constants.js

@@ -14,3 +14,4 @@ export const TYPE_SITE_PROFILE = 'DastSiteProfile';
 export const TYPE_USER = 'User';
 export const TYPE_VULNERABILITY = 'Vulnerability';
 export const TYPE_NOTE = 'Note';
+export const TYPE_DISCUSSION = 'Discussion';

ee/app/assets/javascripts/security_dashboard/graphql/fragments/note.fragment.graphql

+fragment SecurityDashboardNote on Note {
+  id
+  system
+  body
+  bodyHtml
+  updatedAt
+  systemNoteIconName
+  userPermissions {
+    adminNote
+  }
+  author {
+    id
+    name
+    webPath
+    username
+  }
+}

ee/app/assets/javascripts/security_dashboard/graphql/mutations/note_create.mutation.graphql

+#import "../fragments/note.fragment.graphql"
+
+mutation securityDashboardCreateNote(
+  $noteableId: NoteableID!
+  $discussionId: DiscussionID
+  $confidential: Boolean
+  $body: String!
+) {
+  createNote(
+    input: {
+      noteableId: $noteableId
+      body: $body
+      confidential: $confidential
+      discussionId: $discussionId
+    }
+  ) {
+    errors
+    note {
+      ...SecurityDashboardNote
+    }
+  }
+}

ee/app/assets/javascripts/security_dashboard/graphql/mutations/note_delete.mutation.graphql → ee/app/assets/javascripts/security_dashboard/graphql/mutations/note_destroy.mutation.graphql

-mutation($id: ID!) {
+mutation securityDashboardDestroyNote($id: ID!) {
   destroyNote(input: { id: $id }) {
     errors
     note {

ee/app/assets/javascripts/security_dashboard/graphql/mutations/note_update.mutation.graphql

+#import "../fragments/note.fragment.graphql"
+
+mutation securityDashboardUpdateNote($id: NoteID!, $confidential: Boolean, $body: String!) {
+  updateNote(input: { id: $id, body: $body, confidential: $confidential }) {
+    errors
+    note {
+      ...SecurityDashboardNote
+    }
+  }
+}

ee/app/assets/javascripts/vulnerabilities/components/footer.vue

@@ -71,7 +71,7 @@ export default {
           this.createNotesPoll();
 
           if (!Visibility.hidden()) {
-            this.poll.makeRequest();
+            this.fetchDiscussions();
           }
 
           Visibility.change(() => {
@@ -148,6 +148,9 @@ export default {
     }
   },
   methods: {
+    fetchDiscussions() {
+      return this.poll.makeRequest();
+    },
     findDiscussion(id) {
       return this.discussions.find((d) => d.id === id);
     },
@@ -253,7 +256,6 @@ export default {
         v-for="discussion in discussions"
         :key="discussion.id"
         :discussion="discussion"
-        :notes-url="vulnerability.notesUrl"
       />
     </ul>
   </div>

ee/app/assets/javascripts/vulnerabilities/components/history_comment.vue

 <script>
 import { GlButton, GlSafeHtmlDirective as SafeHtml, GlLoadingIcon } from '@gitlab/ui';
-import deleteNoteMutation from 'ee/security_dashboard/graphql/mutations/note_delete.mutation.graphql';
+import createNoteMutation from 'ee/security_dashboard/graphql/mutations/note_create.mutation.graphql';
+import destroyNoteMutation from 'ee/security_dashboard/graphql/mutations/note_destroy.mutation.graphql';
+import updateNoteMutation from 'ee/security_dashboard/graphql/mutations/note_update.mutation.graphql';
 import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
 import createFlash from '~/flash';
-import { TYPE_NOTE } from '~/graphql_shared/constants';
+import { TYPE_NOTE, TYPE_DISCUSSION, TYPE_VULNERABILITY } from '~/graphql_shared/constants';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
-import axios from '~/lib/utils/axios_utils';
 import { __, s__ } from '~/locale';
+import { normalizeGraphQLNote } from '../helpers';
 import HistoryCommentEditor from './history_comment_editor.vue';
 
 export default {
@@ -21,6 +23,8 @@ export default {
     SafeHtml,
   },
 
+  inject: ['vulnerabilityId'],
+
   props: {
     comment: {
       type: Object,
@@ -32,10 +36,6 @@ export default {
       required: false,
       default: undefined,
     },
-    notesUrl: {
-      type: String,
-      required: true,
-    },
   },
 
   data() {
@@ -63,7 +63,7 @@ export default {
       ];
     },
     initialComment() {
-      return this.comment && this.comment.note;
+      return this.comment?.note;
     },
     canEditComment() {
       return this.comment.currentUser?.canEdit;
@@ -85,51 +85,79 @@ export default {
     showCommentInput() {
       this.isEditingComment = true;
     },
-    getSaveConfig(note) {
-      const isUpdatingComment = Boolean(this.comment);
-      const method = isUpdatingComment ? 'put' : 'post';
-      const url = isUpdatingComment ? this.comment.path : this.notesUrl;
-      const data = { note: { note } };
-      const emitName = isUpdatingComment ? 'onCommentUpdated' : 'onCommentAdded';
-
-      // If we're saving a new comment, use the discussion ID in the request data.
-      if (!isUpdatingComment) {
-        data.in_reply_to_discussion_id = this.discussionId;
+    async insertComment(body) {
+      const { data } = await this.$apollo.mutate({
+        mutation: createNoteMutation,
+        variables: {
+          noteableId: convertToGraphQLId(TYPE_VULNERABILITY, this.vulnerabilityId),
+          discussionId: convertToGraphQLId(TYPE_DISCUSSION, this.discussionId),
+          body,
+        },
+      });
+
+      const { note, errors } = data.createNote;
+
+      if (errors?.length > 0) {
+        throw errors;
       }
 
-      return { method, url, data, emitName };
+      this.$emit('onCommentAdded', normalizeGraphQLNote(note));
+    },
+    async updateComment(body) {
+      const { data } = await this.$apollo.mutate({
+        mutation: updateNoteMutation,
+        variables: {
+          id: convertToGraphQLId(TYPE_NOTE, this.comment.id),
+          body,
         },
-    saveComment(note) {
+      });
+
+      const { note, errors } = data.updateNote;
+
+      if (errors?.length > 0) {
+        throw errors;
+      }
+
+      this.cancelEditingComment();
+      this.$emit('onCommentUpdated', normalizeGraphQLNote(note));
+    },
+    async saveComment(body) {
       this.isSavingComment = true;
-      const { method, url, data, emitName } = this.getSaveConfig(note);
+      const isUpdatingComment = Boolean(this.comment);
 
-      // note: this direct API call will be replaced when migrating the vulnerability details page to GraphQL
-      // related epic: https://gitlab.com/groups/gitlab-org/-/epics/3657
-      axios({ method, url, data })
-        .then(({ data: responseData }) => {
-          this.isEditingComment = false;
-          this.$emit(emitName, { response: responseData, comment: this.comment });
-        })
-        .catch(() => {
+      try {
+        if (isUpdatingComment) {
+          await this.updateComment(body);
+        } else {
+          await this.insertComment(body);
+        }
+      } catch {
         createFlash({
           message: s__(
             'VulnerabilityManagement|Something went wrong while trying to save the comment. Please try again later.',
           ),
         });
-        });
+      }
+
+      this.isSavingComment = false;
     },
     async deleteComment() {
       this.isDeletingComment = true;
 
       try {
-        await this.$apollo.mutate({
-          mutation: deleteNoteMutation,
+        const { data } = await this.$apollo.mutate({
+          mutation: destroyNoteMutation,
           variables: {
             id: convertToGraphQLId(TYPE_NOTE, this.comment.id),
           },
         });
+
+        if (data.errors?.length > 0) {
+          throw data.errors;
+        }
+
         this.$emit('onCommentDeleted', this.comment);
-      } catch (e) {
+      } catch {
         createFlash({
           message: s__(
             'VulnerabilityManagement|Something went wrong while trying to delete the comment. Please try again later.',

ee/app/assets/javascripts/vulnerabilities/components/history_entry.vue

 <script>
 import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
-import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import HistoryComment from './history_comment.vue';
 
 export default {
@@ -10,10 +9,6 @@ export default {
       type: Object,
       required: true,
     },
-    notesUrl: {
-      type: String,
-      required: true,
-    },
   },
   data() {
     return {
@@ -34,14 +29,14 @@ export default {
     },
   },
   methods: {
-    addComment({ response }) {
-      this.notes.push(convertObjectPropsToCamelCase(response));
+    addComment(note) {
+      this.notes.push(note);
     },
-    updateComment({ response, comment }) {
-      const index = this.notes.indexOf(comment);
+    updateComment(note) {
+      const index = this.notes.findIndex((n) => Number(n.id) === note.id);
 
       if (index > -1) {
-        this.notes.splice(index, 1, { ...comment, ...convertObjectPropsToCamelCase(response) });
+        this.notes.splice(index, 1, note);
       }
     },
     removeComment(comment) {
@@ -76,7 +71,6 @@ export default {
         ref="existingComment"
         :comment="comment"
         :discussion-id="discussion.replyId"
-        :notes-url="notesUrl"
         @onCommentUpdated="updateComment"
         @onCommentDeleted="removeComment"
       />
@@ -86,7 +80,6 @@ export default {
       v-else
       ref="newComment"
       :discussion-id="discussion.replyId"
-      :notes-url="notesUrl"
       @onCommentAdded="addComment"
     />
   </li>

ee/app/assets/javascripts/vulnerabilities/helpers.js

+import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { isAbsolute, isSafeURL } from '~/lib/utils/url_utility';
 import { REGEXES, gidPrefix, uidPrefix } from './constants';
 
@@ -28,6 +29,27 @@ export const getAddRelatedIssueRequestParams = (reference, defaultProjectId) =>
   return { target_issue_iid: issueId, target_project_id: projectId };
 };
 
+export const normalizeGraphQLNote = (note) => {
+  if (!note) {
+    return null;
+  }
+
+  return {
+    ...note,
+    id: getIdFromGraphQLId(note.id),
+    note: note.body,
+    noteHtml: note.bodyHtml,
+    currentUser: {
+      canEdit: note.userPermissions?.adminNote,
+    },
+    author: {
+      ...note.author,
+      id: getIdFromGraphQLId(note.author.id),
+      path: note.author.webPath,
+    },
+  };
+};
+
 export const normalizeGraphQLVulnerability = (vulnerability) => {
   if (!vulnerability) {
     return null;

ee/spec/frontend/vulnerabilities/footer_spec.js

@@ -141,12 +141,10 @@ describe('Vulnerability Footer', () => {
 
       expect(findDiscussions().at(0).props()).toEqual({
         discussion: { ...discussion1, notes: [convertObjectPropsToCamelCase(notes[0])] },
-        notesUrl: vulnerability.notesUrl,
       });
 
       expect(findDiscussions().at(1).props()).toEqual({
         discussion: { ...discussion2, notes: [convertObjectPropsToCamelCase(notes[1])] },
-        notesUrl: vulnerability.notesUrl,
       });
     });
 

ee/spec/frontend/vulnerabilities/history_entry_spec.js

 import { shallowMount } from '@vue/test-utils';
 import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
 import HistoryEntry from 'ee/vulnerabilities/components/history_entry.vue';
-import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
 
 describe('History Entry', () => {
   let wrapper;
@@ -84,9 +83,7 @@ describe('History Entry', () => {
 
   it('adds a new comment correctly', async () => {
     createWrapper(systemNote);
-    newComment().vm.$emit('onCommentAdded', {
-      response: convertObjectPropsToSnakeCase(commentNote),
-    });
+    newComment().vm.$emit('onCommentAdded', commentNote);
 
     await wrapper.vm.$nextTick();
 
@@ -96,13 +93,13 @@ describe('History Entry', () => {
   });
 
   it('updates an existing comment correctly', async () => {
-    const response = { note: 'new note' };
+    const updatedNote = { ...commentNote, note: 'new note' };
     createWrapper(systemNote, commentNote);
-    commentAt(0).vm.$emit('onCommentUpdated', { response, comment: commentNote });
+    commentAt(0).vm.$emit('onCommentUpdated', updatedNote);
 
     await wrapper.vm.$nextTick();
 
-    expect(commentAt(0).props('comment')).toEqual({ ...commentNote, note: response.note });
+    expect(commentAt(0).props('comment')).toBe(updatedNote);
   });
 
   it('deletes an existing comment correctly', async () => {

ee/spec/frontend/vulnerabilities/vulnerability_spec.js

@@ -4,6 +4,7 @@ import Footer from 'ee/vulnerabilities/components/footer.vue';
 import Header from 'ee/vulnerabilities/components/header.vue';
 import Main from 'ee/vulnerabilities/components/vulnerability.vue';
 import Details from 'ee/vulnerabilities/components/vulnerability_details.vue';
+import { stubComponent } from 'helpers/stub_component';
 
 const mockAxios = new AxiosMockAdapter();
 
@@ -47,6 +48,10 @@ describe('Vulnerability', () => {
       propsData: {
         vulnerability,
       },
+      stubs: {
+        VulnerabilityHeader: stubComponent(Header),
+        VulnerabilityFooter: stubComponent(Footer),
+      },
     });
   };
 
@@ -77,28 +82,25 @@ describe('Vulnerability', () => {
   });
 
   describe('vulnerability state change event', () => {
-    let fetchDiscussions;
+    let makeRequest;
     let refreshVulnerability;
 
     beforeEach(() => {
-      fetchDiscussions = jest.fn();
-      refreshVulnerability = jest.fn();
-
-      findHeader().vm.refreshVulnerability = refreshVulnerability;
-      findFooter().vm.fetchDiscussions = fetchDiscussions;
+      refreshVulnerability = jest.spyOn(findHeader().vm, 'refreshVulnerability');
+      makeRequest = jest.spyOn(findFooter().vm, 'fetchDiscussions');
     });
 
     it('updates the footer notes when the vulnerbility state was changed', () => {
       findHeader().vm.$emit('vulnerability-state-change');
 
-      expect(fetchDiscussions).toHaveBeenCalledTimes(1);
+      expect(makeRequest).toHaveBeenCalledTimes(1);
       expect(refreshVulnerability).not.toHaveBeenCalled();
     });
 
     it('updates the header when the footer received a state-change note', () => {
       findFooter().vm.$emit('vulnerability-state-change');
 
-      expect(fetchDiscussions).not.toHaveBeenCalled();
+      expect(makeRequest).not.toHaveBeenCalled();
       expect(refreshVulnerability).toHaveBeenCalledTimes(1);
     });
   });