diff --git a/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue b/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue index d4985a8a3ddf777d9adfa1e604016b25505522c7..0e11b52cee21996be482c58ce2c0693dee62da48 100644 --- a/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue +++ b/app/assets/javascripts/ml/model_registry/apps/show_ml_model_version.vue @@ -18,6 +18,7 @@ export default { return { projectPath: this.projectPath, canWriteModelRegistry: this.canWriteModelRegistry, + importPath: this.importPath, }; }, props: { @@ -45,6 +46,10 @@ export default { type: Boolean, required: true, }, + importPath: { + type: String, + required: true, + }, }, apollo: { modelWithModelVersion: { @@ -100,7 +105,7 @@ export default { <div> <title-area :title="title" /> <load-or-error-or-show :is-loading="isLoading" :error-message="errorMessage"> - <model-version-detail :model-version="modelVersion" /> + <model-version-detail :model-version="modelVersion" allow-artifact-import /> </load-or-error-or-show> </div> </template> 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 new file mode 100644 index 0000000000000000000000000000000000000000..812d07a0337f8468178e41adbbaf7fac146e753d --- /dev/null +++ b/app/assets/javascripts/ml/model_registry/components/import_artifact_zone.vue @@ -0,0 +1,94 @@ +<script> +import { GlLoadingIcon } from '@gitlab/ui'; +import { createAlert } from '~/alert'; +import axios from '~/lib/utils/axios_utils'; +import { contentTypeMultipartFormData } from '~/lib/utils/headers'; +import { numberToHumanSize } from '~/lib/utils/number_utils'; +import { joinPaths } from '~/lib/utils/url_utility'; +import { s__ } from '~/locale'; +import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue'; + +export default { + name: 'ImportArtifactZone', + components: { + GlLoadingIcon, + UploadDropzone, + }, + props: { + path: { + type: String, + required: true, + }, + }, + data() { + return { + file: null, + loading: false, + }; + }, + computed: { + formattedFileSize() { + return numberToHumanSize(this.file.size); + }, + importUrl() { + return joinPaths(this.path, encodeURIComponent(this.file.name)); + }, + }, + methods: { + resetFile() { + this.file = null; + this.loading = false; + }, + submitRequest() { + const formData = new FormData(); + formData.append('file', this.file); + return axios + .put(this.importUrl, formData, { + headers: { + ...contentTypeMultipartFormData, + }, + }) + .then(() => { + this.$emit('change'); + this.resetFile(); + }) + .catch(() => { + this.resetFile(); + createAlert({ message: this.$options.i18n.errorMessage }); + }); + }, + uploadFile(file) { + this.file = file; + this.loading = true; + + return this.submitRequest(); + }, + }, + i18n: { + dropToStartMessage: s__('MLOps|Drop to start upload'), + uploadSingleMessage: s__('MLOps|Drop or %{linkStart}select%{linkEnd} artifact to attach'), + errorMessage: s__('MLOps|Error importing artifact. Please try again.'), + }, + validFileMimetypes: [], +}; +</script> +<template> + <upload-dropzone + single-file-selection + display-as-card + :valid-file-mimetypes="$options.validFileMimetypes" + :upload-single-message="$options.i18n.uploadSingleMessage" + :drop-to-start-message="$options.i18n.dropToStartMessage" + :is-file-valid="() => true" + @change="uploadFile" + > + <div + v-if="file" + class="card upload-dropzone-card upload-dropzone-border gl-w-full gl-h-full align-items-center justify-content-center gl-p-3" + > + <gl-loading-icon class="gl-p-5" size="sm" /> + <div data-testid="formatted-file-size">{{ formattedFileSize }}</div> + <div data-testid="file-name">{{ file.name }}</div> + </div> + </upload-dropzone> +</template> diff --git a/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue b/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue index dd9f333dc79b53e9724b553d8e8528a4601d1f45..e898163bacc63a96edb1a10eb32f054adc126667 100644 --- a/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue +++ b/app/assets/javascripts/ml/model_registry/components/model_version_detail.vue @@ -9,13 +9,19 @@ export default { PackageFiles: () => import('~/packages_and_registries/package_registry/components/details/package_files.vue'), CandidateDetail, + ImportArtifactZone: () => import('./import_artifact_zone.vue'), }, - inject: ['projectPath', 'canWriteModelRegistry'], + inject: ['projectPath', 'canWriteModelRegistry', 'importPath'], props: { modelVersion: { type: Object, required: true, }, + allowArtifactImport: { + type: Boolean, + required: false, + default: true, + }, }, computed: { packageType() { @@ -27,6 +33,9 @@ export default { packageId() { return this.modelVersion.packageId; }, + showImportArtifactZone() { + return this.canWriteModelRegistry && this.importPath && this.allowArtifactImport; + }, }, i18n, }; @@ -50,7 +59,11 @@ export default { :delete-all-files="true" :project-path="projectPath" :package-type="packageType" - /> + > + <template v-if="showImportArtifactZone" #upload="{ refetch }"> + <import-artifact-zone :path="importPath" @change="refetch" /> + </template> + </package-files> </template> <div class="gl-mt-5"> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue index ba7bfe5fcc3a93a75b8f31fc7637186437a059aa..4147135ecaca2317afc67bcc3cdcfde9d333ef34 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/details/package_files.vue @@ -328,6 +328,9 @@ export default { focusable.focus(); } }, + refetchPackageFiles() { + this.$apollo.getClient().refetchQueries({ include: [getPackageFilesQuery] }); + }, }, i18n: { deleteFile: s__('PackageRegistry|Delete asset'), @@ -496,6 +499,7 @@ export default { @next="fetchNextFilesPage" /> </div> + <slot name="upload" :refetch="refetchPackageFiles"></slot> </template> <gl-modal diff --git a/app/assets/javascripts/vue_shared/components/upload_dropzone/upload_dropzone.vue b/app/assets/javascripts/vue_shared/components/upload_dropzone/upload_dropzone.vue index 23fbf211d5410d78ff0a11654150cff42356a8cd..6e7ee2690fd7ae19eebd1e525fb90c7ad5486f7e 100644 --- a/app/assets/javascripts/vue_shared/components/upload_dropzone/upload_dropzone.vue +++ b/app/assets/javascripts/vue_shared/components/upload_dropzone/upload_dropzone.vue @@ -21,6 +21,16 @@ export default { required: false, default: false, }, + uploadSingleMessage: { + type: String, + required: false, + default: __('Drop or %{linkStart}upload%{linkEnd} file to attach'), + }, + uploadMultipleMessage: { + type: String, + required: false, + default: __('Drop or %{linkStart}upload%{linkEnd} files to attach'), + }, dropToStartMessage: { type: String, required: false, @@ -162,16 +172,10 @@ export default { <p class="gl-mb-0" data-testid="upload-text"> <slot name="upload-text" :open-file-upload="openFileUpload"> <gl-sprintf - :message=" - singleFileSelection - ? __('Drop or %{linkStart}upload%{linkEnd} file to attach') - : __('Drop or %{linkStart}upload%{linkEnd} files to attach') - " + :message="singleFileSelection ? uploadSingleMessage : uploadMultipleMessage" > <template #link="{ content }"> - <gl-link @click.stop="openFileUpload"> - {{ content }} - </gl-link> + <gl-link @click.stop="openFileUpload">{{ content }}</gl-link> </template> </gl-sprintf> </slot> diff --git a/app/helpers/projects/ml/model_registry_helper.rb b/app/helpers/projects/ml/model_registry_helper.rb index 27a3fdd644a08e26c834f832f370b6bf0d66b841..5ab64e62ebad0fc8900bd443ff3d30e8e3ba7df7 100644 --- a/app/helpers/projects/ml/model_registry_helper.rb +++ b/app/helpers/projects/ml/model_registry_helper.rb @@ -40,7 +40,8 @@ def show_ml_model_version_data(model_version, user) model_version_id: model_version.id, model_name: model_version.name, version_name: model_version.version, - can_write_model_registry: can_write_model_registry?(user, project) + can_write_model_registry: can_write_model_registry?(user, project), + import_path: model_version_artifact_import_path(project.id, model_version.id) } to_json(data) @@ -48,6 +49,14 @@ def show_ml_model_version_data(model_version, user) private + def model_version_artifact_import_path(project_id, model_version_id) + path = api_v4_projects_packages_ml_models_files___path___path( + id: project_id, model_version_id: model_version_id, path: '', file_name: '' + ) + + path.delete_suffix('(/path/)') + end + def can_write_model_registry?(user, project) user&.can?(:write_model_registry, project) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 62c4c8ce18912ffd2380c02306738a5085952d6f..1d724f58e9c7a2557d1441d09d8d443915db4640 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -30995,6 +30995,15 @@ msgstr "" msgid "MLExperimentTracking|Delete experiment?" msgstr "" +msgid "MLOps|Drop or %{linkStart}select%{linkEnd} artifact to attach" +msgstr "" + +msgid "MLOps|Drop to start upload" +msgstr "" + +msgid "MLOps|Error importing artifact. Please try again." +msgstr "" + msgid "MRApprovals|Approvals" msgstr "" diff --git a/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js b/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js index 03156601c8a2a29aec397d7bc4b919169ebfb545..ec2f8f7783c6271bd643eca1b942421088ebd790 100644 --- a/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js +++ b/spec/frontend/ml/model_registry/apps/show_ml_model_version_spec.js @@ -33,6 +33,7 @@ describe('ml/model_registry/apps/show_model_version.vue', () => { modelVersionId: 2, projectPath: 'path/to/project', canWriteModelRegistry: true, + importPath: 'path/to/import', }, apolloProvider, stubs: { 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 new file mode 100644 index 0000000000000000000000000000000000000000..bd3b763d319129302ccd0d120f0df692620bae58 --- /dev/null +++ b/spec/frontend/ml/model_registry/components/import_artifact_zone_spec.js @@ -0,0 +1,110 @@ +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; +import { GlLoadingIcon } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { createAlert } from '~/alert'; +import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import ImportArtifactZone from '~/ml/model_registry/components/import_artifact_zone.vue'; +import UploadDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue'; + +jest.mock('~/alert'); + +describe('ImportArtifactZone', () => { + let wrapper; + let axiosMock; + + const file = { name: 'file.txt', size: 1024 }; + const initialProps = { + path: 'some/path', + }; + const filePath = 'some/path/file.txt'; + const formattedFileSizeDiv = () => wrapper.findByTestId('formatted-file-size'); + const fileNameDiv = () => wrapper.findByTestId('file-name'); + const zone = () => wrapper.findComponent(UploadDropzone); + const loadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const emulateFileDrop = () => zone().vm.$emit('change', file); + + beforeEach(() => { + wrapper = shallowMountExtended(ImportArtifactZone, { + propsData: { + ...initialProps, + }, + }); + + axiosMock = new MockAdapter(axios); + axiosMock.onPut(filePath).replyOnce(HTTP_STATUS_OK, {}); + }); + + afterEach(() => { + axiosMock.restore(); + }); + + describe('successful upload', () => { + it('displays the formatted file size', async () => { + await emulateFileDrop(); + expect(formattedFileSizeDiv().text()).toContain('1.00 KiB'); + }); + + it('displays the formatted file name', async () => { + await emulateFileDrop(); + expect(fileNameDiv().text()).toContain('file.txt'); + }); + + it('displays the loading icon', async () => { + await emulateFileDrop(); + + expect(loadingIcon().exists()).toBe(true); + }); + + it('resets the file and loading state', async () => { + await emulateFileDrop(); + + await waitForPromises(); + expect(loadingIcon().exists()).toBe(false); + expect(formattedFileSizeDiv().exists()).toBe(false); + expect(fileNameDiv().exists()).toBe(false); + }); + + it('submits the request', async () => { + await emulateFileDrop(); + await waitForPromises(); + + expect(axiosMock.history.put).toHaveLength(1); + const uploadRequest = axiosMock.history.put[0]; + expect(uploadRequest.url).toBe('some/path/file.txt'); + }); + + it('emits a change event on success', async () => { + await emulateFileDrop(); + await waitForPromises(); + + expect(wrapper.emitted('change')).toStrictEqual([[]]); + }); + + describe('failed uploads', () => { + it('displays an error on failure', async () => { + axiosMock.reset(); + axiosMock.onPut(filePath).replyOnce(HTTP_STATUS_INTERNAL_SERVER_ERROR, {}); + + await emulateFileDrop(); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ + message: 'Error importing artifact. Please try again.', + }); + }); + + it('resets the state on failure', async () => { + axiosMock.reset(); + axiosMock.onPut(filePath).timeout(); + + await emulateFileDrop(); + await waitForPromises(); + expect(loadingIcon().exists()).toBe(false); + expect(formattedFileSizeDiv().exists()).toBe(false); + expect(fileNameDiv().exists()).toBe(false); + }); + }); + }); +}); diff --git a/spec/frontend/ml/model_registry/components/model_version_detail_spec.js b/spec/frontend/ml/model_registry/components/model_version_detail_spec.js index aa20ad15856bbec667d2fdd6cabaa3fc3a4c17d9..1bf8cd5c12f4ae462f7a00baad19ad4613edf7ec 100644 --- a/spec/frontend/ml/model_registry/components/model_version_detail_spec.js +++ b/spec/frontend/ml/model_registry/components/model_version_detail_spec.js @@ -4,8 +4,11 @@ import VueApollo from 'vue-apollo'; import ModelVersionDetail from '~/ml/model_registry/components/model_version_detail.vue'; import PackageFiles from '~/packages_and_registries/package_registry/components/details/package_files.vue'; import CandidateDetail from '~/ml/model_registry/components/candidate_detail.vue'; +import ImportArtifactZone from '~/ml/model_registry/components/import_artifact_zone.vue'; import createMockApollo from 'helpers/mock_apollo_helper'; import { convertCandidateFromGraphql } from '~/ml/model_registry/utils'; +import getPackageFiles from '~/packages_and_registries/package_registry/graphql/queries/get_package_files.query.graphql'; +import { packageFilesQuery } from 'jest/packages_and_registries/package_registry/mock_data'; import { modelVersionWithCandidate } from '../graphql_mock_data'; Vue.use(VueApollo); @@ -15,20 +18,27 @@ const makeGraphqlModelVersion = (overrides = {}) => { }; let wrapper; -const createWrapper = (modelVersion = modelVersionWithCandidate) => { - const apolloProvider = createMockApollo([]); +const createWrapper = (modelVersion = modelVersionWithCandidate, props = {}, provide = {}) => { + const requestHandlers = [ + [getPackageFiles, jest.fn().mockResolvedValue(packageFilesQuery({ files: [] }))], + ]; + + const apolloProvider = createMockApollo(requestHandlers); wrapper = shallowMount(ModelVersionDetail, { apolloProvider, - propsData: { modelVersion }, + propsData: { modelVersion, ...props }, provide: { projectPath: 'path/to/project', canWriteModelRegistry: true, + importPath: 'path/to/import', + ...provide, }, }); }; const findPackageFiles = () => wrapper.findComponent(PackageFiles); const findCandidateDetail = () => wrapper.findComponent(CandidateDetail); +const findImportArtifactZone = () => wrapper.findComponent(ImportArtifactZone); describe('ml/model_registry/components/model_version_detail.vue', () => { describe('base behaviour', () => { @@ -61,6 +71,12 @@ describe('ml/model_registry/components/model_version_detail.vue', () => { deleteAllFiles: true, }); }); + + it('renders import artifact zone', () => { + expect(findImportArtifactZone().props()).toEqual({ + path: 'path/to/import', + }); + }); }); describe('if package does not exist', () => { @@ -71,6 +87,30 @@ describe('ml/model_registry/components/model_version_detail.vue', () => { }); }); + describe('if permission does not exist', () => { + beforeEach(() => createWrapper(undefined, undefined, { canWriteModelRegistry: false })); + + it('does not render import artifact zone', () => { + expect(findImportArtifactZone().exists()).toBe(false); + }); + }); + + describe('if import path does not exist', () => { + beforeEach(() => createWrapper(undefined, undefined, { importPath: undefined })); + + it('does not render import artifact zone', () => { + expect(findImportArtifactZone().exists()).toBe(false); + }); + }); + + describe('if artifact import is allowed', () => { + beforeEach(() => createWrapper(undefined, { allowArtifactImport: false })); + + it('does not render import artifact zone', () => { + expect(findImportArtifactZone().exists()).toBe(false); + }); + }); + describe('if model version does not have description', () => { beforeEach(() => createWrapper(makeGraphqlModelVersion({ description: null }))); diff --git a/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js b/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js index 46109156770d0fb11fc1ce2b7f9814201609fc23..dc1c65b7ee8c3b929b627dbbc4ed1d7dc4bf869f 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/components/details/package_files_spec.js @@ -784,7 +784,6 @@ describe('Package Files', () => { }); }); }); - describe('additional details', () => { describe('details toggle button', () => { it('exists', async () => { @@ -865,4 +864,31 @@ describe('Package Files', () => { }); }); }); + describe('upload slot', () => { + const resolver = jest.fn().mockResolvedValue(packageFilesQuery({ files: [file] })); + const uploadSlotSpy = jest.fn(); + + const callFetchSlotProp = () => uploadSlotSpy.mock.calls[0][0].refetch(); + + beforeEach(async () => { + createComponent({ + resolver, + options: { + scopedSlots: { + upload: uploadSlotSpy, + }, + }, + }); + await waitForPromises(); + }); + + it('should render slot content', () => { + expect(uploadSlotSpy).toHaveBeenLastCalledWith({ refetch: expect.anything() }); + }); + + it('should refetch on change', () => { + callFetchSlotProp(); + expect(resolver).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/upload_dropzone/upload_dropzone_spec.js b/spec/frontend/vue_shared/components/upload_dropzone/upload_dropzone_spec.js index 776395b9717ce6d90053f0f01001752fb165c964..e34b2f768b31f62d37a1d15797098e0c7c02264e 100644 --- a/spec/frontend/vue_shared/components/upload_dropzone/upload_dropzone_spec.js +++ b/spec/frontend/vue_shared/components/upload_dropzone/upload_dropzone_spec.js @@ -186,6 +186,27 @@ describe('Upload dropzone component', () => { expect(wrapper.element).toMatchSnapshot(); }); + it('correctly overrides single upload messages', () => { + createComponent({ + props: { + singleFileSelection: true, + uploadSingleMessage: 'Drop or select file to attach', + }, + }); + expect(findUploadText()).toContain('Drop or select file to attach'); + }); + + it('correctly overrides multiple upload messages', () => { + createComponent({ + props: { + singleFileSelection: false, + uploadMultipleMessage: 'Drop or select files to attach', + }, + }); + + expect(findUploadText()).toContain('Drop or select files to attach'); + }); + describe('file input form name', () => { it('applies inputFieldName as file input name', () => { createComponent({ props: { inputFieldName: 'test_field_name' } }); diff --git a/spec/helpers/projects/ml/model_registry_helper_spec.rb b/spec/helpers/projects/ml/model_registry_helper_spec.rb index fd99078a88144eaf1be9dd667d7ef9ec49cd9a00..d33798d63ca75b6f1d76a38f7bb057bd8a7e37c6 100644 --- a/spec/helpers/projects/ml/model_registry_helper_spec.rb +++ b/spec/helpers/projects/ml/model_registry_helper_spec.rb @@ -85,7 +85,8 @@ "modelVersionId" => model_version.id, "modelName" => model_version.name, "versionName" => model_version.version, - "canWriteModelRegistry" => true + "canWriteModelRegistry" => true, + "importPath" => "/api/v4/projects/#{project.id}/packages/ml_models/#{model_version.id}/files/" }) end