Upadte archived models and archived versions components

opendatahub-io · Sep 13, 2024 · 467d440 · 467d440
1 parent d0b5ce9
commit 467d440
Show file tree

Hide file tree

Showing 18 changed files with 567 additions and 233 deletions.
diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive.ts
@@ -66,6 +66,12 @@ class ModelArchive {
     cy.visit(`/modelRegistry/${preferredModelRegistry}/registeredModels/archive/${rmId}`);
   }
 
+  visitArchiveModelVersionList() {
+    const rmId = '2';
+    const preferredModelRegistry = 'modelregistry-sample';
+    cy.visit(`/modelRegistry/${preferredModelRegistry}/registeredModels/archive/${rmId}/versions`);
+  }
+
   visitModelList() {
     cy.visit('/modelRegistry/modelregistry-sample');
     this.wait();

diff --git a/...end/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts b/...end/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts
@@ -48,7 +48,7 @@ const initIntercepts = ({
   ],
   modelVersions = [
     mockModelVersion({ author: 'Author 1' }),
-    mockModelVersion({ name: 'model version' }),
+    mockModelVersion({ name: 'model version', registeredModelId: '2' }),
   ],
 }: HandlersProps) => {
   cy.interceptOdh(
@@ -74,13 +74,25 @@ const initIntercepts = ({
     mockRegisteredModelList({ items: registeredModels }),
   );
 
+  cy.interceptOdh(
+    `GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/model_versions/:modelVersionId`,
+    {
+      path: {
+        serviceName: 'modelregistry-sample',
+        apiVersion: MODEL_REGISTRY_API_VERSION,
+        modelVersionId: 1,
+      },
+    },
+    mockModelVersion({ id: '1', name: 'Version 2' }),
+  );
+
   cy.interceptOdh(
     `GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/registered_models/:registeredModelId/versions`,
     {
       path: {
         serviceName: 'modelregistry-sample',
         apiVersion: MODEL_REGISTRY_API_VERSION,
-        registeredModelId: 1,
+        registeredModelId: 2,
       },
     },
     mockModelVersionList({ items: modelVersions }),
@@ -123,6 +135,22 @@ describe('Model archive list', () => {
     registeredModelArchive.findArchiveModelTable().should('be.visible');
   });
 
+  it('Archived model flow', () => {
+    initIntercepts({});
+    registeredModelArchive.visitArchiveModelVersionList();
+    verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/archive/2/versions');
+
+    modelRegistry.findModelVersionsTable().should('be.visible');
+    modelRegistry.findModelVersionsTableRows().should('have.length', 2);
+    const version = modelRegistry.getModelVersionRow('model version');
+    version.findModelVersionName().contains('model version').click();
+    verifyRelativeURL(
+      '/modelRegistry/modelregistry-sample/registeredModels/archive/2/versions/1/details',
+    );
+    cy.go('back');
+    verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/archive/2/versions');
+  });
+
   it('Archive models list', () => {
     initIntercepts({});
     registeredModelArchive.visit();
@@ -299,3 +327,5 @@ describe('Archiving model', () => {
     });
   });
 });
+
+//  add flow for archived model page to version details page... and check read crumb and url when back
diff --git a/frontend/src/components/EditableTextDescriptionListGroup.tsx b/frontend/src/components/EditableTextDescriptionListGroup.tsx
@@ -11,12 +11,14 @@ type EditableTextDescriptionListGroupProps = Pick<
   value: string;
   saveEditedValue: (value: string) => Promise<void>;
   testid?: string;
+  isArchive?: boolean;
 };
 
 const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
   title,
   contentWhenEmpty,
   value,
+  isArchive,
   saveEditedValue,
   testid,
 }) => {
@@ -29,7 +31,7 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
       title={title}
       isEmpty={!value}
       contentWhenEmpty={contentWhenEmpty}
-      isEditable
+      isEditable={!isArchive}
       isEditing={isEditing}
       isSavingEdits={isSavingEdits}
       contentWhenEditing={

diff --git a/frontend/src/pages/modelRegistry/ModelRegistryRoutes.tsx b/frontend/src/pages/modelRegistry/ModelRegistryRoutes.tsx
@@ -13,6 +13,7 @@ import RegisteredModelsArchiveDetails from './screens/RegisteredModelsArchive/Re
 import RegisterModel from './screens/RegisterModel/RegisterModel';
 import RegisterVersion from './screens/RegisterModel/RegisterVersion';
 import { modelRegistryUrl } from './screens/routeUtils';
+import ArchiveModelVersionDetails from './screens/ModelVersionsArchive/ArchiveModelVersionDetails';
 
 const ModelRegistryRoutes: React.FC = () => (
   <Routes>
@@ -91,6 +92,25 @@ const ModelRegistryRoutes: React.FC = () => (
               <RegisteredModelsArchiveDetails tab={ModelVersionsTab.VERSIONS} empty={false} />
             }
           />
+          <Route path="versions/:modelVersionId">
+            <Route index element={<Navigate to={ModelVersionDetailsTab.DETAILS} replace />} />
+            <Route
+              path={ModelVersionDetailsTab.DETAILS}
+              element={
+                <ArchiveModelVersionDetails tab={ModelVersionDetailsTab.DETAILS} empty={false} />
+              }
+            />
+            <Route
+              path={ModelVersionDetailsTab.DEPLOYMENTS}
+              element={
+                <ArchiveModelVersionDetails
+                  tab={ModelVersionDetailsTab.DEPLOYMENTS}
+                  empty={false}
+                />
+              }
+            />
+            <Route path="*" element={<Navigate to="." />} />
+          </Route>
           <Route path="*" element={<Navigate to="." />} />
         </Route>
         <Route path="*" element={<Navigate to="." />} />

diff --git a/frontend/src/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx b/frontend/src/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx
@@ -11,11 +11,13 @@ import { getProperties, mergeUpdatedProperty } from './utils';
 
 type ModelPropertiesDescriptionListGroupProps = {
   customProperties: ModelRegistryCustomProperties;
+  isArchive?: boolean;
   saveEditedCustomProperties: (properties: ModelRegistryCustomProperties) => Promise<unknown>;
 };
 
 const ModelPropertiesDescriptionListGroup: React.FC<ModelPropertiesDescriptionListGroupProps> = ({
   customProperties = {},
+  isArchive,
   saveEditedCustomProperties,
 }) => {
   const [editingPropertyKeys, setEditingPropertyKeys] = React.useState<string[]>([]);
@@ -51,16 +53,18 @@ const ModelPropertiesDescriptionListGroup: React.FC<ModelPropertiesDescriptionLi
     <DashboardDescriptionListGroup
       title="Properties"
       action={
-        <Button
-          isInline
-          variant="link"
-          icon={<PlusCircleIcon />}
-          iconPosition="start"
-          isDisabled={isAdding || isSavingEdits}
-          onClick={() => setIsAdding(true)}
-        >
-          Add property
-        </Button>
+        !isArchive && (
+          <Button
+            isInline
+            variant="link"
+            icon={<PlusCircleIcon />}
+            iconPosition="start"
+            isDisabled={isAdding || isSavingEdits}
+            onClick={() => setIsAdding(true)}
+          >
+            Add property
+          </Button>
+        )
       }
       isEmpty={!isAdding && keys.length === 0}
       contentWhenEmpty="No properties"
@@ -70,13 +74,14 @@ const ModelPropertiesDescriptionListGroup: React.FC<ModelPropertiesDescriptionLi
           <Tr>
             <Th>Key {isEditingSomeRow && requiredAsterisk}</Th>
             <Th>Value {isEditingSomeRow && requiredAsterisk}</Th>
-            <Th />
+            <Th screenReaderText="Actions" />
           </Tr>
         </Thead>
         <Tbody>
           {shownKeys.map((key) => (
             <ModelPropertiesTableRow
               key={key}
+              isArchive={isArchive}
               keyValuePair={{ key, value: filteredProperties[key].string_value }}
               allExistingKeys={allExistingKeys}
               isEditing={editingPropertyKeys.includes(key)}

diff --git a/frontend/src/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx b/frontend/src/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx
@@ -18,6 +18,7 @@ type ModelPropertiesTableRowProps = {
   allExistingKeys: string[];
   setIsEditing: (isEditing: boolean) => void;
   isSavingEdits: boolean;
+  isArchive?: boolean;
   setIsSavingEdits: (isSaving: boolean) => void;
   saveEditedProperty: (oldKey: string, newPair: KeyValuePair) => Promise<unknown>;
 } & EitherNotBoth<
@@ -38,6 +39,7 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({
   setIsEditing,
   isSavingEdits,
   setIsSavingEdits,
+  isArchive,
   saveEditedProperty,
 }) => {
   const { key, value } = keyValuePair;
@@ -143,43 +145,45 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({
           </ExpandableSection>
         )}
       </Td>
-      <Td isActionCell width={10}>
-        {isEditing ? (
-          <ActionList isIconList>
-            <ActionListItem>
-              <Button
-                data-testid={`save-edit-button-property-${key}`}
-                aria-label={`Save edits to property with key ${key}`}
-                variant="link"
-                onClick={onSaveEditsClick}
-                isDisabled={isSavingEdits || !unsavedKey || !unsavedValue || !!keyValidationError}
-              >
-                <CheckIcon />
-              </Button>
-            </ActionListItem>
-            <ActionListItem>
-              <Button
-                data-testid={`discard-edit-button-property-${key}`}
-                aria-label={`Discard edits to property with key ${key}`}
-                variant="plain"
-                onClick={onDiscardEditsClick}
-                isDisabled={isSavingEdits}
-              >
-                <TimesIcon />
-              </Button>
-            </ActionListItem>
-          </ActionList>
-        ) : (
-          <ActionsColumn
-            isDisabled={isSavingEdits}
-            popperProps={{ direction: 'up' }}
-            items={[
-              { title: 'Edit', onClick: onEditClick, isDisabled: isSavingEdits },
-              { title: 'Delete', onClick: onDeleteClick, isDisabled: isSavingEdits },
-            ]}
-          />
-        )}
-      </Td>
+      {!isArchive && (
+        <Td isActionCell width={10}>
+          {isEditing ? (
+            <ActionList isIconList>
+              <ActionListItem>
+                <Button
+                  data-testid={`save-edit-button-property-${key}`}
+                  aria-label={`Save edits to property with key ${key}`}
+                  variant="link"
+                  onClick={onSaveEditsClick}
+                  isDisabled={isSavingEdits || !unsavedKey || !unsavedValue || !!keyValidationError}
+                >
+                  <CheckIcon />
+                </Button>
+              </ActionListItem>
+              <ActionListItem>
+                <Button
+                  data-testid={`discard-edit-button-property-${key}`}
+                  aria-label={`Discard edits to property with key ${key}`}
+                  variant="plain"
+                  onClick={onDiscardEditsClick}
+                  isDisabled={isSavingEdits}
+                >
+                  <TimesIcon />
+                </Button>
+              </ActionListItem>
+            </ActionList>
+          ) : (
+            <ActionsColumn
+              isDisabled={isSavingEdits}
+              popperProps={{ direction: 'up' }}
+              items={[
+                { title: 'Edit', onClick: onEditClick, isDisabled: isSavingEdits },
+                { title: 'Delete', onClick: onDeleteClick, isDisabled: isSavingEdits },
+              ]}
+            />
+          )}
+        </Td>
+      )}
     </Tr>
   );
 };

diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx
@@ -14,6 +14,7 @@ type ModelVersionDetailTabsProps = {
   modelVersion: ModelVersion;
   inferenceServices: FetchStateObject<InferenceServiceKind[]>;
   servingRuntimes: FetchStateObject<ServingRuntimeKind[]>;
+  isArchiveVersion?: boolean;
   refresh: () => void;
 };
 
@@ -22,6 +23,7 @@ const ModelVersionDetailsTabs: React.FC<ModelVersionDetailTabsProps> = ({
   modelVersion: mv,
   inferenceServices,
   servingRuntimes,
+  isArchiveVersion,
   refresh,
 }) => {
   const navigate = useNavigate();
@@ -40,7 +42,11 @@ const ModelVersionDetailsTabs: React.FC<ModelVersionDetailTabsProps> = ({
         data-testid="model-versions-details-tab"
       >
         <PageSection isFilled variant="light" data-testid="model-versions-details-tab-content">
-          <ModelVersionDetailsView modelVersion={mv} refresh={refresh} />
+          <ModelVersionDetailsView
+            modelVersion={mv}
+            refresh={refresh}
+            isArchiveVersion={isArchiveVersion}
+          />
         </PageSection>
       </Tab>
       <Tab