Skip to content

Commit

Permalink
Upadte archived models and archived versions components
Browse files Browse the repository at this point in the history
  • Loading branch information
@ppadti
ppadti committed Sep 22, 2024
1 parent d0b5ce9 commit 5cf7e0d
Show file tree
Hide file tree
Showing 23 changed files with 631 additions and 217 deletions.
4 changes: 4 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ class ModelRegistry {
cy.findByTestId('empty-model-versions').should('exist');
}

shouldArchveModelVersionsEmpty() {
cy.findByTestId('empty-archive-model-versions').should('exist');
}

shouldModelRegistrySelectorExist() {
cy.findByTestId('model-registry-selector-dropdown').should('exist');
}
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
67 changes: 60 additions & 7 deletions ...end/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const initIntercepts = ({
mockRegisteredModel({ id: '4', name: 'model 4' }),
],
modelVersions = [
mockModelVersion({ author: 'Author 1' }),
mockModelVersion({ author: 'Author 1', registeredModelId: '2' }),
mockModelVersion({ name: 'model version' }),
],
}: HandlersProps) => {
Expand All @@ -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 }),
Expand All @@ -97,6 +109,18 @@ const initIntercepts = ({
},
mockRegisteredModel({ id: '2', name: 'model 2', state: ModelState.ARCHIVED }),
);

cy.interceptOdh(
'GET /api/service/modelregistry/:serviceName/api/model_registry/:apiVersion/registered_models/:registeredModelId',
{
path: {
serviceName: 'modelregistry-sample',
apiVersion: MODEL_REGISTRY_API_VERSION,
registeredModelId: 3,
},
},
mockRegisteredModel({ id: '3', name: 'model 3' }),
);
};

describe('Model archive list', () => {
Expand All @@ -123,6 +147,32 @@ describe('Model archive list', () => {
registeredModelArchive.findArchiveModelTable().should('be.visible');
});

it('Archived model with no versions', () => {
initIntercepts({ modelVersions: [] });
registeredModelArchive.visit();
verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/archive');
registeredModelArchive.findArchiveModelBreadcrumbItem().contains('Archived models');
const archiveModelRow = registeredModelArchive.getRow('model 2');
archiveModelRow.findName().contains('model 2').click();
modelRegistry.shouldArchveModelVersionsEmpty();
});

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();
Expand Down Expand Up @@ -180,7 +230,7 @@ it('Opens the detail page when we select "View Details" from action menu', () =>
archiveModelRow.findKebabAction('View details').click();
cy.location('pathname').should(
'be.equals',
'/modelRegistry/modelregistry-sample/registeredModels/2/details',
'/modelRegistry/modelregistry-sample/registeredModels/archive/2/details',
);
});

Expand Down Expand Up @@ -276,21 +326,24 @@ describe('Archiving model', () => {
path: {
serviceName: 'modelregistry-sample',
apiVersion: MODEL_REGISTRY_API_VERSION,
registeredModelId: 2,
registeredModelId: 3,
},
},
mockRegisteredModel({ id: '2', name: 'model 2', state: ModelState.ARCHIVED }),
mockRegisteredModel({ id: '3', name: 'model 3', state: ModelState.ARCHIVED }),
).as('modelArchived');

initIntercepts({});
registeredModelArchive.visitModelDetails();
registeredModelArchive.visitModelList();

const modelRow = modelRegistry.getRow('model 3');
modelRow.findName().contains('model 3').click();
registeredModelArchive
.findModelVersionsDetailsHeaderAction()
.findDropdownItem('Archive model')
.click();

archiveModelModal.findArchiveButton().should('be.disabled');
archiveModelModal.findModalTextInput().fill('model 2');
archiveModelModal.findModalTextInput().fill('model 3');
archiveModelModal.findArchiveButton().should('be.enabled').click();
cy.wait('@modelArchived').then((interception) => {
expect(interception.request.body).to.eql({
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/EditableLabelsDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ type EditableTextDescriptionListGroupProps = Partial<
labels: string[];
saveEditedLabels: (labels: string[]) => Promise<unknown>;
allExistingKeys?: string[];
isArchive?: boolean;
};

const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGroupProps> = ({
title = 'Labels',
contentWhenEmpty = 'No labels',
labels,
saveEditedLabels,
isArchive,
allExistingKeys = labels,
}) => {
const [isEditing, setIsEditing] = React.useState(false);
Expand Down Expand Up @@ -98,7 +100,7 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
title={title}
isEmpty={labels.length === 0}
contentWhenEmpty={contentWhenEmpty}
isEditable
isEditable={!isArchive}
isEditing={isEditing}
isSavingEdits={isSavingEdits}
contentWhenEditing={
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/EditableTextDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}) => {
Expand All @@ -29,7 +31,7 @@ const EditableTextDescriptionListGroup: React.FC<EditableTextDescriptionListGrou
title={title}
isEmpty={!value}
contentWhenEmpty={contentWhenEmpty}
isEditable
isEditable={!isArchive}
isEditing={isEditing}
isSavingEdits={isSavingEdits}
contentWhenEditing={
Expand Down
20 changes: 20 additions & 0 deletions frontend/src/pages/modelRegistry/ModelRegistryRoutes.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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="." />} />
Expand Down
27 changes: 16 additions & 11 deletions frontend/src/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[]>([]);
Expand Down Expand Up @@ -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"
Expand All @@ -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)}
Expand Down
78 changes: 41 additions & 37 deletions frontend/src/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand All @@ -38,6 +39,7 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({
setIsEditing,
isSavingEdits,
setIsSavingEdits,
isArchive,
saveEditedProperty,
}) => {
const { key, value } = keyValuePair;
Expand Down Expand Up @@ -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>
);
};
Expand Down
Loading

0 comments on commit 5cf7e0d

Please sign in to comment.