diff options
-rw-r--r-- | app/assets/javascripts/notes/components/comment_form.vue | 35 | ||||
-rw-r--r-- | app/assets/stylesheets/framework/modal.scss | 16 | ||||
-rw-r--r-- | app/views/projects/issues/import_csv/_modal.html.haml | 4 | ||||
-rw-r--r-- | app/views/shared/issuable/csv_export/_modal.html.haml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/33283-remove-svg.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/tor-defect-network-error-message-show-api-errors.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 3 | ||||
-rw-r--r-- | spec/frontend/notes/components/comment_form_spec.js | 114 |
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 }); |