Labels editing and validation (#3426)

* saving * saving * lint errors fix * addressed comments * test * fixed last label red * added tests * Manaswini's comments addressed * saving * addressed new requirments * fix test * latest version * final * final1 * Force LabelGroup to expand after saving edits Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu> --------- Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu> Co-authored-by: Mike Turley <mike.turley@alum.cs.umass.edu>
opendatahub-io · Nov 26, 2024 · 6b1b8e5 · 6b1b8e5
1 parent b4c7a5a
commit 6b1b8e5
Show file tree

Hide file tree

Showing 6 changed files with 307 additions and 186 deletions.
diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts
@@ -126,6 +126,30 @@ class ModelVersionDetails {
       this.findTable().find(`[data-label=Key]`).contains(name).parents('tr'),
     );
   }
+
+  findEditLabelsButton() {
+    return cy.findByTestId('editable-labels-group-edit');
+  }
+
+  findAddLabelButton() {
+    return cy.findByTestId('add-label-button');
+  }
+
+  findLabelInput(label: string) {
+    return cy.findByTestId(`edit-label-input-${label}`);
+  }
+
+  findLabel(label: string) {
+    return cy.findByTestId(`editable-label-${label}`);
+  }
+
+  findLabelErrorAlert() {
+    return cy.findByTestId('label-error-alert');
+  }
+
+  findSaveLabelsButton() {
+    return cy.findByTestId('editable-labels-group-save');
+  }
 }
 
 class PropertyRow extends TableRow {}

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts
@@ -238,19 +238,6 @@ describe('Model version details', () => {
     it('Model version details tab', () => {
       modelVersionDetails.findVersionId().contains('1');
       modelVersionDetails.findDescription().should('have.text', 'Description of model version');
-      modelVersionDetails.findMoreLabelsButton().contains('6 more');
-      modelVersionDetails.findMoreLabelsButton().click();
-      modelVersionDetails.shouldContainsModalLabels([
-        'Testing label',
-        'Financial',
-        'Financial data',
-        'Fraud detection',
-        'Machine learning',
-        'Next data to be overflow',
-        'Label x',
-        'Label y',
-        'Label z',
-      ]);
       modelVersionDetails.findStorageEndpoint().contains('test-endpoint');
       modelVersionDetails.findStorageRegion().contains('test-region');
       modelVersionDetails.findStorageBucket().contains('test-bucket');
@@ -346,6 +333,82 @@ describe('Model version details', () => {
       modelVersionDetails.findModelVersionDropdownItem('Version 2').click();
       modelVersionDetails.findVersionId().contains('2');
     });
+
+    it('should handle label editing', () => {
+      modelVersionDetails.findEditLabelsButton().click();
+
+      modelVersionDetails.findAddLabelButton().click();
+      cy.findByTestId('editable-label-group')
+        .should('exist')
+        .within(() => {
+          cy.contains('New Label').should('exist').click();
+          cy.focused().type('First Label{enter}');
+        });
+
+      modelVersionDetails.findAddLabelButton().click();
+      cy.findByTestId('editable-label-group')
+        .should('exist')
+        .within(() => {
+          cy.contains('New Label').should('exist').click();
+          cy.focused().type('Second Label{enter}');
+        });
+
+      cy.findByTestId('editable-label-group').within(() => {
+        cy.contains('First Label').should('exist').click();
+        cy.focused().type('Updated First Label{enter}');
+      });
+
+      cy.findByTestId('editable-label-group').within(() => {
+        cy.contains('Second Label').parent().find('[data-testid^="remove-label-"]').click();
+      });
+
+      modelVersionDetails.findSaveLabelsButton().should('exist').click();
+    });
+
+    it('should validate label length', () => {
+      modelVersionDetails.findEditLabelsButton().click();
+
+      const longLabel = 'a'.repeat(64);
+      modelVersionDetails.findAddLabelButton().click();
+      cy.findByTestId('editable-label-group')
+        .should('exist')
+        .within(() => {
+          cy.contains('New Label').should('exist').click();
+          cy.focused().type(`${longLabel}{enter}`);
+        });
+
+      cy.findByTestId('label-error-alert')
+        .should('be.visible')
+        .within(() => {
+          cy.contains(`can't exceed 63 characters`).should('exist');
+        });
+    });
+
+    it('should validate duplicate labels', () => {
+      modelVersionDetails.findEditLabelsButton().click();
+
+      modelVersionDetails.findAddLabelButton().click();
+      cy.findByTestId('editable-label-group')
+        .should('exist')
+        .within(() => {
+          cy.get('[data-testid^="editable-label-"]').last().click();
+          cy.focused().type('{selectall}{backspace}Testing label{enter}');
+        });
+
+      modelVersionDetails.findAddLabelButton().click();
+      cy.findByTestId('editable-label-group')
+        .should('exist')
+        .within(() => {
+          cy.get('[data-testid^="editable-label-"]').last().click();
+          cy.focused().type('{selectall}{backspace}Testing label{enter}');
+        });
+
+      cy.findByTestId('label-error-alert')
+        .should('be.visible')
+        .within(() => {
+          cy.contains('Testing label already exists').should('exist');
+        });
+    });
   });
 
   describe('Registered deployments tab', () => {

diff --git a/frontend/src/components/DashboardDescriptionListGroup.tsx b/frontend/src/components/DashboardDescriptionListGroup.tsx
@@ -33,6 +33,7 @@ type EditableProps = {
   editButtonTestId?: string;
   saveButtonTestId?: string;
   cancelButtonTestId?: string;
+  discardButtonTestId?: string;
 };
 
 export type DashboardDescriptionListGroupProps = {
@@ -43,6 +44,7 @@ export type DashboardDescriptionListGroupProps = {
   contentWhenEmpty?: React.ReactNode;
   children: React.ReactNode;
   groupTestId?: string;
+  isSaveDisabled?: boolean;
 } & (({ isEditable: true } & EditableProps) | ({ isEditable?: false } & Partial<EditableProps>));
 
 const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps> = (props) => {
@@ -64,6 +66,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
     editButtonTestId,
     saveButtonTestId,
     cancelButtonTestId,
+    isSaveDisabled,
   } = props;
   return (
     <DescriptionListGroup data-testid={groupTestId}>
@@ -81,7 +84,7 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
                         aria-label={`Save edits to ${title}`}
                         variant="link"
                         onClick={onSaveEditsClick}
-                        isDisabled={isSavingEdits}
+                        isDisabled={isSavingEdits || isSaveDisabled}
                       >
                         <CheckIcon />
                       </Button>