Provide the functionality to search models and model version by "labe…

…ls" (#3288) * working version * addressed comments and added test for labels * comments and more coverage
opendatahub-io · Oct 10, 2024 · 65ddbb9 · 65ddbb9
1 parent a3c2b58
commit 65ddbb9
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 21 deletions.
diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersions.cy.ts
@@ -200,6 +200,12 @@ describe('Model Versions', () => {
     modelRegistry.findModelVersionsTableRows().contains('new model version');
     modelRegistry.findModelVersionsTableSearch().focused().clear();
 
+    // filtering by label
+    modelRegistry.findModelVersionsTableSearch().type('Financial');
+    modelRegistry.findModelVersionsTableRows().should('have.length', 1);
+    modelRegistry.findModelVersionsTableRows().contains('new model version');
+    modelRegistry.findModelVersionsTableSearch().focused().clear();
+
     // filtering by model version author
     modelRegistry.findModelVersionsTableFilter().findSelectOption('Author').click();
     modelRegistry.findModelVersionsTableSearch().type('Test author');

diff --git a/frontend/src/concepts/modelRegistry/types.ts b/frontend/src/concepts/modelRegistry/types.ts
@@ -107,6 +107,7 @@ export type ModelVersion = ModelRegistryBase & {
   state?: ModelState;
   author?: string;
   registeredModelId: string;
+  labels?: string[];
 };
 
 export type RegisteredModel = ModelRegistryBase & {

diff --git a/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts b/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts
@@ -297,14 +297,15 @@ 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', () => {
@@ -317,6 +318,11 @@ describe('filterRegisteredModels', () => {
     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);
     expect(filtered).toEqual(registeredModels);

diff --git a/frontend/src/pages/modelRegistry/screens/utils.ts b/frontend/src/pages/modelRegistry/screens/utils.ts
@@ -79,30 +79,31 @@ 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;
     }
 
     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) => {
@@ -115,23 +116,29 @@ export const filterRegisteredModels = (
   unfilteredRegisteredModels: RegisteredModel[],
   search: string,
   searchType: SearchType,
-): RegisteredModel[] =>
-  unfilteredRegisteredModels.filter((rm: RegisteredModel) => {
+): RegisteredModel[] => {
+  const searchLower = search.toLowerCase();
+
+  return unfilteredRegisteredModels.filter((rm: RegisteredModel) => {
     if (!search) {
       return true;
     }
 
     switch (searchType) {
-      case SearchType.KEYWORD:
+      case SearchType.KEYWORD: {
         return (
-          rm.name.toLowerCase().includes(search.toLowerCase()) ||
-          (rm.description && rm.description.toLowerCase().includes(search.toLowerCase()))
+          rm.name.toLowerCase().includes(searchLower) ||
+          (rm.description && rm.description.toLowerCase().includes(searchLower)) ||
+          getLabels(rm.customProperties).some((label) => label.toLowerCase().includes(searchLower))
         );
+      }
 
-      case SearchType.OWNER:
-        return rm.owner && rm.owner.toLowerCase().includes(search.toLowerCase());
+      case SearchType.OWNER: {
+        return rm.owner && rm.owner.toLowerCase().includes(searchLower);
+      }
 
       default:
         return true;
     }
   });
+};