diff --git a/app/assets/javascripts/ml/model_registry/components/import_artifact_zone.vue b/app/assets/javascripts/ml/model_registry/components/import_artifact_zone.vue index 394e362cc86b241c24b42eb5a24b527245e0f82d..b79f4813e02d119cd0ce30d118f46a32fca7d113 100644 --- a/app/assets/javascripts/ml/model_registry/components/import_artifact_zone.vue +++ b/app/assets/javascripts/ml/model_registry/components/import_artifact_zone.vue @@ -64,6 +64,14 @@ export default { (s) => s === STATUS.FAILED || s === STATUS.CANCELED || s === STATUS.SUCCEEDED, ); }, + allErrors() { + return this.uploads + .map((upload, index) => { + return this.errors[index] ? `${upload.file.name}: ${this.errors[index]}` : null; + }) + .filter(Boolean) + .join(' '); + }, alert() { if (!this.states.length) { return null; @@ -98,6 +106,10 @@ export default { }, }, methods: { + setError(index, message) { + this.errors.splice(index, 1, message); + this.$emit('error', this.allErrors); + }, buildUploads(files) { this.states = new Array(files.length).fill(STATUS.CREATING); this.loads = new Array(files.length).fill(0); @@ -116,7 +128,7 @@ export default { upload.axiosSource.cancel(); } else if (this.states[index] === STATUS.CREATING) { this.states.splice(index, 1, STATUS.CANCELED); - this.errors.splice(index, 1, this.$options.i18n.cancelMessage); + this.setError(index, this.$options.i18n.cancelMessage); } }; upload.onUploadProgress = (event) => { @@ -173,10 +185,10 @@ export default { .catch((error) => { if (axios.isCancel(error)) { this.states.splice(index, 1, STATUS.CANCELED); - this.errors.splice(index, 1, this.$options.i18n.cancelMessage); + this.setError(index, this.$options.i18n.cancelMessage); } else { this.states.splice(index, 1, STATUS.FAILED); - this.errors.splice(index, 1, error); + this.setError(index, error); } }); }, @@ -204,7 +216,7 @@ export default { clearButtonText: __('Clear uploads'), uploadMessage: s__('MlModelRegistry|Drop or %{linkStart}select%{linkEnd} artifacts to attach'), subfolderLabel: s__('MlModelRegistry|Subfolder'), - allSucceeded: s__('MlModelRegistry|Artifact uploaded successfully.'), + allSucceeded: s__('MlModelRegistry|Artifacts uploaded successfully.'), allFailedOrCanceled: s__('MlModelRegistry|All artifact uploads failed or were canceled.'), someFailed: s__('MlModelRegistry|Artifact uploads completed with errors.'), subfolderPlaceholder: s__('MlModelRegistry|folder name'), diff --git a/app/assets/javascripts/ml/model_registry/components/model_create.vue b/app/assets/javascripts/ml/model_registry/components/model_create.vue index 217d93841d717cb5fc870b0bfe014a2e48da4ea8..bb36a0a4727dea503e3fce18926136b90aa13fea 100644 --- a/app/assets/javascripts/ml/model_registry/components/model_create.vue +++ b/app/assets/javascripts/ml/model_registry/components/model_create.vue @@ -9,7 +9,7 @@ import { GlModalDirective, } from '@gitlab/ui'; import { __, s__ } from '~/locale'; -import { visitUrl } from '~/lib/utils/url_utility'; +import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; import { helpPagePath } from '~/helpers/help_page_helper'; @@ -52,6 +52,7 @@ export default { versionData: null, markdownDocPath: helpPagePath('user/markdown'), markdownEditorRestrictedToolBarItems: ['full-screen'], + importErrorsText: null, }; }, computed: { @@ -91,6 +92,15 @@ export default { versionDescriptionText() { return !this.version ? this.$options.modal.versionDescription : ''; }, + importErrorsAlert() { + return { + id: 'import-artifact-alert', + variant: this.importErrorsText ? 'danger' : 'info', + message: this.importErrorsText + ? `${this.$options.i18n.someFailed} ${this.importErrorsText}` + : this.$options.i18n.allSucceeded, + }; + }, }, methods: { async createModel() { @@ -143,11 +153,11 @@ export default { const { showPath, importPath } = this.versionData.mlModelVersionCreate.modelVersion._links; await this.$refs.importArtifactZoneRef.uploadArtifact(importPath); - visitUrl(showPath); + visitUrlWithAlerts(showPath, [this.importErrorsAlert]); } } else { const { showPath } = this.modelData.mlModelCreate.model._links; - visitUrl(showPath); + visitUrlWithAlerts(showPath, [this.importErrorsAlert]); } } catch (error) { Sentry.captureException(error); @@ -162,6 +172,7 @@ export default { this.errorMessage = null; this.modelData = null; this.versionData = null; + this.importErrorsText = null; }, hideAlert() { this.errorMessage = null; @@ -176,8 +187,14 @@ export default { this.versionDescription = newVersionText; } }, + onImportError(error) { + this.importErrorsText = error; + }, + }, + i18n: { + allSucceeded: s__('MlModelRegistry|Artifacts uploaded successfully.'), + someFailed: s__('MlModelRegistry|Artifact uploads completed with errors.'), }, - i18n: {}, descriptionFormFieldProps: { placeholder: s__('MlModelRegistry|Enter a model description'), id: 'model-description', @@ -333,6 +350,7 @@ export default { ref="importArtifactZoneRef" class="gl-px-3 gl-py-0" :submit-on-select="false" + @error="onImportError" /> </gl-form-group> </gl-form> diff --git a/app/assets/javascripts/ml/model_registry/components/model_version_create.vue b/app/assets/javascripts/ml/model_registry/components/model_version_create.vue index 5c0549371feabb6e61660f9c832cc1ee380eef31..cf73f78bf653690c9a4b8517a2b2244997d0039b 100644 --- a/app/assets/javascripts/ml/model_registry/components/model_version_create.vue +++ b/app/assets/javascripts/ml/model_registry/components/model_version_create.vue @@ -9,7 +9,7 @@ import { GlModalDirective, } from '@gitlab/ui'; import { __, s__, sprintf } from '~/locale'; -import { visitUrl } from '~/lib/utils/url_utility'; +import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { semverRegex } from '~/lib/utils/regexp'; import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue'; @@ -53,6 +53,7 @@ export default { submitButtonDisabled: true, markdownDocPath: helpPagePath('user/markdown'), markdownEditorRestrictedToolBarItems: ['full-screen'], + importErrorsText: null, }; }, computed: { @@ -91,6 +92,15 @@ export default { this.submitAvailable(); return null; }, + importErrorsAlert() { + return { + id: 'import-artifact-alert', + variant: this.importErrorsText ? 'danger' : 'info', + message: this.importErrorsText + ? `${this.$options.i18n.someFailed} ${this.importErrorsText}` + : this.$options.i18n.allSucceeded, + }; + }, }, methods: { submitDisabled() { @@ -129,7 +139,7 @@ export default { const { showPath, importPath } = this.versionData.mlModelVersionCreate.modelVersion._links; await this.$refs.importArtifactZoneRef.uploadArtifact(importPath); - visitUrl(showPath); + visitUrlWithAlerts(showPath, [this.importErrorsAlert]); } } catch (error) { Sentry.captureException(error); @@ -141,6 +151,7 @@ export default { this.description = ''; this.errorMessage = null; this.versionData = null; + this.importErrorsText = null; }, hideAlert() { this.errorMessage = null; @@ -150,8 +161,14 @@ export default { this.description = newText; } }, + onImportError(error) { + this.importErrorsText = error; + }, + }, + i18n: { + allSucceeded: s__('MlModelRegistry|Artifacts uploaded successfully.'), + someFailed: s__('MlModelRegistry|Artifact uploads completed with errors.'), }, - i18n: {}, descriptionFormFieldProps: { placeholder: s__('MlModelRegistry|Enter a model version description'), id: 'model-version-description', @@ -242,6 +259,7 @@ export default { ref="importArtifactZoneRef" class="gl-px-3 gl-py-0" :submit-on-select="false" + @error="onImportError" /> </gl-form-group> </gl-form> diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fdf262e8f2c83bb91a09eff7d8e2c956aad21063..6c1c0c53d08b99c237dc897808072e3e7db9e5d1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -34743,15 +34743,15 @@ msgstr "" msgid "MlModelRegistry|Artifact file is too large" msgstr "" -msgid "MlModelRegistry|Artifact uploaded successfully." -msgstr "" - msgid "MlModelRegistry|Artifact uploads completed with errors." msgstr "" msgid "MlModelRegistry|Artifacts" msgstr "" +msgid "MlModelRegistry|Artifacts uploaded successfully." +msgstr "" + msgid "MlModelRegistry|CI Info" msgstr "" diff --git a/spec/frontend/ml/model_registry/components/import_artifact_zone_spec.js b/spec/frontend/ml/model_registry/components/import_artifact_zone_spec.js index b2f61759c364d479d62e1bf11dcceb632340452e..a346c0f2ad7495431fcbc54298c4d91b3bb0bf38 100644 --- a/spec/frontend/ml/model_registry/components/import_artifact_zone_spec.js +++ b/spec/frontend/ml/model_registry/components/import_artifact_zone_spec.js @@ -139,7 +139,7 @@ describe('ImportArtifactZone', () => { await emulateFileDrop(); await waitForPromises(); - expect(alert().text()).toBe('Artifact uploaded successfully.'); + expect(alert().text()).toBe('Artifacts uploaded successfully.'); expect(alert().props().variant).toBe('success'); }); }); @@ -259,7 +259,16 @@ describe('ImportArtifactZone', () => { expect(alert().props().variant).toBe('warning'); }); - it('displays an error on failure', async () => { + it('emits an error event on a failure', async () => { + uploadModel.mockRejectedValueOnce('File is too big.'); + + await emulateFileDrop(); + await waitForPromises(); + + expect(wrapper.emitted('error')).toStrictEqual([['file.txt: File is too big.']]); + }); + + it('displays an error on failure and cancelation', async () => { uploadModel.mockRejectedValueOnce('File is too big.'); uploadModel.mockRejectedValueOnce('File name is invalid.'); @@ -271,6 +280,19 @@ describe('ImportArtifactZone', () => { expect(alert().props().variant).toBe('danger'); }); + it('emits an error event on multiple failures', async () => { + uploadModel.mockRejectedValueOnce('File is too big.'); + uploadModel.mockRejectedValueOnce('File name is invalid.'); + + await emulateFileDrop(); + await waitForPromises(); + + expect(wrapper.emitted('error')).toStrictEqual([ + ['file.txt: File is too big.'], + ['file.txt: File is too big. another file.txt: File name is invalid.'], + ]); + }); + it('keeps the failed table', async () => { uploadModel.mockRejectedValueOnce('Internal server error.'); await emulateFileDrop(); diff --git a/spec/frontend/ml/model_registry/components/model_create_spec.js b/spec/frontend/ml/model_registry/components/model_create_spec.js index ff77d418180b225dfc664a86f7728e1acbb7fcdf..6642ec9cbd3b42a3f46f473392014d3bb739497f 100644 --- a/spec/frontend/ml/model_registry/components/model_create_spec.js +++ b/spec/frontend/ml/model_registry/components/model_create_spec.js @@ -3,7 +3,7 @@ import VueApollo from 'vue-apollo'; import { GlModal } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; -import { visitUrl } from '~/lib/utils/url_utility'; +import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; import ModelCreate from '~/ml/model_registry/components/model_create.vue'; import ImportArtifactZone from '~/ml/model_registry/components/import_artifact_zone.vue'; import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue'; @@ -22,7 +22,7 @@ Vue.use(VueApollo); jest.mock('~/lib/utils/url_utility', () => ({ ...jest.requireActual('~/lib/utils/url_utility'), - visitUrl: jest.fn(), + visitUrlWithAlerts: jest.fn(), })); jest.mock('~/ml/model_registry/services/upload_model', () => ({ @@ -407,7 +407,13 @@ describe('ModelCreate', () => { }); it('Visits the model versions page upon successful create mutation', () => { - expect(visitUrl).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1'); + expect(visitUrlWithAlerts).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1', [ + { + id: 'import-artifact-alert', + message: 'Artifacts uploaded successfully.', + variant: 'info', + }, + ]); }); }); @@ -422,7 +428,13 @@ describe('ModelCreate', () => { }); it('Visits the model page upon successful create mutation without a version', () => { - expect(visitUrl).toHaveBeenCalledWith('/some/project/-/ml/models/1'); + expect(visitUrlWithAlerts).toHaveBeenCalledWith('/some/project/-/ml/models/1', [ + { + id: 'import-artifact-alert', + message: 'Artifacts uploaded successfully.', + variant: 'info', + }, + ]); }); }); @@ -495,7 +507,13 @@ describe('ModelCreate', () => { it('Visits the model versions page upon successful create mutation', async () => { await submitForm(); // retry submit - expect(visitUrl).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1'); + expect(visitUrlWithAlerts).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1', [ + { + id: 'import-artifact-alert', + message: 'Artifact uploads completed with errors. file.txt: Artifact import error.', + variant: 'danger', + }, + ]); }); it('Uploads a file mutation upon confirm', async () => { diff --git a/spec/frontend/ml/model_registry/components/model_version_create_spec.js b/spec/frontend/ml/model_registry/components/model_version_create_spec.js index ec41bba69cdbb71f0a2833916b9ab8ca92cbf863..32fe87ea0877cba10d3b4d8a1a4172e285dc87b6 100644 --- a/spec/frontend/ml/model_registry/components/model_version_create_spec.js +++ b/spec/frontend/ml/model_registry/components/model_version_create_spec.js @@ -3,7 +3,7 @@ import VueApollo from 'vue-apollo'; import { GlAlert, GlModal } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; -import { visitUrl } from '~/lib/utils/url_utility'; +import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; import ModelVersionCreate from '~/ml/model_registry/components/model_version_create.vue'; import ImportArtifactZone from '~/ml/model_registry/components/import_artifact_zone.vue'; import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue'; @@ -21,7 +21,7 @@ Vue.use(VueApollo); jest.mock('~/lib/utils/url_utility', () => ({ ...jest.requireActual('~/lib/utils/url_utility'), - visitUrl: jest.fn(), + visitUrlWithAlerts: jest.fn(), })); jest.mock('~/ml/model_registry/services/upload_model', () => ({ @@ -275,7 +275,13 @@ describe('ModelVersionCreate', () => { await submitForm(); - expect(visitUrl).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1'); + expect(visitUrlWithAlerts).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1', [ + { + id: 'import-artifact-alert', + message: 'Artifacts uploaded successfully.', + variant: 'info', + }, + ]); }); it('clicking on secondary button clears the form', async () => { @@ -313,7 +319,13 @@ describe('ModelVersionCreate', () => { it('Visits the model versions page upon successful create mutation', async () => { await submitForm(); - expect(visitUrl).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1'); + expect(visitUrlWithAlerts).toHaveBeenCalledWith('/some/project/-/ml/models/1/versions/1', [ + { + id: 'import-artifact-alert', + message: 'Artifact uploads completed with errors. file.txt: Artifact import error.', + variant: 'danger', + }, + ]); }); it('Uploads the model upon retry', async () => {