Model registry: version validation

app/assets/javascripts/lib/utils/regexp.js

@@ -9,3 +9,5 @@ export const unicodeLetters =
 
 export const semverRegex =
   /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/;
+
+export const noSpacesRegex = /^\S+$/;

app/assets/javascripts/ml/model_registry/components/model_create.vue

@@ -12,6 +12,7 @@ import {
 import { __, s__ } from '~/locale';
 import { visitUrl } from '~/lib/utils/url_utility';
 import * as Sentry from '~/sentry/sentry_browser_wrapper';
+import { semverRegex, noSpacesRegex } from '~/lib/utils/regexp';
 import { uploadModel } from '../services/upload_model';
 import createModelVersionMutation from '../graphql/mutations/create_model_version.mutation.graphql';
 import createModelMutation from '../graphql/mutations/create_model.mutation.graphql';
@@ -49,6 +50,36 @@ export default {
     showImportArtifactZone() {
       return this.version && this.name;
     },
+    modelNameIsValid() {
+      return this.name && noSpacesRegex.test(this.name);
+    },
+    isSemver() {
+      return semverRegex.test(this.version);
+    },
+    isVersionValid() {
+      return !this.version || this.isSemver;
+    },
+    submitButtonDisabled() {
+      return !this.isVersionValid || !this.modelNameIsValid;
+    },
+    actionPrimary() {
+      return {
+        text: s__('MlModelRegistry|Create'),
+        attributes: { variant: 'confirm', disabled: this.submitButtonDisabled },
+      };
+    },
+    validVersionFeedback() {
+      if (this.isSemver) {
+        return this.$options.modal.versionValid;
+      }
+      return null;
+    },
+    modelNameDescription() {
+      return !this.name || this.modelNameIsValid ? this.$options.modal.nameDescription : '';
+    },
+    versionDescriptionText() {
+      return !this.version ? this.$options.modal.versionDescription : '';
+    },
   },
   methods: {
     async createModel() {
@@ -138,29 +169,28 @@ export default {
   i18n: {},
   modal: {
     id: MODEL_CREATION_MODAL_ID,
-    actionPrimary: {
-      text: __('Create'),
-      attributes: { variant: 'confirm' },
-    },
     actionSecondary: {
       text: __('Cancel'),
       attributes: { variant: 'default' },
     },
-    nameDescription: s__(
-      'MlModelRegistry|Model name must not contain spaces or upper case letter.',
-    ),
-    namePlaceholder: s__('MlModelRegistry|For example my-model'),
-    versionDescription: s__('MlModelRegistry|Leave empty to skip version creation.'),
-    versionPlaceholder: s__('MlModelRegistry|For example 1.0.0. Must be a semantic version.'),
-    descriptionPlaceholder: s__('MlModelRegistry|Enter a model description'),
+    nameDescriptionLabel: s__('MlModelRegistry|Must be unique. May not contain spaces.'),
+    nameDescription: s__('MlModelRegistry|Example: my-model'),
+    nameInvalid: s__('MlModelRegistry|May not contain spaces.'),
+    namePlaceholder: s__('MlModelRegistry|Enter a model name'),
+    versionDescription: s__('MlModelRegistry|Example: 1.0.0'),
+    versionPlaceholder: s__('MlModelRegistry|Enter a semantic version'),
+    nameDescriptionPlaceholder: s__('MlModelRegistry|Enter a model description'),
     versionDescriptionTitle: s__('MlModelRegistry|Version description'),
-    versionDescriptionPlaceholder: s__(
-      'MlModelRegistry|Enter a description for this version of the model.',
+    versionDescriptionLabel: s__(
+      'MlModelRegistry|Must be a semantic version. Leave blank to skip version creation.',
     ),
+    versionValid: s__('MlModelRegistry|Version is a valid semantic version.'),
+    versionInvalid: s__('MlModelRegistry|Must be a semantic version. Example: 1.0.0'),
+    versionDescriptionPlaceholder: s__('MlModelRegistry|Enter a version description'),
     buttonTitle: s__('MlModelRegistry|Create model'),
     title: s__('MlModelRegistry|Create model, version & import artifacts'),
     modelName: s__('MlModelRegistry|Model name'),
-    modelDescription: __('Description'),
+    modelDescription: __('Model description'),
     version: __('Version'),
     uploadLabel: __('Upload artifacts'),
     modelSuccessButVersionArtifactFailAlert: {
@@ -170,6 +200,7 @@ export default {
       ),
       variant: 'warning',
     },
+    optionalText: s__('MlModelRegistry|Optional'),
   },
 };
 </script>
@@ -180,7 +211,7 @@ export default {
     <gl-modal
       :modal-id="$options.modal.id"
       :title="$options.modal.title"
-      :action-primary="$options.modal.actionPrimary"
+      :action-primary="actionPrimary"
       :action-secondary="$options.modal.actionSecondary"
       size="sm"
       @primary="create"
@@ -189,8 +220,12 @@ export default {
       <gl-form>
         <gl-form-group
           :label="$options.modal.modelName"
+          :label-description="$options.modal.nameDescriptionLabel"
           label-for="nameId"
-          :description="$options.modal.nameDescription"
+          data-testid="nameGroupId"
+          :state="modelNameIsValid"
+          :invalid-feedback="$options.modal.nameInvalid"
+          :description="modelNameDescription"
         >
           <gl-form-input
             id="nameId"
@@ -200,18 +235,30 @@ export default {
             :placeholder="$options.modal.namePlaceholder"
           />
         </gl-form-group>
-        <gl-form-group :label="$options.modal.modelDescription" label-for="descriptionId">
+        <gl-form-group
+          :label="$options.modal.modelDescription"
+          label-for="descriptionId"
+          optional
+          :optional-text="$options.modal.optionalText"
+        >
           <gl-form-textarea
             id="descriptionId"
             v-model="description"
             data-testid="descriptionId"
-            :placeholder="$options.modal.descriptionPlaceholder"
+            :placeholder="$options.modal.nameDescriptionPlaceholder"
           />
         </gl-form-group>
         <gl-form-group
           :label="$options.modal.version"
+          :label-description="$options.modal.versionDescriptionLabel"
+          data-testid="versionGroupId"
           label-for="versionId"
-          :description="$options.modal.versionDescription"
+          :state="isVersionValid"
+          :invalid-feedback="$options.modal.versionInvalid"
+          :valid-feedback="validVersionFeedback"
+          :description="versionDescriptionText"
+          optional
+          :optional-text="$options.modal.optionalText"
         >
           <gl-form-input
             id="versionId"
@@ -225,6 +272,8 @@ export default {
         <gl-form-group
           :label="$options.modal.versionDescriptionTitle"
           label-for="versionDescriptionId"
+          optional
+          :optional-text="$options.modal.optionalText"
         >
           <gl-form-textarea
             id="versionDescriptionId"
@@ -251,11 +300,11 @@ export default {
 
       <gl-alert
         v-if="errorMessage"
-        data-testid="modal-create-alert"
+        data-testid="modalCreateAlert"
         variant="danger"
         @dismiss="hideAlert"
-        >{{ errorMessage }}</gl-alert
-      >
+        >{{ errorMessage }}
+      </gl-alert>
     </gl-modal>
   </div>
 </template>

locale/gitlab.pot

@@ -33463,6 +33463,9 @@ msgstr ""
 msgid "MlModelRegistry|Candidate not linked to a CI build"
 msgstr ""
+msgid "MlModelRegistry|Create"
+msgstr ""
+
 msgid "MlModelRegistry|Create & import"
 msgstr ""
@@ -33523,10 +33526,13 @@ msgstr ""
 msgid "MlModelRegistry|Drop to start upload"
 msgstr ""
-msgid "MlModelRegistry|Enter a description for this version of the model."
+msgid "MlModelRegistry|Enter a model description"
 msgstr ""
-msgid "MlModelRegistry|Enter a model description"
+msgid "MlModelRegistry|Enter a model name"
+msgstr ""
+
+msgid "MlModelRegistry|Enter a semantic version"
 msgstr ""
 msgid "MlModelRegistry|Enter a semantic version."
@@ -33538,9 +33544,18 @@ msgstr ""
 msgid "MlModelRegistry|Enter a subfolder name to organize your artifacts."
 msgstr ""
+msgid "MlModelRegistry|Enter a version description"
+msgstr ""
+
 msgid "MlModelRegistry|Enter some description"
 msgstr ""
+msgid "MlModelRegistry|Example: 1.0.0"
+msgstr ""
+
+msgid "MlModelRegistry|Example: my-model"
+msgstr ""
+
 msgid "MlModelRegistry|Experiment"
 msgstr ""
@@ -33562,12 +33577,6 @@ msgstr ""
 msgid "MlModelRegistry|For example 1.0.0"
 msgstr ""
-msgid "MlModelRegistry|For example 1.0.0. Must be a semantic version."
-msgstr ""
-
-msgid "MlModelRegistry|For example my-model"
-msgstr ""
-
 msgid "MlModelRegistry|ID"
 msgstr ""
@@ -33580,9 +33589,6 @@ msgstr ""
 msgid "MlModelRegistry|Latest version"
 msgstr ""
-msgid "MlModelRegistry|Leave empty to skip version creation."
-msgstr ""
-
 msgid "MlModelRegistry|Logging artifacts"
 msgstr ""
@@ -33598,6 +33604,9 @@ msgstr ""
 msgid "MlModelRegistry|Manage versions of your machine learning modelManage versions of your machine learning model"
 msgstr ""
+msgid "MlModelRegistry|May not contain spaces."
+msgstr ""
+
 msgid "MlModelRegistry|Metadata"
 msgstr ""
@@ -33610,9 +33619,6 @@ msgstr ""
 msgid "MlModelRegistry|Model name"
 msgstr ""
-msgid "MlModelRegistry|Model name must not contain spaces or upper case letter."
-msgstr ""
-
 msgid "MlModelRegistry|Model performance"
 msgstr ""
@@ -33622,6 +33628,15 @@ msgstr ""
 msgid "MlModelRegistry|Model version %{versionName} deleted successfully"
 msgstr ""
+msgid "MlModelRegistry|Must be a semantic version. Example: 1.0.0"
+msgstr ""
+
+msgid "MlModelRegistry|Must be a semantic version. Leave blank to skip version creation."
+msgstr ""
+
+msgid "MlModelRegistry|Must be unique. May not contain spaces."
+msgstr ""
+
 msgid "MlModelRegistry|New model"
 msgstr ""
@@ -33640,6 +33655,9 @@ msgstr ""
 msgid "MlModelRegistry|No registered versions"
 msgstr ""
+msgid "MlModelRegistry|Optional"
+msgstr ""
+
 msgid "MlModelRegistry|Package creation failed"
 msgstr ""
@@ -33688,6 +33706,9 @@ msgstr ""
 msgid "MlModelRegistry|Version description"
 msgstr ""
+msgid "MlModelRegistry|Version is a valid semantic version."
+msgstr ""
+
 msgid "MlModelRegistry|Version is not a valid semantic version."
 msgstr ""
@@ -33723,6 +33744,9 @@ msgstr ""
 msgid "Model"
 msgstr ""
+msgid "Model description"
+msgstr ""
+
 msgid "Model experiments"
 msgstr ""

spec/frontend/ml/model_registry/components/model_create_spec.js

-import Vue from 'vue';
+import Vue, { nextTick } from 'vue';
 import VueApollo from 'vue-apollo';
 import { GlModal } from '@gitlab/ui';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
@@ -74,17 +74,19 @@ describe('ModelCreate', () => {
   const findModalButton = () => wrapper.findByText('Create model');
   const findNameInput = () => wrapper.findByTestId('nameId');
   const findVersionInput = () => wrapper.findByTestId('versionId');
+  const findVersionGroup = () => wrapper.findByTestId('versionGroupId');
   const findDescriptionInput = () => wrapper.findByTestId('descriptionId');
   const findVersionDescriptionInput = () => wrapper.findByTestId('versionDescriptionId');
   const findImportArtifactZone = () => wrapper.findComponent(ImportArtifactZone);
   const zone = () => wrapper.findComponent(UploadDropzone);
   const findGlModal = () => wrapper.findComponent(GlModal);
-  const findGlAlert = () => wrapper.findByTestId('modal-create-alert');
+  const findGlAlert = () => wrapper.findByTestId('modalCreateAlert');
   const submitForm = async () => {
     findGlModal().vm.$emit('primary', new Event('primary'));
     await waitForPromises();
   };
   const findArtifactZoneLabel = () => wrapper.findByTestId('importArtifactZoneLabel');
+  const findModelNameGroup = () => wrapper.findByTestId('nameGroupId');
 
   describe('Initial state', () => {
     describe('Modal closed', () => {
@@ -117,14 +119,46 @@ describe('ModelCreate', () => {
         expect(findNameInput().exists()).toBe(true);
       });
 
+      it('renders the model name group description', () => {
+        expect(findModelNameGroup().attributes('description')).toBe(
+          ModelCreate.modal.nameDescription,
+        );
+      });
+
+      it('renders the name label', () => {
+        expect(findModelNameGroup().attributes('label')).toBe(ModelCreate.modal.modelName);
+      });
+
       it('renders the version input', () => {
         expect(findVersionInput().exists()).toBe(true);
       });
 
+      it('renders the version label', () => {
+        expect(findVersionGroup().attributes('label')).toBe('Version');
+      });
+
+      it('renders the version placeholder', () => {
+        expect(findVersionInput().attributes('placeholder')).toBe(
+          ModelCreate.modal.versionPlaceholder,
+        );
+      });
+
+      it('renders the version group description', () => {
+        expect(findVersionGroup().attributes('description')).toBe(
+          ModelCreate.modal.versionDescription,
+        );
+      });
+
       it('renders the description input', () => {
         expect(findDescriptionInput().exists()).toBe(true);
       });
 
+      it('renders the description input text', () => {
+        expect(findVersionGroup().attributes('valid-feedback')).toBe(
+          ModelCreate.modal.validVersion,
+        );
+      });
+
       it('renders the version description input', () => {
         expect(findVersionDescriptionInput().exists()).toBe(true);
       });
@@ -166,7 +200,7 @@ describe('ModelCreate', () => {
 
       it('renders the create button in the modal', () => {
         expect(findGlModal().props('actionPrimary')).toEqual({
-          attributes: { variant: 'confirm' },
+          attributes: { variant: 'confirm', disabled: true },
           text: 'Create',
         });
       });
@@ -183,6 +217,81 @@ describe('ModelCreate', () => {
       });
     });
 
+    describe('It reacts to semantic version input', () => {
+      beforeEach(() => {
+        createWrapper();
+      });
+      it('renders the version input label for initial state', () => {
+        expect(findVersionGroup().attributes('state')).toBe('true');
+        expect(findGlModal().props('actionPrimary')).toEqual({
+          attributes: { variant: 'confirm', disabled: true },
+          text: 'Create',
+        });
+      });
+      it.each(['1.0', '1', 'abc', '1.abc', '1.0.0.0'])(
+        'renders the version input label for invalid state',
+        async (version) => {
+          findVersionInput().vm.$emit('input', version);
+          await nextTick();
+          expect(findVersionGroup().attributes()).not.toContain('state');
+          expect(findVersionGroup().attributes('invalid-feedback')).toBe(
+            ModelCreate.modal.versionInvalid,
+          );
+          expect(findVersionGroup().attributes('description')).toBe('');
+          expect(findGlModal().props('actionPrimary')).toEqual({
+            attributes: { variant: 'confirm', disabled: true },
+            text: 'Create',
+          });
+        },
+      );
+      it.each(['1.0.0', '0.0.0-b', '24.99.99-b99'])(
+        'renders the version input label for valid state',
+        async (version) => {
+          findVersionInput().vm.$emit('input', version);
+          await nextTick();
+          expect(findVersionGroup().attributes('state')).toBe('true');
+          expect(findVersionGroup().attributes('valid-feedback')).toBe(
+            ModelCreate.modal.versionValid,
+          );
+          expect(findVersionGroup().attributes('description')).toBe('');
+          expect(findGlModal().props('actionPrimary')).toEqual({
+            attributes: { variant: 'confirm', disabled: true },
+            text: 'Create',
+          });
+        },
+      );
+      it.each(['1.0.0', '0.0.0-b', '24.99.99-b99'])(
+        'renders the version input label for valid state',
+        async (version) => {
+          findNameInput().vm.$emit('input', 'gpt-alice-1');
+          findVersionInput().vm.$emit('input', version);
+          await nextTick();
+          expect(findVersionGroup().attributes('state')).toBe('true');
+          expect(findGlModal().props('actionPrimary')).toEqual({
+            attributes: { variant: 'confirm', disabled: false },
+            text: 'Create',
+          });
+        },
+      );
+
+      it.each(['model name', ' modelname', 'modelname ', ' ', ''])(
+        'renders the modelnames as invalid',
+        async (name) => {
+          findNameInput().vm.$emit('input', name);
+          await nextTick();
+          expect(findModelNameGroup().attributes()).not.toContain('state');
+        },
+      );
+      it.each(['modelname', 'model-name', 'MODELname', 'model_name'])(
+        'renders the modelnames as invalid',
+        async (name) => {
+          findNameInput().vm.$emit('input', name);
+          await nextTick();
+          expect(findModelNameGroup().attributes('state')).toBe('true');
+        },
+      );
+    });
+
     it('clicking on secondary button clears the form', async () => {
       createWrapper();