Fix validation in the storage modal mount path

opendatahub-io · Sep 23, 2024 · 15d1363 · 15d1363
1 parent a509cf4
commit 15d1363
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 10 deletions.
diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts
@@ -1,7 +1,7 @@
 import _ from 'lodash-es';
 import { KnownLabels, NotebookKind } from '~/k8sTypes';
 import { DEFAULT_NOTEBOOK_SIZES } from '~/pages/projects/screens/spawner/const';
-import { ContainerResources, TolerationEffect, TolerationOperator } from '~/types';
+import { ContainerResources, TolerationEffect, TolerationOperator, VolumeMount } from '~/types';
 import { genUID } from '~/__mocks__/mockUtils';
 import { RecursivePartial } from '~/typeHelpers';
 
@@ -17,6 +17,7 @@ type MockResourceConfigType = {
   lastImageSelection?: string;
   opts?: RecursivePartial<NotebookKind>;
   uid?: string;
+  additionalVolumeMounts?: VolumeMount[];
 };
 
 export const mockNotebookK8sResource = ({
@@ -31,6 +32,7 @@ export const mockNotebookK8sResource = ({
   lastImageSelection = 's2i-minimal-notebook:py3.8-v1',
   opts = {},
   uid = genUID('notebook'),
+  additionalVolumeMounts = [],
 }: MockResourceConfigType): NotebookKind =>
   _.merge(
     {
@@ -144,6 +146,7 @@ export const mockNotebookK8sResource = ({
                     mountPath: '/opt/app-root/src',
                     name,
                   },
+                  ...additionalVolumeMounts,
                 ],
                 workingDir: '/opt/app-root/src',
               },

diff --git a/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts b/frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts
@@ -49,6 +49,10 @@ class ClusterStorageModal extends Modal {
     return this.find().findByTestId('mount-path-folder-value');
   }
 
+  findMountFieldHelperText() {
+    return this.find().findByTestId('mount-path-folder-helper-text');
+  }
+
   findWorkbenchRestartAlert() {
     return this.find().findByTestId('notebook-restart-alert');
   }

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/clusterStorage.cy.ts
@@ -53,7 +53,19 @@ const initInterceptors = ({ isEmpty = false, storageClassName }: HandlersProps)
           ],
     ),
   );
-  cy.interceptK8sList(NotebookModel, mockK8sResourceList([mockNotebookK8sResource({})]));
+  cy.interceptK8sList(
+    NotebookModel,
+    mockK8sResourceList([
+      mockNotebookK8sResource({
+        additionalVolumeMounts: [
+          {
+            mountPath: '/opt/app-root/src/test-dupe',
+            name: 'test-dupe-pvc-path',
+          },
+        ],
+      }),
+    ]),
+  );
 };
 
 const [openshiftDefaultStorageClass, otherStorageClass] = mockStorageClasses;
@@ -133,6 +145,35 @@ describe('ClusterStorage', () => {
       .findWorkbenchConnectionSelect()
       .findSelectOption('Test Notebook')
       .click();
+
+    // don't allow duplicate path
+    addClusterStorageModal.findMountField().clear();
+    addClusterStorageModal.findMountField().fill('test-dupe');
+    addClusterStorageModal
+      .findMountFieldHelperText()
+      .should('have.text', 'Mount folder is already in use for this workbench.');
+
+    // don't allow number in the path
+    addClusterStorageModal.findMountField().clear();
+    addClusterStorageModal.findMountField().fill('test2');
+    addClusterStorageModal
+      .findMountFieldHelperText()
+      .should('have.text', 'Must only consist of lower case letters and dashes.');
+
+    // Allow trailing slash
+    addClusterStorageModal.findMountField().clear();
+    addClusterStorageModal.findMountField().fill('test/');
+    addClusterStorageModal
+      .findMountFieldHelperText()
+      .should('have.text', 'Must consist of lower case letters and dashes.');
+
+    addClusterStorageModal.findMountField().clear();
+    addClusterStorageModal
+      .findMountFieldHelperText()
+      .should(
+        'have.text',
+        'Enter a path to a model or folder. This path cannot point to a root folder.',
+      );
     addClusterStorageModal.findMountField().fill('data');
     addClusterStorageModal.findWorkbenchRestartAlert().should('exist');
 

diff --git a/frontend/src/pages/projects/pvc/MountPathField.tsx b/frontend/src/pages/projects/pvc/MountPathField.tsx
@@ -37,11 +37,11 @@ const MountPathField: React.FC<MountPathFieldProps> = ({
           onChange={(e, value) => {
             let error = '';
             if (value.length === 0) {
-              error = 'Required';
-            } else if (!/^[a-z-]+$/.test(value)) {
-              error = 'Must only consist of lower case letters and dashes';
+              error = 'Enter a path to a model or folder. This path cannot point to a root folder.';
+            } else if (!/^[a-z-]+\/?$/.test(value)) {
+              error = 'Must only consist of lower case letters and dashes.';
             } else if (inUseMountPaths.includes(`/${value}`)) {
-              error = 'Mount folder is already in use for this workbench';
+              error = 'Mount folder is already in use for this workbench.';
             }
             setMountPath({ value, error });
           }}
@@ -50,10 +50,11 @@ const MountPathField: React.FC<MountPathFieldProps> = ({
     </InputGroup>
     <FormHelperText>
       <HelperText>
-        <HelperTextItem variant={mountPath.error ? 'error' : 'default'}>
-          {mountPath.error
-            ? 'Enter a path to a model or folder. This path cannot point to a root folder.'
-            : 'Must consist of lower case letters and dashes.'}
+        <HelperTextItem
+          variant={mountPath.error ? 'error' : 'default'}
+          data-testid="mount-path-folder-helper-text"
+        >
+          {mountPath.error || 'Must consist of lower case letters and dashes.'}
         </HelperTextItem>
       </HelperText>
     </FormHelperText>