Merge branch 'tor/defect/network-error-message/show-api-errors' into 'master'

Show API errors when a command-only comment fails

See merge request gitlab-org/gitlab!55457

app/assets/javascripts/notes/components/comment_form.vue

 <script>
 import {
+  GlAlert,
   GlButton,
   GlIcon,
   GlFormCheckbox,
@@ -14,6 +15,7 @@ import { mapActions, mapGetters, mapState } from 'vuex';
 import Autosave from '~/autosave';
 import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
 import { deprecatedCreateFlash as Flash } from '~/flash';
+import httpStatusCodes from '~/lib/utils/http_status';
 import {
   capitalizeFirstCharacter,
   convertToCamelCase,
@@ -34,6 +36,8 @@ import CommentFieldLayout from './comment_field_layout.vue';
 import discussionLockedWidget from './discussion_locked_widget.vue';
 import noteSignedOutWidget from './note_signed_out_widget.vue';
 
+const { UNPROCESSABLE_ENTITY } = httpStatusCodes;
+
 export default {
   name: 'CommentForm',
   i18n: COMMENT_FORM,
@@ -43,6 +47,7 @@ export default {
     noteSignedOutWidget,
     discussionLockedWidget,
     markdownField,
+    GlAlert,
     GlButton,
     TimelineEntryItem,
     GlIcon,
@@ -66,6 +71,7 @@ export default {
     return {
       note: '',
       noteType: constants.COMMENT,
+      errors: [],
       noteIsConfidential: false,
       isSubmitting: false,
     };
@@ -201,11 +207,19 @@ export default {
       'reopenIssuable',
       'toggleIssueLocalState',
     ]),
+    handleSaveError({ data, status }) {
+      if (status === UNPROCESSABLE_ENTITY && data.errors?.commands_only?.length) {
+        this.errors = data.errors.commands_only;
+      } else {
+        this.errors = [this.$options.i18n.GENERIC_UNSUBMITTABLE_NETWORK];
+      }
+    },
     handleSave(withIssueAction) {
+      this.errors = [];
+
       if (this.note.length) {
         const noteData = {
           endpoint: this.endpoint,
-          flashContainer: this.$el,
           data: {
             note: {
               noteable_type: this.noteableType,
@@ -236,10 +250,10 @@ export default {
               this.toggleIssueState();
             }
           })
-          .catch(() => {
+          .catch(({ response }) => {
+            this.handleSaveError(response);
+
             this.discard(false);
-            const msg = this.$options.i18n.GENERIC_UNSUBMITTABLE_NETWORK;
-            Flash(msg, 'alert', this.$el);
             this.note = noteData.data.note.note; // Restore textarea content.
             this.removePlaceholderNotes();
           })
@@ -318,6 +332,9 @@ export default {
     hasEmailParticipants() {
       return this.getNoteableData.issue_email_participants?.length;
     },
+    dismissError(index) {
+      this.errors.splice(index, 1);
+    },
   },
 };
 </script>
@@ -328,7 +345,15 @@ export default {
     <discussion-locked-widget v-else-if="!canCreateNote" :issuable-type="issuableTypeTitle" />
     <ul v-else-if="canCreateNote" class="notes notes-form timeline">
       <timeline-entry-item class="note-form">
-        <div class="flash-container error-alert timeline-content"></div>
+        <gl-alert
+          v-for="(error, index) in errors"
+          :key="index"
+          variant="danger"
+          class="gl-mb-2"
+          @dismiss="() => dismissError(index)"
+        >
+          {{ error }}
+        </gl-alert>
         <div class="timeline-content timeline-content-form">
           <form ref="commentForm" class="new-note common-note-form gfm-form js-main-target-form">
             <comment-field-layout

changelogs/unreleased/tor-defect-network-error-message-show-api-errors.yml

+---
+title: Show API errors when a command-only comment fails
+merge_request: 55457
+author:
+type: other

spec/frontend/notes/components/comment_form_spec.js

-import { GlDropdown } from '@gitlab/ui';
+import { GlDropdown, GlAlert } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import Autosize from 'autosize';
 import MockAdapter from 'axios-mock-adapter';
-import { nextTick } from 'vue';
+import Vue, { nextTick } from 'vue';
+import Vuex from 'vuex';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
 import { deprecatedCreateFlash as flash } from '~/flash';
@@ -10,7 +11,8 @@ import axios from '~/lib/utils/axios_utils';
 import CommentForm from '~/notes/components/comment_form.vue';
 import * as constants from '~/notes/constants';
 import eventHub from '~/notes/event_hub';
-import createStore from '~/notes/stores';
+import { COMMENT_FORM } from '~/notes/i18n';
+import notesModule from '~/notes/stores/modules';
 import { loggedOutnoteableData, notesDataMock, userDataMock, noteableDataMock } from '../mock_data';
 
 jest.mock('autosize');
@@ -18,6 +20,8 @@ jest.mock('~/commons/nav/user_merge_requests');
 jest.mock('~/flash');
 jest.mock('~/gl_form');
 
+Vue.use(Vuex);
+
 describe('issue_comment_form component', () => {
   let store;
   let wrapper;
@@ -28,6 +32,33 @@ describe('issue_comment_form component', () => {
   const findConfidentialNoteCheckbox = () => wrapper.findByTestId('confidential-note-checkbox');
   const findCommentGlDropdown = () => wrapper.find(GlDropdown);
   const findCommentButton = () => findCommentGlDropdown().find('button');
+  const findErrorAlerts = () => wrapper.findAllComponents(GlAlert).wrappers;
+
+  async function clickCommentButton({ waitForComponent = true, waitForNetwork = true } = {}) {
+    findCommentButton().trigger('click');
+
+    if (waitForComponent || waitForNetwork) {
+      // Wait for the click to bubble out and trigger the handler
+      await nextTick();
+
+      if (waitForNetwork) {
+        // Wait for the network request promise to resolve
+        await nextTick();
+      }
+    }
+  }
+
+  function createStore({ actions = {} } = {}) {
+    const baseModule = notesModule();
+
+    return new Vuex.Store({
+      ...baseModule,
+      actions: {
+        ...baseModule.actions,
+        ...actions,
+      },
+    });
+  }
 
   const createNotableDataMock = (data = {}) => {
     return {
@@ -103,6 +134,83 @@ describe('issue_comment_form component', () => {
         expect(wrapper.vm.resizeTextarea).toHaveBeenCalled();
       });
 
+      it('does not report errors in the UI when the save succeeds', async () => {
+        mountComponent({ mountFunction: mount, initialData: { note: '/label ~sdfghj' } });
+
+        jest.spyOn(wrapper.vm, 'saveNote').mockResolvedValue();
+
+        await clickCommentButton();
+
+        // findErrorAlerts().exists returns false if *any* wrapper is empty,
+        //   not necessarily that there aren't any at all.
+        // We want to check here that there are none found, so we use the
+        //   raw wrapper array length instead.
+        expect(findErrorAlerts().length).toBe(0);
+      });
+
+      it.each`
+        httpStatus | errors
+        ${400}     | ${[COMMENT_FORM.GENERIC_UNSUBMITTABLE_NETWORK]}
+        ${422}     | ${['error 1']}
+        ${422}     | ${['error 1', 'error 2']}
+        ${422}     | ${['error 1', 'error 2', 'error 3']}
+      `(
+        'displays the correct errors ($errors) for a $httpStatus network response',
+        async ({ errors, httpStatus }) => {
+          store = createStore({
+            actions: {
+              saveNote: jest.fn().mockRejectedValue({
+                response: { status: httpStatus, data: { errors: { commands_only: errors } } },
+              }),
+            },
+          });
+
+          mountComponent({ mountFunction: mount, initialData: { note: '/label ~sdfghj' } });
+
+          await clickCommentButton();
+
+          const errorAlerts = findErrorAlerts();
+
+          expect(errorAlerts.length).toBe(errors.length);
+          errors.forEach((msg, index) => {
+            const alert = errorAlerts[index];
+
+            expect(alert.text()).toBe(msg);
+          });
+        },
+      );
+
+      it('should remove the correct error from the list when it is dismissed', async () => {
+        const commandErrors = ['1', '2', '3'];
+        store = createStore({
+          actions: {
+            saveNote: jest.fn().mockRejectedValue({
+              response: { status: 422, data: { errors: { commands_only: [...commandErrors] } } },
+            }),
+          },
+        });
+
+        mountComponent({ mountFunction: mount, initialData: { note: '/label ~sdfghj' } });
+
+        await clickCommentButton();
+
+        let errorAlerts = findErrorAlerts();
+
+        expect(errorAlerts.length).toBe(commandErrors.length);
+
+        // dismiss the second error
+        extendedWrapper(errorAlerts[1]).findByTestId('close-icon').trigger('click');
+        // Wait for the dismissal to bubble out of the Alert component and be handled in this component
+        await nextTick();
+        // Refresh the list of alerts
+        errorAlerts = findErrorAlerts();
+
+        expect(errorAlerts.length).toBe(commandErrors.length - 1);
+        // We want to know that the *correct* error was dismissed, not just that any one is gone
+        expect(errorAlerts[0].text()).toBe(commandErrors[0]);
+        expect(errorAlerts[1].text()).toBe(commandErrors[2]);
+      });
+
       it('should toggle issue state when no note', () => {
         mountComponent({ mountFunction: mount });