Add latest changes from gitlab-org/gitlab@master

8 files changed, 153 insertions, 33 deletions

diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue
index dd6f05a6ae7..08d7c745791 100644
--- a/app/assets/javascripts/notes/components/comment_form.vue
+++ b/app/assets/javascripts/notes/components/comment_form.vue
@@ -1,5 +1,6 @@
 <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
diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss
index 2dbeacb0f8c..ec433434573 100644
--- a/app/assets/stylesheets/framework/modal.scss
+++ b/app/assets/stylesheets/framework/modal.scss
@@ -130,22 +130,6 @@ body.modal-open {
 
 .issues-import-modal,
 .issuable-export-modal {
-  .modal-header {
-    justify-content: flex-start;
-
-    .import-export-svg-container {
-      flex-grow: 1;
-      height: 56px;
-      padding: $gl-btn-padding $gl-btn-padding 0;
-      text-align: right;
-
-      .illustration {
-        height: inherit;
-        width: initial;
-      }
-    }
-  }
-
   .modal-body {
     padding: 0;
 
diff --git a/app/views/projects/issues/import_csv/_modal.html.haml b/app/views/projects/issues/import_csv/_modal.html.haml
index e928a71b940..ac370210e12 100644
--- a/app/views/projects/issues/import_csv/_modal.html.haml
+++ b/app/views/projects/issues/import_csv/_modal.html.haml
@@ -3,10 +3,8 @@
     .modal-content
       = form_tag import_csv_namespace_project_issues_path, multipart: true do
         .modal-header
-          %h3
+          %h4.gl-m-0
             = _('Import issues')
-          .svg-content.import-export-svg-container
-            = image_tag 'illustrations/export-import.svg', alt: _('Import/Export illustration'), class: 'illustration'
           %a.close{ href: '#', 'data-dismiss' => 'modal' } ×
         .modal-body
           .modal-text
diff --git a/app/views/shared/issuable/csv_export/_modal.html.haml b/app/views/shared/issuable/csv_export/_modal.html.haml
index 4a4c6b90cd9..17990de8522 100644
--- a/app/views/shared/issuable/csv_export/_modal.html.haml
+++ b/app/views/shared/issuable/csv_export/_modal.html.haml
@@ -4,10 +4,8 @@
     .modal-dialog
       .modal-content{ data: { qa_selector: "export_issuable_modal" } }
         .modal-header
-          %h3
+          %h4.gl-m-0
             = _("Export %{issuable_type}" % { issuable_type: issuable_type.humanize(capitalize: false) })
-          .svg-content.import-export-svg-container
-            = image_tag 'illustrations/export-import.svg', role: "presentation", class: 'illustration'
           %button.close{ type: "button", "data-dismiss": "modal", "aria-label" => _('Close') }
             = sprite_icon('close', css_class: 'gl-icon')
         .modal-body
diff --git a/changelogs/unreleased/33283-remove-svg.yml b/changelogs/unreleased/33283-remove-svg.yml
new file mode 100644
index 00000000000..67b5775c639
--- /dev/null
+++ b/changelogs/unreleased/33283-remove-svg.yml
@@ -0,0 +1,5 @@
+---
+title: Remove illustration in export/import CSV modal
+merge_request: 54136
+author: Yogi (@yo)
+type: changed
diff --git a/changelogs/unreleased/tor-defect-network-error-message-show-api-errors.yml b/changelogs/unreleased/tor-defect-network-error-message-show-api-errors.yml
new file mode 100644
index 00000000000..627dc8bf741
--- /dev/null
+++ b/changelogs/unreleased/tor-defect-network-error-message-show-api-errors.yml
@@ -0,0 +1,5 @@
+---
+title: Show API errors when a command-only comment fails
+merge_request: 55457
+author:
+type: other
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d503936eebf..36c206f2bd7 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -15489,9 +15489,6 @@ msgstr ""
 msgid "Import/Export Rate Limits"
 msgstr ""
 
-msgid "Import/Export illustration"
-msgstr ""
-
 msgid "ImportButtons|Connect repositories from"
 msgstr ""
 
diff --git a/spec/frontend/notes/components/comment_form_spec.js b/spec/frontend/notes/components/comment_form_spec.js
index 6393394b68c..bab90723578 100644
--- a/spec/frontend/notes/components/comment_form_spec.js
+++ b/spec/frontend/notes/components/comment_form_spec.js
@@ -1,8 +1,9 @@
-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 });