diff --git a/clients/ui/frontend/.env b/clients/ui/frontend/.env index cf90978c5..7ff436e10 100644 --- a/clients/ui/frontend/.env +++ b/clients/ui/frontend/.env @@ -7,4 +7,5 @@ LOGO_DARK=logo-dark-theme.svg FAVICON=favicon.ico PRODUCT_NAME="Model Registry" STYLE_THEME=mui-theme +PLATFORM_MODE=kubeflow diff --git a/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts b/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts index 56fed2e3f..ade07d64f 100644 --- a/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts +++ b/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts @@ -8,7 +8,7 @@ type MockModelRegistry = { export const mockModelRegistry = ({ name = 'modelregistry-sample', - description = 'New model registry', + description = 'Model registry description', displayName = 'Model Registry Sample', }: MockModelRegistry): ModelRegistry => ({ name, diff --git a/clients/ui/frontend/src/__mocks__/mockModelVersion.ts b/clients/ui/frontend/src/__mocks__/mockModelVersion.ts index 80a6f3107..018b12f01 100644 --- a/clients/ui/frontend/src/__mocks__/mockModelVersion.ts +++ b/clients/ui/frontend/src/__mocks__/mockModelVersion.ts @@ -1,23 +1,22 @@ -import { ModelVersion, ModelState } from '~/app/types'; -import { createModelRegistryLabelsObject } from './utils'; +import { ModelVersion, ModelState, ModelRegistryCustomProperties } from '~/app/types'; type MockModelVersionType = { author?: string; id?: string; registeredModelId?: string; name?: string; - labels?: string[]; state?: ModelState; description?: string; createTimeSinceEpoch?: string; lastUpdateTimeSinceEpoch?: string; + customProperties?: ModelRegistryCustomProperties; }; export const mockModelVersion = ({ author = 'Test author', registeredModelId = '1', name = 'new model version', - labels = [], + customProperties = {}, id = '1', state = ModelState.LIVE, description = 'Description of model version', @@ -26,7 +25,7 @@ export const mockModelVersion = ({ }: MockModelVersionType): ModelVersion => ({ author, createTimeSinceEpoch, - customProperties: createModelRegistryLabelsObject(labels), + customProperties, id, lastUpdateTimeSinceEpoch, name, diff --git a/clients/ui/frontend/src/__mocks__/mockRegisteredModel.ts b/clients/ui/frontend/src/__mocks__/mockRegisteredModel.ts index 7c45fbe57..61886e311 100644 --- a/clients/ui/frontend/src/__mocks__/mockRegisteredModel.ts +++ b/clients/ui/frontend/src/__mocks__/mockRegisteredModel.ts @@ -1,5 +1,4 @@ -import { ModelState, RegisteredModel } from '~/app/types'; -import { createModelRegistryLabelsObject } from './utils'; +import { ModelRegistryCustomProperties, ModelState, RegisteredModel } from '~/app/types'; type MockRegisteredModelType = { id?: string; @@ -7,7 +6,7 @@ type MockRegisteredModelType = { owner?: string; state?: ModelState; description?: string; - labels?: string[]; + customProperties?: ModelRegistryCustomProperties; }; export const mockRegisteredModel = ({ @@ -15,7 +14,7 @@ export const mockRegisteredModel = ({ owner = 'Author 1', state = ModelState.LIVE, description = '', - labels = [], + customProperties = {}, id = '1', }: MockRegisteredModelType): RegisteredModel => ({ createTimeSinceEpoch: '1710404288975', @@ -26,5 +25,5 @@ export const mockRegisteredModel = ({ name, state, owner, - customProperties: createModelRegistryLabelsObject(labels), + customProperties, }); diff --git a/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts b/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts index ddb525afe..1695c410d 100644 --- a/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts +++ b/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts @@ -1,4 +1,4 @@ -import { RegisteredModelList } from '~/app/types'; +import { ModelRegistryMetadataType, RegisteredModelList } from '~/app/types'; import { mockRegisteredModel } from './mockRegisteredModel'; export const mockRegisteredModelList = ({ @@ -10,39 +10,35 @@ export const mockRegisteredModelList = ({ name: 'Fraud detection model', description: 'A machine learning model trained to detect fraudulent transactions in financial data', - labels: [ - 'Financial data', - 'Fraud detection', - 'Test label', - 'Machine learning', - 'Next data to be overflow', - ], + customProperties: { + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: '', + }, + }, }), mockRegisteredModel({ name: 'Credit Scoring', - labels: [ - 'Credit Score Predictor', - 'Creditworthiness scoring system', - 'Default Risk Analyzer', - 'Portfolio Management', - 'Risk Assessment', - ], + customProperties: { + 'Credit Score Predictor': { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: '', + }, + }, }), mockRegisteredModel({ name: 'Label modal', description: 'A machine learning model trained to detect fraudulent transactions in financial data', - labels: [ - 'Testing label', - 'Financial data', - 'Fraud detection', - 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc', - 'Machine learning', - 'Next data to be overflow', - 'Label x', - 'Label y', - 'Label z', - ], + customProperties: { + 'Testing label': { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: '', + }, + }, }), ], }: Partial): RegisteredModelList => ({ diff --git a/clients/ui/frontend/src/__mocks__/utils.ts b/clients/ui/frontend/src/__mocks__/utils.ts index 43d1768e8..f1599462b 100644 --- a/clients/ui/frontend/src/__mocks__/utils.ts +++ b/clients/ui/frontend/src/__mocks__/utils.ts @@ -1,20 +1,4 @@ -import { - ModelRegistryMetadataType, - ModelRegistryBody, - ModelRegistryStringCustomProperties, -} from '~/app/types'; - -export const createModelRegistryLabelsObject = ( - labels: string[], -): ModelRegistryStringCustomProperties => - labels.reduce((acc, label) => { - acc[label] = { - metadataType: ModelRegistryMetadataType.STRING, - // eslint-disable-next-line camelcase - string_value: '', - }; - return acc; - }, {} as ModelRegistryStringCustomProperties); +import { ModelRegistryBody } from '~/app/types'; export const mockBFFResponse = (data: T): ModelRegistryBody => ({ data, diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SearchSelector.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SearchSelector.ts new file mode 100644 index 000000000..21591ae2a --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SearchSelector.ts @@ -0,0 +1,59 @@ +import { SubComponentBase } from '~/__tests__/cypress/cypress/pages/components/subComponents/SubComponentBase'; + +export class SearchSelector extends SubComponentBase { + constructor( + private selectorId: string, + contextSelectorId?: string, + ) { + super(contextSelectorId); + } + + private findContextualItem(suffix: string): Cypress.Chainable> { + return this.findScope().document().findByTestId(`${this.selectorId}-${suffix}`); + } + + findItem(name: string, useMenuList: boolean): Cypress.Chainable> { + const list = useMenuList ? this.findMenuList() : this.findResultTableList(); + return list.contains(name).should('exist'); + } + + selectItem(name: string, useMenuList = false): void { + this.findItem(name, useMenuList).click(); + } + + findSearchInput(): Cypress.Chainable> { + return this.findContextualItem('search'); + } + + findToggleButton(): Cypress.Chainable> { + return this.findContextualItem('toggle'); + } + + findResultTableList(): Cypress.Chainable> { + return this.findContextualItem('table-list'); + } + + findSearchHelpText(): Cypress.Chainable> { + return this.findContextualItem('searchHelpText'); + } + + findMenu(): Cypress.Chainable> { + return this.findContextualItem('menu'); + } + + findMenuList(): Cypress.Chainable> { + return this.findContextualItem('menuList'); + } + + // Search for an item by typing into the search input + searchItem(name: string): void { + this.findSearchInput().clear().type(name); + } + + // Perform the entire process: open, search, and select + openAndSelectItem(name: string, useMenuList = false): void { + this.findToggleButton().click(); + this.searchItem(name); + this.selectItem(name, useMenuList); + } +} diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SubComponentBase.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SubComponentBase.ts new file mode 100644 index 000000000..e1706e07e --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/components/subComponents/SubComponentBase.ts @@ -0,0 +1,35 @@ +/** + * A SubComponent is a component that doesn't make up a full page and will be consumed in other page + * objects. This could be a complex field, a group of fields, or some section. Typically not large + * enough to warrant its own standalone page object. + * + * Primary use-case example: + * class Foo extends SubComponentBase { + * constructor(private myTestId: string, scopedTestId?: string) { + * super(scopedTestId); + * } + * + * private find(suffix: string) { + * return this.findScope().getByTestId(`${this.myTestId}-${suffix}`); + * } + * + * selectItem(name: string) { + * // "list" would be an internal suffix for your component to know where the "items" are + * return this.find('list').findDropdownItem(name); + * } + * } + * + * Search uses of this component to see further examples + */ +export class SubComponentBase { + constructor(private scopedBaseTestId?: string) {} + + /** Allows for extended classes to make use of a simple one-check for their `find()` calls */ + protected findScope(): (Cypress.cy & CyEventEmitter) | Cypress.Chainable> { + if (this.scopedBaseTestId) { + return cy.findByTestId(this.scopedBaseTestId); + } + + return cy; + } +} diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts index 6e4b28591..35e45db8f 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts @@ -87,14 +87,38 @@ class ModelRegistry { return cy.findByTestId('empty-model-registries-state'); } + findModelRegistryEmptyTableState() { + return cy.findByTestId('dashboard-empty-table-state'); + } + shouldregisteredModelsEmpty() { cy.findByTestId('empty-registered-models').should('exist'); } + findViewDetailsButton() { + return cy.findByTestId('view-details-button'); + } + + findDetailsPopover() { + return cy.findByTestId('mr-details-popover'); + } + + findHelpContentButton() { + return cy.findByTestId('model-registry-help-button'); + } + + findHelpContentPopover() { + return cy.findByTestId('model-registry-help-content'); + } + shouldmodelVersionsEmpty() { 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'); } @@ -103,10 +127,6 @@ class ModelRegistry { cy.findByTestId('registered-models-table-toolbar').should('exist'); } - shouldArchiveModelVersionsEmpty() { - cy.findByTestId('empty-archive-model-versions').should('exist'); - } - tabEnabled() { appChrome.findNavItem('Model Registry').should('exist'); return this; diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionArchive.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionArchive.ts index 951b3b04c..a52fe97e1 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionArchive.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionArchive.ts @@ -97,6 +97,14 @@ class ModelVersionArchive { return cy.findByTestId('archive-version-page-breadcrumb'); } + findVersionDetailsTab() { + return cy.findByTestId('model-versions-details-tab'); + } + + findVersionDeploymentTab() { + return cy.findByTestId('deployments-tab'); + } + findArchiveVersionTable() { return cy.findByTestId('model-versions-archive-table'); } diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails.ts index c3d44c5f7..5106f1948 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails.ts @@ -1,3 +1,5 @@ +import { TableRow } from '~/__tests__/cypress/cypress/pages/components/table'; + class ModelVersionDetails { visit() { const preferredModelRegistry = 'modelregistry-sample'; @@ -20,6 +22,14 @@ class ModelVersionDetails { return cy.findByTestId('model-version-description'); } + findSourceModelFormat(subComponent: 'group' | 'edit' | 'save' | 'cancel') { + return cy.findByTestId(`source-model-format-${subComponent}`); + } + + findSourceModelVersion(subComponent: 'group' | 'edit' | 'save' | 'cancel') { + return cy.findByTestId(`source-model-version-${subComponent}`); + } + findMoreLabelsButton() { return cy.findByTestId('label-group').find('button'); } @@ -68,6 +78,78 @@ class ModelVersionDetails { findRegisteredDeploymentsTab() { return cy.findByTestId('deployments-tab'); } + + findAddPropertyButton() { + return cy.findByTestId('add-property-button'); + } + + findAddKeyInput() { + return cy.findByTestId('add-property-key-input'); + } + + findAddValueInput() { + return cy.findByTestId('add-property-value-input'); + } + + findKeyEditInput(key: string) { + return cy.findByTestId(['edit-property-key-input', key]); + } + + findValueEditInput(value: string) { + return cy.findByTestId(['edit-property-value-input', value]); + } + + findSaveButton() { + return cy.findByTestId('save-edit-button-property'); + } + + findCancelButton() { + return cy.findByTestId('discard-edit-button-property'); + } + + findExpandControlButton() { + return cy.findByTestId('expand-control-button'); + } + + private findTable() { + return cy.findByTestId('properties-table'); + } + + findPropertiesTableRows() { + return this.findTable().find('tbody tr'); + } + + getRow(name: string) { + return new PropertyRow(() => + this.findTable().find(`[data-label=Key]`).contains(name).parents('tr'), + ); + } + + findEditLabelsButton() { + return cy.findByTestId('editable-labels-group-edit'); + } + + findAddLabelButton() { + return cy.findByTestId('add-label-button'); + } + + findLabelInput(label: string) { + return cy.findByTestId(`edit-label-input-${label}`); + } + + findLabel(label: string) { + return cy.findByTestId(`editable-label-${label}`); + } + + findLabelErrorAlert() { + return cy.findByTestId('label-error-alert'); + } + + findSaveLabelsButton() { + return cy.findByTestId('editable-labels-group-save'); + } } +class PropertyRow extends TableRow {} + export const modelVersionDetails = new ModelVersionDetails(); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts index 80d38bd61..4e9f4af7c 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts @@ -1,3 +1,5 @@ +import { SearchSelector } from '~/__tests__/cypress/cypress/pages/components/subComponents/SearchSelector'; + export enum FormFieldSelector { MODEL_NAME = '#model-name', MODEL_DESCRIPTION = '#model-description', @@ -15,6 +17,8 @@ export enum FormFieldSelector { } class RegisterModelPage { + projectDropdown = new SearchSelector('project-selector', 'connection-autofill-modal'); + visit() { const preferredModelRegistry = 'modelregistry-sample'; cy.visit(`/model-registry/${preferredModelRegistry}/registerModel`); @@ -41,10 +45,6 @@ class RegisterModelPage { return cy.findByTestId('connection-autofill-modal'); } - findProjectSelector() { - return this.findConnectionAutofillModal().findByTestId('project-selector-dropdown'); - } - findConnectionSelector() { return this.findConnectionAutofillModal().findByTestId('select-data-connection'); } diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerVersionPage.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerVersionPage.ts new file mode 100644 index 000000000..3b9df6e9a --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerVersionPage.ts @@ -0,0 +1,51 @@ +export enum FormFieldSelector { + REGISTERED_MODEL = '#registered-model-container .pf-m-typeahead', + VERSION_NAME = '#version-name', + VERSION_DESCRIPTION = '#version-description', + SOURCE_MODEL_FORMAT = '#source-model-format', + SOURCE_MODEL_FORMAT_VERSION = '#source-model-format-version', + LOCATION_TYPE_OBJECT_STORAGE = '#location-type-object-storage', + LOCATION_ENDPOINT = '#location-endpoint', + LOCATION_BUCKET = '#location-bucket', + LOCATION_REGION = '#location-region', + LOCATION_PATH = '#location-path', + LOCATION_TYPE_URI = '#location-type-uri', + LOCATION_URI = '#location-uri', +} + +class RegisterVersionPage { + visit(registeredModelId?: string) { + const preferredModelRegistry = 'modelregistry-sample'; + cy.visit( + registeredModelId + ? `/model-registry/${preferredModelRegistry}/registeredModels/${registeredModelId}/registerVersion` + : `/model-registry/${preferredModelRegistry}/registerVersion`, + ); + this.wait(); + } + + private wait() { + const preferredModelRegistry = 'modelregistry-sample'; + cy.findByTestId('app-page-title').should('exist'); + cy.findByTestId('app-page-title').contains('Register new version'); + cy.findByText(`Model registry - ${preferredModelRegistry}`).should('exist'); + cy.testA11y(); + } + + findFormField(selector: FormFieldSelector) { + return cy.get(selector); + } + + selectRegisteredModel(name: string) { + this.findFormField(FormFieldSelector.REGISTERED_MODEL) + .findByRole('button', { name: 'Typeahead menu toggle' }) + .findSelectOption(name) + .click(); + } + + findSubmitButton() { + return cy.findByTestId('create-button'); + } +} + +export const registerVersionPage = new RegisterVersionPage(); 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 f4fc6018d..e560ebf26 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 @@ -50,6 +50,13 @@ declare global { }, response: ApiResponse, ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions', + options: { + path: { modelRegistryName: string; apiVersion: string }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & (( type: 'POST /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions', options: { diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts index 897b01148..abbb30e23 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts @@ -5,7 +5,12 @@ import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; -import type { ModelRegistry, ModelVersion, RegisteredModel } from '~/app/types'; +import { + ModelRegistryMetadataType, + type ModelRegistry, + type ModelVersion, + type RegisteredModel, +} from '~/app/types'; import { be } from '~/__tests__/cypress/cypress/utils/should'; import { MODEL_REGISTRY_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api'; @@ -19,13 +24,11 @@ const initIntercepts = ({ 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', + description: '', + displayName: 'modelregistry-sample-2', }), ], registeredModels = [ @@ -33,29 +36,72 @@ const initIntercepts = ({ name: 'Fraud detection model', description: 'A machine learning model trained to detect fraudulent transactions in financial data', - labels: [ - 'Financial data', - 'Fraud detection', - 'Test label', - 'Machine learning', - 'Next data to be overflow', - ], + customProperties: { + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, }), mockRegisteredModel({ name: 'Label modal', description: 'A machine learning model trained to detect fraudulent transactions in financial data', - labels: [ - 'Testing label', - 'Financial data', - 'Fraud detection', - 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc', - 'Machine learning', - 'Next data to be overflow', - 'Label x', - 'Label y', - 'Label z', - ], + customProperties: { + 'Testing label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc': + { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, }), ], modelVersions = [ @@ -71,6 +117,14 @@ const initIntercepts = ({ modelRegistries, ); + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions`, + { + path: { modelRegistryName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockModelVersionList({ items: modelVersions }), + ); + cy.interceptApi( `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models`, { @@ -95,20 +149,7 @@ const initIntercepts = ({ describe('Model Registry core', () => { it('Model Registry Enabled in the cluster', () => { initIntercepts({ - registeredModels: [ - mockRegisteredModel({ - name: 'Fraud detection model', - description: - 'A machine learning model trained to detect fraudulent transactions in financial data', - labels: [ - 'Financial data', - 'Fraud detection', - 'Test label', - 'Machine learning', - 'Next data to be overflow', - ], - }), - ], + registeredModels: [], }); modelRegistry.visit(); @@ -126,7 +167,6 @@ describe('Model Registry core', () => { modelRegistry.navigate(); modelRegistry.findModelRegistryEmptyState().should('exist'); }); - it('No registered models in the selected Model Registry', () => { initIntercepts({ registeredModels: [], @@ -136,6 +176,26 @@ describe('Model Registry core', () => { modelRegistry.navigate(); modelRegistry.shouldModelRegistrySelectorExist(); modelRegistry.shouldregisteredModelsEmpty(); + + modelRegistry.findViewDetailsButton().click(); + modelRegistry.findDetailsPopover().should('exist'); + modelRegistry.findDetailsPopover().findByText('Model registry description').should('exist'); + + // Model registry with no description + modelRegistry.findModelRegistry().findSelectOption('modelregistry-sample-2').click(); + modelRegistry.findViewDetailsButton().click(); + modelRegistry.findDetailsPopover().should('exist'); + modelRegistry.findDetailsPopover().findByText('No description').should('exist'); + + // Model registry help content + modelRegistry.findHelpContentButton().click(); + modelRegistry.findHelpContentPopover().should('exist'); + modelRegistry + .findHelpContentPopover() + .findByText( + 'To request access to a new or existing model registry, contact your administrator.', + ) + .should('exist'); }); describe('Registered model table', () => { diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionArchive.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts similarity index 86% rename from clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionArchive.cy.ts rename to clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts index eeed18eda..278281f2a 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionArchive.cy.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionArchive.cy.ts @@ -6,7 +6,7 @@ import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; import type { ModelRegistry, ModelVersion } from '~/app/types'; -import { ModelState } from '~/app/types'; +import { ModelRegistryMetadataType, ModelState } from '~/app/types'; import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockBFFResponse } from '~/__mocks__/utils'; import { @@ -30,16 +30,40 @@ const initIntercepts = ({ name: 'model version 1', 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', - ], + customProperties: { + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, state: ModelState.ARCHIVED, }), mockModelVersion({ id: '2', name: 'model version 2', state: ModelState.ARCHIVED }), @@ -202,7 +226,7 @@ describe('Restoring archive version', () => { modelVersionArchive.visit(); const archiveVersionRow = modelVersionArchive.getRow('model version 2'); - archiveVersionRow.findKebabAction('Restore version').click(); + archiveVersionRow.findKebabAction('Restore model version').click(); restoreVersionModal.findRestoreButton().click(); @@ -273,6 +297,13 @@ describe('Archiving version', () => { }); }); + it('Archived version details page does not have the Deployments tab', () => { + initIntercepts({}); + modelVersionArchive.visitArchiveVersionDetail(); + modelVersionArchive.findVersionDetailsTab().should('exist'); + modelVersionArchive.findVersionDeploymentTab().should('not.exist'); + }); + it('Archive version from versions details', () => { cy.interceptApi( 'PATCH /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId', @@ -290,7 +321,7 @@ describe('Archiving version', () => { modelVersionArchive.visitModelVersionDetails(); modelVersionArchive .findModelVersionsDetailsHeaderAction() - .findDropdownItem('Archive version') + .findDropdownItem('Archive model version') .click(); archiveVersionModal.findArchiveButton().should('be.disabled'); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts new file mode 100644 index 000000000..3b5771d50 --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDetails.cy.ts @@ -0,0 +1,397 @@ +/* eslint-disable camelcase */ +import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; +import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockModelArtifactList } from '~/__mocks__/mockModelArtifactList'; +import { ModelRegistryMetadataType, ModelState, type ModelRegistry } from '~/app/types'; +import { MODEL_REGISTRY_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api'; +import { modelVersionDetails } from '~/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails'; +import { mockBFFResponse } from '~/__mocks__/utils'; + +const mockModelVersions = mockModelVersion({ + id: '1', + name: 'Version 1', + customProperties: { + a1: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v1', + }, + a2: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v2', + }, + a3: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v3', + }, + a4: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v4', + }, + a5: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v5', + }, + a6: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v1', + }, + a7: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'v7', + }, + 'Testing label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc': + { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, +}); + +type HandlersProps = { + modelRegistries?: ModelRegistry[]; +}; + +const initIntercepts = ({ + 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', + }), + ], +}: HandlersProps) => { + cy.interceptApi( + `GET /api/:apiVersion/model_registry`, + { + path: { apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + modelRegistries, + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + registeredModelId: 1, + }, + }, + mockRegisteredModel({}), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + registeredModelId: 1, + }, + }, + mockModelVersionList({ + items: [ + mockModelVersion({ name: 'Version 1', author: 'Author 1', registeredModelId: '1' }), + mockModelVersion({ + author: 'Author 2', + registeredModelId: '1', + id: '2', + name: 'Version 2', + }), + mockModelVersion({ + author: 'Author 3', + registeredModelId: '1', + id: '3', + name: 'Version 3', + state: ModelState.ARCHIVED, + }), + ], + }), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 1, + }, + }, + mockModelVersions, + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 2, + }, + }, + mockModelVersion({ id: '2', name: 'Version 2' }), + ); + + cy.interceptApi( + `PATCH /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 1, + }, + }, + mockModelVersions, + ).as('UpdatePropertyRow'); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId/artifacts`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 1, + }, + }, + mockModelArtifactList({}), + ); +}; + +describe('Model version details', () => { + describe('Details tab', () => { + beforeEach(() => { + initIntercepts({}); + modelVersionDetails.visit(); + }); + + it('Model version details page header', () => { + verifyRelativeURL( + '/model-registry/modelregistry-sample/registeredModels/1/versions/1/details', + ); + cy.findByTestId('app-page-title').should('have.text', 'Version 1'); + cy.findByTestId('breadcrumb-version-name').should('have.text', 'Version 1'); + }); + + it('should add a property', () => { + modelVersionDetails.findAddPropertyButton().click(); + modelVersionDetails.findAddKeyInput().type('new_key'); + modelVersionDetails.findAddValueInput().type('new_value'); + modelVersionDetails.findCancelButton().click(); + + modelVersionDetails.findAddPropertyButton().click(); + modelVersionDetails.findAddKeyInput().type('new_key'); + modelVersionDetails.findAddValueInput().type('new_value'); + modelVersionDetails.findSaveButton().click(); + cy.wait('@UpdatePropertyRow'); + }); + + it('should edit a property row', () => { + modelVersionDetails.findExpandControlButton().should('have.text', 'Show 2 more properties'); + modelVersionDetails.findExpandControlButton().click(); + const propertyRow = modelVersionDetails.getRow('a6'); + propertyRow.find().findKebabAction('Edit').click(); + modelVersionDetails.findKeyEditInput('a6').clear().type('edit_key'); + modelVersionDetails.findValueEditInput('v1').clear().type('edit_value'); + + modelVersionDetails.findCancelButton().click(); + propertyRow.find().findKebabAction('Edit').click(); + modelVersionDetails.findKeyEditInput('a6').clear().type('edit_key'); + modelVersionDetails.findValueEditInput('v1').clear().type('edit_value'); + modelVersionDetails.findSaveButton().click(); + cy.wait('@UpdatePropertyRow').then((interception) => { + expect(interception.request.body).to.eql( + mockBFFResponse({ + customProperties: { + a1: { metadataType: 'MetadataStringValue', string_value: 'v1' }, + a2: { metadataType: 'MetadataStringValue', string_value: 'v2' }, + a3: { metadataType: 'MetadataStringValue', string_value: 'v3' }, + a4: { metadataType: 'MetadataStringValue', string_value: 'v4' }, + a5: { metadataType: 'MetadataStringValue', string_value: 'v5' }, + a7: { metadataType: 'MetadataStringValue', string_value: 'v7' }, + 'Testing label': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Financial data': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Fraud detection': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc': + { metadataType: 'MetadataStringValue', string_value: '' }, + 'Machine learning': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Next data to be overflow': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label x': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label y': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label z': { metadataType: 'MetadataStringValue', string_value: '' }, + edit_key: { string_value: 'edit_value', metadataType: 'MetadataStringValue' }, + }, + }), + ); + }); + }); + + it('should delete a property row', () => { + modelVersionDetails.findExpandControlButton().should('have.text', 'Show 2 more properties'); + modelVersionDetails.findExpandControlButton().click(); + const propertyRow = modelVersionDetails.getRow('a6'); + modelVersionDetails.findPropertiesTableRows().should('have.length', 7); + propertyRow.find().findKebabAction('Delete').click(); + cy.wait('@UpdatePropertyRow').then((interception) => { + expect(interception.request.body).to.eql( + mockBFFResponse({ + customProperties: { + a1: { metadataType: 'MetadataStringValue', string_value: 'v1' }, + a2: { metadataType: 'MetadataStringValue', string_value: 'v2' }, + a3: { metadataType: 'MetadataStringValue', string_value: 'v3' }, + a4: { metadataType: 'MetadataStringValue', string_value: 'v4' }, + a5: { metadataType: 'MetadataStringValue', string_value: 'v5' }, + a7: { metadataType: 'MetadataStringValue', string_value: 'v7' }, + 'Testing label': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Financial data': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Fraud detection': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc': + { metadataType: 'MetadataStringValue', string_value: '' }, + 'Machine learning': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Next data to be overflow': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label x': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label y': { metadataType: 'MetadataStringValue', string_value: '' }, + 'Label z': { metadataType: 'MetadataStringValue', string_value: '' }, + }, + }), + ); + }); + }); + + it('Switching model versions', () => { + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId/artifacts`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + modelVersionId: 2, + }, + }, + mockModelArtifactList({}), + ); + modelVersionDetails.findVersionId().contains('1'); + modelVersionDetails.findModelVersionDropdownButton().click(); + modelVersionDetails.findModelVersionDropdownItem('Version 3').should('not.exist'); + modelVersionDetails.findModelVersionDropdownSearch().fill('Version 2'); + modelVersionDetails.findModelVersionDropdownItem('Version 2').click(); + modelVersionDetails.findVersionId().contains('2'); + }); + + it('should handle label editing', () => { + modelVersionDetails.findEditLabelsButton().click(); + + modelVersionDetails.findAddLabelButton().click(); + cy.findByTestId('editable-label-group') + .should('exist') + .within(() => { + cy.contains('New Label').should('exist').click(); + cy.focused().type('First Label{enter}'); + }); + + modelVersionDetails.findAddLabelButton().click(); + cy.findByTestId('editable-label-group') + .should('exist') + .within(() => { + cy.contains('New Label').should('exist').click(); + cy.focused().type('Second Label{enter}'); + }); + + cy.findByTestId('editable-label-group').within(() => { + cy.contains('First Label').should('exist').click(); + cy.focused().type('Updated First Label{enter}'); + }); + + cy.findByTestId('editable-label-group').within(() => { + cy.contains('Second Label').parent().find('[data-testid^="remove-label-"]').click(); + }); + + modelVersionDetails.findSaveLabelsButton().should('exist').click(); + }); + + it('should validate label length', () => { + modelVersionDetails.findEditLabelsButton().click(); + + const longLabel = 'a'.repeat(64); + modelVersionDetails.findAddLabelButton().click(); + cy.findByTestId('editable-label-group') + .should('exist') + .within(() => { + cy.contains('New Label').should('exist').click(); + cy.focused().type(`${longLabel}{enter}`); + }); + + cy.findByTestId('label-error-alert') + .should('be.visible') + .within(() => { + cy.contains(`can't exceed 63 characters`).should('exist'); + }); + }); + + it('should validate duplicate labels', () => { + modelVersionDetails.findEditLabelsButton().click(); + + modelVersionDetails.findAddLabelButton().click(); + cy.findByTestId('editable-label-group') + .should('exist') + .within(() => { + cy.get('[data-testid^="editable-label-"]').last().click(); + cy.focused().type('{selectall}{backspace}Testing label{enter}'); + }); + + modelVersionDetails.findAddLabelButton().click(); + cy.findByTestId('editable-label-group') + .should('exist') + .within(() => { + cy.get('[data-testid^="editable-label-"]').last().click(); + cy.focused().type('{selectall}{backspace}Testing label{enter}'); + }); + + cy.findByTestId('label-error-alert') + .should('be.visible') + .within(() => { + cy.contains('Testing label already exists').should('exist'); + }); + }); + }); +}); 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/modelRegistry/modelVersions.cy.ts similarity index 85% rename from clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts rename to clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts index 842392f6b..292cbee0c 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersions.cy.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts @@ -4,7 +4,7 @@ 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 { ModelRegistryMetadataType, type ModelRegistry, type ModelVersion } from '~/app/types'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; @@ -34,16 +34,40 @@ const initIntercepts = ({ 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', - ], + customProperties: { + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, }), mockModelVersion({ id: '2', name: 'model version' }), ], @@ -56,6 +80,14 @@ const initIntercepts = ({ modelRegistries, ); + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions`, + { + path: { modelRegistryName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockModelVersionList({ items: modelVersions }), + ); + cy.interceptApi( `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models`, { diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/registeredModelArchive.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts similarity index 90% rename from clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/registeredModelArchive.cy.ts rename to clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts index 6601e3290..5de788501 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/registeredModelArchive.cy.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts @@ -7,7 +7,7 @@ import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/mod import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { be } from '~/__tests__/cypress/cypress/utils/should'; import type { ModelRegistry, ModelVersion, RegisteredModel } from '~/app/types'; -import { ModelState } from '~/app/types'; +import { ModelRegistryMetadataType, ModelState } from '~/app/types'; import { mockBFFResponse } from '~/__mocks__/utils'; import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; import { MODEL_REGISTRY_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api'; @@ -29,16 +29,40 @@ const initIntercepts = ({ mockRegisteredModel({ name: 'model 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', - ], + customProperties: { + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Test label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, state: ModelState.ARCHIVED, }), mockRegisteredModel({ id: '2', name: 'model 2', state: ModelState.ARCHIVED }), @@ -70,6 +94,14 @@ const initIntercepts = ({ modelRegistries, ); + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions`, + { + path: { modelRegistryName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockModelVersionList({ items: modelVersions }), + ); + cy.interceptApi( `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models`, { @@ -158,7 +190,7 @@ describe('Model archive list', () => { registeredModelArchive.findArchiveModelBreadcrumbItem().contains('Archived models'); const archiveModelRow = registeredModelArchive.getRow('model 2'); archiveModelRow.findName().contains('model 2').click(); - modelRegistry.shouldArchiveModelVersionsEmpty(); + modelRegistry.shouldArchveModelVersionsEmpty(); }); it('Archived model flow', () => { diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionDetails.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionDetails.cy.ts deleted file mode 100644 index 58f28ac6a..000000000 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelVersionDetails.cy.ts +++ /dev/null @@ -1,178 +0,0 @@ -/* eslint-disable camelcase */ -import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; -import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; -import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; -import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; -import { mockModelVersion } from '~/__mocks__/mockModelVersion'; -import { mockModelArtifactList } from '~/__mocks__/mockModelArtifactList'; -import { mockModelArtifact } from '~/__mocks__/mockModelArtifact'; -import type { ModelRegistry } from '~/app/types'; -import { MODEL_REGISTRY_API_VERSION } from '~/__tests__/cypress/cypress/support/commands/api'; -import { modelVersionDetails } from '~/__tests__/cypress/cypress/pages/modelRegistryView/modelVersionDetails'; - -type HandlersProps = { - modelRegistries?: ModelRegistry[]; -}; - -const initIntercepts = ({ - 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', - }), - ], -}: HandlersProps) => { - cy.interceptApi( - `GET /api/:apiVersion/model_registry`, - { - path: { apiVersion: MODEL_REGISTRY_API_VERSION }, - }, - modelRegistries, - ); - - cy.interceptApi( - `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId`, - { - path: { - modelRegistryName: 'modelregistry-sample', - apiVersion: MODEL_REGISTRY_API_VERSION, - registeredModelId: 1, - }, - }, - mockRegisteredModel({}), - ); - - cy.interceptApi( - `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions`, - { - path: { - modelRegistryName: 'modelregistry-sample', - apiVersion: MODEL_REGISTRY_API_VERSION, - registeredModelId: 1, - }, - }, - mockModelVersionList({ - items: [ - mockModelVersion({ name: 'Version 1', author: 'Author 1', registeredModelId: '1' }), - mockModelVersion({ - author: 'Author 2', - registeredModelId: '1', - id: '2', - name: 'Version 2', - }), - ], - }), - ); - - 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: 'Version 1', - labels: [ - 'Testing label', - 'Financial data', - 'Fraud detection', - 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc', - 'Machine learning', - 'Next data to be overflow', - 'Label x', - 'Label y', - 'Label z', - ], - }), - ); - - cy.interceptApi( - `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId`, - { - path: { - modelRegistryName: 'modelregistry-sample', - apiVersion: MODEL_REGISTRY_API_VERSION, - modelVersionId: 2, - }, - }, - mockModelVersion({ id: '2', name: 'Version 2' }), - ); - - cy.interceptApi( - `GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId/artifacts`, - { - path: { - modelRegistryName: 'modelregistry-sample', - apiVersion: MODEL_REGISTRY_API_VERSION, - modelVersionId: 1, - }, - }, - mockModelArtifactList({ - items: [ - mockModelArtifact({}), - mockModelArtifact({ - author: 'Author 2', - id: '2', - name: 'Artifact 2', - }), - ], - }), - ); -}; - -describe('Model version details', () => { - describe('Details tab', () => { - beforeEach(() => { - initIntercepts({}); - modelVersionDetails.visit(); - }); - - it('Model version details page header', () => { - verifyRelativeURL( - '/model-registry/modelregistry-sample/registeredModels/1/versions/1/details', - ); - cy.findByTestId('app-page-title').should('have.text', 'Version 1'); - cy.findByTestId('breadcrumb-version-name').should('have.text', 'Version 1'); - }); - - it('Model version details tab', () => { - modelVersionDetails.findVersionId().contains('1'); - modelVersionDetails.findDescription().should('have.text', 'Description of model version'); - modelVersionDetails.findMoreLabelsButton().contains('6 more'); - modelVersionDetails.findMoreLabelsButton().click(); - modelVersionDetails.shouldContainsModalLabels([ - 'Testing label', - 'Financial', - 'Financial data', - 'Fraud detection', - 'Machine learning', - 'Next data to be overflow', - 'Label x', - 'Label y', - 'Label z', - ]); - modelVersionDetails.findStorageEndpoint().contains('test-endpoint'); - modelVersionDetails.findStorageRegion().contains('test-region'); - modelVersionDetails.findStorageBucket().contains('test-bucket'); - modelVersionDetails.findStoragePath().contains('demo-models/test-path'); - }); - - it('Switching model versions', () => { - modelVersionDetails.findVersionId().contains('1'); - modelVersionDetails.findModelVersionDropdownButton().click(); - modelVersionDetails.findModelVersionDropdownSearch().fill('Version 2'); - modelVersionDetails.findModelVersionDropdownItem('Version 2').click(); - modelVersionDetails.findVersionId().contains('2'); - }); - }); -}); diff --git a/clients/ui/frontend/src/app/AppRoutes.tsx b/clients/ui/frontend/src/app/AppRoutes.tsx index 60a69d480..2be853ffb 100644 --- a/clients/ui/frontend/src/app/AppRoutes.tsx +++ b/clients/ui/frontend/src/app/AppRoutes.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { Navigate, Route, Routes } from 'react-router-dom'; -import { NotFound } from './pages/notFound/NotFound'; +import NotFound from '~/shared/components/notFound/NotFound'; import ModelRegistrySettingsRoutes from './pages/settings/ModelRegistrySettingsRoutes'; import ModelRegistryRoutes from './pages/modelRegistry/ModelRegistryRoutes'; import useUser from './hooks/useUser'; diff --git a/clients/ui/frontend/src/app/__tests__/updateTimestamps.test.ts b/clients/ui/frontend/src/app/__tests__/updateTimestamps.test.ts new file mode 100644 index 000000000..7824ed5a3 --- /dev/null +++ b/clients/ui/frontend/src/app/__tests__/updateTimestamps.test.ts @@ -0,0 +1,132 @@ +import { + ModelRegistryAPIs, + ModelState, + ModelRegistryMetadataType, + ModelVersion, + RegisteredModel, +} from '~/app/types'; +import { + bumpModelVersionTimestamp, + bumpRegisteredModelTimestamp, + bumpBothTimestamps, +} from '~/app/utils/updateTimestamps'; + +describe('updateTimestamps', () => { + const mockApi = jest.mocked({ + createRegisteredModel: jest.fn(), + createModelVersionForRegisteredModel: jest.fn(), + createModelArtifactForModelVersion: jest.fn(), + getRegisteredModel: jest.fn(), + getModelVersion: jest.fn(), + listModelVersions: jest.fn(), + listRegisteredModels: jest.fn(), + getModelVersionsByRegisteredModel: jest.fn(), + getModelArtifactsByModelVersion: jest.fn(), + patchRegisteredModel: jest.fn(), + patchModelVersion: jest.fn(), + patchModelArtifact: jest.fn(), + }); + const fakeModelVersionId = 'test-model-version-id'; + const fakeRegisteredModelId = 'test-registered-model-id'; + + beforeEach(() => { + jest.spyOn(Date.prototype, 'toISOString').mockReturnValue('2024-01-01T00:00:00.000Z'); + }); + + describe('bumpModelVersionTimestamp', () => { + it('should successfully update model version timestamp', async () => { + await bumpModelVersionTimestamp(mockApi, fakeModelVersionId); + + expect(mockApi.patchModelVersion).toHaveBeenCalledWith( + {}, + { + state: ModelState.LIVE, + customProperties: { + _lastModified: { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: '2024-01-01T00:00:00.000Z', + }, + }, + }, + fakeModelVersionId, + ); + }); + + it('should throw error if modelVersionId is empty', async () => { + await expect(bumpModelVersionTimestamp(mockApi, '')).rejects.toThrow( + 'Model version ID is required', + ); + }); + + it('should handle API errors appropriately', async () => { + const errorMessage = 'API Error'; + // Use proper type for mock function + const mockFn = mockApi.patchModelVersion; + mockFn.mockRejectedValue(new Error(errorMessage)); + + await expect(bumpModelVersionTimestamp(mockApi, fakeModelVersionId)).rejects.toThrow( + `Failed to update model version timestamp: ${errorMessage}`, + ); + }); + }); + + describe('bumpRegisteredModelTimestamp', () => { + it('should successfully update registered model timestamp', async () => { + await bumpRegisteredModelTimestamp(mockApi, fakeRegisteredModelId); + + expect(mockApi.patchRegisteredModel).toHaveBeenCalledWith( + {}, + { + state: ModelState.LIVE, + customProperties: { + _lastModified: { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: '2024-01-01T00:00:00.000Z', + }, + }, + }, + fakeRegisteredModelId, + ); + }); + + it('should throw error if registeredModelId is empty', async () => { + await expect(bumpRegisteredModelTimestamp(mockApi, '')).rejects.toThrow( + 'Registered model ID is required', + ); + }); + + it('should handle API errors appropriately', async () => { + const errorMessage = 'API Error'; + // Use proper type for mock function + const mockFn = mockApi.patchRegisteredModel; + mockFn.mockRejectedValue(new Error(errorMessage)); + + await expect(bumpRegisteredModelTimestamp(mockApi, fakeRegisteredModelId)).rejects.toThrow( + `Failed to update registered model timestamp: ${errorMessage}`, + ); + }); + }); + + describe('bumpBothTimestamps', () => { + it('should update both timestamps successfully', async () => { + mockApi.patchModelVersion.mockResolvedValue({} as ModelVersion); + mockApi.patchRegisteredModel.mockResolvedValue({} as RegisteredModel); + + await bumpBothTimestamps(mockApi, fakeModelVersionId, fakeRegisteredModelId); + + expect(mockApi.patchModelVersion).toHaveBeenCalled(); + expect(mockApi.patchRegisteredModel).toHaveBeenCalled(); + }); + + it('should handle errors from either update', async () => { + const errorMessage = 'API Error'; + mockApi.patchModelVersion.mockRejectedValue(new Error(errorMessage)); + + await expect( + bumpBothTimestamps(mockApi, fakeModelVersionId, fakeRegisteredModelId), + ).rejects.toThrow(); + }); + }); +}); diff --git a/clients/ui/frontend/src/app/__tests__/utils.spec.ts b/clients/ui/frontend/src/app/__tests__/utils.spec.ts new file mode 100644 index 000000000..82cf97644 --- /dev/null +++ b/clients/ui/frontend/src/app/__tests__/utils.spec.ts @@ -0,0 +1,230 @@ +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { + filterArchiveModels, + filterArchiveVersions, + filterLiveModels, + filterLiveVersions, + getLastCreatedItem, + ObjectStorageFields, + objectStorageFieldsToUri, + uriToObjectStorageFields, +} from '~/app/utils'; +import { RegisteredModel, ModelState, ModelVersion } from '~/app/types'; + +describe('objectStorageFieldsToUri', () => { + it('converts fields to URI with all fields present', () => { + const uri = objectStorageFieldsToUri({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + region: 'us-east-1', + path: 'demo-models/flan-t5-small-caikit', + }); + expect(uri).toEqual( + 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', + ); + }); + + it('converts fields to URI with region missing', () => { + const uri = objectStorageFieldsToUri({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + path: 'demo-models/flan-t5-small-caikit', + }); + expect(uri).toEqual( + 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F', + ); + }); + + it('converts fields to URI with region empty', () => { + const uri = objectStorageFieldsToUri({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + region: '', + path: 'demo-models/flan-t5-small-caikit', + }); + expect(uri).toEqual( + 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F', + ); + }); + + it('falls back to null if endpoint is empty', () => { + const uri = objectStorageFieldsToUri({ + endpoint: '', + bucket: 'test-bucket', + region: 'us-east-1', + path: 'demo-models/flan-t5-small-caikit', + }); + expect(uri).toEqual(null); + }); + + it('falls back to null if bucket is empty', () => { + const uri = objectStorageFieldsToUri({ + endpoint: 'http://s3.amazonaws.com/', + bucket: '', + region: 'us-east-1', + path: 'demo-models/flan-t5-small-caikit', + }); + expect(uri).toEqual(null); + }); + + it('falls back to null if path is empty', () => { + const uri = objectStorageFieldsToUri({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + region: 'us-east-1', + path: '', + }); + expect(uri).toEqual(null); + }); +}); + +describe('uriToObjectStorageFields', () => { + it('converts URI to fields with all params present', () => { + const fields = uriToObjectStorageFields( + 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', + ); + expect(fields).toEqual({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + region: 'us-east-1', + path: 'demo-models/flan-t5-small-caikit', + } satisfies ObjectStorageFields); + }); + + it('converts URI to fields with region missing', () => { + const fields = uriToObjectStorageFields( + 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F', + ); + expect(fields).toEqual({ + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + path: 'demo-models/flan-t5-small-caikit', + region: undefined, + } satisfies ObjectStorageFields); + }); + + it('falls back to null if endpoint is missing', () => { + const fields = uriToObjectStorageFields('s3://test-bucket/demo-models/flan-t5-small-caikit'); + expect(fields).toBeNull(); + }); + + it('falls back to null if path is missing', () => { + const fields = uriToObjectStorageFields( + 's3://test-bucket/?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', + ); + expect(fields).toBeNull(); + }); + + it('falls back to null if bucket is missing', () => { + const fields = uriToObjectStorageFields( + 's3://?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', + ); + expect(fields).toBeNull(); + }); + + it('falls back to null if the URI is malformed', () => { + const fields = uriToObjectStorageFields('test-bucket/demo-models/flan-t5-small-caikit'); + expect(fields).toBeNull(); + }); +}); + +describe('getLastCreatedItem', () => { + it('returns the latest item correctly', () => { + const items = [ + { + foo: 'a', + createTimeSinceEpoch: '1712234877179', // Apr 04 2024 + }, + { + foo: 'b', + createTimeSinceEpoch: '1723659611927', // Aug 14 2024 + }, + ]; + expect(getLastCreatedItem(items)).toBe(items[1]); + }); + + it('returns first item if items have no createTimeSinceEpoch', () => { + const items = [ + { foo: 'a', createTimeSinceEpoch: undefined }, + { foo: 'b', createTimeSinceEpoch: undefined }, + ]; + expect(getLastCreatedItem(items)).toBe(items[0]); + }); +}); + +describe('Filter model state', () => { + const models: RegisteredModel[] = [ + mockRegisteredModel({ name: 'Test 1', state: ModelState.ARCHIVED }), + mockRegisteredModel({ + name: 'Test 2', + state: ModelState.LIVE, + description: 'Description2', + }), + mockRegisteredModel({ name: 'Test 3', state: ModelState.ARCHIVED }), + mockRegisteredModel({ name: 'Test 4', state: ModelState.ARCHIVED }), + mockRegisteredModel({ name: 'Test 5', state: ModelState.LIVE }), + ]; + + describe('filterArchiveModels', () => { + it('should filter out only the archived versions', () => { + const archivedModels = filterArchiveModels(models); + expect(archivedModels).toEqual([models[0], models[2], models[3]]); + }); + + it('should return an empty array if the input array is empty', () => { + const result = filterArchiveModels([]); + expect(result).toEqual([]); + }); + }); + + describe('filterLiveModels', () => { + it('should filter out only the live models', () => { + const liveModels = filterLiveModels(models); + expect(liveModels).toEqual([models[1], models[4]]); + }); + + it('should return an empty array if the input array is empty', () => { + const result = filterLiveModels([]); + expect(result).toEqual([]); + }); + }); +}); + +describe('Filter model version state', () => { + const modelVersions: ModelVersion[] = [ + mockModelVersion({ name: 'Test 1', state: ModelState.ARCHIVED }), + mockModelVersion({ + name: 'Test 2', + state: ModelState.LIVE, + description: 'Description2', + }), + mockModelVersion({ name: 'Test 3', author: 'Author3', state: ModelState.ARCHIVED }), + mockModelVersion({ name: 'Test 4', state: ModelState.ARCHIVED }), + mockModelVersion({ name: 'Test 5', state: ModelState.LIVE }), + ]; + + describe('filterArchiveVersions', () => { + it('should filter out only the archived versions', () => { + const archivedVersions = filterArchiveVersions(modelVersions); + expect(archivedVersions).toEqual([modelVersions[0], modelVersions[2], modelVersions[3]]); + }); + + it('should return an empty array if the input array is empty', () => { + const result = filterArchiveVersions([]); + expect(result).toEqual([]); + }); + }); + + describe('filterLiveVersions', () => { + it('should filter out only the live versions', () => { + const liveVersions = filterLiveVersions(modelVersions); + expect(liveVersions).toEqual([modelVersions[1], modelVersions[4]]); + }); + + it('should return an empty array if the input array is empty', () => { + const result = filterLiveVersions([]); + expect(result).toEqual([]); + }); + }); +}); diff --git a/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx b/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx index 9b1309dbf..941881b91 100644 --- a/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx +++ b/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx @@ -9,6 +9,7 @@ export type ModelRegistrySelectorContextType = { modelRegistries: ModelRegistry[]; preferredModelRegistry: ModelRegistry | undefined; updatePreferredModelRegistry: (modelRegistry: ModelRegistry | undefined) => void; + //refreshRulesReview: () => void; TODO: [Midstream] Reimplement this }; type ModelRegistrySelectorContextProviderProps = { @@ -21,6 +22,7 @@ export const ModelRegistrySelectorContext = React.createContext undefined, + //refreshRulesReview: () => undefined, }); export const ModelRegistrySelectorContextProvider: React.FC< @@ -34,6 +36,8 @@ export const ModelRegistrySelectorContextProvider: React.FC< const EnabledModelRegistrySelectorContextProvider: React.FC< ModelRegistrySelectorContextProviderProps > = ({ children }) => { + // TODO: [Midstream] Add area check for enablement + const queryParams = useQueryParamNamespaces(); const [modelRegistries, isLoaded, error] = useModelRegistries(queryParams); @@ -49,6 +53,7 @@ const EnabledModelRegistrySelectorContextProvider: React.FC< modelRegistries, preferredModelRegistry: preferredModelRegistry ?? firstModelRegistry ?? undefined, updatePreferredModelRegistry: setPreferredModelRegistry, + // refreshRulesReview, }), [isLoaded, error, modelRegistries, preferredModelRegistry, firstModelRegistry], ); diff --git a/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts b/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts index aefe82671..18df6295e 100644 --- a/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts +++ b/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts @@ -19,11 +19,13 @@ const mockModelRegistryAPIs: ModelRegistryAPIs = { createModelArtifactForModelVersion: jest.fn(), getRegisteredModel: jest.fn(), getModelVersion: jest.fn(), + listModelVersions: jest.fn(), listRegisteredModels: jest.fn(), getModelVersionsByRegisteredModel: jest.fn(), getModelArtifactsByModelVersion: jest.fn(), patchRegisteredModel: jest.fn(), patchModelVersion: jest.fn(), + patchModelArtifact: jest.fn(), }; describe('useModelArtifactsByVersionId', () => { diff --git a/clients/ui/frontend/src/app/hooks/useModelVersions.ts b/clients/ui/frontend/src/app/hooks/useModelVersions.ts new file mode 100644 index 000000000..d4b59b73c --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelVersions.ts @@ -0,0 +1,27 @@ +import * as React from 'react'; +import useFetchState, { + FetchState, + FetchStateCallbackPromise, +} from '~/shared/utilities/useFetchState'; +import { ModelVersionList } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useModelVersions = (): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + const callback = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + return api.listModelVersions(opts).then((r) => r); + }, + [api, apiAvailable], + ); + return useFetchState( + callback, + { items: [], size: 0, pageSize: 0, nextPageToken: '' }, + { initialPromisePurity: true }, + ); +}; + +export default useModelVersions; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx index 0e6cac17a..1066ab20d 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx @@ -1,12 +1,12 @@ import * as React from 'react'; import { Navigate, Outlet, useParams } from 'react-router-dom'; -import { Bullseye, Alert, Popover, List, ListItem, Button } from '@patternfly/react-core'; -import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; +import { Bullseye, Alert } from '@patternfly/react-core'; import ApplicationsPage from '~/shared/components/ApplicationsPage'; import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; import { ProjectObjectType, typedEmptyImage } from '~/shared/components/design/utils'; import { ModelRegistryContextProvider } from '~/app/context/ModelRegistryContext'; import TitleWithIcon from '~/shared/components/design/TitleWithIcon'; +import WhosMyAdministrator from '~/shared/components/WhosMyAdministrator'; import EmptyModelRegistryState from './screens/components/EmptyModelRegistryState'; import InvalidModelRegistry from './screens/InvalidModelRegistry'; import ModelRegistrySelectorNavigator from './screens/ModelRegistrySelectorNavigator'; @@ -27,7 +27,6 @@ const ModelRegistryCoreLoader: React.FC = ({ getInvalidRedirectPath, }) => { const { modelRegistry } = useParams<{ modelRegistry: string }>(); - const { modelRegistriesLoaded, modelRegistriesLoadError, @@ -69,27 +68,7 @@ const ModelRegistryCoreLoader: React.FC = ({ headerIcon={() => ( )} - customAction={ - - - The person who gave you your username, or who helped you to log in for the first - time - - Someone in your IT department or help desk - A project manager or developer - - } - > - - - } + customAction={} /> ), headerContent: null, @@ -104,7 +83,6 @@ const ModelRegistryCoreLoader: React.FC = ({ ); } - // They ended up on a non-valid project path renderStateProps = { empty: true, diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx index f6fd0f486..d23e5feb8 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import { Navigate, Route, Routes } from 'react-router-dom'; +import { isPlatformDefault } from '~/shared/utilities/const'; import ModelRegistry from './screens/ModelRegistry'; import ModelRegistryCoreLoader from './ModelRegistryCoreLoader'; import { modelRegistryUrl } from './screens/routeUtils'; @@ -44,6 +45,14 @@ const ModelRegistryRoutes: React.FC = () => ( path={ModelVersionDetailsTab.DETAILS} element={} /> + {isPlatformDefault() && ( + + } + /> + )} } /> @@ -56,6 +65,18 @@ const ModelRegistryRoutes: React.FC = () => ( } /> + {isPlatformDefault() && ( + + } + /> + )} + } /> } /> @@ -86,6 +107,18 @@ const ModelRegistryRoutes: React.FC = () => ( } /> + {isPlatformDefault() && ( + + } + /> + )} + } /> } /> diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx index 60a11b82c..115c5cd3f 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup.tsx @@ -55,12 +55,15 @@ const ModelPropertiesDescriptionListGroup: React.FC} iconPosition="start" isDisabled={isAdding || isSavingEdits} - onClick={() => setIsAdding(true)} + onClick={() => { + setIsShowingMoreProperties(true); + setIsAdding(true); + }} > Add property @@ -120,6 +123,7 @@ const ModelPropertiesDescriptionListGroup: React.FC setIsShowingMoreProperties(!isShowingMoreProperties)} > {isShowingMoreProperties diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx index 9cde58d83..7d92932f4 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx @@ -93,12 +93,13 @@ const ModelPropertiesTableRow: React.FC = ({ const propertyKeyInput = ( setUnsavedKey(str)} validated={keyValidationError ? 'error' : 'default'} @@ -107,7 +108,7 @@ const ModelPropertiesTableRow: React.FC = ({ const propertyValueInput = ( = ({ + /> + /> ) : ( @@ -195,6 +194,7 @@ const ModelPropertiesTableRow: React.FC = ({ popperProps={{ direction: 'up' }} items={[ { title: 'Edit', onClick: onEditClick, isDisabled: isSavingEdits }, + { isSeparator: true }, { title: 'Delete', onClick: onDeleteClick, isDisabled: isSavingEdits }, ]} /> diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx index 4c55f9f8d..d089b444f 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx @@ -3,7 +3,7 @@ import ApplicationsPage from '~/shared/components/ApplicationsPage'; import TitleWithIcon from '~/shared/components/design/TitleWithIcon'; import { ProjectObjectType } from '~/shared/components/design/utils'; import useRegisteredModels from '~/app/hooks/useRegisteredModels'; -import { filterLiveModels } from '~/app/pages/modelRegistry/screens/utils'; +import useModelVersions from '~/app/hooks/useModelVersions'; import ModelRegistrySelectorNavigator from './ModelRegistrySelectorNavigator'; import RegisteredModelListView from './RegisteredModels/RegisteredModelListView'; import { modelRegistryUrl } from './routeUtils'; @@ -20,7 +20,16 @@ type ModelRegistryProps = Omit< >; const ModelRegistry: React.FC = ({ ...pageProps }) => { - const [registeredModels, loaded, loadError, refresh] = useRegisteredModels(); + const [registeredModels, modelsLoaded, modelsLoadError, refreshModels] = useRegisteredModels(); + const [modelVersions, versionsLoaded, versionsLoadError, refreshVersions] = useModelVersions(); + + const loaded = modelsLoaded && versionsLoaded; + const loadError = modelsLoadError || versionsLoadError; + + const refresh = React.useCallback(() => { + refreshModels(); + refreshVersions(); + }, [refreshModels, refreshVersions]); return ( = ({ ...pageProps }) => { removeChildrenTopPadding > diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx index 303135b4d..267d9498a 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx @@ -6,23 +6,21 @@ import { DescriptionListDescription, DescriptionListGroup, DescriptionListTerm, - Divider, Flex, FlexItem, Icon, - MenuToggle, Popover, - Select, - SelectGroup, - SelectList, - SelectOption, + PopoverPosition, Tooltip, } from '@patternfly/react-core'; +import text from '@patternfly/react-styles/css/utilities/Text/text'; import truncateStyles from '@patternfly/react-styles/css/components/Truncate/truncate'; import { InfoCircleIcon, BlueprintIcon } from '@patternfly/react-icons'; import { useBrowserStorage } from '~/shared/components/browserStorage'; import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; import { ModelRegistry } from '~/app/types'; +import SimpleSelect, { SimpleSelectOption } from '~/shared/components/SimpleSelect'; +import WhosMyAdministrator from '~/shared/components/WhosMyAdministrator'; const MODEL_REGISTRY_FAVORITE_STORAGE_KEY = 'kubeflow.dashboard.model.registry.favorite'; @@ -42,7 +40,6 @@ const ModelRegistrySelector: React.FC = ({ ); const selection = modelRegistries.find((mr) => mr.name === modelRegistry); - const [isOpen, setIsOpen] = React.useState(false); const [favorites, setFavorites] = useBrowserStorage( MODEL_REGISTRY_FAVORITE_STORAGE_KEY, [], @@ -53,7 +50,7 @@ const ModelRegistrySelector: React.FC = ({ const toggleLabel = modelRegistries.length === 0 ? 'No model registries' : selectionDisplayName; const getMRSelectDescription = (mr: ModelRegistry) => { - const desc = mr.description || mr.name; + const desc = mr.description || ''; if (!desc) { return; } @@ -74,69 +71,48 @@ const ModelRegistrySelector: React.FC = ({ ); }; - const options = [ - - - {modelRegistries.map((mr) => ( - - {mr.displayName} - - ))} - - , - ]; - - const createFavorites = (favIds: string[]) => { - const favorite: JSX.Element[] = []; + const allOptions: SimpleSelectOption[] = modelRegistries.map((mr) => ({ + key: mr.name, + label: mr.name, + dropdownLabel: mr.displayName, + description: getMRSelectDescription(mr), + isFavorited: favorites.includes(mr.name), + })); - options.forEach((item) => { - if (item.type === SelectList) { - item.props.children.filter( - (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), - ); - } else if (item.type === SelectGroup) { - item.props.children.props.children.filter( - (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), - ); - } else if (favIds.includes(item.props.value)) { - favorite.push(item); - } - }); - - return favorite; - }; + const favoriteOptions = (favIds: string[]) => + allOptions.filter((option) => favIds.includes(option.key)); const selector = ( - + /> ); if (primary) { @@ -166,29 +132,52 @@ const ModelRegistrySelector: React.FC = ({ } return ( - - - - - + + + + + + + + Model registry + + {selector} + {selection && ( - Model registry + + + Description + + {selection.description || 'No description'} + + + + } + > + + - {selector} - {selection && selection.description && ( - - - - - - )} - + )} + + + ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx index 7c606fd31..262bef4eb 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx @@ -11,15 +11,17 @@ const ModelRegistrySelectorNavigator: React.FC { const navigate = useNavigate(); - const { modelRegistry } = useParams<{ modelRegistry: string }>(); + const { modelRegistry: currentModelRegistry } = useParams<{ modelRegistry: string }>(); return ( { - navigate(getRedirectPath(modelRegistryName)); + if (modelRegistryName !== currentModelRegistry) { + navigate(getRedirectPath(modelRegistryName)); + } }} - modelRegistry={modelRegistry ?? ''} + modelRegistry={currentModelRegistry ?? ''} /> ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx index 61523b501..53ff6ba22 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetails.tsx @@ -90,7 +90,7 @@ const ModelVersionsDetails: React.FC = ({ tab, ...page /> - + ) diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx index 740f0538a..8b1863500 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsHeaderActions.tsx @@ -1,18 +1,27 @@ import * as React from 'react'; -import { Dropdown, DropdownList, MenuToggle, DropdownItem } from '@patternfly/react-core'; +import { + Dropdown, + DropdownList, + MenuToggle, + DropdownItem, + ButtonVariant, + ActionList, +} from '@patternfly/react-core'; import { useNavigate } from 'react-router'; import { ModelState, ModelVersion } from '~/app/types'; import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; import { ArchiveModelVersionModal } from '~/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal'; -import { modelVersionArchiveDetailsUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; +import { modelVersionListUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; interface ModelVersionsDetailsHeaderActionsProps { mv: ModelVersion; + hasDeployment?: boolean; } const ModelVersionsDetailsHeaderActions: React.FC = ({ mv, + hasDeployment = false, }) => { const { apiState } = React.useContext(ModelRegistryContext); const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); @@ -23,15 +32,15 @@ const ModelVersionsDetailsHeaderActions: React.FC(null); return ( - <> + setOpenActionDropdown(false)} onOpenChange={(open) => setOpenActionDropdown(open)} - popperProps={{ position: 'right' }} + popperProps={{ position: 'right', appendTo: 'inline' }} toggle={(toggleRef) => ( setOpenActionDropdown(!isOpenActionDropdown)} isExpanded={isOpenActionDropdown} @@ -44,41 +53,40 @@ const ModelVersionsDetailsHeaderActions: React.FC setIsArchiveModalOpen(true)} + tooltipProps={ + hasDeployment ? { content: 'Deployed model versions cannot be archived' } : undefined + } ref={tooltipRef} > - Archive version + Archive model version - setIsArchiveModalOpen(false)} - onSubmit={() => - apiState.api - .patchModelVersion( - {}, - { - state: ModelState.ARCHIVED, - }, - mv.id, - ) - .then(() => - navigate( - modelVersionArchiveDetailsUrl( - mv.id, - mv.registeredModelId, - preferredModelRegistry?.name, - ), - ), - ) - } - isOpen={isArchiveModalOpen} - modelVersionName={mv.name} - /> - + {isArchiveModalOpen ? ( + setIsArchiveModalOpen(false)} + onSubmit={() => + apiState.api + .patchModelVersion( + {}, + { + state: ModelState.ARCHIVED, + }, + mv.id, + ) + .then(() => + navigate(modelVersionListUrl(mv.registeredModelId, preferredModelRegistry?.name)), + ) + } + modelVersionName={mv.name} + /> + ) : null} + ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx index 2747ce088..4aaa05097 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsTabs.tsx @@ -34,8 +34,8 @@ const ModelVersionDetailsTabs: React.FC = ({ data-testid="model-versions-details-tab" > = ({ isArchiveVersion, refresh, }) => { - const [modelArtifact] = useModelArtifactsByVersionId(mv.id); + const [modelArtifacts, modelArtifactsLoaded, modelArtifactsLoadError, refreshModelArtifacts] = + useModelArtifactsByVersionId(mv.id); + + const modelArtifact = modelArtifacts.items.length ? modelArtifacts.items[0] : null; const { apiState } = React.useContext(ModelRegistryContext); - const storageFields = uriToObjectStorageFields(modelArtifact.items[0]?.uri || ''); + const storageFields = uriToObjectStorageFields(modelArtifact?.uri || ''); + + if (!modelArtifactsLoaded) { + return ( + + + + ); + } + const handleVersionUpdate = async (updatePromise: Promise): Promise => { + await updatePromise; + + if (!mv.registeredModelId) { + return; + } + + await bumpRegisteredModelTimestamp(apiState.api, mv.registeredModelId); + refresh(); + }; + + const handleArtifactUpdate = async (updatePromise: Promise): Promise => { + try { + await updatePromise; + await bumpBothTimestamps(apiState.api, mv.id, mv.registeredModelId); + refreshModelArtifacts(); + } catch (error) { + throw new Error( + `Failed to update artifact: ${error instanceof Error ? error.message : String(error)}`, + ); + } + }; return ( = ({ - apiState.api - .patchModelVersion( - {}, - { - description: value, - }, - mv.id, - ) - .then(refresh) + handleVersionUpdate(apiState.api.patchModelVersion({}, { description: value }, mv.id)) } /> - apiState.api - .patchModelVersion( + title="Labels" + contentWhenEmpty="No labels" + onLabelsChange={(editedLabels) => + handleVersionUpdate( + apiState.api.patchModelVersion( {}, - { - customProperties: mergeUpdatedLabels(mv.customProperties, editedLabels), - }, + { customProperties: mergeUpdatedLabels(mv.customProperties, editedLabels) }, mv.id, - ) - .then(refresh) + ), + ) } + data-testid="model-version-labels" /> = ({ - + <Title style={{ margin: '1em 0' }} headingLevel={ContentVariants.h3}> Model location - - {storageFields && ( - <> - - - - - - - - - - - - - - )} - {!storageFields && ( - <> - - - - - )} - - {modelArtifact.items[0]?.modelFormatName} - + {modelArtifactsLoadError ? ( + + {modelArtifactsLoadError.message} + + ) : ( + <> + + {storageFields && ( + <> + + + + + + + + + + + + + + )} + {!storageFields && ( + <> + + + + + )} + + + + Source model format + + + + handleArtifactUpdate( + apiState.api.patchModelArtifact( + {}, + { modelFormatName: value }, + modelArtifact?.id || '', + ), + ) + } + title="Model Format" + contentWhenEmpty="No model format specified" + /> + + handleArtifactUpdate( + apiState.api.patchModelArtifact( + {}, + { modelFormatVersion: newVersion }, + modelArtifact?.id || '', + ), + ) + } + title="Version" + contentWhenEmpty="No source model format version" + /> + + + )} + + - } + popover="The author is the user who registered the model version." > {mv.author} diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionSelector.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionSelector.tsx index 119b9a843..bdf2556b9 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionSelector.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionSelector.tsx @@ -14,6 +14,7 @@ import { } from '@patternfly/react-core'; import { ModelVersion } from '~/app/types'; import useModelVersionsByRegisteredModel from '~/app/hooks/useModelVersionsByRegisteredModel'; +import { filterLiveVersions } from '~/app/utils'; type ModelVersionSelectorProps = { rmId?: string; @@ -33,8 +34,9 @@ const ModelVersionSelector: React.FC = ({ const menuRef = React.useRef(null); const [modelVersions] = useModelVersionsByRegisteredModel(rmId); + const liveModelVersions = filterLiveVersions(modelVersions.items); - const menuListItems = modelVersions.items + const menuListItems = liveModelVersions .filter((item) => !input || item.name.toLowerCase().includes(input.toString().toLowerCase())) .map((mv, index) => ( @@ -42,7 +44,7 @@ const ModelVersionSelector: React.FC = ({ )); - if (input && modelVersions.size === 0) { + if (input && liveModelVersions.length === 0) { menuListItems.push( No results found @@ -74,8 +76,8 @@ const ModelVersionSelector: React.FC = ({ /> - - {`Type a name to search your ${modelVersions.size} versions.`} + + {`Type a name to search your ${liveModelVersions.length} versions.`} diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/const.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/const.ts index ded505d14..30b76e22b 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/const.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionDetails/const.ts @@ -1,7 +1,9 @@ export enum ModelVersionDetailsTab { DETAILS = 'details', + DEPLOYMENTS = 'deployments', } export enum ModelVersionDetailsTabTitle { DETAILS = 'Details', + DEPLOYMENTS = 'Deployments', } 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 index 0f97fc080..89b8b280d 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelDetailsView.tsx @@ -3,7 +3,7 @@ import { ClipboardCopy, DescriptionList, Flex, FlexItem, Content } from '@patter import { RegisteredModel } from '~/app/types'; import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; import EditableTextDescriptionListGroup from '~/shared/components/EditableTextDescriptionListGroup'; -import EditableLabelsDescriptionListGroup from '~/shared/components/EditableLabelsDescriptionListGroup'; +import { EditableLabelsDescriptionListGroup } from '~/shared/components/EditableLabelsDescriptionListGroup'; import { getLabels, mergeUpdatedLabels } from '~/app/pages/modelRegistry/screens/utils'; import ModelPropertiesDescriptionListGroup from '~/app/pages/modelRegistry/screens/ModelPropertiesDescriptionListGroup'; import DashboardDescriptionListGroup from '~/shared/components/DashboardDescriptionListGroup'; @@ -30,6 +30,7 @@ const ModelDetailsView: React.FC = ({ = ({ labels={getLabels(rm.customProperties)} isArchive={isArchiveModel} allExistingKeys={Object.keys(rm.customProperties)} - saveEditedLabels={(editedLabels) => + title="Labels" + contentWhenEmpty="No labels" + onLabelsChange={(editedLabels) => apiState.api .patchRegisteredModel( {}, @@ -86,7 +89,10 @@ const ModelDetailsView: React.FC = ({ {rm.id} - + {rm.owner || '-'} 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 index fd927bddb..b8a11ce4f 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx @@ -30,25 +30,31 @@ import { modelVersionArchiveUrl, registerVersionForModelUrl, } from '~/app/pages/modelRegistry/screens/routeUtils'; -import { asEnumMember } from '~/app/utils'; +import { asEnumMember } from '~/shared/utilities/utils'; import ModelVersionsTable from '~/app/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable'; import SimpleSelect from '~/shared/components/SimpleSelect'; import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; import { isMUITheme } from '~/shared/utilities/const'; +import { filterArchiveVersions, filterLiveVersions } from '~/app/utils'; type ModelVersionListViewProps = { modelVersions: ModelVersion[]; - registeredModel?: RegisteredModel; + registeredModel: RegisteredModel; isArchiveModel?: boolean; refresh: () => void; }; const ModelVersionListView: React.FC = ({ - modelVersions: unfilteredModelVersions, + modelVersions, registeredModel: rm, isArchiveModel, refresh, }) => { + const unfilteredModelVersions = isArchiveModel + ? modelVersions + : filterLiveVersions(modelVersions); + + const archiveModelVersions = filterArchiveVersions(modelVersions); const navigate = useNavigate(); const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); @@ -61,7 +67,7 @@ const ModelVersionListView: React.FC = ({ React.useState(false); const filteredModelVersions = filterModelVersions(unfilteredModelVersions, search, searchType); - const date = rm?.lastUpdateTimeSinceEpoch && new Date(parseInt(rm.lastUpdateTimeSinceEpoch)); + const date = rm.lastUpdateTimeSinceEpoch && new Date(parseInt(rm.lastUpdateTimeSinceEpoch)); if (unfilteredModelVersions.length === 0) { if (isArchiveModel) { @@ -75,7 +81,7 @@ const ModelVersionListView: React.FC = ({ alt="missing version" /> )} - description={`${rm?.name} has no registered versions.`} + description={`${rm.name} has no registered versions.`} /> ); } @@ -89,14 +95,16 @@ const ModelVersionListView: React.FC = ({ alt="missing version" /> )} - description={`${rm?.name} has no registered versions. Register a version to this model.`} + 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)); + navigate(registerVersionForModelUrl(rm.id, preferredModelRegistry?.name)); }} + secondaryActionText={ + archiveModelVersions.length !== 0 ? 'View archived versions' : undefined + } secondaryActionOnClick={() => { - navigate(modelVersionArchiveUrl(rm?.id, preferredModelRegistry?.name)); + navigate(modelVersionArchiveUrl(rm.id, preferredModelRegistry?.name)); }} /> ); @@ -186,9 +194,9 @@ const ModelVersionListView: React.FC = ({ <> } description={} @@ -89,7 +85,7 @@ const ModelVersionsArchiveDetails: React.FC = /> )} - {mv !== null && ( + {mv !== null && isRestoreModalOpen ? ( setIsRestoreModalOpen(false)} onSubmit={() => @@ -103,10 +99,9 @@ const ModelVersionsArchiveDetails: React.FC = ) .then(() => navigate(modelVersionUrl(mv.id, rm?.id, preferredModelRegistry?.name))) } - isOpen={isRestoreModalOpen} modelVersionName={mv.name} /> - )} + ) : null} ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx index 3d57fe309..7bf680180 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchive.tsx @@ -7,7 +7,7 @@ import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelecto import useRegisteredModelById from '~/app/hooks/useRegisteredModelById'; import useModelVersionsByRegisteredModel from '~/app/hooks/useModelVersionsByRegisteredModel'; import { registeredModelUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; -import { filterArchiveVersions } from '~/app/pages/modelRegistry/screens/utils'; +import { filterArchiveVersions } from '~/app/utils'; import ModelVersionsArchiveListView from './ModelVersionsArchiveListView'; type ModelVersionsArchiveProps = Omit< diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveListView.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveListView.tsx index 097d60f1a..bc96f4082 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveListView.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveListView.tsx @@ -8,11 +8,11 @@ import { ToolbarItem, ToolbarToggleGroup, } from '@patternfly/react-core'; -import { FilterIcon } from '@patternfly/react-icons'; +import { FilterIcon, SearchIcon } from '@patternfly/react-icons'; import { ModelVersion } from '~/app/types'; import { SearchType } from '~/shared/components/DashboardSearchField'; import SimpleSelect from '~/shared/components/SimpleSelect'; -import { asEnumMember } from '~/app/utils'; +import { asEnumMember } from '~/shared/utilities/utils'; import { filterModelVersions } from '~/app/pages/modelRegistry/screens/utils'; import EmptyModelRegistryState from '~/app/pages/modelRegistry/screens/components/EmptyModelRegistryState'; import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; @@ -38,9 +38,10 @@ const ModelVersionsArchiveListView: React.FC if (unfilteredmodelVersions.length === 0) { return ( ); } diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveTable.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveTable.tsx index 932f50480..d44f87e71 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveTable.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersionsArchive/ModelVersionsArchiveTable.tsx @@ -22,10 +22,11 @@ const ModelVersionsArchiveTable: React.FC = ({ data={modelVersions} columns={mvColumns} toolbarContent={toolbarContent} - enablePagination="compact" + enablePagination + onClearFilters={clearFilters} emptyTableView={} defaultSortColumn={1} - rowRenderer={(mv: ModelVersion) => ( + rowRenderer={(mv) => ( )} /> diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModel.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModel.tsx index 230e9b7cc..a93fc98b8 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModel.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModel.tsx @@ -4,6 +4,9 @@ import { BreadcrumbItem, Form, FormGroup, + FormHelperText, + HelperText, + HelperTextItem, PageSection, Stack, StackItem, @@ -17,36 +20,53 @@ import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormField import FormSection from '~/shared/components/pf-overrides/FormSection'; import ApplicationsPage from '~/shared/components/ApplicationsPage'; import { modelRegistryUrl, registeredModelUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; -import { ValueOf } from '~/shared/typeHelpers'; import { isMUITheme } from '~/shared/utilities/const'; -import { useRegisterModelData, RegistrationCommonFormData } from './useRegisterModelData'; -import { isRegisterModelSubmitDisabled, registerModel } from './utils'; -import { useRegistrationCommonState } from './useRegistrationCommonState'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; +import { AppContext } from '~/app/AppContext'; +import { useRegisterModelData } from './useRegisterModelData'; +import { isNameValid, isRegisterModelSubmitDisabled, registerModel } from './utils'; import RegistrationCommonFormSections from './RegistrationCommonFormSections'; import RegistrationFormFooter from './RegistrationFormFooter'; +import { MR_CHARACTER_LIMIT, SubmitLabel } from './const'; +import PrefilledModelRegistryField from './PrefilledModelRegistryField'; const RegisterModel: React.FC = () => { const { modelRegistry: mrName } = useParams(); const navigate = useNavigate(); - - const { isSubmitting, submitError, setSubmitError, handleSubmit, apiState, author } = - useRegistrationCommonState(); - + const { apiState } = React.useContext(ModelRegistryContext); + const { user } = React.useContext(AppContext); + const author = user.userId || ''; + const [isSubmitting, setIsSubmitting] = React.useState(false); + const [submitError, setSubmitError] = React.useState(undefined); const [formData, setData] = useRegisterModelData(); + const isModelNameValid = isNameValid(formData.modelName); const isSubmitDisabled = isSubmitting || isRegisterModelSubmitDisabled(formData); const { modelName, modelDescription } = formData; + const [registeredModelName, setRegisteredModelName] = React.useState(''); + const [versionName, setVersionName] = React.useState(''); + const [errorName, setErrorName] = React.useState(undefined); - const onSubmit = () => - handleSubmit(async () => { - const { registeredModel } = await registerModel(apiState, formData, author); + const handleSubmit = async () => { + setIsSubmitting(true); + setSubmitError(undefined); + + const { + data: { registeredModel, modelVersion, modelArtifact }, + errors, + } = await registerModel(apiState, formData, author); + if (registeredModel && modelVersion && modelArtifact) { navigate(registeredModelUrl(registeredModel.id, mrName)); - }); + } else if (Object.keys(errors).length > 0) { + setIsSubmitting(false); + setRegisteredModelName(formData.modelName); + setVersionName(formData.versionName); + const resourceName = Object.keys(errors)[0]; + setErrorName(resourceName); + setSubmitError(errors[resourceName]); + } + }; const onCancel = () => navigate(modelRegistryUrl(mrName)); - const modelRegistryInput = ( - - ); - const modelNameInput = ( {
- - {isMUITheme() ? ( - - ) : ( - modelRegistryInput - )} - + { ) : ( modelNameInput )} + {!isModelNameValid && ( + + + + Cannot exceed {MR_CHARACTER_LIMIT} characters + + + + )} { , - ) => setData(propKey, propValue)} + setData={setData} isFirstVersion /> @@ -137,13 +152,15 @@ const RegisterModel: React.FC = () => {
); diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModelErrors.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModelErrors.tsx new file mode 100644 index 000000000..d14b8548f --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterModelErrors.tsx @@ -0,0 +1,114 @@ +import React from 'react'; +import { Alert, AlertActionCloseButton, StackItem } from '@patternfly/react-core'; +import { ErrorName, SubmitLabel } from './const'; + +type RegisterModelErrorProp = { + submitLabel: string; + submitError: Error; + errorName?: string; + versionName?: string; + modelName?: string; +}; + +const RegisterModelErrors: React.FC = ({ + submitLabel, + submitError, + errorName, + versionName = '', + modelName = '', +}) => { + const [showAlert, setShowAlert] = React.useState(true); + + if (submitLabel === SubmitLabel.REGISTER_MODEL && errorName === ErrorName.MODEL_VERSION) { + return ( + <> + {showAlert && ( + + setShowAlert(false)} />} + /> + + )} + + + {submitError.message} + + + + ); + } + + if (submitLabel === SubmitLabel.REGISTER_VERSION && errorName === ErrorName.MODEL_VERSION) { + return ( + + + {submitError.message} + + + ); + } + + if (submitLabel === SubmitLabel.REGISTER_MODEL && errorName === ErrorName.MODEL_ARTIFACT) { + return ( + <> + {showAlert && ( + + setShowAlert(false)} />} + /> + + )} + + + {submitError.message} + + + + ); + } + + if (submitLabel === SubmitLabel.REGISTER_VERSION && errorName === ErrorName.MODEL_ARTIFACT) { + return ( + <> + {showAlert && ( + + setShowAlert(false)} />} + /> + + )} + + + {submitError.message} + + + + ); + } + + return ( + + + {submitError.message} + + + ); +}; +export default RegisterModelErrors; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterVersion.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterVersion.tsx index 6a3524588..e25072a8d 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterVersion.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisterVersion.tsx @@ -15,27 +15,31 @@ import { Link } from 'react-router-dom'; import ApplicationsPage from '~/shared/components/ApplicationsPage'; import { modelRegistryUrl, registeredModelUrl } from '~/app/pages/modelRegistry/screens/routeUtils'; import useRegisteredModels from '~/app/hooks/useRegisteredModels'; -import { ValueOf } from '~/shared/typeHelpers'; -import { filterLiveModels } from '~/app/pages/modelRegistry/screens/utils'; -import { RegistrationCommonFormData, useRegisterVersionData } from './useRegisterModelData'; +import { filterLiveModels } from '~/app/utils'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; +import { AppContext } from '~/app/AppContext'; +import { useRegisterVersionData } from './useRegisterModelData'; import { isRegisterVersionSubmitDisabled, registerVersion } from './utils'; import RegistrationCommonFormSections from './RegistrationCommonFormSections'; -import { useRegistrationCommonState } from './useRegistrationCommonState'; import PrefilledModelRegistryField from './PrefilledModelRegistryField'; import RegistrationFormFooter from './RegistrationFormFooter'; import RegisteredModelSelector from './RegisteredModelSelector'; import { usePrefillRegisterVersionFields } from './usePrefillRegisterVersionFields'; +import { SubmitLabel } from './const'; const RegisterVersion: React.FC = () => { const { modelRegistry: mrName, registeredModelId: prefilledRegisteredModelId } = useParams(); - const navigate = useNavigate(); - - const { isSubmitting, submitError, setSubmitError, handleSubmit, apiState, author } = - useRegistrationCommonState(); - + const { apiState } = React.useContext(ModelRegistryContext); + const { user } = React.useContext(AppContext); + const author = user.userId || ''; + const [isSubmitting, setIsSubmitting] = React.useState(false); const [formData, setData] = useRegisterVersionData(prefilledRegisteredModelId); const isSubmitDisabled = isSubmitting || isRegisterVersionSubmitDisabled(formData); + const [submitError, setSubmitError] = React.useState(undefined); + const [errorName, setErrorName] = React.useState(undefined); + const [versionName, setVersionName] = React.useState(''); + const { registeredModelId } = formData; const [allRegisteredModels, loadedRegisteredModels, loadRegisteredModelsError] = @@ -49,15 +53,29 @@ const RegisterVersion: React.FC = () => { setData, }); - const onSubmit = () => { + const handleSubmit = async () => { if (!registeredModel) { return; // We shouldn't be able to hit this due to form validation } - handleSubmit(async () => { - await registerVersion(apiState, registeredModel, formData, author); + setIsSubmitting(true); + setSubmitError(undefined); + + const { + data: { modelVersion, modelArtifact }, + errors, + } = await registerVersion(apiState, registeredModel, formData, author); + + if (modelVersion && modelArtifact) { navigate(registeredModelUrl(registeredModel.id, mrName)); - }); + } else if (Object.keys(errors).length > 0) { + const resourceName = Object.keys(errors)[0]; + setVersionName(formData.versionName); + setErrorName(resourceName); + setSubmitError(errors[resourceName]); + setIsSubmitting(false); + } }; + const onCancel = () => navigate( prefilledRegisteredModelId && registeredModel @@ -101,6 +119,7 @@ const RegisterVersion: React.FC = () => { { , - ) => setData(propKey, propValue)} + setData={setData} isFirstVersion={false} latestVersion={latestVersion} /> @@ -130,13 +146,14 @@ const RegisterVersion: React.FC = () => { ); diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx index 9ea05f4e8..80fed1ab7 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx @@ -1,9 +1,9 @@ import React from 'react'; import { FormGroup, TextInput } from '@patternfly/react-core'; -import { TypeaheadSelect, TypeaheadSelectOption } from '@patternfly/react-templates'; import { RegisteredModel } from '~/app/types'; import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; import { isMUITheme } from '~/shared/utilities/const'; +import TypeaheadSelect, { TypeaheadSelectOption } from '~/shared/components/TypeaheadSelect'; type RegisteredModelSelectorProps = { registeredModels: RegisteredModel[]; @@ -60,7 +60,8 @@ const RegisteredModelSelector: React.FC = ({ return ( setRegisteredModelId('')} + selectOptions={options} placeholder="Select a registered model" noOptionsFoundMessage={(filter) => `No results found for "${filter}"`} onSelect={(_event, selection) => { diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx index 8a1063259..e8e169477 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx @@ -9,8 +9,8 @@ import { HelperText, HelperTextItem, FormHelperText, - TextInputGroup, - TextInputGroupMain, + InputGroupItem, + InputGroupText, } from '@patternfly/react-core'; import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; import { UpdateObjectAtPropAndValue } from '~/shared/types'; @@ -21,25 +21,33 @@ import FormSection from '~/shared/components/pf-overrides/FormSection'; import { ModelVersion } from '~/app/types'; import { isMUITheme } from '~/shared/utilities/const'; import { ModelLocationType, RegistrationCommonFormData } from './useRegisterModelData'; +import { isNameValid } from './utils'; +import { MR_CHARACTER_LIMIT } from './const'; // import { ConnectionModal } from './ConnectionModal'; -type RegistrationCommonFormSectionsProps = { - formData: RegistrationCommonFormData; - setData: UpdateObjectAtPropAndValue; +type RegistrationCommonFormSectionsProps = { + formData: D; + setData: UpdateObjectAtPropAndValue; isFirstVersion: boolean; latestVersion?: ModelVersion; }; -const RegistrationCommonFormSections: React.FC = ({ +const RegistrationCommonFormSections = ({ formData, setData, isFirstVersion, latestVersion, -}) => { - // TODO: [Data connections] Check wether we should use data connections +}: RegistrationCommonFormSectionsProps): React.ReactNode => { // const [isAutofillModalOpen, setAutofillModalOpen] = React.useState(false); + const isVersionNameValid = isNameValid(formData.versionName); - // const connectionDataMap: Record = { + // const connectionDataMap: Record< + // string, + // keyof Pick< + // RegistrationCommonFormData, + // 'modelLocationEndpoint' | 'modelLocationBucket' | 'modelLocationRegion' + // > + // > = { // AWS_S3_ENDPOINT: 'modelLocationEndpoint', // AWS_S3_BUCKET: 'modelLocationBucket', // AWS_DEFAULT_REGION: 'modelLocationRegion', @@ -72,6 +80,7 @@ const RegistrationCommonFormSections: React.FC setData('versionName', value)} + validated={isVersionNameValid ? 'default' : 'error'} /> ); @@ -140,16 +149,16 @@ const RegistrationCommonFormSections: React.FC - + setData('modelLocationPath', value)} /> - + ); const uriInput = ( @@ -179,19 +188,22 @@ const RegistrationCommonFormSections: React.FC - {latestVersion && ( - - Current version is {latestVersion.name} - + {latestVersion && ( + + Current version is {latestVersion.name} + + )} + {!isVersionNameValid && ( + + + Cannot exceed {MR_CHARACTER_LIMIT} characters + + + )} - )} - + + {isMUITheme() ? ( ) : ( @@ -267,46 +279,50 @@ const RegistrationCommonFormSections: React.FC : regionInput} - {isMUITheme() ? : pathInput} - - + + + / + + + {isMUITheme() ? : pathInput} + + Enter a path to a model or folder. This path cannot point to a root folder. - - - )} - - - { - setData('modelLocationType', ModelLocationType.URI); - }} - label="URI" - id="location-type-uri" - /> - - - {modelLocationType === ModelLocationType.URI && ( - <> - - {isMUITheme() ? : uriInput} )} + { + setData('modelLocationType', ModelLocationType.URI); + }} + label="URI" + id="location-type-uri" + body={ + modelLocationType === ModelLocationType.URI && ( + <> + + {isMUITheme() ? : uriInput} + + + ) + } + /> - {/* setAutofillModalOpen(false)} - onSubmit={(connection) => { - fillObjectStorageByConnection(connection); - setAutofillModalOpen(false); - }} - /> */} + {/* {isAutofillModalOpen ? ( + setAutofillModalOpen(false)} + onSubmit={(connection) => { + fillObjectStorageByConnection(connection); + setAutofillModalOpen(false); + }} + /> + ) : null} */} ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationFormFooter.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationFormFooter.tsx index 86ca64789..3430df9ef 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationFormFooter.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/RegistrationFormFooter.tsx @@ -3,61 +3,74 @@ import { PageSection, Stack, StackItem, - Alert, - AlertActionCloseButton, - ActionGroup, Button, + ActionList, + ActionListItem, + ActionListGroup, } from '@patternfly/react-core'; +import RegisterModelErrors from './RegisterModelErrors'; type RegistrationFormFooterProps = { submitLabel: string; submitError?: Error; - setSubmitError: (e?: Error) => void; isSubmitDisabled: boolean; isSubmitting: boolean; onSubmit: () => void; onCancel: () => void; + errorName?: string; + versionName?: string; + modelName?: string; }; const RegistrationFormFooter: React.FC = ({ submitLabel, submitError, - setSubmitError, isSubmitDisabled, isSubmitting, onSubmit, onCancel, + errorName, + versionName, + modelName, }) => ( {submitError && ( - - setSubmitError(undefined)} />} - > - {submitError.message} - - + )} - - - - + + + + + + + + + + diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/const.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/const.ts new file mode 100644 index 000000000..a2ca8196b --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/const.ts @@ -0,0 +1,12 @@ +export const MR_CHARACTER_LIMIT = 128; + +export enum SubmitLabel { + REGISTER_MODEL = 'Register model', + REGISTER_VERSION = 'Register new version', +} + +export enum ErrorName { + REGISTERED_MODEL = 'registeredModel', + MODEL_VERSION = 'modelVersion', + MODEL_ARTIFACT = 'modelArtifact', +} diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts index be9e3601b..61538c5f1 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts @@ -1,10 +1,6 @@ import React from 'react'; import { RegisteredModel, ModelVersion, ModelArtifact } from '~/app/types'; -import { - filterLiveVersions, - getLastCreatedItem, - uriToObjectStorageFields, -} from '~/app/pages/modelRegistry/screens/utils'; +import { filterLiveVersions, getLastCreatedItem, uriToObjectStorageFields } from '~/app/utils'; import { UpdateObjectAtPropAndValue } from '~/shared/types'; import useModelArtifactsByVersionId from '~/app/hooks/useModelArtifactsByVersionId'; import useModelVersionsByRegisteredModel from '~/app/hooks/useModelVersionsByRegisteredModel'; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts deleted file mode 100644 index 8d3510d68..000000000 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/useRegistrationCommonState.ts +++ /dev/null @@ -1,41 +0,0 @@ -import React from 'react'; -import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; -import { ModelRegistryAPIState } from '~/app/hooks/useModelRegistryAPIState'; -import useUser from '~/app/hooks/useUser'; - -type RegistrationCommonState = { - isSubmitting: boolean; - setIsSubmitting: React.Dispatch>; - submitError: Error | undefined; - setSubmitError: React.Dispatch>; - handleSubmit: (doSubmit: () => Promise) => void; - apiState: ModelRegistryAPIState; - author: string; -}; - -export const useRegistrationCommonState = (): RegistrationCommonState => { - const [isSubmitting, setIsSubmitting] = React.useState(false); - const [submitError, setSubmitError] = React.useState(undefined); - - const { apiState } = React.useContext(ModelRegistryContext); - const { userId } = useUser(); - - const handleSubmit = (doSubmit: () => Promise) => { - setIsSubmitting(true); - setSubmitError(undefined); - doSubmit().catch((e: Error) => { - setIsSubmitting(false); - setSubmitError(e); - }); - }; - - return { - isSubmitting, - setIsSubmitting, - submitError, - setSubmitError, - handleSubmit, - apiState, - author: userId, - }; -}; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/utils.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/utils.ts index 22fef4d04..f3a74b541 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/utils.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisterModel/utils.ts @@ -6,45 +6,60 @@ import { RegisteredModel, } from '~/app/types'; import { ModelRegistryAPIState } from '~/app/hooks/useModelRegistryAPIState'; -import { objectStorageFieldsToUri } from '~/app/pages/modelRegistry/screens/utils'; +import { objectStorageFieldsToUri } from '~/app/utils'; import { ModelLocationType, RegisterModelFormData, RegisterVersionFormData, RegistrationCommonFormData, } from './useRegisterModelData'; +import { ErrorName, MR_CHARACTER_LIMIT } from './const'; export type RegisterModelCreatedResources = RegisterVersionCreatedResources & { - registeredModel: RegisteredModel; + registeredModel?: RegisteredModel; }; export type RegisterVersionCreatedResources = { - modelVersion: ModelVersion; - modelArtifact: ModelArtifact; + modelVersion?: ModelVersion; + modelArtifact?: ModelArtifact; }; export const registerModel = async ( apiState: ModelRegistryAPIState, formData: RegisterModelFormData, author: string, -): Promise => { - const registeredModel = await apiState.api.createRegisteredModel( - {}, - { - name: formData.modelName, - description: formData.modelDescription, - customProperties: {}, - owner: author, - state: ModelState.LIVE, - }, - ); - const { modelVersion, modelArtifact } = await registerVersion( - apiState, - registeredModel, - formData, - author, - ); - return { registeredModel, modelVersion, modelArtifact }; +): Promise<{ + data: RegisterModelCreatedResources; + errors: { [key: string]: Error | undefined }; +}> => { + let registeredModel; + const error: { [key: string]: Error | undefined } = {}; + try { + registeredModel = await apiState.api.createRegisteredModel( + {}, + { + name: formData.modelName, + description: formData.modelDescription, + customProperties: {}, + owner: author, + state: ModelState.LIVE, + }, + ); + } catch (e) { + if (e instanceof Error) { + error[ErrorName.REGISTERED_MODEL] = e; + } + return { data: { registeredModel }, errors: error }; + } + const { + data: { modelVersion, modelArtifact }, + errors, + } = await registerVersion(apiState, registeredModel, formData, author); + + return { + data: { registeredModel, modelVersion, modelArtifact }, + errors, + }; }; export const registerVersion = async ( @@ -52,39 +67,59 @@ export const registerVersion = async ( registeredModel: RegisteredModel, formData: Omit, author: string, -): Promise => { - const modelVersion = await apiState.api.createModelVersionForRegisteredModel( - {}, - registeredModel.id, - { +): Promise<{ + data: RegisterVersionCreatedResources; + errors: { [key: string]: Error | undefined }; +}> => { + let modelVersion; + let modelArtifact; + const errors: { [key: string]: Error | undefined } = {}; + try { + modelVersion = await apiState.api.createModelVersionForRegisteredModel({}, registeredModel.id, { name: formData.versionName, description: formData.versionDescription, customProperties: {}, state: ModelState.LIVE, author, registeredModelId: registeredModel.id, - }, - ); - const modelArtifact = await apiState.api.createModelArtifactForModelVersion({}, modelVersion.id, { - name: `${registeredModel.name}-${formData.versionName}-artifact`, - description: formData.versionDescription, - customProperties: {}, - state: ModelArtifactState.LIVE, - author, - modelFormatName: formData.sourceModelFormat, - modelFormatVersion: formData.sourceModelFormatVersion, - uri: - formData.modelLocationType === ModelLocationType.ObjectStorage - ? objectStorageFieldsToUri({ - endpoint: formData.modelLocationEndpoint, - bucket: formData.modelLocationBucket, - region: formData.modelLocationRegion, - path: formData.modelLocationPath, - }) || '' // We'll only hit this case if required fields are empty strings, so form validation should catch it. - : formData.modelLocationURI, - artifactType: 'model-artifact', - }); - return { modelVersion, modelArtifact }; + }); + } catch (e) { + if (e instanceof Error) { + errors[ErrorName.MODEL_VERSION] = e; + } + return { data: { modelVersion, modelArtifact }, errors }; + } + + try { + modelArtifact = await apiState.api.createModelArtifactForModelVersion({}, modelVersion.id, { + name: `${formData.versionName}`, + description: formData.versionDescription, + customProperties: {}, + state: ModelArtifactState.LIVE, + author, + modelFormatName: formData.sourceModelFormat, + modelFormatVersion: formData.sourceModelFormatVersion, + // TODO fill in the name of the data connection we used to prefill if we used one + // TODO this should be done as part of https://issues.redhat.com/browse/RHOAIENG-9914 + // storageKey: 'TODO', + uri: + formData.modelLocationType === ModelLocationType.ObjectStorage + ? objectStorageFieldsToUri({ + endpoint: formData.modelLocationEndpoint, + bucket: formData.modelLocationBucket, + region: formData.modelLocationRegion, + path: formData.modelLocationPath, + }) || '' // We'll only hit this case if required fields are empty strings, so form validation should catch it. + : formData.modelLocationURI, + artifactType: 'model-artifact', + }); + } catch (e) { + if (e instanceof Error) { + errors[ErrorName.MODEL_ARTIFACT] = e; + } + } + + return { data: { modelVersion, modelArtifact }, errors }; }; const isSubmitDisabledForCommonFields = (formData: RegistrationCommonFormData): boolean => { @@ -100,12 +135,17 @@ const isSubmitDisabledForCommonFields = (formData: RegistrationCommonFormData): !versionName || (modelLocationType === ModelLocationType.URI && !modelLocationURI) || (modelLocationType === ModelLocationType.ObjectStorage && - (!modelLocationBucket || !modelLocationEndpoint || !modelLocationPath)) + (!modelLocationBucket || !modelLocationEndpoint || !modelLocationPath)) || + !isNameValid(versionName) ); }; export const isRegisterModelSubmitDisabled = (formData: RegisterModelFormData): boolean => - !formData.modelName || isSubmitDisabledForCommonFields(formData); + !formData.modelName || + isSubmitDisabledForCommonFields(formData) || + !isNameValid(formData.modelName); export const isRegisterVersionSubmitDisabled = (formData: RegisterVersionFormData): boolean => !formData.registeredModelId || isSubmitDisabledForCommonFields(formData); + +export const isNameValid = (name: string): boolean => name.length <= MR_CHARACTER_LIMIT; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelListView.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelListView.tsx index 4dbfcbeb1..1d921acbb 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelListView.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelListView.tsx @@ -8,11 +8,10 @@ import { } from '@patternfly/react-core'; import { FilterIcon } from '@patternfly/react-icons'; import { useNavigate } from 'react-router-dom'; -import { RegisteredModel } from '~/app/types'; +import { ModelVersion, RegisteredModel } from '~/app/types'; import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; import { SearchType } from '~/shared/components/DashboardSearchField'; import { ProjectObjectType, typedEmptyImage } from '~/shared/components/design/utils'; -import { asEnumMember, filterRegisteredModels } from '~/app/utils'; import SimpleSelect from '~/shared/components/SimpleSelect'; import { registeredModelArchiveUrl, @@ -21,23 +20,29 @@ import { import EmptyModelRegistryState from '~/app/pages/modelRegistry/screens/components/EmptyModelRegistryState'; import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; import { isMUITheme } from '~/shared/utilities/const'; +import { filterRegisteredModels } from '~/app/pages/modelRegistry/screens/utils'; +import { asEnumMember } from '~/shared/utilities/utils'; +import { filterArchiveModels, filterLiveModels } from '~/app/utils'; import RegisteredModelTable from './RegisteredModelTable'; import RegisteredModelsTableToolbar from './RegisteredModelsTableToolbar'; type RegisteredModelListViewProps = { registeredModels: RegisteredModel[]; + modelVersions: ModelVersion[]; refresh: () => void; }; const RegisteredModelListView: React.FC = ({ - registeredModels: unfilteredRegisteredModels, + registeredModels, + modelVersions, refresh, }) => { const navigate = useNavigate(); const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); const [searchType, setSearchType] = React.useState(SearchType.KEYWORD); const [search, setSearch] = React.useState(''); - + const unfilteredRegisteredModels = filterLiveModels(registeredModels); + const archiveRegisteredModels = filterArchiveModels(registeredModels); const searchTypes = React.useMemo(() => [SearchType.KEYWORD, SearchType.OWNER], []); if (unfilteredRegisteredModels.length === 0) { @@ -51,9 +56,13 @@ const RegisteredModelListView: React.FC = ({ alt="missing model" /> )} - description={`${preferredModelRegistry?.name} has no active registered models. Register a model in this registry, or select a different registry.`} + description={`${ + preferredModelRegistry?.name ?? '' + } has no active registered models. Register a model in this registry, or select a different registry.`} primaryActionText="Register model" - secondaryActionText="View archived models" + secondaryActionText={ + archiveRegisteredModels.length !== 0 ? 'View archived models' : undefined + } primaryActionOnClick={() => { navigate(registerModelUrl(preferredModelRegistry?.name)); }} @@ -66,6 +75,7 @@ const RegisteredModelListView: React.FC = ({ const filteredRegisteredModels = filterRegisteredModels( unfilteredRegisteredModels, + modelVersions, search, searchType, ); @@ -78,8 +88,8 @@ const RegisteredModelListView: React.FC = ({ setSearch('')} - deleteLabelGroup={() => setSearch('')} + deleteLabel={resetFilters} + deleteLabelGroup={resetFilters} categoryName="Keyword" > = ({ icon={} /> - + {isMUITheme() ? ( = ({ onChange={(_, searchValue) => { setSearch(searchValue); }} - onClear={() => setSearch('')} + onClear={resetFilters} style={{ minWidth: '200px' }} data-testid="registered-model-table-search" /> @@ -136,7 +146,12 @@ const RegisteredModelListView: React.FC = ({ refresh={refresh} clearFilters={resetFilters} registeredModels={filteredRegisteredModels} - toolbarContent={} + toolbarContent={ + + } /> ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx index 6ef939886..c842fc1e4 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx @@ -23,10 +23,16 @@ const RegisteredModelTable: React.FC = ({ columns={rmColumns} toolbarContent={toolbarContent} defaultSortColumn={2} - enablePagination="compact" + onClearFilters={clearFilters} + enablePagination emptyTableView={} rowRenderer={(rm) => ( - + )} /> ); 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 feab14d6c..cc5e3dd91 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 @@ -19,12 +19,14 @@ import { ModelVersionsTab } from '~/app/pages/modelRegistry/screens/ModelVersion type RegisteredModelTableRowProps = { registeredModel: RegisteredModel; isArchiveRow?: boolean; + hasDeploys?: boolean; refresh: () => void; }; const RegisteredModelTableRow: React.FC = ({ registeredModel: rm, isArchiveRow, + hasDeploys = false, refresh, }) => { const { apiState } = React.useContext(ModelRegistryContext); @@ -49,15 +51,24 @@ const RegisteredModelTableRow: React.FC = ({ } }, }, - isArchiveRow - ? { - title: 'Restore model', - onClick: () => setIsRestoreModalOpen(true), - } - : { - title: 'Archive model', - onClick: () => setIsArchiveModalOpen(true), - }, + ...(isArchiveRow + ? [ + { + title: 'Restore model', + onClick: () => setIsRestoreModalOpen(true), + }, + ] + : [ + { isSeparator: true }, + { + title: 'Archive model', + onClick: () => setIsArchiveModalOpen(true), + isAriaDisabled: hasDeploys, + tooltipProps: hasDeploys + ? { content: 'Models with deployed versions cannot be archived.' } + : undefined, + }, + ]), ]; return ( @@ -95,38 +106,40 @@ const RegisteredModelTableRow: React.FC = ({ - setIsArchiveModalOpen(false)} - onSubmit={() => - apiState.api - .patchRegisteredModel( - {}, - { - state: ModelState.ARCHIVED, - }, - rm.id, - ) - .then(refresh) - } - isOpen={isArchiveModalOpen} - registeredModelName={rm.name} - /> - setIsRestoreModalOpen(false)} - onSubmit={() => - apiState.api - .patchRegisteredModel( - {}, - { - state: ModelState.LIVE, - }, - rm.id, - ) - .then(() => navigate(registeredModelUrl(rm.id, preferredModelRegistry?.name))) - } - isOpen={isRestoreModalOpen} - registeredModelName={rm.name} - /> + {isArchiveModalOpen ? ( + setIsArchiveModalOpen(false)} + onSubmit={() => + apiState.api + .patchRegisteredModel( + {}, + { + state: ModelState.ARCHIVED, + }, + rm.id, + ) + .then(refresh) + } + registeredModelName={rm.name} + /> + ) : null} + {isRestoreModalOpen ? ( + setIsRestoreModalOpen(false)} + onSubmit={() => + apiState.api + .patchRegisteredModel( + {}, + { + state: ModelState.LIVE, + }, + rm.id, + ) + .then(() => navigate(registeredModelUrl(rm.id, preferredModelRegistry?.name))) + } + registeredModelName={rm.name} + /> + ) : null} ); diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts index 7ca6bce8e..af228730b 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts @@ -28,9 +28,9 @@ export const rmColumns: SortableData[] = [ label: 'Owner', sortable: true, info: { - tooltip: 'The owner is the user who registered the model.', - tooltipProps: { - isContentLeftAligned: true, + popover: 'The owner is the user who registered the model.', + popoverProps: { + position: 'left', }, }, }, diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx index 3209d51e2..66b3a4677 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableToolbar.tsx @@ -22,10 +22,12 @@ import { type RegisteredModelsTableToolbarProps = { toggleGroupItems?: React.ReactNode; + onClearAllFilters?: () => void; }; const RegisteredModelsTableToolbar: React.FC = ({ toggleGroupItems: tableToggleGroupItems, + onClearAllFilters, }) => { const navigate = useNavigate(); const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); @@ -35,7 +37,7 @@ const RegisteredModelsTableToolbar: React.FC const tooltipRef = React.useRef(null); return ( - + } breakpoint="xl"> {tableToggleGroupItems} @@ -45,7 +47,7 @@ const RegisteredModelsTableToolbar: React.FC isOpen={isRegisterNewVersionOpen} onSelect={() => setIsRegisterNewVersionOpen(false)} onOpenChange={(isOpen) => setIsRegisterNewVersionOpen(isOpen)} - toggle={(toggleRef: React.Ref) => ( + toggle={(toggleRef) => ( - - {rm.name} - - - - + + {rm.name} + ) } @@ -84,7 +80,7 @@ const RegisteredModelsArchiveDetails: React.FC - {rm !== null && ( + {rm !== null && isRestoreModalOpen ? ( setIsRestoreModalOpen(false)} onSubmit={() => @@ -98,10 +94,9 @@ const RegisteredModelsArchiveDetails: React.FC navigate(registeredModelUrl(rm.id, preferredModelRegistry?.name))) } - isOpen={isRestoreModalOpen} registeredModelName={rm.name} /> - )} + ) : null} ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx index 0311e5cbb..566e6b7e0 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchive.tsx @@ -3,8 +3,9 @@ import { Breadcrumb, BreadcrumbItem } from '@patternfly/react-core'; import { Link } from 'react-router-dom'; import ApplicationsPage from '~/shared/components/ApplicationsPage'; import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; -import { filterArchiveModels } from '~/app/pages/modelRegistry/screens/utils'; +import { filterArchiveModels } from '~/app/utils'; import useRegisteredModels from '~/app/hooks/useRegisteredModels'; +import useModelVersions from '~/app/hooks/useModelVersions'; import RegisteredModelsArchiveListView from './RegisteredModelsArchiveListView'; type RegisteredModelsArchiveProps = Omit< @@ -14,7 +15,16 @@ type RegisteredModelsArchiveProps = Omit< const RegisteredModelsArchive: React.FC = ({ ...pageProps }) => { const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext); - const [registeredModels, loaded, loadError, refresh] = useRegisteredModels(); + const [registeredModels, modelsLoaded, modelsLoadError, refreshModels] = useRegisteredModels(); + const [modelVersions, versionsLoaded, versionsLoadError, refreshVersions] = useModelVersions(); + + const loaded = modelsLoaded && versionsLoaded; + const loadError = modelsLoadError || versionsLoadError; + + const refresh = React.useCallback(() => { + refreshModels(); + refreshVersions(); + }, [refreshModels, refreshVersions]); return ( = ({ ...pa } - title={`Archived models of ${preferredModelRegistry?.name}`} + title={`Archived models of ${preferredModelRegistry?.name ?? ''}`} loadError={loadError} loaded={loaded} provideChildrenPadding > diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveListView.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveListView.tsx index 74eeb07d7..a49aa4aa5 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveListView.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveListView.tsx @@ -8,33 +8,35 @@ import { ToolbarItem, ToolbarToggleGroup, } from '@patternfly/react-core'; -import { FilterIcon } from '@patternfly/react-icons'; -import { RegisteredModel } from '~/app/types'; +import { FilterIcon, SearchIcon } from '@patternfly/react-icons'; +import { ModelVersion, RegisteredModel } from '~/app/types'; import { SearchType } from '~/shared/components/DashboardSearchField'; import { filterRegisteredModels } from '~/app/pages/modelRegistry/screens/utils'; import EmptyModelRegistryState from '~/app/pages/modelRegistry/screens/components/EmptyModelRegistryState'; import SimpleSelect from '~/shared/components/SimpleSelect'; -import { asEnumMember } from '~/app/utils'; +import { asEnumMember } from '~/shared/utilities/utils'; import FormFieldset from '~/app/pages/modelRegistry/screens/components/FormFieldset'; import { isMUITheme } from '~/shared/utilities/const'; import RegisteredModelsArchiveTable from './RegisteredModelsArchiveTable'; type RegisteredModelsArchiveListViewProps = { registeredModels: RegisteredModel[]; + modelVersions: ModelVersion[]; refresh: () => void; }; const RegisteredModelsArchiveListView: React.FC = ({ registeredModels: unfilteredRegisteredModels, + modelVersions, refresh, }) => { const [searchType, setSearchType] = React.useState(SearchType.KEYWORD); const [search, setSearch] = React.useState(''); - const searchTypes = [SearchType.KEYWORD, SearchType.AUTHOR]; - + const searchTypes = [SearchType.KEYWORD, SearchType.OWNER]; const filteredRegisteredModels = filterRegisteredModels( unfilteredRegisteredModels, + modelVersions, search, searchType, ); @@ -42,6 +44,7 @@ const RegisteredModelsArchiveListView: React.FC} rowRenderer={(rm) => ( 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 index 77c5726eb..7760389e7 100644 --- 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 @@ -297,28 +297,39 @@ describe('filterModelVersions', () => { describe('filterRegisteredModels', () => { const registeredModels: RegisteredModel[] = [ - mockRegisteredModel({ name: 'Test 1', state: ModelState.ARCHIVED }), + mockRegisteredModel({ name: 'Test 1', state: ModelState.ARCHIVED, owner: 'Alice' }), mockRegisteredModel({ name: 'Test 2', description: 'Description2', + owner: 'Bob', }), - mockRegisteredModel({ name: 'Test 3', state: ModelState.ARCHIVED }), - mockRegisteredModel({ name: 'Test 4', state: ModelState.ARCHIVED }), - mockRegisteredModel({ name: 'Test 5' }), + mockRegisteredModel({ name: 'Test 3', state: ModelState.ARCHIVED, owner: 'Charlie' }), + mockRegisteredModel({ name: 'Test 4', state: ModelState.ARCHIVED, owner: 'Alice' }), + mockRegisteredModel({ name: 'Test 5', owner: 'Bob' }), ]; test('filters by name', () => { - const filtered = filterRegisteredModels(registeredModels, 'Test 1', SearchType.KEYWORD); + const filtered = filterRegisteredModels(registeredModels, [], 'Test 1', SearchType.KEYWORD); expect(filtered).toEqual([registeredModels[0]]); }); test('filters by description', () => { - const filtered = filterRegisteredModels(registeredModels, 'Description2', SearchType.KEYWORD); + const filtered = filterRegisteredModels( + registeredModels, + [], + 'Description2', + SearchType.KEYWORD, + ); expect(filtered).toEqual([registeredModels[1]]); }); + test('filters by owner', () => { + const filtered = filterRegisteredModels(registeredModels, [], 'Alice', SearchType.OWNER); + expect(filtered).toEqual([registeredModels[0], registeredModels[3]]); + }); + test('does not filter when search is empty', () => { - const filtered = filterRegisteredModels(registeredModels, '', SearchType.KEYWORD); + const filtered = filterRegisteredModels(registeredModels, [], '', SearchType.KEYWORD); expect(filtered).toEqual(registeredModels); }); }); 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 index f4e018cb6..8fae89be0 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveModelVersionModal.tsx @@ -1,34 +1,25 @@ import * as React from 'react'; -import { - Alert, - Form, - FormGroup, - Modal, - ModalBody, - ModalHeader, - TextInput, -} from '@patternfly/react-core'; +import { Flex, FlexItem, Stack, StackItem, TextInput } from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; import DashboardModalFooter from '~/shared/components/DashboardModalFooter'; import { useNotification } from '~/app/hooks/useNotification'; interface ArchiveModelVersionModalProps { onCancel: () => void; onSubmit: () => void; - isOpen: boolean; modelVersionName: string; } export const ArchiveModelVersionModal: React.FC = ({ onCancel, onSubmit, - isOpen, modelVersionName, }) => { + const notification = useNotification(); const [isSubmitting, setIsSubmitting] = React.useState(false); const [error, setError] = React.useState(); const [confirmInputValue, setConfirmInputValue] = React.useState(''); const isDisabled = confirmInputValue.trim() !== modelVersionName || isSubmitting; - const notification = useNotification(); const onClose = React.useCallback(() => { setConfirmInputValue(''); @@ -49,34 +40,37 @@ export const ArchiveModelVersionModal: React.FC = } finally { setIsSubmitting(false); } - }, [notification, modelVersionName, onSubmit, onClose]); - - const description = ( - <> - {modelVersionName} will be archived and unavailable for use unless it is restored. -
-
- Type {modelVersionName} to confirm archiving: - - ); + }, [onSubmit, onClose, notification, modelVersionName]); return ( + } data-testid="archive-model-version-modal" > - - -
- {error && ( - - {error.message} - - )} - - {description} + + + {modelVersionName} will be archived and unavailable for use unless it is restored. + + + + + Type {modelVersionName} to confirm archiving: + = } }} /> - -
- - + + + ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal.tsx index e5de53054..2a6738ed8 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal.tsx @@ -1,27 +1,18 @@ import * as React from 'react'; -import { - Alert, - Form, - FormGroup, - Modal, - ModalBody, - ModalHeader, - TextInput, -} from '@patternfly/react-core'; +import { Flex, FlexItem, Stack, StackItem, TextInput } from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; import DashboardModalFooter from '~/shared/components/DashboardModalFooter'; import { useNotification } from '~/app/hooks/useNotification'; interface ArchiveRegisteredModelModalProps { onCancel: () => void; onSubmit: () => void; - isOpen: boolean; registeredModelName: string; } export const ArchiveRegisteredModelModal: React.FC = ({ onCancel, onSubmit, - isOpen, registeredModelName, }) => { const notification = useNotification(); @@ -49,36 +40,38 @@ export const ArchiveRegisteredModelModal: React.FC - {registeredModelName} and all of its versions will be archived and unavailable for use - unless it is restored. -
-
- Type {registeredModelName} to confirm archiving: - - ); + }, [onSubmit, onClose, notification, registeredModelName]); return ( + } data-testid="archive-registered-model-modal" > - - -
- {error && ( - - {error.message} - - )} - - {description} + + + {registeredModelName} and all of its versions will be archived and unavailable for + use unless it is restored. + + + + + Type {registeredModelName} to confirm archiving: + - -
- - + + + ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ModelLabels.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ModelLabels.tsx index 10983cc11..081ba5d30 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ModelLabels.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/ModelLabels.tsx @@ -65,11 +65,11 @@ const ModelLabels: React.FC = ({ name, customProperties }) => ); - const labelModal = ( + const labelModal = isLabelModalOpen ? ( setIsLabelModalOpen(false)} description={ @@ -100,7 +100,7 @@ const ModelLabels: React.FC = ({ name, customProperties }) => {labelsComponent(filteredLabels, '50ch')} - ); + ) : null; if (Object.keys(customProperties).length === 0) { return '-'; 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 index e7dc7a341..f1f382564 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreModelVersionModal.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreModelVersionModal.tsx @@ -1,24 +1,22 @@ import * as React from 'react'; -import { Form, Modal, ModalHeader, ModalBody, Alert } from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; import DashboardModalFooter from '~/shared/components/DashboardModalFooter'; import { useNotification } from '~/app/hooks/useNotification'; interface RestoreModelVersionModalProps { onCancel: () => void; onSubmit: () => void; - isOpen: boolean; modelVersionName: string; } export const RestoreModelVersionModal: React.FC = ({ onCancel, onSubmit, - isOpen, modelVersionName, }) => { + const notification = useNotification(); const [isSubmitting, setIsSubmitting] = React.useState(false); const [error, setError] = React.useState(); - const notification = useNotification(); const onClose = React.useCallback(() => { onCancel(); @@ -38,40 +36,28 @@ export const RestoreModelVersionModal: React.FC = } finally { setIsSubmitting(false); } - }, [notification, modelVersionName, onSubmit, onClose]); - - const description = ( - <> - {modelVersionName} will be restored and returned to the versions list. - - ); + }, [onSubmit, onClose, notification, modelVersionName]); return ( + } data-testid="restore-model-version-modal" > - - -
- {error && ( - - {error.message} - - )} -
- {description} - - + {modelVersionName} will be restored and returned to the versions list. ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreRegisteredModel.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreRegisteredModel.tsx index e1348b4ee..8eb3e23a1 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreRegisteredModel.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/RestoreRegisteredModel.tsx @@ -1,19 +1,17 @@ import * as React from 'react'; -import { Alert, Form, ModalHeader, Modal, ModalBody } from '@patternfly/react-core'; +import { Modal } from '@patternfly/react-core/deprecated'; import DashboardModalFooter from '~/shared/components/DashboardModalFooter'; import { useNotification } from '~/app/hooks/useNotification'; interface RestoreRegisteredModelModalProps { onCancel: () => void; onSubmit: () => void; - isOpen: boolean; registeredModelName: string; } export const RestoreRegisteredModelModal: React.FC = ({ onCancel, onSubmit, - isOpen, registeredModelName, }) => { const notification = useNotification(); @@ -38,40 +36,29 @@ export const RestoreRegisteredModelModal: React.FC - {registeredModelName} and all of its versions will be restored and returned to the - registered models list. - - ); + }, [onSubmit, onClose, notification, registeredModelName]); return ( + } data-testid="restore-registered-model-modal" > - - -
- {error && ( - - {error.message} - - )} -
- {description} - - + {registeredModelName} and all of its versions will be restored and returned to the + registered models list. ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts index 30d531ec5..0a6d34862 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts @@ -1,23 +1,28 @@ -export const modelRegistryUrl = (preferredModelRegistry?: string): string => +export const modelRegistryUrl = (preferredModelRegistry = ''): string => `/model-registry/${preferredModelRegistry}`; export const registeredModelsUrl = (preferredModelRegistry?: string): string => `${modelRegistryUrl(preferredModelRegistry)}/registeredModels`; -export const registeredModelUrl = (rmId?: string, preferredModelRegistry?: string): string => +export const registeredModelUrl = (rmId = '', preferredModelRegistry?: string): string => `${registeredModelsUrl(preferredModelRegistry)}/${rmId}`; export const registeredModelArchiveUrl = (preferredModelRegistry?: string): string => `${registeredModelsUrl(preferredModelRegistry)}/archive`; export const registeredModelArchiveDetailsUrl = ( - rmId?: string, + rmId = '', preferredModelRegistry?: string, ): string => `${registeredModelArchiveUrl(preferredModelRegistry)}/${rmId}`; export const modelVersionListUrl = (rmId?: string, preferredModelRegistry?: string): string => `${registeredModelUrl(rmId, preferredModelRegistry)}/versions`; +export const archiveModelVersionListUrl = ( + rmId?: string, + preferredModelRegistry?: string, +): string => `${registeredModelArchiveDetailsUrl(rmId, preferredModelRegistry)}/versions`; + export const modelVersionUrl = ( mvId: string, rmId?: string, @@ -27,6 +32,12 @@ export const modelVersionUrl = ( export const modelVersionArchiveUrl = (rmId?: string, preferredModelRegistry?: string): string => `${modelVersionListUrl(rmId, preferredModelRegistry)}/archive`; +export const archiveModelVersionDetailsUrl = ( + mvId: string, + rmId?: string, + preferredModelRegistry?: string, +): string => `${archiveModelVersionListUrl(rmId, preferredModelRegistry)}/${mvId}`; + export const modelVersionArchiveDetailsUrl = ( mvId: string, rmId?: string, @@ -44,13 +55,8 @@ export const registerVersionForModelUrl = ( preferredModelRegistry?: string, ): string => `${registeredModelUrl(rmId, preferredModelRegistry)}/registerVersion`; -export const archiveModelVersionListUrl = ( - rmId?: string, - preferredModelRegistry?: string, -): string => `${registeredModelArchiveDetailsUrl(rmId, preferredModelRegistry)}/versions`; - -export const archiveModelVersionDetailsUrl = ( +export const modelVersionDeploymentsUrl = ( mvId: string, rmId?: string, preferredModelRegistry?: string, -): string => `${archiveModelVersionListUrl(rmId, preferredModelRegistry)}/${mvId}`; +): string => `${modelVersionUrl(mvId, rmId, preferredModelRegistry)}/deployments`; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/utils.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/utils.ts index 964cd1ea0..a811369e4 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/screens/utils.ts +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/utils.ts @@ -3,7 +3,6 @@ import { ModelRegistryCustomProperties, ModelRegistryMetadataType, ModelRegistryStringCustomProperties, - ModelState, ModelVersion, RegisteredModel, } from '~/app/types'; @@ -51,6 +50,12 @@ export const getProperties = ( ): ModelRegistryStringCustomProperties => { const initial: ModelRegistryStringCustomProperties = {}; return Object.keys(customProperties).reduce((acc, key) => { + // _lastModified is a property that is required to update the timestamp on the backend and we have a workaround for it. It should be resolved by + // backend team See https://issues.redhat.com/browse/RHOAIENG-17614 . + if (key === '_lastModified') { + return acc; + } + const prop = customProperties[key]; if (prop.metadataType === ModelRegistryMetadataType.STRING && prop.string_value !== '') { return { ...acc, [key]: prop }; @@ -87,8 +92,10 @@ export const filterModelVersions = ( unfilteredModelVersions: ModelVersion[], search: string, searchType: SearchType, -): ModelVersion[] => - unfilteredModelVersions.filter((mv: ModelVersion) => { +): ModelVersion[] => { + const searchLower = search.toLowerCase(); + + return unfilteredModelVersions.filter((mv: ModelVersion) => { if (!search) { return true; } @@ -96,21 +103,20 @@ export const filterModelVersions = ( switch (searchType) { case SearchType.KEYWORD: return ( - mv.name.toLowerCase().includes(search.toLowerCase()) || - (mv.description && mv.description.toLowerCase().includes(search.toLowerCase())) + mv.name.toLowerCase().includes(searchLower) || + (mv.description && mv.description.toLowerCase().includes(searchLower)) || + getLabels(mv.customProperties).some((label) => label.toLowerCase().includes(searchLower)) ); - case SearchType.AUTHOR: - return ( - mv.author && - (mv.author.toLowerCase().includes(search.toLowerCase()) || - (mv.author && mv.author.toLowerCase().includes(search.toLowerCase()))) - ); + case SearchType.AUTHOR: { + return mv.author && mv.author.toLowerCase().includes(searchLower); + } default: return true; } }); +}; export const sortModelVersionsByCreateTime = (registeredModels: ModelVersion[]): ModelVersion[] => registeredModels.toSorted((a, b) => { @@ -121,82 +127,46 @@ export const sortModelVersionsByCreateTime = (registeredModels: ModelVersion[]): export const filterRegisteredModels = ( unfilteredRegisteredModels: RegisteredModel[], + unfilteredModelVersions: ModelVersion[], search: string, searchType: SearchType, -): RegisteredModel[] => - unfilteredRegisteredModels.filter((rm: RegisteredModel) => { +): RegisteredModel[] => { + const searchLower = search.toLowerCase(); + + return unfilteredRegisteredModels.filter((rm: RegisteredModel) => { if (!search) { return true; } + const modelVersions = unfilteredModelVersions.filter((mv) => mv.registeredModelId === rm.id); + switch (searchType) { - case SearchType.KEYWORD: - return ( - rm.name.toLowerCase().includes(search.toLowerCase()) || - (rm.description && rm.description.toLowerCase().includes(search.toLowerCase())) + case SearchType.KEYWORD: { + const matchesModel = + rm.name.toLowerCase().includes(searchLower) || + (rm.description && rm.description.toLowerCase().includes(searchLower)) || + getLabels(rm.customProperties).some((label) => label.toLowerCase().includes(searchLower)); + + const matchesVersion = modelVersions.some( + (mv: ModelVersion) => + mv.name.toLowerCase().includes(searchLower) || + (mv.description && mv.description.toLowerCase().includes(searchLower)) || + getLabels(mv.customProperties).some((label) => + label.toLowerCase().includes(searchLower), + ), ); - case SearchType.OWNER: - return rm.owner && rm.owner.toLowerCase().includes(search.toLowerCase()); + return matchesModel || matchesVersion; + } + case SearchType.OWNER: { + return rm.owner && rm.owner.toLowerCase().includes(searchLower); + } default: return true; } }); - -export const objectStorageFieldsToUri = (fields: ObjectStorageFields): string | null => { - const { endpoint, bucket, region, path } = fields; - if (!endpoint || !bucket || !path) { - return null; - } - const searchParams = new URLSearchParams(); - searchParams.set('endpoint', endpoint); - if (region) { - searchParams.set('defaultRegion', region); - } - return `s3://${bucket}/${path}?${searchParams.toString()}`; }; -export const getLastCreatedItem = ( - items?: T[], -): T | undefined => - items?.toSorted( - ({ createTimeSinceEpoch: createTimeA }, { createTimeSinceEpoch: createTimeB }) => { - if (!createTimeA || !createTimeB) { - return 0; - } - return Number(createTimeB) - Number(createTimeA); - }, - )[0]; - -export const filterArchiveVersions = (modelVersions: ModelVersion[]): ModelVersion[] => - modelVersions.filter((mv) => mv.state === ModelState.ARCHIVED); - -export const filterLiveVersions = (modelVersions: ModelVersion[]): ModelVersion[] => - modelVersions.filter((mv) => mv.state === ModelState.LIVE); - -export const filterArchiveModels = (registeredModels: RegisteredModel[]): RegisteredModel[] => - registeredModels.filter((rm) => rm.state === ModelState.ARCHIVED); - -export const filterLiveModels = (registeredModels: RegisteredModel[]): RegisteredModel[] => - registeredModels.filter((rm) => rm.state === ModelState.LIVE); - -export const uriToObjectStorageFields = (uri: string): ObjectStorageFields | null => { - try { - const urlObj = new URL(uri); - // Some environments include the first token after the protocol (our bucket) in the pathname and some have it as the hostname - const [bucket, ...pathSplit] = `${urlObj.hostname}/${urlObj.pathname}` - .split('/') - .filter(Boolean); - const path = pathSplit.join('/'); - const searchParams = new URLSearchParams(urlObj.search); - const endpoint = searchParams.get('endpoint'); - const region = searchParams.get('defaultRegion'); - if (endpoint && bucket && path) { - return { endpoint, bucket, region: region || undefined, path }; - } - return null; - } catch { - return null; - } -}; +// export const getServerAddress = (resource: ServiceKind): string => +// resource.metadata.annotations?.['routing.opendatahub.io/external-address-rest'] || ''; diff --git a/clients/ui/frontend/src/app/pages/notFound/NotFound.tsx b/clients/ui/frontend/src/app/pages/notFound/NotFound.tsx deleted file mode 100644 index 794b06422..000000000 --- a/clients/ui/frontend/src/app/pages/notFound/NotFound.tsx +++ /dev/null @@ -1,37 +0,0 @@ -import * as React from 'react'; -import { ExclamationTriangleIcon } from '@patternfly/react-icons'; -import { - Button, - EmptyState, - EmptyStateBody, - EmptyStateFooter, - PageSection, -} from '@patternfly/react-core'; -import { useNavigate } from 'react-router-dom'; - -const NotFound: React.FunctionComponent = () => { - function GoHomeBtn() { - const navigate = useNavigate(); - - function handleClick() { - navigate('/'); - } - - return ; - } - - return ( - - - - We didn't find a page that matches the address you navigated to. - - - - - - - ); -}; - -export { NotFound }; diff --git a/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTable.tsx b/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTable.tsx index e05cacd6a..e9096cb3c 100644 --- a/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTable.tsx +++ b/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTable.tsx @@ -9,12 +9,21 @@ type ModelRegistriesTableProps = { }; const ModelRegistriesTable: React.FC = ({ modelRegistries }) => ( - // TODO: [Model Registry RBAC] Add toolbar once we manage permissions + // TODO: [Midstream] Complete once we have permissions } + rowRenderer={(mr) => ( + {}} + // eslint-disable-next-line @typescript-eslint/no-empty-function + onEditRegistry={() => {}} + /> + )} variant="compact" /> ); diff --git a/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx b/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx index 6289a6c61..9ea9d3527 100644 --- a/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx +++ b/clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx @@ -1,22 +1,92 @@ import React from 'react'; -import { Td, Tr } from '@patternfly/react-table'; +import { ActionsColumn, Td, Tr } from '@patternfly/react-table'; +import { Button, Tooltip } from '@patternfly/react-core'; +import { useNavigate } from 'react-router'; import { ModelRegistry } from '~/app/types'; +import ResourceNameTooltip from '~/shared/components/ResourceNameTooltip'; +import { convertToK8sResourceCommon } from '~/app/utils'; +import { isPlatformDefault } from '~/shared/utilities/const'; +import { ModelRegistryTableRowStatus } from './ModelRegistryTableRowStatus'; type ModelRegistriesTableRowProps = { modelRegistry: ModelRegistry; + // roleBindings: ContextResourceData; // TODO: [Midstream] Filter role bindings for this model registry + onEditRegistry: (obj: ModelRegistry) => void; + onDeleteRegistry: (obj: ModelRegistry) => void; }; -const ModelRegistriesTableRow: React.FC = ({ modelRegistry: mr }) => ( - <> +const ModelRegistriesTableRow: React.FC = ({ + modelRegistry: mr, + // roleBindings, // TODO: [Midstream] Filter role bindings for this model registry + onEditRegistry, + onDeleteRegistry, +}) => { + const navigate = useNavigate(); + const filteredRoleBindings = []; // TODO: [Midstream] Filter role bindings for this model registry + + return ( + + {isPlatformDefault() && ( + + )} + {isPlatformDefault() && ( + + )} - -); - -// TODO: [Model Registry RBAC] Get rest of columns once we manage permissions + ); +}; export default ModelRegistriesTableRow; diff --git a/clients/ui/frontend/src/app/pages/settings/ModelRegistrySettings.tsx b/clients/ui/frontend/src/app/pages/settings/ModelRegistrySettings.tsx index 6e320bb2a..95068a0bb 100644 --- a/clients/ui/frontend/src/app/pages/settings/ModelRegistrySettings.tsx +++ b/clients/ui/frontend/src/app/pages/settings/ModelRegistrySettings.tsx @@ -10,18 +10,20 @@ import ModelRegistriesTable from './ModelRegistriesTable'; const ModelRegistrySettings: React.FC = () => { const queryParams = useQueryParamNamespaces(); + const [modelRegistries, mrloaded, loadError] = useModelRegistries(queryParams); + // TODO: [Midstream] Implement this when adding logic for rules review + const loaded = mrloaded; //&& roleBindings.loaded; - const [modelRegistries, loaded, loadError] = useModelRegistries(queryParams); return ( <> } - description="List all the model registries deployed in your environment." + description="Manage model registry settings for all users in your organization." loaded={loaded} loadError={loadError} errorMessage="Unable to load model registries." @@ -34,7 +36,9 @@ const ModelRegistrySettings: React.FC = () => { variant={EmptyStateVariant.lg} data-testid="mr-settings-empty-state" > - To get started, create a model registry. + + To get started, create a model registry. You can manage permissions after creation. + } provideChildrenPadding diff --git a/clients/ui/frontend/src/app/pages/settings/ModelRegistryTableRowStatus.tsx b/clients/ui/frontend/src/app/pages/settings/ModelRegistryTableRowStatus.tsx new file mode 100644 index 000000000..37afed67c --- /dev/null +++ b/clients/ui/frontend/src/app/pages/settings/ModelRegistryTableRowStatus.tsx @@ -0,0 +1,164 @@ +import React from 'react'; + +import { Label, Popover, Stack, StackItem } from '@patternfly/react-core'; +import { + CheckCircleIcon, + ExclamationCircleIcon, + ExclamationTriangleIcon, + InProgressIcon, +} from '@patternfly/react-icons'; + +import { K8sCondition } from '~/shared/k8sTypes'; + +enum ModelRegistryStatus { + Progressing = 'Progressing', + Degraded = 'Degraded', + Available = 'Available', + IstioAvailable = 'IstioAvailable', + GatewayAvailable = 'GatewayAvailable', +} + +enum ModelRegistryStatusLabel { + Progressing = 'Progressing', + Available = 'Available', + Degrading = 'Degrading', + Unavailable = 'Unavailable', +} + +enum ConditionStatus { + True = 'True', + False = 'False', +} +interface ModelRegistryTableRowStatusProps { + conditions: K8sCondition[] | undefined; +} + +export const ModelRegistryTableRowStatus: React.FC = ({ + conditions, +}) => { + const conditionsMap = + conditions?.reduce((acc: Record, condition) => { + acc[condition.type] = condition; + return acc; + }, {}) ?? {}; + let statusLabel: string = ModelRegistryStatusLabel.Progressing; + let icon = ; + let status: React.ComponentProps['status']; + let popoverMessages: string[] = []; + let popoverTitle = ''; + + if (Object.values(conditionsMap).length) { + const { + [ModelRegistryStatus.Available]: availableCondition, + [ModelRegistryStatus.Progressing]: progressCondition, + [ModelRegistryStatus.Degraded]: degradedCondition, + } = conditionsMap; + + popoverMessages = + availableCondition?.status === ConditionStatus.False + ? Object.values(conditionsMap).reduce((messages: string[], condition) => { + if (condition?.status === ConditionStatus.False && condition.message) { + messages.push(condition.message); + } + return messages; + }, []) + : []; + + // Unavailable + if ( + availableCondition?.status === ConditionStatus.False && + !popoverMessages.some((message) => message.includes('ContainerCreating')) + ) { + statusLabel = ModelRegistryStatusLabel.Unavailable; + icon = ; + status = 'warning'; + } + // Degrading + else if (degradedCondition?.status === ConditionStatus.True) { + statusLabel = ModelRegistryStatusLabel.Degrading; + icon = ; + popoverTitle = 'Service is degrading'; + } + // Available + else if (availableCondition?.status === ConditionStatus.True) { + statusLabel = ModelRegistryStatusLabel.Available; + icon = ; + status = 'success'; + } + // Progressing + else if (progressCondition?.status === ConditionStatus.True) { + statusLabel = ModelRegistryStatusLabel.Progressing; + icon = ; + status = 'info'; + } + } + // Handle popover logic for Unavailable status + if (statusLabel === ModelRegistryStatusLabel.Unavailable) { + const { + [ModelRegistryStatus.IstioAvailable]: istioAvailableCondition, + [ModelRegistryStatus.GatewayAvailable]: gatewayAvailableCondition, + } = conditionsMap; + + if ( + istioAvailableCondition?.status === ConditionStatus.False && + gatewayAvailableCondition?.status === ConditionStatus.False + ) { + popoverTitle = 'Istio resources and Istio Gateway resources are both unavailable'; + } else if (istioAvailableCondition?.status === ConditionStatus.False) { + popoverTitle = 'Istio resources are unavailable'; + } else if (gatewayAvailableCondition?.status === ConditionStatus.False) { + popoverTitle = 'Istio Gateway resources are unavailable'; + } else if ( + istioAvailableCondition?.status === ConditionStatus.True && + gatewayAvailableCondition?.status === ConditionStatus.True + ) { + popoverTitle = 'Deployment is unavailable'; + } else { + popoverTitle = 'Service is unavailable'; + } + } + + const isClickable = popoverTitle && popoverMessages.length; + + const label = ( + + ); + + return popoverTitle && popoverMessages.length ? ( + , + } + : { alertSeverityVariant: 'danger', headerIcon: })} + bodyContent={ + + {popoverMessages.map((message, index) => ( + {message} + ))} + + } + > + {label} + + ) : ( + label + ); +}; diff --git a/clients/ui/frontend/src/app/pages/settings/columns.ts b/clients/ui/frontend/src/app/pages/settings/columns.ts index 36ae2ea29..57092e765 100644 --- a/clients/ui/frontend/src/app/pages/settings/columns.ts +++ b/clients/ui/frontend/src/app/pages/settings/columns.ts @@ -1,5 +1,6 @@ -import { SortableData } from '~/shared/components/table'; +import { kebabTableColumn, SortableData } from '~/shared/components/table'; import { ModelRegistry } from '~/app/types'; +import { isPlatformDefault } from '~/shared/utilities/const'; export const modelRegistryColumns: SortableData[] = [ { @@ -8,16 +9,19 @@ export const modelRegistryColumns: SortableData[] = [ sortable: (a, b) => a.name.localeCompare(b.name), width: 30, }, - // TODO: [Model Registry RBAC] Add once we manage permissions - // { - // field: 'status', - // label: 'Status', - // sortable: false, - // }, - // { - // field: 'manage permissions', - // label: '', - // sortable: false, - // }, - // kebabTableColumn(), + { + field: 'status', + label: 'Status', + sortable: false, + }, + ...(isPlatformDefault() + ? [ + { + field: 'manage permissions', + label: '', + sortable: false, + }, + kebabTableColumn(), + ] + : []), ]; diff --git a/clients/ui/frontend/src/app/types.ts b/clients/ui/frontend/src/app/types.ts index a2c1dafb0..0a2ee0da9 100644 --- a/clients/ui/frontend/src/app/types.ts +++ b/clients/ui/frontend/src/app/types.ts @@ -162,6 +162,8 @@ export type GetRegisteredModel = ( export type GetModelVersion = (opts: APIOptions, modelversionId: string) => Promise; +export type GetListModelVersions = (opts: APIOptions) => Promise; + export type GetListRegisteredModels = (opts: APIOptions) => Promise; export type GetModelVersionsByRegisteredModel = ( @@ -186,17 +188,25 @@ export type PatchModelVersion = ( modelversionId: string, ) => Promise; +export type PatchModelArtifact = ( + opts: APIOptions, + data: Partial, + modelartifactId: string, +) => Promise; + export type ModelRegistryAPIs = { createRegisteredModel: CreateRegisteredModel; createModelVersionForRegisteredModel: CreateModelVersionForRegisteredModel; createModelArtifactForModelVersion: CreateModelArtifactForModelVersion; getRegisteredModel: GetRegisteredModel; getModelVersion: GetModelVersion; + listModelVersions: GetListModelVersions; listRegisteredModels: GetListRegisteredModels; getModelVersionsByRegisteredModel: GetModelVersionsByRegisteredModel; getModelArtifactsByModelVersion: GetModelArtifactsByModelVersion; patchRegisteredModel: PatchRegisteredModel; patchModelVersion: PatchModelVersion; + patchModelArtifact: PatchModelArtifact; }; export type Notification = { diff --git a/clients/ui/frontend/src/app/utils.ts b/clients/ui/frontend/src/app/utils.ts index cc2e412e3..7da7d2c78 100644 --- a/clients/ui/frontend/src/app/utils.ts +++ b/clients/ui/frontend/src/app/utils.ts @@ -1,45 +1,86 @@ -import { SearchType } from '~/shared/components/DashboardSearchField'; -import { RegisteredModel } from '~/app/types'; - -export const asEnumMember = ( - member: T[keyof T] | string | number | undefined | null, - e: T, -): T[keyof T] | null => (isEnumMember(member, e) ? member : null); - -export const isEnumMember = ( - member: T[keyof T] | string | number | undefined | unknown | null, - e: T, -): member is T[keyof T] => { - if (member != null) { - return Object.entries(e) - .filter(([key]) => Number.isNaN(Number(key))) - .map(([, value]) => value) - .includes(member); +import { ModelRegistry, ModelState, ModelVersion, RegisteredModel } from '~/app/types'; +import { K8sResourceCommon } from '~/shared/types'; + +export type ObjectStorageFields = { + endpoint: string; + bucket: string; + region?: string; + path: string; +}; + +export const objectStorageFieldsToUri = (fields: ObjectStorageFields): string | null => { + const { endpoint, bucket, region, path } = fields; + if (!endpoint || !bucket || !path) { + return null; } - return false; + const searchParams = new URLSearchParams(); + searchParams.set('endpoint', endpoint); + if (region) { + searchParams.set('defaultRegion', region); + } + return `s3://${bucket}/${path}?${searchParams.toString()}`; }; -export const filterRegisteredModels = ( - unfilteredRegisteredModels: RegisteredModel[], - search: string, - searchType: SearchType, -): RegisteredModel[] => - unfilteredRegisteredModels.filter((rm: RegisteredModel) => { - if (!search) { - return true; +export const uriToObjectStorageFields = (uri: string): ObjectStorageFields | null => { + try { + const urlObj = new URL(uri); + // Some environments include the first token after the protocol (our bucket) in the pathname and some have it as the hostname + const [bucket, ...pathSplit] = `${urlObj.hostname}/${urlObj.pathname}` + .split('/') + .filter(Boolean); + const path = pathSplit.join('/'); + const searchParams = new URLSearchParams(urlObj.search); + const endpoint = searchParams.get('endpoint'); + const region = searchParams.get('defaultRegion'); + if (endpoint && bucket && path) { + return { endpoint, bucket, region: region || undefined, path }; } + return null; + } catch { + return null; + } +}; - switch (searchType) { - case SearchType.KEYWORD: - return ( - rm.name.toLowerCase().includes(search.toLowerCase()) || - (rm.description && rm.description.toLowerCase().includes(search.toLowerCase())) - ); +export const getLastCreatedItem = ( + items?: T[], +): T | undefined => + items?.toSorted( + ({ createTimeSinceEpoch: createTimeA }, { createTimeSinceEpoch: createTimeB }) => { + if (!createTimeA || !createTimeB) { + return 0; + } + return Number(createTimeB) - Number(createTimeA); + }, + )[0]; - case SearchType.OWNER: - return rm.owner && rm.owner.toLowerCase().includes(search.toLowerCase()); +export const filterArchiveVersions = (modelVersions: ModelVersion[]): ModelVersion[] => + modelVersions.filter((mv) => mv.state === ModelState.ARCHIVED); - default: - return true; - } - }); +export const filterLiveVersions = (modelVersions: ModelVersion[]): ModelVersion[] => + modelVersions.filter((mv) => mv.state === ModelState.LIVE); + +export const filterArchiveModels = (registeredModels: RegisteredModel[]): RegisteredModel[] => + registeredModels.filter((rm) => rm.state === ModelState.ARCHIVED); + +export const filterLiveModels = (registeredModels: RegisteredModel[]): RegisteredModel[] => + registeredModels.filter((rm) => rm.state === ModelState.LIVE); + +export const convertToK8sResourceCommon = (modelRegistry: ModelRegistry): K8sResourceCommon => ({ + apiVersion: 'v1', + kind: 'ModelRegistry', + metadata: { + name: modelRegistry.name, + }, + spec: { + // Add any additional fields from ModelRegistry to K8sResourceCommon spec if needed + }, + status: { + conditions: [ + { + type: 'Degrading', + status: 'True', + lastTransitionTime: new Date().toISOString(), + }, + ], + }, +}); diff --git a/clients/ui/frontend/src/app/utils/updateTimestamps.ts b/clients/ui/frontend/src/app/utils/updateTimestamps.ts new file mode 100644 index 000000000..21e6a2214 --- /dev/null +++ b/clients/ui/frontend/src/app/utils/updateTimestamps.ts @@ -0,0 +1,84 @@ +import { ModelRegistryAPIs, ModelState, ModelRegistryMetadataType } from '~/app/types'; + +type MinimalModelRegistryAPI = Pick; + +export const bumpModelVersionTimestamp = async ( + api: ModelRegistryAPIs, + modelVersionId: string, +): Promise => { + if (!modelVersionId) { + throw new Error('Model version ID is required'); + } + + try { + const currentTime = new Date().toISOString(); + await api.patchModelVersion( + {}, + { + // This is a workaround to update the timestamp on the backend. There is a bug opened for model registry team + // to fix this issue. see https://issues.redhat.com/browse/RHOAIENG-17614 + state: ModelState.LIVE, + customProperties: { + _lastModified: { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: currentTime, + }, + }, + }, + modelVersionId, + ); + } catch (error) { + throw new Error( + `Failed to update model version timestamp: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +}; + +export const bumpRegisteredModelTimestamp = async ( + api: MinimalModelRegistryAPI, + registeredModelId: string, +): Promise => { + if (!registeredModelId) { + throw new Error('Registered model ID is required'); + } + + try { + const currentTime = new Date().toISOString(); + await api.patchRegisteredModel( + {}, + { + state: ModelState.LIVE, + customProperties: { + // This is a workaround to update the timestamp on the backend. There is a bug opened for model registry team + // to fix this issue. see https://issues.redhat.com/browse/RHOAIENG-17614 + _lastModified: { + metadataType: ModelRegistryMetadataType.STRING, + // eslint-disable-next-line camelcase + string_value: currentTime, + }, + }, + }, + registeredModelId, + ); + } catch (error) { + throw new Error( + `Failed to update registered model timestamp: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +}; + +export const bumpBothTimestamps = async ( + api: ModelRegistryAPIs, + modelVersionId: string, + registeredModelId: string, +): Promise => { + await Promise.all([ + bumpModelVersionTimestamp(api, modelVersionId), + bumpRegisteredModelTimestamp(api, registeredModelId), + ]); +}; diff --git a/clients/ui/frontend/src/shared/components/DashboardDescriptionListGroup.tsx b/clients/ui/frontend/src/shared/components/DashboardDescriptionListGroup.tsx index a9ab8b55e..23da670b5 100644 --- a/clients/ui/frontend/src/shared/components/DashboardDescriptionListGroup.tsx +++ b/clients/ui/frontend/src/shared/components/DashboardDescriptionListGroup.tsx @@ -8,13 +8,20 @@ import { DescriptionListTerm, Flex, FlexItem, + Popover, 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 { + CheckIcon, + PencilAltIcon, + TimesIcon, + OutlinedQuestionCircleIcon, +} from '@patternfly/react-icons'; -import '~/shared/components/DashboardDescriptionListGroup.scss'; +import './DashboardDescriptionListGroup.scss'; +import DashboardPopupIconButton from '~/shared/components/dashboard/DashboardPopupIconButton'; type EditableProps = { isEditing: boolean; @@ -23,21 +30,27 @@ type EditableProps = { onEditClick: () => void; onSaveEditsClick: () => void; onDiscardEditsClick: () => void; + editButtonTestId?: string; + saveButtonTestId?: string; + cancelButtonTestId?: string; + discardButtonTestId?: string; }; export type DashboardDescriptionListGroupProps = { title: string; - tooltip?: React.ReactNode; + popover?: React.ReactNode; action?: React.ReactNode; isEmpty?: boolean; contentWhenEmpty?: React.ReactNode; children: React.ReactNode; + groupTestId?: string; + isSaveDisabled?: boolean; } & (({ isEditable: true } & EditableProps) | ({ isEditable?: false } & Partial)); const DashboardDescriptionListGroup: React.FC = (props) => { const { title, - tooltip, + popover, action, isEmpty, contentWhenEmpty, @@ -49,11 +62,16 @@ const DashboardDescriptionListGroup: React.FC + {action || isEditable ? ( - + {title} @@ -62,35 +80,32 @@ const DashboardDescriptionListGroup: React.FC + isDisabled={isSavingEdits || isSaveDisabled} + /> + /> ) : ( - - + // make sure alert uses the full width + + {error && ( + + + {error.message} + + + )} + + + + + + + + + + + + + ); export default DashboardModalFooter; diff --git a/clients/ui/frontend/src/shared/components/DashboardSearchField.tsx b/clients/ui/frontend/src/shared/components/DashboardSearchField.tsx index 82f8f4359..f74d5a81e 100644 --- a/clients/ui/frontend/src/shared/components/DashboardSearchField.tsx +++ b/clients/ui/frontend/src/shared/components/DashboardSearchField.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { InputGroup, InputGroupItem, SearchInput } from '@patternfly/react-core'; import SimpleSelect from '~/shared/components/SimpleSelect'; -import { asEnumMember } from '~/app/utils'; +import { asEnumMember } from '~/shared/utilities/utils'; // List all the possible search fields here export enum SearchType { diff --git a/clients/ui/frontend/src/shared/components/EditableLabelsDescriptionListGroup.tsx b/clients/ui/frontend/src/shared/components/EditableLabelsDescriptionListGroup.tsx index 392987637..bbdfc7d67 100644 --- a/clients/ui/frontend/src/shared/components/EditableLabelsDescriptionListGroup.tsx +++ b/clients/ui/frontend/src/shared/components/EditableLabelsDescriptionListGroup.tsx @@ -1,220 +1,248 @@ -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 '~/shared/components/DashboardDescriptionListGroup'; - -type EditableTextDescriptionListGroupProps = Partial< - Pick -> & { +import React, { useState } from 'react'; +import { Label, LabelGroup, Alert, AlertVariant } from '@patternfly/react-core'; +import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; +import DashboardDescriptionListGroup from '~/shared/components/DashboardDescriptionListGroup'; + +interface EditableLabelsProps { labels: string[]; - saveEditedLabels: (labels: string[]) => Promise; - allExistingKeys?: string[]; + onLabelsChange: (labels: string[]) => Promise; isArchive?: boolean; -}; + title?: string; + contentWhenEmpty?: string; + allExistingKeys: string[]; +} -const EditableLabelsDescriptionListGroup: React.FC = ({ +export const EditableLabelsDescriptionListGroup: React.FC = ({ title = 'Labels', contentWhenEmpty = 'No labels', labels, - saveEditedLabels, + onLabelsChange, isArchive, - allExistingKeys = labels, + allExistingKeys, }) => { - const [isEditing, setIsEditing] = React.useState(false); - const [unsavedLabels, setUnsavedLabels] = React.useState(labels); - const [isSavingEdits, setIsSavingEdits] = React.useState(false); + const [isEditing, setIsEditing] = useState(false); + const [isSavingEdits, setIsSavingEdits] = useState(false); + const [hasSavedEdits, setHasSavedEdits] = useState(false); + const [unsavedLabels, setUnsavedLabels] = useState(labels); - const editUnsavedLabel = (newText: string, index: number) => { - if (isSavingEdits) { + const validateLabels = (): string[] => { + const errors: string[] = []; + + const duplicatesMap = new Map(); + unsavedLabels.forEach((label) => { + duplicatesMap.set(label, (duplicatesMap.get(label) || 0) + 1); + }); + + const duplicateLabels: string[] = []; + duplicatesMap.forEach((count, label) => { + if (count > 1) { + duplicateLabels.push(label); + } + }); + + if (duplicateLabels.length > 0) { + if (duplicateLabels.length === 1) { + const label = duplicateLabels[0] ?? ''; + errors.push(`**${label}** already exists as a label. Ensure that each label is unique.`); + } else { + const lastLabel = duplicateLabels.pop() ?? ''; + const formattedLabels = duplicateLabels.map((label) => `**${label}**`).join(', '); + errors.push( + `${formattedLabels} and **${lastLabel}** already exist as labels. Ensure that each label is unique.`, + ); + } + } + unsavedLabels.forEach((label) => { + if (allExistingKeys.includes(label) && !labels.includes(label)) { + errors.push( + `**${label}** already exists as a property key. Labels cannot use the same name as existing properties.`, + ); + } + if (label.length > 63) { + errors.push(`**${label}** can't exceed 63 characters`); + } + }); + + return errors; + }; + + const handleEditComplete = ( + _event: MouseEvent | KeyboardEvent, + newText: string, + currentLabel?: string, + ) => { + if (!newText) { return; } - const copy = [...unsavedLabels]; - copy[index] = newText; - setUnsavedLabels(copy); + + setUnsavedLabels((prev) => { + if (currentLabel) { + const index = prev.indexOf(currentLabel); + if (index === -1) { + return [...prev, newText]; + } + + const newLabels = [...prev]; + newLabels[index] = newText; + return newLabels; + } + return [...prev, newText]; + }); }; + const removeUnsavedLabel = (text: string) => { if (isSavingEdits) { return; } setUnsavedLabels(unsavedLabels.filter((label) => label !== text)); }; - const addUnsavedLabel = (text: string) => { + + const addNewLabel = () => { if (isSavingEdits) { return; } - setUnsavedLabels([...unsavedLabels, text]); - }; + const baseLabel = 'New Label'; + let counter = 1; + let newLabel = baseLabel; - // 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); + while (unsavedLabels.includes(newLabel)) { + newLabel = `${baseLabel} ${counter}`; + counter++; + } + + setUnsavedLabels((prev) => { + const updated = [...prev, newLabel]; + return updated; + }); }; - React.useEffect(() => { - if (isAddLabelModalOpen && addLabelInputRef.current) { - addLabelInputRef.current.focus(); + + const labelErrors = validateLabels(); + const shouldBeRed = (label: string, index: number): boolean => { + const firstIndex = unsavedLabels.findIndex((l) => l === label); + + if (firstIndex !== index) { + return true; } - }, [isAddLabelModalOpen]); - - const addLabelModalSubmitDisabled = !addLabelInputValue || !!addLabelValidationError; - const submitAddLabelModal = (event?: React.FormEvent) => { - event?.preventDefault(); - if (!addLabelModalSubmitDisabled) { - addUnsavedLabel(addLabelInputValue); - toggleAddLabelModal(); + + if (allExistingKeys.includes(label) && !labels.includes(label)) { + return true; } + + return false; }; + // Add a ref for the alert + const alertRef = React.useRef(null); + return ( - <> - Add label - ) + ) : undefined } > {unsavedLabels.map((label, index) => ( ))} + {labelErrors.length > 0 && ( + +
    + {labelErrors.map((error, index) => ( +
  • + {error + .split('**') + .map((part, i) => (i % 2 === 0 ? part : {part}))} +
    • + ))} +
+ + )} + + } + onEditClick={() => { + setUnsavedLabels(labels); + setIsEditing(true); + }} + onSaveEditsClick={async () => { + if (labelErrors.length > 0) { + return; } - onEditClick={() => { - setUnsavedLabels(labels); - setIsEditing(true); - }} - onSaveEditsClick={async () => { - setIsSavingEdits(true); - try { - await saveEditedLabels(unsavedLabels); - } finally { - setIsSavingEdits(false); - } + setIsSavingEdits(true); + try { + await onLabelsChange(unsavedLabels); + } finally { + setHasSavedEdits(true); + setIsSavingEdits(false); setIsEditing(false); - }} - onDiscardEditsClick={() => { - setUnsavedLabels(labels); - setIsEditing(false); - }} - > - - {labels.map((label) => ( - - ))} - - - - Save - , - , - ]} + } + }} + onDiscardEditsClick={() => { + setUnsavedLabels(labels); + setIsEditing(false); + }} + > + -
- - , value: string) => - setAddLabelInputValue(value) - } - ref={addLabelInputRef} - isRequired - validated={addLabelValidationError ? 'error' : 'default'} - /> - {addLabelValidationError && ( - - - } variant="error"> - {addLabelValidationError} - - - - )} - - - - + {labels.map((label) => ( + + ))} + + ); }; - -export default EditableLabelsDescriptionListGroup; diff --git a/clients/ui/frontend/src/shared/components/EditableTextDescriptionListGroup.tsx b/clients/ui/frontend/src/shared/components/EditableTextDescriptionListGroup.tsx index be1527d79..112ede6fc 100644 --- a/clients/ui/frontend/src/shared/components/EditableTextDescriptionListGroup.tsx +++ b/clients/ui/frontend/src/shared/components/EditableTextDescriptionListGroup.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { ExpandableSection, TextArea } from '@patternfly/react-core'; +import { ExpandableSection, TextArea, TextInput } from '@patternfly/react-core'; import DashboardDescriptionListGroup, { DashboardDescriptionListGroupProps, } from '~/shared/components/DashboardDescriptionListGroup'; @@ -12,8 +12,9 @@ type EditableTextDescriptionListGroupProps = Pick< > & { value: string; saveEditedValue: (value: string) => Promise; - testid?: string; + baseTestId?: string; isArchive?: boolean; + editableVariant: 'TextInput' | 'TextArea'; }; const EditableTextDescriptionListGroup: React.FC = ({ @@ -22,23 +23,36 @@ const EditableTextDescriptionListGroup: React.FC { const [isEditing, setIsEditing] = React.useState(false); const [unsavedValue, setUnsavedValue] = React.useState(value); const [isSavingEdits, setIsSavingEdits] = React.useState(false); const [isTextExpanded, setIsTextExpanded] = React.useState(false); - const editableTextArea = ( -
- {mr.displayName || mr.name} + + {mr.name} + {mr.description &&

{mr.description}

} 		+ + + {filteredRoleBindings.length === 0 ? ( + + + + ) : ( + + )} + + { + onEditRegistry(mr); + }, + }, + { + title: 'Delete model registry', + disabled: !isPlatformDefault(), + onClick: () => { + onDeleteRegistry(mr); + }, + }, + ]} + /> +