Merge branch '118609-design-comment-edit-comment-text' into 'master'

Resolve "Design Comment: Edit Comment text"

See merge request gitlab-org/gitlab!30479

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

@@ -103,7 +103,13 @@ export default {
       class="design-discussion bordered-box position-relative"
       data-qa-selector="design_discussion_content"
     >
-      <design-note v-for="note in discussion.notes" :key="note.id" :note="note" />
+      <design-note
+        v-for="note in discussion.notes"
+        :key="note.id"
+        :note="note"
+        :markdown-preview-path="markdownPreviewPath"
+        @error="$emit('updateNoteError', $event)"
+      />
       <div class="reply-wrapper">
         <reply-placeholder
           v-if="!isFormRendered"

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

 <script>
+import { ApolloMutation } from 'vue-apollo';
+import { GlTooltipDirective, GlIcon } from '@gitlab/ui';
+import updateNoteMutation from '../../graphql/mutations/update_note.mutation.graphql';
 import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import DesignReplyForm from './design_reply_form.vue';
 import { findNoteId } from '../../utils/design_management_utils';
+import { hasErrors } from '../../utils/cache_update';
 
 export default {
   components: {
     UserAvatarLink,
     TimelineEntryItem,
     TimeAgoTooltip,
+    DesignReplyForm,
+    ApolloMutation,
+    GlIcon,
+  },
+  directives: {
+    GlTooltip: GlTooltipDirective,
   },
   props: {
     note: {
       type: Object,
       required: true,
     },
+    markdownPreviewPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  data() {
+    return {
+      noteText: this.note.body,
+      isEditing: false,
+    };
   },
   computed: {
     author() {
@@ -26,12 +48,31 @@ export default {
     isNoteLinked() {
       return this.$route.hash === `#note_${this.noteAnchorId}`;
     },
+    mutationPayload() {
+      return {
+        id: this.note.id,
+        body: this.noteText,
+      };
+    },
   },
   mounted() {
     if (this.isNoteLinked) {
       this.$refs.anchor.$el.scrollIntoView({ behavior: 'smooth', inline: 'start' });
     }
   },
+  methods: {
+    hideForm() {
+      this.isEditing = false;
+      this.noteText = this.note.body;
+    },
+    onDone({ data }) {
+      this.hideForm();
+      if (hasErrors(data.updateNote)) {
+        this.$emit('error', data.errors[0]);
+      }
+    },
+  },
+  updateNoteMutation,
 };
 </script>
 
@@ -43,26 +84,65 @@ export default {
       :img-alt="author.username"
       :img-size="40"
     />
-    <a
-      v-once
-      :href="author.webUrl"
-      class="js-user-link"
-      :data-user-id="author.id"
-      :data-username="author.username"
-    >
-      <span class="note-header-author-name bold">{{ author.name }}</span>
-      <span v-if="author.status_tooltip_html" v-html="author.status_tooltip_html"></span>
-      <span class="note-headline-light">@{{ author.username }}</span>
-    </a>
-    <span class="note-headline-light note-headline-meta">
-      <span class="system-note-message"> <slot></slot> </span>
-      <template v-if="note.createdAt">
-        <span class="system-note-separator"></span>
-        <a class="note-timestamp system-note-separator" :href="`#note_${noteAnchorId}`">
-          <time-ago-tooltip :time="note.createdAt" tooltip-placement="bottom" />
+    <div class="d-flex justify-content-between">
+      <div>
+        <a
+          v-once
+          :href="author.webUrl"
+          class="js-user-link"
+          :data-user-id="author.id"
+          :data-username="author.username"
+        >
+          <span class="note-header-author-name bold">{{ author.name }}</span>
+          <span v-if="author.status_tooltip_html" v-html="author.status_tooltip_html"></span>
+          <span class="note-headline-light">@{{ author.username }}</span>
         </a>
-      </template>
-    </span>
-    <div class="note-text md" data-qa-selector="note_content" v-html="note.bodyHtml"></div>
+        <span class="note-headline-light note-headline-meta">
+          <span class="system-note-message"> <slot></slot> </span>
+          <template v-if="note.createdAt">
+            <span class="system-note-separator"></span>
+            <a class="note-timestamp system-note-separator" :href="`#note_${noteAnchorId}`">
+              <time-ago-tooltip :time="note.createdAt" tooltip-placement="bottom" />
+            </a>
+          </template>
+        </span>
+      </div>
+      <button
+        v-if="!isEditing"
+        v-gl-tooltip
+        type="button"
+        title="Edit comment"
+        class="note-action-button js-note-edit btn btn-transparent qa-note-edit-button"
+        @click="isEditing = true"
+      >
+        <gl-icon name="pencil" class="link-highlight" />
+      </button>
+    </div>
+    <div
+      v-if="!isEditing"
+      class="note-text js-note-text md"
+      data-qa-selector="note_content"
+      v-html="note.bodyHtml"
+    ></div>
+    <apollo-mutation
+      v-else
+      #default="{ mutate, loading }"
+      :mutation="$options.updateNoteMutation"
+      :variables="{
+        input: mutationPayload,
+      }"
+      @error="$emit('error', $event)"
+      @done="onDone"
+    >
+      <design-reply-form
+        v-model="noteText"
+        :is-saving="loading"
+        :markdown-preview-path="markdownPreviewPath"
+        :is-new-comment="false"
+        class="mt-5"
+        @submitForm="mutate"
+        @cancelForm="hideForm"
+      />
+    </apollo-mutation>
   </timeline-entry-item>
 </template>

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

 <script>
 import { GlDeprecatedButton, GlModal } from '@gitlab/ui';
 import MarkdownField from '~/vue_shared/components/markdown/field.vue';
+import { s__ } from '~/locale';
 
 export default {
   name: 'DesignReplyForm',
@@ -23,11 +24,42 @@ export default {
       type: Boolean,
       required: true,
     },
+    isNewComment: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+  },
+  data() {
+    return {
+      formText: this.value,
+    };
   },
   computed: {
     hasValue() {
       return this.value.trim().length > 0;
     },
+    modalSettings() {
+      if (this.isNewComment) {
+        return {
+          title: s__('DesignManagement|Cancel comment confirmation'),
+          okTitle: s__('DesignManagement|Discard comment'),
+          cancelTitle: s__('DesignManagement|Keep comment'),
+          content: s__('DesignManagement|Are you sure you want to cancel creating this comment?'),
+        };
+      }
+      return {
+        title: s__('DesignManagement|Cancel comment update confirmation'),
+        okTitle: s__('DesignManagement|Cancel changes'),
+        cancelTitle: s__('DesignManagement|Keep changes'),
+        content: s__('DesignManagement|Are you sure you want to cancel changes to this comment?'),
+      };
+    },
+    buttonText() {
+      return this.isNewComment
+        ? s__('DesignManagement|Comment')
+        : s__('DesignManagement|Save comment');
+    },
   },
   mounted() {
     this.$refs.textarea.focus();
@@ -37,7 +69,7 @@ export default {
       if (this.hasValue) this.$emit('submitForm');
     },
     cancelComment() {
-      if (this.hasValue) {
+      if (this.hasValue && this.formText !== this.value) {
         this.$refs.cancelCommentModal.show();
       } else {
         this.$emit('cancelForm');
@@ -85,7 +117,7 @@ export default {
         data-qa-selector="save_comment_button"
         @click="$emit('submitForm')"
       >
-        {{ __('Comment') }}
+        {{ buttonText }}
       </gl-deprecated-button>
       <gl-deprecated-button ref="cancelButton" @click="cancelComment">{{
         __('Cancel')
@@ -94,12 +126,12 @@ export default {
     <gl-modal
       ref="cancelCommentModal"
       ok-variant="danger"
-      :title="s__('DesignManagement|Cancel comment confirmation')"
-      :ok-title="s__('DesignManagement|Discard comment')"
-      :cancel-title="s__('DesignManagement|Keep comment')"
+      :title="modalSettings.title"
+      :ok-title="modalSettings.okTitle"
+      :cancel-title="modalSettings.cancelTitle"
       modal-id="cancel-comment-modal"
       @ok="$emit('cancelForm')"
-      >{{ s__('DesignManagement|Are you sure you want to cancel creating this comment?') }}
+      >{{ modalSettings.content }}
     </gl-modal>
   </form>
 </template>

app/assets/javascripts/design_management/graphql/mutations/update_note.mutation.graphql

+#import "../fragments/designNote.fragment.graphql"
+
+mutation updateNote($input: UpdateNoteInput!) {
+  updateNote(input: $input) {
+    note {
+      ...DesignNote
+    }
+    errors
+  }
+}

app/assets/javascripts/design_management/pages/design/index.vue

@@ -32,6 +32,7 @@ import {
   UPDATE_IMAGE_DIFF_NOTE_ERROR,
   DESIGN_NOT_FOUND_ERROR,
   DESIGN_VERSION_NOT_EXIST_ERROR,
+  UPDATE_NOTE_ERROR,
   designDeletionError,
 } from '../../utils/error_messages';
 import { DESIGNS_ROUTE_NAME } from '../../router/constants';
@@ -231,6 +232,9 @@ export default {
     onCreateImageDiffNoteError(e) {
       this.onError(ADD_IMAGE_DIFF_NOTE_ERROR, e);
     },
+    onUpdateNoteError(e) {
+      this.onError(UPDATE_NOTE_ERROR, e);
+    },
     onDesignDiscussionError(e) {
       this.onError(ADD_DISCUSSION_COMMENT_ERROR, e);
     },
@@ -329,6 +333,7 @@ export default {
             :discussion-index="index + 1"
             :markdown-preview-path="markdownPreviewPath"
             @error="onDesignDiscussionError"
+            @updateNoteError="onUpdateNoteError"
           />
           <apollo-mutation
             v-if="annotationCoordinates"
@@ -345,7 +350,7 @@ export default {
               v-model="comment"
               :is-saving="loading"
               :markdown-preview-path="markdownPreviewPath"
-              @submitForm="mutate()"
+              @submitForm="mutate"
               @cancelForm="closeCommentForm"
             />
           </apollo-mutation>

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

@@ -214,7 +214,7 @@ const onError = (data, message) => {
   throw new Error(data.errors);
 };
 
-const hasErrors = ({ errors = [] }) => errors?.length;
+export const hasErrors = ({ errors = [] }) => errors?.length;
 
 /**
  * Updates a store after design deletion

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

@@ -12,6 +12,8 @@ export const UPDATE_IMAGE_DIFF_NOTE_ERROR = s__(
   'DesignManagement|Could not update discussion. Please try again.',
 );
 
+export const UPDATE_NOTE_ERROR = s__('DesignManagement|Could not update note. Please try again.');
+
 export const UPLOAD_DESIGN_ERROR = s__(
   'DesignManagement|Error uploading a new design. Please try again.',
 );

changelogs/unreleased/118609-design-comment-edit-comment-text.yml

+---
+title: 'Resolve Design Comment: Edit Comment text'
+merge_request: 30479
+author:
+type: added

locale/gitlab.pot

@@ -7220,15 +7220,27 @@ msgstr ""
 msgid "DesignManagement|Adding a design with the same filename replaces the file in a new version."
 msgstr ""
 
+msgid "DesignManagement|Are you sure you want to cancel changes to this comment?"
+msgstr ""
+
 msgid "DesignManagement|Are you sure you want to cancel creating this comment?"
 msgstr ""
 
 msgid "DesignManagement|Are you sure you want to delete the selected designs?"
 msgstr ""
 
+msgid "DesignManagement|Cancel changes"
+msgstr ""
+
 msgid "DesignManagement|Cancel comment confirmation"
 msgstr ""
 
+msgid "DesignManagement|Cancel comment update confirmation"
+msgstr ""
+
+msgid "DesignManagement|Comment"
+msgstr ""
+
 msgid "DesignManagement|Could not add a new comment. Please try again."
 msgstr ""
 
@@ -7238,6 +7250,9 @@ msgstr ""
 msgid "DesignManagement|Could not update discussion. Please try again."
 msgstr ""
 
+msgid "DesignManagement|Could not update note. Please try again."
+msgstr ""
+
 msgid "DesignManagement|Delete"
 msgstr ""
 
@@ -7265,12 +7280,18 @@ msgstr ""
 msgid "DesignManagement|Go to previous design"
 msgstr ""
 
+msgid "DesignManagement|Keep changes"
+msgstr ""
+
 msgid "DesignManagement|Keep comment"
 msgstr ""
 
 msgid "DesignManagement|Requested design version does not exist. Showing latest version instead"
 msgstr ""
 
+msgid "DesignManagement|Save comment"
+msgstr ""
+
 msgid "DesignManagement|Select all"
 msgstr ""
 

spec/frontend/design_management/components/design_notes/__snapshots__/design_note_spec.js.snap

@@ -3,7 +3,7 @@
 exports[`Design note component should match the snapshot 1`] = `
 <timeline-entry-item-stub
   class="design-note note-form"
-  id="note_undefined"
+  id="note_123"
 >
   <user-avatar-link-stub
     imgalt=""
@@ -16,50 +16,55 @@ exports[`Design note component should match the snapshot 1`] = `
     username=""
   />
    
-  <a
-    class="js-user-link"
-    data-user-id="author-id"
-  >
-    <span
-      class="note-header-author-name bold"
-    >
-      
-    </span>
-     
-    <!---->
-     
-    <span
-      class="note-headline-light"
-    >
-      @
-    </span>
-  </a>
-   
-  <span
-    class="note-headline-light note-headline-meta"
+  <div
+    class="d-flex justify-content-between"
   >
-    <span
-      class="system-note-message"
-    />
-     
-    <span
-      class="system-note-separator"
-    />
+    <div>
+      <a
+        class="js-user-link"
+        data-user-id="author-id"
+      >
+        <span
+          class="note-header-author-name bold"
+        >
+          
+        </span>
+         
+        <!---->
+         
+        <span
+          class="note-headline-light"
+        >
+          @
+        </span>
+      </a>
+       
+      <span
+        class="note-headline-light note-headline-meta"
+      >
+        <span
+          class="system-note-message"
+        />
+         
+        <!---->
+      </span>
+    </div>
      
-    <a
-      class="note-timestamp system-note-separator"
-      href="#note_undefined"
+    <button
+      class="note-action-button js-note-edit btn btn-transparent qa-note-edit-button"
+      title="Edit comment"
+      type="button"
     >
-      <time-ago-tooltip-stub
-        cssclass=""
-        time="2019-07-26T15:02:20Z"
-        tooltipplacement="bottom"
+      <gl-icon-stub
+        class="link-highlight"
+        name="pencil"
+        size="16"
       />
-    </a>
-  </span>
+    </button>
+  </div>
    
   <div
-    class="note-text md"
+    class="note-text js-note-text md"
     data-qa-selector="note_content"
   />
 </timeline-entry-item-stub>

spec/frontend/design_management/components/design_notes/__snapshots__/design_reply_form_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Design reply form component renders button text as "Comment" when creating a comment 1`] = `
+"<button data-track-event=\\"click_button\\" data-qa-selector=\\"save_comment_button\\" type=\\"submit\\" disabled=\\"disabled\\" class=\\"btn btn-success btn-md disabled\\">
+  <!---->
+  Comment
+</button>"
+`;
+
+exports[`Design reply form component renders button text as "Save comment" when creating a comment 1`] = `
+"<button data-track-event=\\"click_button\\" data-qa-selector=\\"save_comment_button\\" type=\\"submit\\" disabled=\\"disabled\\" class=\\"btn btn-success btn-md disabled\\">
+  <!---->
+  Save comment
+</button>"
+`;

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

 import { shallowMount } from '@vue/test-utils';
+import { ApolloMutation } from 'vue-apollo';
 import DesignNote from '~/design_management/components/design_notes/design_note.vue';
 import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue';
 
 const scrollIntoViewMock = jest.fn();
+const note = {
+  id: 'gid://gitlab/DiffNote/123',
+  author: {
+    id: 'author-id',
+  },
+  body: 'test',
+};
 HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;
 
 const $route = {
   hash: '#note_123',
 };
 
+const mutate = jest.fn().mockResolvedValue({ data: { updateNote: {} } });
+
 describe('Design note component', () => {
   let wrapper;
 
   const findUserAvatar = () => wrapper.find(UserAvatarLink);
   const findUserLink = () => wrapper.find('.js-user-link');
+  const findReplyForm = () => wrapper.find(DesignReplyForm);
+  const findEditButton = () => wrapper.find('.js-note-edit');
+  const findNoteContent = () => wrapper.find('.js-note-text');
 
-  function createComponent(props = {}) {
+  function createComponent(props = {}, data = { isEditing: false }) {
     wrapper = shallowMount(DesignNote, {
       propsData: {
         note: {},
         ...props,
       },
+      data() {
+        return {
+          ...data,
+        };
+      },
       mocks: {
         $route,
+        $apollo: {
+          mutate,
+        },
+      },
+      stubs: {
+        ApolloMutation,
       },
     });
   }
@@ -34,13 +59,7 @@ describe('Design note component', () => {
 
   it('should match the snapshot', () => {
     createComponent({
-      note: {
-        id: '1',
-        createdAt: '2019-07-26T15:02:20Z',
-        author: {
-          id: 'author-id',
-        },
-      },
+      note,
     });
 
     expect(wrapper.element).toMatchSnapshot();
@@ -48,12 +67,7 @@ describe('Design note component', () => {
 
   it('should render an author', () => {
     createComponent({
-      note: {
-        id: '1',
-        author: {
-          id: 'author-id',
-        },
-      },
+      note,
     });
 
     expect(findUserAvatar().exists()).toBe(true);
@@ -63,11 +77,8 @@ describe('Design note component', () => {
   it('should render a time ago tooltip if note has createdAt property', () => {
     createComponent({
       note: {
-        id: '1',
+        ...note,
         createdAt: '2019-07-26T15:02:20Z',
-        author: {
-          id: 'author-id',
-        },
       },
     });
 
@@ -76,14 +87,61 @@ describe('Design note component', () => {
 
   it('should trigger a scrollIntoView method', () => {
     createComponent({
-      note: {
-        id: 'gid://gitlab/DiffNote/123',
-        author: {
-          id: 'author-id',
-        },
-      },
+      note,
     });
 
     expect(scrollIntoViewMock).toHaveBeenCalled();
   });
+
+  it('should open an edit form on edit button click', () => {
+    createComponent({
+      note,
+    });
+
+    findEditButton().trigger('click');
+
+    return wrapper.vm.$nextTick().then(() => {
+      expect(findReplyForm().exists()).toBe(true);
+      expect(findNoteContent().exists()).toBe(false);
+    });
+  });
+
+  describe('when edit form is rendered', () => {
+    beforeEach(() => {
+      createComponent(
+        {
+          note,
+        },
+        { isEditing: true },
+      );
+    });
+
+    it('should not render note content and should render reply form', () => {
+      expect(findNoteContent().exists()).toBe(false);
+      expect(findReplyForm().exists()).toBe(true);
+    });
+
+    it('hides the form on hideForm event', () => {
+      findReplyForm().vm.$emit('cancelForm');
+
+      return wrapper.vm.$nextTick().then(() => {
+        expect(findReplyForm().exists()).toBe(false);
+        expect(findNoteContent().exists()).toBe(true);
+      });
+    });
+
+    it('calls a mutation on submitForm event and hides a form', () => {
+      findReplyForm().vm.$emit('submitForm');
+      expect(mutate).toHaveBeenCalled();
+
+      return mutate()
+        .then(() => {
+          return wrapper.vm.$nextTick();
+        })
+        .then(() => {
+          expect(findReplyForm().exists()).toBe(false);
+          expect(findNoteContent().exists()).toBe(true);
+        });
+    });
+  });
 });

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

 import { mount } from '@vue/test-utils';
 import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue';
 
+const showModal = jest.fn();
+
+const GlModal = {
+  template: '<div><slot name="modal-title"></slot><slot></slot><slot name="modal-ok"></slot></div>',
+  methods: {
+    show: showModal,
+  },
+};
+
 describe('Design reply form component', () => {
   let wrapper;
 
@@ -16,6 +25,7 @@ describe('Design reply form component', () => {
         isSaving: false,
         ...props,
       },
+      stubs: { GlModal },
     });
   }
 
@@ -29,6 +39,18 @@ describe('Design reply form component', () => {
     expect(findTextarea().element).toEqual(document.activeElement);
   });
 
+  it('renders button text as "Comment" when creating a comment', () => {
+    createComponent();
+
+    expect(findSubmitButton().html()).toMatchSnapshot();
+  });
+
+  it('renders button text as "Save comment" when creating a comment', () => {
+    createComponent({ isNewComment: false });
+
+    expect(findSubmitButton().html()).toMatchSnapshot();
+  });
+
   describe('when form has no text', () => {
     beforeEach(() => {
       createComponent({
@@ -120,16 +142,34 @@ describe('Design reply form component', () => {
       });
     });
 
-    it('opens confirmation modal on pressing Escape button', () => {
+    it('emits cancelForm event on Escape key if text was not changed', () => {
       findTextarea().trigger('keyup.esc');
 
-      expect(findModal().exists()).toBe(true);
+      expect(wrapper.emitted('cancelForm')).toBeTruthy();
     });
 
-    it('opens confirmation modal on Cancel button click', () => {
-      findCancelButton().vm.$emit('click');
+    it('opens confirmation modal on Escape key when text has changed', () => {
+      wrapper.setProps({ value: 'test2' });
 
-      expect(findModal().exists()).toBe(true);
+      return wrapper.vm.$nextTick().then(() => {
+        findTextarea().trigger('keyup.esc');
+        expect(showModal).toHaveBeenCalled();
+      });
+    });
+
+    it('emits cancelForm event on Cancel button click if text was not changed', () => {
+      findCancelButton().trigger('click');
+
+      expect(wrapper.emitted('cancelForm')).toBeTruthy();
+    });
+
+    it('opens confirmation modal on Cancel button click when text has changed', () => {
+      wrapper.setProps({ value: 'test2' });
+
+      return wrapper.vm.$nextTick().then(() => {
+        findCancelButton().trigger('click');
+        expect(showModal).toHaveBeenCalled();
+      });
     });
 
     it('emits cancelForm event on modal Ok button click', () => {