Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changed the source of Owner and updated field to be searchable and so… #3095

Merged
merged 6 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/src/__mocks__/mockRegisteredModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { createModelRegistryLabelsObject } from './utils';
type MockRegisteredModelType = {
id?: string;
name?: string;
owner?: string;
state?: ModelState;
description?: string;
labels?: string[];
};

export const mockRegisteredModel = ({
name = 'test',
owner = 'Author 1',
state = ModelState.LIVE,
description = '',
labels = [],
Expand All @@ -23,5 +25,6 @@ export const mockRegisteredModel = ({
lastUpdateTimeSinceEpoch: '1710404288975',
name,
state,
owner,
customProperties: createModelRegistryLabelsObject(labels),
});
4 changes: 2 additions & 2 deletions frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class ModelRegistryTableRow extends TableRow {
return this.find().findByTestId('description');
}

findAuthor() {
return this.find().findByTestId('registered-model-author');
findOwner() {
return this.find().findByTestId('registered-model-owner');
}

findLabelPopoverText() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe('Model Registry core', () => {
.contains(
'A machine learning model trained to detect fraudulent transactions in financial data',
);
registeredModelRow.findAuthor().contains('Author 1');
registeredModelRow.findOwner().contains('Author 1');

// Label popover
registeredModelRow.findLabelPopoverText().contains('2 more');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ describe('Restoring archive model', () => {
description: '',
externalID: '1234132asdfasdf',
state: 'LIVE',
owner: 'Author 1',
});
});
});
Expand Down Expand Up @@ -200,6 +201,7 @@ describe('Restoring archive model', () => {
description: '',
externalID: '1234132asdfasdf',
state: 'LIVE',
owner: 'Author 1',
});
});
});
Expand Down Expand Up @@ -233,6 +235,7 @@ describe('Archiving model', () => {
description: '',
externalID: '1234132asdfasdf',
state: 'ARCHIVED',
owner: 'Author 1',
});
});
});
Expand Down Expand Up @@ -266,6 +269,7 @@ describe('Archiving model', () => {
description: '',
externalID: '1234132asdfasdf',
state: 'ARCHIVED',
owner: 'Author 1',
});
});
});
Expand Down
1 change: 1 addition & 0 deletions frontend/src/concepts/dashboard/DashboardSearchField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export enum SearchType {
IDENTIFIER = 'Identifier',
KEYWORD = 'Keyword',
AUTHOR = 'Author',
OWNER = 'Owner',
}

type DashboardSearchFieldProps = {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/concepts/modelRegistry/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export type ModelVersion = ModelRegistryBase & {

export type RegisteredModel = ModelRegistryBase & {
state?: ModelState;
owner?: string;
};

export type InferenceService = ModelRegistryBase & {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import * as React from 'react';
import { ClipboardCopy, DescriptionList, Flex, FlexItem } from '@patternfly/react-core';
import { ClipboardCopy, DescriptionList, Flex, FlexItem, Text } from '@patternfly/react-core';
import { RegisteredModel } from '~/concepts/modelRegistry/types';
import { ModelRegistryContext } from '~/concepts/modelRegistry/context/ModelRegistryContext';
import DashboardDescriptionListGroup from '~/components/DashboardDescriptionListGroup';
import EditableTextDescriptionListGroup from '~/components/EditableTextDescriptionListGroup';
import EditableLabelsDescriptionListGroup from '~/components/EditableLabelsDescriptionListGroup';
import RegisteredModelAuthor from '~/pages/modelRegistry/screens/RegisteredModels/RegisteredModelAuthor';
import ModelTimestamp from '~/pages/modelRegistry/screens/components/ModelTimestamp';
import {
getLabels,
Expand Down Expand Up @@ -82,8 +81,8 @@ const ModelDetailsView: React.FC<ModelDetailsViewProps> = ({ registeredModel: rm
{rm.id}
</ClipboardCopy>
</DashboardDescriptionListGroup>
<DashboardDescriptionListGroup title="Author">
<RegisteredModelAuthor registeredModelId={rm.id} />
<DashboardDescriptionListGroup title="Owner">
<Text data-testid="registered-model-owner">{rm.owner || '-'}</Text>
</DashboardDescriptionListGroup>
<DashboardDescriptionListGroup
title="Last modified at"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const registerModel = async (
name: formData.modelName,
description: formData.modelDescription,
customProperties: {},
owner: author,
state: ModelState.LIVE,
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const RegisteredModelListView: React.FC<RegisteredModelListViewProps> = ({
const [searchType, setSearchType] = React.useState<SearchType>(SearchType.KEYWORD);
const [search, setSearch] = React.useState('');

const searchTypes = React.useMemo(() => [SearchType.KEYWORD], []); // TODO Add owner once RHOAIENG-7566 is completed.
const searchTypes = React.useMemo(() => [SearchType.KEYWORD, SearchType.OWNER], []);

if (unfilteredRegisteredModels.length === 0) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { ModelRegistryContext } from '~/concepts/modelRegistry/context/ModelRegi
import { ArchiveRegisteredModelModal } from '~/pages/modelRegistry/screens/components/ArchiveRegisteredModelModal';
import { getPatchBodyForRegisteredModel } from '~/pages/modelRegistry/screens/utils';
import { RestoreRegisteredModelModal } from '~/pages/modelRegistry/screens/components/RestoreRegisteredModel';
import RegisteredModelAuthor from './RegisteredModelAuthor';

type RegisteredModelTableRowProps = {
registeredModel: RegisteredModel;
Expand Down Expand Up @@ -76,8 +75,8 @@ const RegisteredModelTableRow: React.FC<RegisteredModelTableRowProps> = ({
<Td dataLabel="Last modified">
<ModelTimestamp timeSinceEpoch={rm.lastUpdateTimeSinceEpoch} />
</Td>
<Td dataLabel="Author">
<RegisteredModelAuthor registeredModelId={rm.id} />
<Td dataLabel="Owner">
<Text data-testid="registered-model-owner">{rm.owner || '-'}</Text>
</Td>
<Td isActionCell>
<ActionsColumn items={actions} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export const rmColumns: SortableData<RegisteredModel>[] = [
},
},
{
field: 'author',
label: 'Author',
sortable: false, // TODO Add sortable once RHOAIENG-7566 is completed.
field: 'owner',
label: 'Owner',
sortable: true,
info: {
tooltip: 'The owner is the user who registered the model.',
tooltipProps: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ describe('getPatchBody', () => {
it('returns a given RegisteredModel with id/name/timestamps removed, customProperties updated and other values unchanged', () => {
const registeredModel = mockRegisteredModel({
id: '1',
owner: 'Author 1',
name: 'test-model',
description: 'Description here',
labels: [],
Expand All @@ -289,6 +290,7 @@ describe('getPatchBody', () => {
customProperties: {
label1: { string_value: '', metadataType: ModelRegistryMetadataType.STRING },
},
owner: 'Author 1',
state: ModelState.LIVE,
externalID: '1234132asdfasdf',
} satisfies Partial<RegisteredModel>);
Expand Down
5 changes: 2 additions & 3 deletions frontend/src/pages/modelRegistry/screens/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,8 @@ export const filterRegisteredModels = (
(rm.description && rm.description.toLowerCase().includes(search.toLowerCase()))
);

case SearchType.AUTHOR:
// TODO Implement owner search functionality once RHOAIENG-7566 is completed.
return;
case SearchType.OWNER:
return rm.owner && rm.owner.toLowerCase().includes(search.toLowerCase());

default:
return true;
Expand Down
Loading