diff --git a/clients/ui/frontend/package-lock.json b/clients/ui/frontend/package-lock.json index e36704e1..3975f834 100644 --- a/clients/ui/frontend/package-lock.json +++ b/clients/ui/frontend/package-lock.json @@ -19,6 +19,7 @@ "npm-run-all": "^4.1.5", "react": "^18", "react-dom": "^18", + "react-router": "^6.26.2", "sass": "^1.78.0", "showdown": "^2.1.0" }, @@ -3686,7 +3687,6 @@ "version": "1.19.2", "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.19.2.tgz", "integrity": "sha512-baiMx18+IMuD1yyvOGaHM9QrVUPGGG0jC+z+IPHnRJWUAUvaKuWKyE8gjDj2rzv3sz9zOGoRSPgeBVHRhZnBlA==", - "dev": true, "license": "MIT", "engines": { "node": ">=14.0.0" @@ -18631,8 +18631,6 @@ "version": "6.26.2", "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.26.2.tgz", "integrity": "sha512-tvN1iuT03kHgOFnLPfLJ8V95eijteveqdOSk+srqfePtQvqCExB8eHOYnlilbOcyJyKnYkr1vJvf7YqotAJu1A==", - "dev": true, - "license": "MIT", "dependencies": { "@remix-run/router": "1.19.2" }, diff --git a/clients/ui/frontend/package.json b/clients/ui/frontend/package.json index eb33f8e3..8748f1b6 100644 --- a/clients/ui/frontend/package.json +++ b/clients/ui/frontend/package.json @@ -24,7 +24,7 @@ "test:fix": "eslint --ext .js,.ts,.jsx,.tsx ./src --fix", "test:lint": "eslint --max-warnings 0 --ext .js,.ts,.jsx,.tsx ./src", "cypress:open": "cypress open --project src/__tests__/cypress", - "cypress:open:mock": "CY_MOCK=1 CY_WS_PORT=9002 npm run cypress:open -- ", + "cypress:open:mock": "CY_MOCK=1 npm run cypress:open -- ", "cypress:run": "cypress run -b chrome --project src/__tests__/cypress", "cypress:run:mock": "CY_MOCK=1 npm run cypress:run -- ", "cypress:server:build": "POLL_INTERVAL=9999999 FAST_POLL_INTERVAL=9999999 npm run build", @@ -40,12 +40,12 @@ "@testing-library/jest-dom": "^6.5.0", "@testing-library/react": "^16.0.0", "@testing-library/user-event": "14.5.2", - "@types/jest": "^29.5.12", - "@types/react-router-dom": "^5.3.3", "@types/classnames": "^2.3.1", "@types/dompurify": "^3.0.5", - "@types/showdown": "^2.0.3", + "@types/jest": "^29.5.12", "@types/lodash-es": "^4.17.8", + "@types/react-router-dom": "^5.3.3", + "@types/showdown": "^2.0.3", "chai-subset": "^1.6.0", "copy-webpack-plugin": "^12.0.2", "core-js": "^3.37.1", @@ -98,6 +98,7 @@ "npm-run-all": "^4.1.5", "react": "^18", "react-dom": "^18", + "react-router": "^6.26.2", "sass": "^1.78.0", "dompurify": "^3.1.6", "showdown": "^2.1.0", diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts index 730064da..edf7c007 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts @@ -58,7 +58,7 @@ declare global { options: { path: { modelRegistryName: string; apiVersion: string; registeredModelId: number }; }, - response: ApiResponse, + response: ApiResponse>, ) => Cypress.Chainable) & (( type: 'PATCH /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId', diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts new file mode 100644 index 00000000..31a7ab37 --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts @@ -0,0 +1,222 @@ +/* eslint-disable camelcase */ +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; +import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; +import { be } from '~/__tests__/cypress/cypress/utils/should'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import type { ModelRegistry, ModelVersion } from '~/app/types'; +import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; +import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockBFFResponse } from '~/__mocks__/utils'; + +const MODEL_REGISTRY_API_VERSION = 'v1'; + +type HandlersProps = { + registeredModelsSize?: number; + modelVersions?: ModelVersion[]; + modelRegistries?: ModelRegistry[]; +}; + +const initIntercepts = ({ + registeredModelsSize = 4, + modelRegistries = [ + mockModelRegistry({ + name: 'modelregistry-sample', + description: 'New model registry', + displayName: 'Model Registry Sample', + }), + mockModelRegistry({ + name: 'modelregistry-sample-2', + description: 'New model registry 2', + displayName: 'Model Registry Sample 2', + }), + ], + modelVersions = [ + mockModelVersion({ + author: 'Author 1', + id: '1', + labels: [ + 'Financial data', + 'Fraud detection', + 'Test label', + 'Machine learning', + 'Next data to be overflow', + 'Test label x', + 'Test label y', + 'Test label z', + ], + }), + mockModelVersion({ id: '2', name: 'model version' }), + ], +}: HandlersProps) => { + cy.interceptApi( + `GET /api/:apiVersion/model_registry`, + { + path: { apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockBFFResponse(modelRegistries), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models`, + { + path: { modelRegistryName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockBFFResponse(mockRegisteredModelList({ size: registeredModelsSize })), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + registeredModelId: 1, + }, + }, + mockBFFResponse(mockModelVersionList({ items: modelVersions })), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + registeredModelId: 1, + }, + }, + mockBFFResponse(mockRegisteredModel({})), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 1, + }, + }, + mockModelVersion({ id: '1', name: 'model version' }), + ); +}; + +describe('Model Versions', () => { + it('No model versions in the selected registered model', () => { + initIntercepts({ + modelVersions: [], + }); + + modelRegistry.visit(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + modelRegistry.shouldmodelVersionsEmpty(); + }); + + it('Model versions table browser back button should lead to Registered models table', () => { + initIntercepts({ + modelVersions: [], + }); + + modelRegistry.visit(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + cy.go('back'); + verifyRelativeURL(`/modelRegistry/modelregistry-sample`); + registeredModelRow.findName().contains('Fraud detection model').should('exist'); + }); + + it('Model versions table', () => { + // TODO: Uncomment when we fix finding dropdown items + + initIntercepts({ + modelRegistries: [ + // mockModelRegistry({ name: 'modelRegistry-1', displayName: 'modelRegistry-1' }), + mockModelRegistry({}), + ], + }); + + modelRegistry.visit(); + //modelRegistry.findModelRegistry().findSelectOption('Model Registry Sample').click(); + //cy.reload(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + modelRegistry.findModelBreadcrumbItem().contains('test'); + //modelRegistry.findModelVersionsTableKebab().findDropdownItem('View archived versions'); + //modelRegistry.findModelVersionsHeaderAction().findDropdownItem('Archive model'); + modelRegistry.findModelVersionsTable().should('be.visible'); + modelRegistry.findModelVersionsTableRows().should('have.length', 2); + + // Label modal + const modelVersionRow = modelRegistry.getModelVersionRow('new model version'); + + modelVersionRow.findLabelModalText().contains('5 more'); + modelVersionRow.findLabelModalText().click(); + labelModal.shouldContainsModalLabels([ + 'Financial', + 'Financial data', + 'Fraud detection', + 'Test label', + 'Machine learning', + 'Next data to be overflow', + 'Test label x', + 'Test label y', + 'Test label y', + ]); + labelModal.findModalSearchInput().type('Financial'); + labelModal.shouldContainsModalLabels(['Financial', 'Financial data']); + labelModal.findCloseModal().click(); + + // sort by model version name + modelRegistry.findModelVersionsTableHeaderButton('Version name').click(); + modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortAscending); + modelRegistry.findModelVersionsTableHeaderButton('Version name').click(); + modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortDescending); + + // sort by Last modified + modelRegistry.findModelVersionsTableHeaderButton('Last modified').click(); + modelRegistry.findModelVersionsTableHeaderButton('Last modified').should(be.sortAscending); + modelRegistry.findModelVersionsTableHeaderButton('Last modified').click(); + modelRegistry.findModelVersionsTableHeaderButton('Last modified').should(be.sortDescending); + + // sort by model version author + modelRegistry.findModelVersionsTableHeaderButton('Author').click(); + modelRegistry.findModelVersionsTableHeaderButton('Author').should(be.sortAscending); + modelRegistry.findModelVersionsTableHeaderButton('Author').click(); + modelRegistry.findModelVersionsTableHeaderButton('Author').should(be.sortDescending); + + // filtering by keyword + modelRegistry.findModelVersionsTableSearch().type('new model version'); + modelRegistry.findModelVersionsTableRows().should('have.length', 1); + modelRegistry.findModelVersionsTableRows().contains('new model version'); + modelRegistry.findModelVersionsTableSearch().focused().clear(); + + // filtering by model version author + modelRegistry.findModelVersionsTableFilter().findSelectOption('Author').click(); + modelRegistry.findModelVersionsTableSearch().type('Test author'); + modelRegistry.findModelVersionsTableRows().should('have.length', 1); + modelRegistry.findModelVersionsTableRows().contains('Test author'); + }); + + it('Model version details back button should lead to versions table', () => { + initIntercepts({}); + + modelRegistry.visit(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + // TODO: Uncomment when we have model version details + // const modelVersionRow = modelRegistry.getModelVersionRow('model version'); + // modelVersionRow.findModelVersionName().contains('model version').click(); + // verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/1/versions/1/details'); + // cy.findByTestId('app-page-title').should('have.text', 'model version'); + // cy.findByTestId('breadcrumb-version-name').should('have.text', 'model version'); + // cy.go('back'); + // verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/1/versions'); + }); +}); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/utils/url.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/utils/url.ts new file mode 100644 index 00000000..b976a070 --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/utils/url.ts @@ -0,0 +1,12 @@ +/** + * Verify the relative route to the cypress host + * e.g. If page is running on `https://localhost:9001/pipelines` + * calling verifyRelativeURL('/pipelines') will check whether the full URL matches the URL above + */ +export const verifyRelativeURL = (relativeURL: string): Cypress.Chainable => { + return cy + .location() + .then((location) => + cy.url().should('eq', `${location.protocol}//${location.host}${relativeURL}`), + ); +}; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx index 6e318440..c7b78d10 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx @@ -4,6 +4,8 @@ import ModelRegistry from './screens/ModelRegistry'; import ModelRegistryCoreLoader from './ModelRegistryCoreLoader'; import { modelRegistryUrl } from './screens/routeUtils'; import RegisteredModelsArchive from './screens/RegisteredModelsArchive/RegisteredModelsArchive'; +import { ModelVersionsTab } from './screens/ModelVersions/const'; +import ModelVersions from './screens/ModelVersions/ModelVersions'; const ModelRegistryRoutes: React.FC = () => ( @@ -16,6 +18,18 @@ const ModelRegistryRoutes: React.FC = () => ( } > } /> + + } /> + } + /> + } + /> + } /> + } /> } /> diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx new file mode 100644 index 00000000..aeb98464 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx @@ -0,0 +1,129 @@ +import * as React from 'react'; +import { Button } from '@patternfly/react-core'; +import { Table, Tbody, Th, Thead, Tr } from '@patternfly/react-table'; +import { PlusCircleIcon } from '@patternfly/react-icons'; +import text from '@patternfly/react-styles/css/utilities/Text/text'; +import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; +import DashboardDescriptionListGroup from '~/components/DashboardDescriptionListGroup'; +import { getProperties, mergeUpdatedProperty } from '~/app/pages/modelRegistry/screens/utils'; +import { ModelRegistryCustomProperties } from '~/app/types'; +import ModelPropertiesTableRow from '~/app/pages/modelRegistry/screens/ModelPropertiesTableRow'; + +type ModelPropertiesDescriptionListGroupProps = { + customProperties: ModelRegistryCustomProperties; + saveEditedCustomProperties: (properties: ModelRegistryCustomProperties) => Promise; +}; + +const ModelPropertiesDescriptionListGroup: React.FC = ({ + customProperties = {}, + saveEditedCustomProperties, +}) => { + const [editingPropertyKeys, setEditingPropertyKeys] = React.useState([]); + const setIsEditingKey = (key: string, isEditing: boolean) => + setEditingPropertyKeys([ + ...editingPropertyKeys.filter((k) => k !== key), + ...(isEditing ? [key] : []), + ]); + const [isAdding, setIsAdding] = React.useState(false); + const isEditingSomeRow = isAdding || editingPropertyKeys.length > 0; + + const [isSavingEdits, setIsSavingEdits] = React.useState(false); + + // We only show string properties with a defined value (no labels or other property types) + const filteredProperties = getProperties(customProperties); + + const [isShowingMoreProperties, setIsShowingMoreProperties] = React.useState(false); + const keys = Object.keys(filteredProperties); + const needExpandControl = keys.length > 5; + const shownKeys = isShowingMoreProperties ? keys : keys.slice(0, 5); + const numHiddenKeys = keys.length - shownKeys.length; + + // Includes keys reserved by non-string properties and labels + const allExistingKeys = Object.keys(customProperties); + + const requiredAsterisk = ( + + ); + + return ( + } + iconPosition="start" + isDisabled={isAdding || isSavingEdits} + onClick={() => setIsAdding(true)} + > + Add property + + } + isEmpty={!isAdding && keys.length === 0} + contentWhenEmpty="No properties" + > + + + + + + + + + {shownKeys.map((key) => ( + setIsEditingKey(key, isEditing)} + isSavingEdits={isSavingEdits} + setIsSavingEdits={setIsSavingEdits} + saveEditedProperty={(oldKey, newPair) => + saveEditedCustomProperties( + mergeUpdatedProperty({ customProperties, op: 'update', oldKey, newPair }), + ) + } + deleteProperty={(oldKey) => + saveEditedCustomProperties( + mergeUpdatedProperty({ customProperties, op: 'delete', oldKey }), + ) + } + /> + ))} + {isAdding && ( + + saveEditedCustomProperties( + mergeUpdatedProperty({ customProperties, op: 'create', newPair }), + ) + } + /> + )} + +
Key {isEditingSomeRow && requiredAsterisk}Value {isEditingSomeRow && requiredAsterisk} +
+ {needExpandControl && ( + + )} + + ); +}; + +export default ModelPropertiesDescriptionListGroup; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx new file mode 100644 index 00000000..c3d75a2c --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx @@ -0,0 +1,187 @@ +import * as React from 'react'; +import { ActionsColumn, Td, Tr } from '@patternfly/react-table'; +import { + ActionList, + ActionListItem, + Button, + ExpandableSection, + FormHelperText, + HelperText, + HelperTextItem, + TextInput, +} from '@patternfly/react-core'; +import { CheckIcon, TimesIcon } from '@patternfly/react-icons'; +import { KeyValuePair } from '~/types'; +import { EitherNotBoth } from '~/typeHelpers'; + +type ModelPropertiesTableRowProps = { + allExistingKeys: string[]; + setIsEditing: (isEditing: boolean) => void; + isSavingEdits: boolean; + setIsSavingEdits: (isSaving: boolean) => void; + saveEditedProperty: (oldKey: string, newPair: KeyValuePair) => Promise; +} & EitherNotBoth< + { isAddRow: true }, + { + isEditing: boolean; + keyValuePair: KeyValuePair; + deleteProperty: (key: string) => Promise; + } +>; + +const ModelPropertiesTableRow: React.FC = ({ + isAddRow, + isEditing = isAddRow, + keyValuePair = { key: '', value: '' }, + deleteProperty = () => Promise.resolve(), + allExistingKeys, + setIsEditing, + isSavingEdits, + setIsSavingEdits, + saveEditedProperty, +}) => { + const { key, value } = keyValuePair; + const [unsavedKey, setUnsavedKey] = React.useState(key); + const [unsavedValue, setUnsavedValue] = React.useState(value); + + const [isValueExpanded, setIsValueExpanded] = React.useState(false); + + let keyValidationError: string | null = null; + if (unsavedKey !== key && allExistingKeys.includes(unsavedKey)) { + keyValidationError = 'Key must not match an existing property key or label'; + } else if (unsavedKey.length > 63) { + keyValidationError = "Key text can't exceed 63 characters"; + } + + const clearUnsavedInputs = () => { + setUnsavedKey(key); + setUnsavedValue(value); + }; + + const onEditClick = () => { + clearUnsavedInputs(); + setIsEditing(true); + }; + + const onDeleteClick = async () => { + setIsSavingEdits(true); + try { + await deleteProperty(key); + } finally { + setIsSavingEdits(false); + } + }; + + const onSaveEditsClick = async () => { + setIsSavingEdits(true); + try { + await saveEditedProperty(key, { key: unsavedKey, value: unsavedValue }); + } finally { + setIsSavingEdits(false); + } + setIsEditing(false); + }; + + const onDiscardEditsClick = () => { + clearUnsavedInputs(); + setIsEditing(false); + }; + + return ( + + + {isEditing ? ( + <> + setUnsavedKey(str)} + validated={keyValidationError ? 'error' : 'default'} + /> + {keyValidationError && ( + + + {keyValidationError} + + + )} + + ) : ( + key + )} + + + {isEditing ? ( + setUnsavedValue(str)} + /> + ) : ( + setIsValueExpanded(isExpanded)} + isExpanded={isValueExpanded} + > + {value} + + )} + + + {isEditing ? ( + + + + + + + + + ) : ( + + )} + + + ); +}; + +export default ModelPropertiesTableRow; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx new file mode 100644 index 00000000..ec145465 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx @@ -0,0 +1,106 @@ +import * as React from 'react'; +import { ClipboardCopy, DescriptionList, Flex, FlexItem, Content } from '@patternfly/react-core'; +import { RegisteredModel } from '~/app/types'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; +import EditableTextDescriptionListGroup from '~/components/EditableTextDescriptionListGroup'; +import EditableLabelsDescriptionListGroup from '~/components/EditableLabelsDescriptionListGroup'; +import { getLabels, mergeUpdatedLabels } from '~/app/pages/modelRegistry/screens/utils'; +import ModelPropertiesDescriptionListGroup from '~/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup'; +import DashboardDescriptionListGroup from '~/components/DashboardDescriptionListGroup'; +import ModelTimestamp from '~/app/pages/modelRegistry/screens/components/ModelTimestamp'; + +type ModelDetailsViewProps = { + registeredModel: RegisteredModel; + refresh: () => void; +}; + +const ModelDetailsView: React.FC = ({ registeredModel: rm, refresh }) => { + const { apiState } = React.useContext(ModelRegistryContext); + return ( + + + + + apiState.api + .patchRegisteredModel( + {}, + { + description: value, + }, + rm.id, + ) + .then(refresh) + } + /> + + apiState.api + .patchRegisteredModel( + {}, + { + customProperties: mergeUpdatedLabels(rm.customProperties, editedLabels), + }, + rm.id, + ) + .then(refresh) + } + /> + + apiState.api + .patchRegisteredModel( + {}, + { + customProperties: editedProperties, + }, + rm.id, + ) + .then(refresh) + } + /> + + + + + + + {rm.id} + + + + + {rm.owner || '-'} + + + + + + + + + + + + ); +}; + +export default ModelDetailsView; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx new file mode 100644 index 00000000..0f9db46c --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx @@ -0,0 +1,176 @@ +import * as React from 'react'; +import { + Button, + Dropdown, + DropdownItem, + DropdownList, + MenuToggle, + MenuToggleElement, + SearchInput, + ToolbarContent, + ToolbarFilter, + ToolbarGroup, + ToolbarItem, + ToolbarToggleGroup, +} from '@patternfly/react-core'; +import { EllipsisVIcon, FilterIcon } from '@patternfly/react-icons'; +import { useNavigate } from 'react-router'; +import { ModelVersion, RegisteredModel } from '~/app/types'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { SearchType } from '~/app/components/DashboardSearchField'; +import { + filterModelVersions, + sortModelVersionsByCreateTime, +} from '~/app/pages/modelRegistry/screens/utils'; +import EmptyModelRegistryState from '~/app/pages/modelRegistry/screens/components/EmptyModelRegistryState'; +import { ProjectObjectType, typedEmptyImage } from '~/app/components/design/utils'; +import { + modelVersionArchiveUrl, + registerVersionForModelUrl, +} from '~/app/pages/modelRegistry/screens/routeUtils'; +import { asEnumMember } from '~/app/utils'; +import ModelVersionsTable from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable'; +import SimpleSelect from '~/app/components/SimpleSelect'; + +type ModelVersionListViewProps = { + modelVersions: ModelVersion[]; + registeredModel?: RegisteredModel; + refresh: () => void; +}; + +const ModelVersionListView: React.FC = ({ + modelVersions: unfilteredModelVersions, + registeredModel: rm, + refresh, +}) => { + const navigate = useNavigate(); + const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); + + const [searchType, setSearchType] = React.useState(SearchType.KEYWORD); + const [search, setSearch] = React.useState(''); + + const searchTypes = [SearchType.KEYWORD, SearchType.AUTHOR]; + + const [isArchivedModelVersionKebabOpen, setIsArchivedModelVersionKebabOpen] = + React.useState(false); + + const filteredModelVersions = filterModelVersions(unfilteredModelVersions, search, searchType); + + if (unfilteredModelVersions.length === 0) { + return ( + ( + missing version + )} + description={`${rm?.name} has no registered versions. Register a version to this model.`} + primaryActionText="Register new version" + secondaryActionText="View archived versions" + primaryActionOnClick={() => { + navigate(registerVersionForModelUrl(rm?.id, preferredModelRegistry?.name)); + }} + secondaryActionOnClick={() => { + navigate(modelVersionArchiveUrl(rm?.id, preferredModelRegistry?.name)); + }} + /> + ); + } + + return ( + setSearch('')} + modelVersions={sortModelVersionsByCreateTime(filteredModelVersions)} + toolbarContent={ + + } breakpoint="xl"> + + setSearch('')} + deleteLabelGroup={() => setSearch('')} + categoryName={searchType} + > + ({ + key, + label: key, + }))} + value={searchType} + onChange={(newSearchType) => { + const enumMember = asEnumMember(newSearchType, SearchType); + if (enumMember !== null) { + setSearchType(enumMember); + } + }} + icon={} + /> + + + { + setSearch(searchValue); + }} + onClear={() => setSearch('')} + style={{ minWidth: '200px' }} + data-testid="model-versions-table-search" + /> + + + + + + + + setIsArchivedModelVersionKebabOpen(false)} + onOpenChange={(isOpen: boolean) => setIsArchivedModelVersionKebabOpen(isOpen)} + toggle={(tr: React.Ref) => ( + + setIsArchivedModelVersionKebabOpen(!isArchivedModelVersionKebabOpen) + } + isExpanded={isArchivedModelVersionKebabOpen} + aria-label="View archived versions" + > + + + )} + shouldFocusToggleOnSelect + > + + + navigate(modelVersionArchiveUrl(rm?.id, preferredModelRegistry?.name)) + } + > + View archived versions + + + + + + } + /> + ); +}; + +export default ModelVersionListView; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx new file mode 100644 index 00000000..9e471aca --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersions.tsx @@ -0,0 +1,64 @@ +import React from 'react'; +import { useParams } from 'react-router'; +import { Breadcrumb, BreadcrumbItem, Truncate } from '@patternfly/react-core'; +import { Link } from 'react-router-dom'; +import { ModelVersionsTab } from '~/app/pages/modelRegistry/screens/ModelVersions/const'; +import ApplicationsPage from '~/app/components/ApplicationsPage'; +import useModelVersionsByRegisteredModel from '~/app/hooks/useModelVersionsByRegisteredModel'; +import useRegisteredModelById from '~/app/hooks/useRegisteredModelById'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { filterLiveVersions } from '~/app/pages/modelRegistry/screens/utils'; +import ModelVersionsHeaderActions from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions'; +import ModelVersionsTabs from './ModelVersionsTabs'; + +type ModelVersionsProps = { + tab: ModelVersionsTab; +} & Omit< + React.ComponentProps, + 'breadcrumb' | 'title' | 'description' | 'loadError' | 'loaded' | 'provideChildrenPadding' +>; + +const ModelVersions: React.FC = ({ tab, ...pageProps }) => { + const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); + const { registeredModelId: rmId } = useParams(); + const [modelVersions, mvLoaded, mvLoadError, mvRefresh] = useModelVersionsByRegisteredModel(rmId); + const [rm, rmLoaded, rmLoadError, rmRefresh] = useRegisteredModelById(rmId); + const loadError = mvLoadError || rmLoadError; + const loaded = mvLoaded && rmLoaded; + + return ( + + ( + Model registry - {preferredModelRegistry?.name} + )} + /> + + {rm?.name || 'Loading...'} + + + } + title={rm?.name} + headerAction={rm && } + description={} + loadError={loadError} + loaded={loaded} + provideChildrenPadding + > + {rm !== null && ( + + )} + + ); +}; + +export default ModelVersions; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx new file mode 100644 index 00000000..1a383430 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsHeaderActions.tsx @@ -0,0 +1,86 @@ +import * as React from 'react'; +import { + Dropdown, + DropdownList, + MenuToggle, + DropdownItem, + Flex, + FlexItem, +} from '@patternfly/react-core'; +import { useNavigate } from 'react-router'; +import { ModelState, RegisteredModel } from '~/app/types'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { ArchiveRegisteredModelModal } from '~/app/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal'; +import { registeredModelsUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; + +interface ModelVersionsHeaderActionsProps { + rm: RegisteredModel; +} + +const ModelVersionsHeaderActions: React.FC = ({ rm }) => { + const { apiState } = React.useContext(ModelRegistryContext); + const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); + + const navigate = useNavigate(); + const [isOpen, setOpen] = React.useState(false); + const tooltipRef = React.useRef(null); + const [isArchiveModalOpen, setIsArchiveModalOpen] = React.useState(false); + + return ( + <> + + + setOpen(false)} + onOpenChange={(open) => setOpen(open)} + popperProps={{ position: 'end' }} + toggle={(toggleRef) => ( + setOpen(!isOpen)} + isExpanded={isOpen} + aria-label="Model version action toggle" + data-testid="model-version-action-toggle" + > + Actions + + )} + > + + setIsArchiveModalOpen(true)} + ref={tooltipRef} + > + Archive model + + + + + + setIsArchiveModalOpen(false)} + onSubmit={() => + apiState.api + .patchRegisteredModel( + {}, + { + state: ModelState.ARCHIVED, + }, + rm.id, + ) + .then(() => navigate(registeredModelsUrl(preferredModelRegistry?.name))) + } + isOpen={isArchiveModalOpen} + registeredModelName={rm.name} + /> + + ); +}; + +export default ModelVersionsHeaderActions; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx new file mode 100644 index 00000000..999dc2aa --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx @@ -0,0 +1,34 @@ +import * as React from 'react'; +import { Table } from '~/app/components/table'; +import { ModelVersion } from '~/app/types'; +import { mvColumns } from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableColumns'; +import DashboardEmptyTableView from '~/app/components/DashboardEmptyTableView'; +import ModelVersionsTableRow from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow'; + +type ModelVersionsTableProps = { + clearFilters: () => void; + modelVersions: ModelVersion[]; + refresh: () => void; +} & Partial, 'toolbarContent'>>; + +const ModelVersionsTable: React.FC = ({ + clearFilters, + modelVersions, + toolbarContent, + refresh, +}) => ( + } + rowRenderer={(mv) => ( + + )} + /> +); + +export default ModelVersionsTable; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableColumns.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableColumns.ts new file mode 100644 index 00000000..f98ea912 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableColumns.ts @@ -0,0 +1,40 @@ +import { SortableData } from '~/app/components/table'; +import { ModelVersion } from '~/app/types'; + +export const mvColumns: SortableData[] = [ + { + field: 'version name', + label: 'Version name', + sortable: (a, b) => a.name.localeCompare(b.name), + width: 40, + }, + { + field: 'last_modified', + label: 'Last modified', + sortable: (a: ModelVersion, b: ModelVersion): number => { + const first = parseInt(a.lastUpdateTimeSinceEpoch); + const second = parseInt(b.lastUpdateTimeSinceEpoch); + return new Date(second).getTime() - new Date(first).getTime(); + }, + }, + { + field: 'author', + label: 'Author', + sortable: (a: ModelVersion, b: ModelVersion): number => { + const first = a.author || ''; + const second = b.author || ''; + return first.localeCompare(second); + }, + }, + { + field: 'labels', + label: 'Labels', + sortable: false, + width: 35, + }, + { + field: 'kebab', + label: '', + sortable: false, + }, +]; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx new file mode 100644 index 00000000..55a4d85b --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTableRow.tsx @@ -0,0 +1,128 @@ +import * as React from 'react'; +import { ActionsColumn, Td, Tr } from '@patternfly/react-table'; +import { Content, ContentVariants, Truncate, FlexItem } from '@patternfly/react-core'; +import { Link, useNavigate } from 'react-router-dom'; +import { ModelState, ModelVersion } from '~/app/types'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; +import { + modelVersionArchiveDetailsUrl, + modelVersionUrl, +} from '~/app/pages/modelRegistry/screens/routeUtils'; +import ModelTimestamp from '~/app/pages/modelRegistry/screens/components/ModelTimestamp'; +import ModelLabels from '~/app/pages/modelRegistry/screens/components/ModelLabels'; +import { ArchiveModelVersionModal } from '~/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal'; +import { RestoreModelVersionModal } from '~/app/pages/modelRegistry/screens/components/RestoreModelVersionModal'; + +type ModelVersionsTableRowProps = { + modelVersion: ModelVersion; + isArchiveRow?: boolean; + refresh: () => void; +}; + +const ModelVersionsTableRow: React.FC = ({ + modelVersion: mv, + isArchiveRow, + refresh, +}) => { + const navigate = useNavigate(); + const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); + const [isArchiveModalOpen, setIsArchiveModalOpen] = React.useState(false); + const [isRestoreModalOpen, setIsRestoreModalOpen] = React.useState(false); + const { apiState } = React.useContext(ModelRegistryContext); + + const actions = isArchiveRow + ? [ + { + title: 'Restore version', + onClick: () => setIsRestoreModalOpen(true), + }, + ] + : [ + { + title: 'Deploy', + onClick: () => setIsDeployModalOpen(true), + }, + { + title: 'Archive model version', + onClick: () => setIsArchiveModalOpen(true), + }, + ]; + + return ( + + + + + + + + ); +}; + +export default ModelVersionsTableRow; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTabs.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTabs.tsx new file mode 100644 index 00000000..7460e663 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTabs.tsx @@ -0,0 +1,63 @@ +import * as React from 'react'; +import { useNavigate } from 'react-router-dom'; +import { PageSection, Tab, Tabs, TabTitleText } from '@patternfly/react-core'; +import ModelDetailsView from '~/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView'; +import { ModelVersion, RegisteredModel } from '~/app/types'; +import { + ModelVersionsTab, + ModelVersionsTabTitle, +} from '~/app/pages/modelRegistry/screens/ModelVersions/const'; +import ModelVersionListView from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView'; + +type ModelVersionsTabProps = { + tab: ModelVersionsTab; + registeredModel: RegisteredModel; + modelVersions: ModelVersion[]; + refresh: () => void; + mvRefresh: () => void; +}; + +const ModelVersionsTabs: React.FC = ({ + tab, + registeredModel: rm, + modelVersions, + refresh, + mvRefresh, +}) => { + const navigate = useNavigate(); + return ( + navigate(`../${eventKey}`, { relative: 'path' })} + > + {ModelVersionsTabTitle.VERSIONS}} + aria-label="Model versions tab" + data-testid="model-versions-tab" + > + + + + + {ModelVersionsTabTitle.DETAILS}} + aria-label="Model Details tab" + data-testid="model-details-tab" + > + + + + + + ); +}; +export default ModelVersionsTabs; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/const.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/const.ts new file mode 100644 index 00000000..42133ed9 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/const.ts @@ -0,0 +1,9 @@ +export enum ModelVersionsTab { + VERSIONS = 'versions', + DETAILS = 'details', +} + +export enum ModelVersionsTabTitle { + VERSIONS = 'Versions', + DETAILS = 'Details', +} diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx index 881e0033..07e0c722 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTableRow.tsx @@ -13,6 +13,7 @@ import { registeredModelArchiveDetailsUrl, registeredModelUrl, } from '~/app/pages/modelRegistry/screens/routeUtils'; +import { ModelVersionsTab } from '~/app/pages/modelRegistry/screens/ModelVersions/const'; type RegisteredModelTableRowProps = { registeredModel: RegisteredModel; @@ -36,7 +37,7 @@ const RegisteredModelTableRow: React.FC = ({ { title: 'View details', // eslint-disable-next-line @typescript-eslint/no-empty-function - onClick: () => {}, // TODO: @Griffin-Sullivan uncomment this once model versions is active ---> navigate(`${rmUrl}/${ModelVersionsTab.DETAILS}`), + onClick: () => navigate(`${rmUrl}/${ModelVersionsTab.DETAILS}`), }, isArchiveRow ? { diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/__tests__/utils.spec.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/__tests__/utils.spec.ts new file mode 100644 index 00000000..d4a102c0 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/__tests__/utils.spec.ts @@ -0,0 +1,348 @@ +/* eslint-disable camelcase */ +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { + ModelRegistryCustomProperties, + ModelRegistryMetadataType, + ModelRegistryStringCustomProperties, + ModelState, + ModelVersion, + RegisteredModel, +} from '~/app/types'; +import { + filterModelVersions, + filterRegisteredModels, + getLabels, + getProperties, + mergeUpdatedLabels, + mergeUpdatedProperty, + sortModelVersionsByCreateTime, +} from '~/app/pages/modelRegistry/screens/utils'; +import { SearchType } from '~/app/components/DashboardSearchField'; + +describe('getLabels', () => { + it('should return an empty array when customProperties is empty', () => { + const customProperties: ModelRegistryCustomProperties = {}; + const result = getLabels(customProperties); + expect(result).toEqual([]); + }); + + it('should return an array of keys with empty string values in customProperties', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + label2: { metadataType: ModelRegistryMetadataType.STRING, string_value: 'non-empty' }, + label3: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + }; + const result = getLabels(customProperties); + expect(result).toEqual(['label1', 'label3']); + }); + + it('should return an empty array when all values in customProperties are non-empty strings', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { metadataType: ModelRegistryMetadataType.STRING, string_value: 'non-empty' }, + label2: { metadataType: ModelRegistryMetadataType.STRING, string_value: 'another-non-empty' }, + }; + const result = getLabels(customProperties); + expect(result).toEqual([]); + }); +}); + +describe('mergeUpdatedLabels', () => { + it('should return an empty object when customProperties and updatedLabels are empty', () => { + const customProperties: ModelRegistryCustomProperties = {}; + const result = mergeUpdatedLabels(customProperties, []); + expect(result).toEqual({}); + }); + + it('should return an unmodified object if updatedLabels match existing labels', () => { + const customProperties: ModelRegistryCustomProperties = { + someUnrelatedProp: { string_value: 'foo', metadataType: ModelRegistryMetadataType.STRING }, + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedLabels(customProperties, ['label1']); + expect(result).toEqual(customProperties); + }); + + it('should return an object with labels added', () => { + const customProperties: ModelRegistryCustomProperties = {}; + const result = mergeUpdatedLabels(customProperties, ['label1', 'label2']); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should return an object with labels removed', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label3: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label4: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedLabels(customProperties, ['label2', 'label4']); + expect(result).toEqual({ + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label4: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should return an object with labels both added and removed', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label3: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedLabels(customProperties, ['label1', 'label3', 'label4']); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label3: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label4: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should not affect non-label properties on the object', () => { + const customProperties: ModelRegistryCustomProperties = { + someUnrelatedStrProp: { string_value: 'foo', metadataType: ModelRegistryMetadataType.STRING }, + someUnrelatedIntProp: { int_value: '3', metadataType: ModelRegistryMetadataType.INT }, + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedLabels(customProperties, ['label2', 'label3']); + expect(result).toEqual({ + someUnrelatedStrProp: { string_value: 'foo', metadataType: ModelRegistryMetadataType.STRING }, + someUnrelatedIntProp: { int_value: '3', metadataType: ModelRegistryMetadataType.INT }, + label2: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + label3: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); +}); + +describe('getProperties', () => { + it('should return an empty object when customProperties is empty', () => { + const customProperties: ModelRegistryCustomProperties = {}; + const result = getProperties(customProperties); + expect(result).toEqual({}); + }); + + it('should return a filtered object including only string properties with a non-empty value', () => { + const customProperties: ModelRegistryCustomProperties = { + property1: { metadataType: ModelRegistryMetadataType.STRING, string_value: 'non-empty' }, + property2: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'another-non-empty', + }, + label1: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + label2: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + int1: { metadataType: ModelRegistryMetadataType.INT, int_value: '1' }, + int2: { metadataType: ModelRegistryMetadataType.INT, int_value: '2' }, + }; + const result = getProperties(customProperties); + expect(result).toEqual({ + property1: { metadataType: ModelRegistryMetadataType.STRING, string_value: 'non-empty' }, + property2: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'another-non-empty', + }, + } satisfies ModelRegistryStringCustomProperties); + }); + + it('should return an empty object when all values in customProperties are empty strings or non-string values', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + label2: { metadataType: ModelRegistryMetadataType.STRING, string_value: '' }, + int1: { metadataType: ModelRegistryMetadataType.INT, int_value: '1' }, + int2: { metadataType: ModelRegistryMetadataType.INT, int_value: '2' }, + }; + const result = getProperties(customProperties); + expect(result).toEqual({}); + }); +}); + +describe('mergeUpdatedProperty', () => { + it('should handle the create operation', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'create', + newPair: { key: 'prop2', value: 'val2' }, + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + prop2: { string_value: 'val2', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should handle the update operation without a key change', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'update', + oldKey: 'prop1', + newPair: { key: 'prop1', value: 'updatedVal1' }, + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'updatedVal1', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should handle the update operation with a key change', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'update', + oldKey: 'prop1', + newPair: { key: 'prop2', value: 'val2' }, + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop2: { string_value: 'val2', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should perform a create if using the update operation with an invalid oldKey', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'update', + oldKey: 'prop2', + newPair: { key: 'prop3', value: 'val3' }, + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + prop3: { string_value: 'val3', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should handle the delete operation', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + prop2: { string_value: 'val2', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'delete', + oldKey: 'prop2', + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); + + it('should do nothing if using the delete operation with an invalid oldKey', () => { + const customProperties: ModelRegistryCustomProperties = { + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + }; + const result = mergeUpdatedProperty({ + customProperties, + op: 'delete', + oldKey: 'prop2', + }); + expect(result).toEqual({ + label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING }, + prop1: { string_value: 'val1', metadataType: ModelRegistryMetadataType.STRING }, + } satisfies ModelRegistryCustomProperties); + }); +}); + +describe('filterModelVersions', () => { + const modelVersions: ModelVersion[] = [ + mockModelVersion({ name: 'Test 1', state: ModelState.ARCHIVED }), + mockModelVersion({ + name: 'Test 2', + description: 'Description2', + }), + mockModelVersion({ name: 'Test 3', author: 'Author3', state: ModelState.ARCHIVED }), + mockModelVersion({ name: 'Test 4', state: ModelState.ARCHIVED }), + mockModelVersion({ name: 'Test 5' }), + ]; + + test('filters by name', () => { + const filtered = filterModelVersions(modelVersions, 'Test 1', SearchType.KEYWORD); + expect(filtered).toEqual([modelVersions[0]]); + }); + + test('filters by description', () => { + const filtered = filterModelVersions(modelVersions, 'Description2', SearchType.KEYWORD); + expect(filtered).toEqual([modelVersions[1]]); + }); + + test('filters by author', () => { + const filtered = filterModelVersions(modelVersions, 'Author3', SearchType.AUTHOR); + expect(filtered).toEqual([modelVersions[2]]); + }); + + test('does not filter when search is empty', () => { + const filtered = filterModelVersions(modelVersions, '', SearchType.KEYWORD); + expect(filtered).toEqual(modelVersions); + }); +}); + +describe('filterRegisteredModels', () => { + const registeredModels: RegisteredModel[] = [ + mockRegisteredModel({ name: 'Test 1', state: ModelState.ARCHIVED }), + mockRegisteredModel({ + name: 'Test 2', + description: 'Description2', + }), + mockRegisteredModel({ name: 'Test 3', state: ModelState.ARCHIVED }), + mockRegisteredModel({ name: 'Test 4', state: ModelState.ARCHIVED }), + mockRegisteredModel({ name: 'Test 5' }), + ]; + + test('filters by name', () => { + const filtered = filterRegisteredModels(registeredModels, 'Test 1', SearchType.KEYWORD); + expect(filtered).toEqual([registeredModels[0]]); + }); + + test('filters by description', () => { + const filtered = filterRegisteredModels(registeredModels, 'Description2', SearchType.KEYWORD); + expect(filtered).toEqual([registeredModels[1]]); + }); + + test('does not filter when search is empty', () => { + const filtered = filterRegisteredModels(registeredModels, '', SearchType.KEYWORD); + expect(filtered).toEqual(registeredModels); + }); +}); + +describe('sortModelVersionsByCreateTime', () => { + it('should return list of sorted modelVersions by create time', () => { + const modelVersions: ModelVersion[] = [ + mockModelVersion({ + name: 'model version 1', + author: 'Author 1', + id: '1', + createTimeSinceEpoch: '1725018764650', + lastUpdateTimeSinceEpoch: '1725030215299', + }), + mockModelVersion({ + name: 'model version 1', + author: 'Author 1', + id: '1', + createTimeSinceEpoch: '1725028468207', + lastUpdateTimeSinceEpoch: '1725030142332', + }), + ]; + + const result = sortModelVersionsByCreateTime(modelVersions); + expect(result).toEqual([modelVersions[1], modelVersions[0]]); + }); +}); diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal.tsx new file mode 100644 index 00000000..3260639a --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal.tsx @@ -0,0 +1,90 @@ +import * as React from 'react'; +import { Flex, FlexItem, Stack, StackItem, TextInput } from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; +import DashboardModalFooter from '~/app/components/DashboardModalFooter'; + +interface ArchiveModelVersionModalProps { + onCancel: () => void; + onSubmit: () => void; + isOpen: boolean; + modelVersionName: string; +} + +export const ArchiveModelVersionModal: React.FC = ({ + onCancel, + onSubmit, + isOpen, + modelVersionName, +}) => { + const [isSubmitting, setIsSubmitting] = React.useState(false); + const [error, setError] = React.useState(); + const [confirmInputValue, setConfirmInputValue] = React.useState(''); + const isDisabled = confirmInputValue.trim() !== modelVersionName || isSubmitting; + + const onClose = React.useCallback(() => { + setConfirmInputValue(''); + onCancel(); + }, [onCancel]); + + const onConfirm = React.useCallback(async () => { + setIsSubmitting(true); + + try { + await onSubmit(); + onClose(); + } catch (e) { + if (e instanceof Error) { + setError(e); + } + } finally { + setIsSubmitting(false); + } + }, [onSubmit, onClose]); + + return ( + + } + data-testid="archive-model-version-modal" + > + + + {modelVersionName} will be archived and unavailable for use unless it is restored. + + + + + Type {modelVersionName} to confirm archiving: + + setConfirmInputValue(newValue)} + onKeyDown={(event) => { + if (event.key === 'Enter' && !isDisabled) { + onConfirm(); + } + }} + /> + + + + + ); +}; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreModelVersionModal.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreModelVersionModal.tsx new file mode 100644 index 00000000..b9c57d85 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreModelVersionModal.tsx @@ -0,0 +1,62 @@ +import * as React from 'react'; +import { Modal } from '@patternfly/react-core/deprecated'; +import DashboardModalFooter from '~/app/components/DashboardModalFooter'; + +interface RestoreModelVersionModalProps { + onCancel: () => void; + onSubmit: () => void; + isOpen: boolean; + modelVersionName: string; +} + +export const RestoreModelVersionModal: React.FC = ({ + onCancel, + onSubmit, + isOpen, + modelVersionName, +}) => { + const [isSubmitting, setIsSubmitting] = React.useState(false); + const [error, setError] = React.useState(); + + const onClose = React.useCallback(() => { + onCancel(); + }, [onCancel]); + + const onConfirm = React.useCallback(async () => { + setIsSubmitting(true); + + try { + await onSubmit(); + onClose(); + } catch (e) { + if (e instanceof Error) { + setError(e); + } + } finally { + setIsSubmitting(false); + } + }, [onSubmit, onClose]); + + return ( + + } + data-testid="restore-model-version-modal" + > + {modelVersionName} will be restored and returned to the versions list. + + ); +}; diff --git a/clients/ui/frontend/src/components/DashboardDescriptionListGroup.scss b/clients/ui/frontend/src/components/DashboardDescriptionListGroup.scss new file mode 100644 index 00000000..aed757d5 --- /dev/null +++ b/clients/ui/frontend/src/components/DashboardDescriptionListGroup.scss @@ -0,0 +1,4 @@ +.kubeflow-custom-description-list-term-with-action > span { + /* Workaround for missing functionality in PF DescriptionList, see https://github.com/patternfly/patternfly/issues/6583 */ + width: 100%; +} \ No newline at end of file diff --git a/clients/ui/frontend/src/components/DashboardDescriptionListGroup.tsx b/clients/ui/frontend/src/components/DashboardDescriptionListGroup.tsx new file mode 100644 index 00000000..4216c86b --- /dev/null +++ b/clients/ui/frontend/src/components/DashboardDescriptionListGroup.tsx @@ -0,0 +1,120 @@ +import * as React from 'react'; +import { + ActionList, + ActionListItem, + Button, + DescriptionListDescription, + DescriptionListGroup, + DescriptionListTerm, + Flex, + FlexItem, + Split, + SplitItem, +} from '@patternfly/react-core'; +import text from '@patternfly/react-styles/css/utilities/Text/text'; +import { CheckIcon, PencilAltIcon, TimesIcon } from '@patternfly/react-icons'; + +import '~/components/DashboardDescriptionListGroup.scss'; + +type EditableProps = { + isEditing: boolean; + contentWhenEditing: React.ReactNode; + isSavingEdits?: boolean; + onEditClick: () => void; + onSaveEditsClick: () => void; + onDiscardEditsClick: () => void; +}; + +export type DashboardDescriptionListGroupProps = { + title: React.ReactNode; + tooltip?: React.ReactNode; + action?: React.ReactNode; + isEmpty?: boolean; + contentWhenEmpty?: React.ReactNode; + children: React.ReactNode; +} & (({ isEditable: true } & EditableProps) | ({ isEditable?: false } & Partial)); + +const DashboardDescriptionListGroup: React.FC = (props) => { + const { + title, + tooltip, + action, + isEmpty, + contentWhenEmpty, + isEditable = false, + isEditing, + contentWhenEditing, + isSavingEdits = false, + onEditClick, + onSaveEditsClick, + onDiscardEditsClick, + children, + } = props; + return ( + + {action || isEditable ? ( + + + {title} + + {action || + (isEditing ? ( + + + + + + + + + ) : ( + + ))} + + + + ) : ( + + + {title} + {tooltip} + + + )} + + {isEditing ? contentWhenEditing : isEmpty ? contentWhenEmpty : children} + + + ); +}; + +export default DashboardDescriptionListGroup; diff --git a/clients/ui/frontend/src/components/EditableLabelsDescriptionListGroup.tsx b/clients/ui/frontend/src/components/EditableLabelsDescriptionListGroup.tsx new file mode 100644 index 00000000..42cc9091 --- /dev/null +++ b/clients/ui/frontend/src/components/EditableLabelsDescriptionListGroup.tsx @@ -0,0 +1,218 @@ +import * as React from 'react'; +import { + Button, + Form, + FormGroup, + FormHelperText, + HelperText, + HelperTextItem, + Label, + LabelGroup, + TextInput, +} from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; +import { ExclamationCircleIcon } from '@patternfly/react-icons'; +import DashboardDescriptionListGroup, { + DashboardDescriptionListGroupProps, +} from '~/components/DashboardDescriptionListGroup'; + +type EditableTextDescriptionListGroupProps = Partial< + Pick +> & { + labels: string[]; + saveEditedLabels: (labels: string[]) => Promise; + allExistingKeys?: string[]; +}; + +const EditableLabelsDescriptionListGroup: React.FC = ({ + title = 'Labels', + contentWhenEmpty = 'No labels', + labels, + saveEditedLabels, + allExistingKeys = labels, +}) => { + const [isEditing, setIsEditing] = React.useState(false); + const [unsavedLabels, setUnsavedLabels] = React.useState(labels); + const [isSavingEdits, setIsSavingEdits] = React.useState(false); + + const editUnsavedLabel = (newText: string, index: number) => { + if (isSavingEdits) { + return; + } + const copy = [...unsavedLabels]; + copy[index] = newText; + setUnsavedLabels(copy); + }; + const removeUnsavedLabel = (text: string) => { + if (isSavingEdits) { + return; + } + setUnsavedLabels(unsavedLabels.filter((label) => label !== text)); + }; + const addUnsavedLabel = (text: string) => { + if (isSavingEdits) { + return; + } + setUnsavedLabels([...unsavedLabels, text]); + }; + + // Don't allow a label that matches a non-label property key or another label (as they stand before saving) + // Note that this means if you remove a label and add it back before saving, that is valid + const reservedKeys = [ + ...allExistingKeys.filter((key) => !labels.includes(key)), + ...unsavedLabels, + ]; + + const [isAddLabelModalOpen, setIsAddLabelModalOpen] = React.useState(false); + const [addLabelInputValue, setAddLabelInputValue] = React.useState(''); + const addLabelInputRef = React.useRef(null); + let addLabelValidationError: string | null = null; + if (reservedKeys.includes(addLabelInputValue)) { + addLabelValidationError = 'Label must not match an existing label or property key'; + } else if (addLabelInputValue.length > 63) { + addLabelValidationError = "Label text can't exceed 63 characters"; + } + + const toggleAddLabelModal = () => { + setAddLabelInputValue(''); + setIsAddLabelModalOpen(!isAddLabelModalOpen); + }; + React.useEffect(() => { + if (isAddLabelModalOpen && addLabelInputRef.current) { + addLabelInputRef.current.focus(); + } + }, [isAddLabelModalOpen]); + + const addLabelModalSubmitDisabled = !addLabelInputValue || !!addLabelValidationError; + const submitAddLabelModal = (event?: React.FormEvent) => { + event?.preventDefault(); + if (!addLabelModalSubmitDisabled) { + addUnsavedLabel(addLabelInputValue); + toggleAddLabelModal(); + } + }; + + return ( + <> + + Add label + + ) + } + > + {unsavedLabels.map((label, index) => ( + + ))} + + } + onEditClick={() => { + setUnsavedLabels(labels); + setIsEditing(true); + }} + onSaveEditsClick={async () => { + setIsSavingEdits(true); + try { + await saveEditedLabels(unsavedLabels); + } finally { + setIsSavingEdits(false); + } + setIsEditing(false); + }} + onDiscardEditsClick={() => { + setUnsavedLabels(labels); + setIsEditing(false); + }} + > + + {labels.map((label) => ( + + ))} + + + + Save + , + , + ]} + > +
+ + , value: string) => + setAddLabelInputValue(value) + } + ref={addLabelInputRef} + isRequired + validated={addLabelValidationError ? 'error' : 'default'} + /> + {addLabelValidationError && ( + + + } variant="error"> + {addLabelValidationError} + + + + )} + + + + + ); +}; + +export default EditableLabelsDescriptionListGroup; diff --git a/clients/ui/frontend/src/components/EditableTextDescriptionListGroup.tsx b/clients/ui/frontend/src/components/EditableTextDescriptionListGroup.tsx new file mode 100644 index 00000000..1ad501ea --- /dev/null +++ b/clients/ui/frontend/src/components/EditableTextDescriptionListGroup.tsx @@ -0,0 +1,78 @@ +import * as React from 'react'; +import { ExpandableSection, TextArea } from '@patternfly/react-core'; +import DashboardDescriptionListGroup, { + DashboardDescriptionListGroupProps, +} from '~/components/DashboardDescriptionListGroup'; + +type EditableTextDescriptionListGroupProps = Pick< + DashboardDescriptionListGroupProps, + 'title' | 'contentWhenEmpty' +> & { + value: string; + saveEditedValue: (value: string) => Promise; + testid?: string; +}; + +const EditableTextDescriptionListGroup: React.FC = ({ + title, + contentWhenEmpty, + value, + saveEditedValue, + testid, +}) => { + const [isEditing, setIsEditing] = React.useState(false); + const [unsavedValue, setUnsavedValue] = React.useState(value); + const [isSavingEdits, setIsSavingEdits] = React.useState(false); + const [isTextExpanded, setIsTextExpanded] = React.useState(false); + return ( + setUnsavedValue(v)} + isDisabled={isSavingEdits} + rows={24} + resizeOrientation="vertical" + /> + } + onEditClick={() => { + setUnsavedValue(value); + setIsEditing(true); + }} + onSaveEditsClick={async () => { + setIsSavingEdits(true); + try { + await saveEditedValue(unsavedValue); + } finally { + setIsSavingEdits(false); + } + setIsEditing(false); + }} + onDiscardEditsClick={() => { + setUnsavedValue(value); + setIsEditing(false); + }} + > + setIsTextExpanded(isExpanded)} + isExpanded={isTextExpanded} + > + {value} + + + ); +}; + +export default EditableTextDescriptionListGroup; diff --git a/clients/ui/frontend/tsconfig.json b/clients/ui/frontend/tsconfig.json index 1ea9486c..674889be 100644 --- a/clients/ui/frontend/tsconfig.json +++ b/clients/ui/frontend/tsconfig.json @@ -6,6 +6,7 @@ "module": "esnext", "target": "es5", "lib": [ + "ESNext.Array", "es6", "dom" ],
+
+ + + + + +
+ {mv.description && ( + + + + )} + 		+ + {mv.author} + + + + setIsArchiveModalOpen(false)} + onSubmit={() => + apiState.api + .patchModelVersion( + {}, + { + state: ModelState.ARCHIVED, + }, + mv.id, + ) + .then(refresh) + } + isOpen={isArchiveModalOpen} + modelVersionName={mv.name} + /> + setIsRestoreModalOpen(false)} + onSubmit={() => + apiState.api + .patchModelVersion( + {}, + { + state: ModelState.LIVE, + }, + mv.id, + ) + .then(() => + navigate( + modelVersionUrl(mv.id, mv.registeredModelId, preferredModelRegistry?.name), + ), + ) + } + isOpen={isRestoreModalOpen} + modelVersionName={mv.name} + /> +