Update MR tables default sort state

opendatahub-io · Sep 3, 2024 · dcadbc8 · dcadbc8
1 parent 3174a74
commit dcadbc8
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 12 deletions.
diff --git a/frontend/src/__mocks__/mockModelVersion.ts b/frontend/src/__mocks__/mockModelVersion.ts
@@ -10,6 +10,7 @@ type MockModelVersionType = {
   state?: ModelState;
   description?: string;
   createTimeSinceEpoch?: string;
+  lastUpdateTimeSinceEpoch?: string;
 };
 
 export const mockModelVersion = ({
@@ -21,12 +22,13 @@ export const mockModelVersion = ({
   state = ModelState.LIVE,
   description = 'Description of model version',
   createTimeSinceEpoch = '1712234877179',
+  lastUpdateTimeSinceEpoch = '1712234877179',
 }: MockModelVersionType): ModelVersion => ({
   author,
   createTimeSinceEpoch,
   customProperties: createModelRegistryLabelsObject(labels),
   id,
-  lastUpdateTimeSinceEpoch: '1712234877179',
+  lastUpdateTimeSinceEpoch,
   name,
   state,
   registeredModelId,

diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive.ts
@@ -90,6 +90,10 @@ class ModelArchive {
     return cy.findByTestId('archive-model-page-breadcrumb');
   }
 
+  findRegisteredModelsArchiveTableHeaderButton(name: string) {
+    return this.findArchiveModelTable().find('thead').findByRole('button', { name });
+  }
+
   findArchiveModelTable() {
     return cy.findByTestId('registered-models-archive-table');
   }

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelRegistry.cy.ts
@@ -209,13 +209,20 @@ describe('Model Registry core', () => {
       labelModal.findCloseModal().click();
     });
 
-    it('Sorts by model name', () => {
+    it('Sort by Model name', () => {
+      modelRegistry.findRegisteredModelTableHeaderButton('Model name').click();
       modelRegistry.findRegisteredModelTableHeaderButton('Model name').should(be.sortAscending);
       modelRegistry.findRegisteredModelTableHeaderButton('Model name').click();
       modelRegistry.findRegisteredModelTableHeaderButton('Model name').should(be.sortDescending);
     });
 
-    it('Filters by keyword', () => {
+    it('Sort by Last modified', () => {
+      modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortAscending);
+      modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click();
+      modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortDescending);
+    });
+
+    it('Filter by keyword', () => {
       modelRegistry.findTableSearch().type('Fraud detection model');
       modelRegistry.findTableRows().should('have.length', 1);
       modelRegistry.findTableRows().contains('Fraud detection model');

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
@@ -91,12 +91,13 @@ const initIntercepts = ({
       path: {
         serviceName: 'modelregistry-sample',
         apiVersion: MODEL_REGISTRY_API_VERSION,
-        modelVersionId: 2,
+        modelVersionId: 1,
       },
     },
-    mockModelVersion({ id: '2', name: 'model version' }),
+    mockModelVersion({ id: '1', name: 'model version' }),
   );
 };
+
 describe('Model Versions', () => {
   it('No model versions in the selected registered model', () => {
     initIntercepts({
@@ -167,10 +168,17 @@ describe('Model Versions', () => {
     labelModal.findCloseModal().click();
 
     // sort by model version name
+    modelRegistry.findModelVersionsTableHeaderButton('Version name').click();
     modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortAscending);
     modelRegistry.findModelVersionsTableHeaderButton('Version name').click();
     modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortDescending);
 
+    // sort by Last modified
+    modelRegistry.findModelVersionsTableHeaderButton('Last modified').click();
+    modelRegistry.findModelVersionsTableHeaderButton('Last modified').should(be.sortAscending);
+    modelRegistry.findModelVersionsTableHeaderButton('Last modified').click();
+    modelRegistry.findModelVersionsTableHeaderButton('Last modified').should(be.sortDescending);
+
     // sort by model version author
     modelRegistry.findModelVersionsTableHeaderButton('Author').click();
     modelRegistry.findModelVersionsTableHeaderButton('Author').should(be.sortAscending);
@@ -200,7 +208,7 @@ describe('Model Versions', () => {
     verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`);
     const modelVersionRow = modelRegistry.getModelVersionRow('model version');
     modelVersionRow.findModelVersionName().contains('model version').click();
-    verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/1/versions/2/details');
+    verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/1/versions/1/details');
     cy.findByTestId('app-page-title').should('have.text', 'model version');
     cy.findByTestId('breadcrumb-version-name').should('have.text', 'model version');
     cy.go('back');

diff --git a/...end/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts b/...end/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts
@@ -16,6 +16,7 @@ import {
 } from '~/__tests__/cypress/cypress/pages/modelRegistry/registeredModelArchive';
 import { mockModelVersionList } from '~/__mocks__/mockModelVersionList';
 import { mockModelRegistryService } from '~/__mocks__/mockModelRegistryService';
+import { be } from '~/__tests__/cypress/cypress/utils/should';
 
 const MODEL_REGISTRY_API_VERSION = 'v1alpha3';
 
@@ -150,6 +151,25 @@ describe('Model archive list', () => {
       'Test label y',
     ]);
     labelModal.findCloseModal().click();
+
+    // sort by Last modified
+    registeredModelArchive
+      .findRegisteredModelsArchiveTableHeaderButton('Last modified')
+      .should(be.sortAscending);
+    registeredModelArchive.findRegisteredModelsArchiveTableHeaderButton('Last modified').click();
+    registeredModelArchive
+      .findRegisteredModelsArchiveTableHeaderButton('Last modified')
+      .should(be.sortDescending);
+
+    // sort by Model name
+    registeredModelArchive.findRegisteredModelsArchiveTableHeaderButton('Model name').click();
+    registeredModelArchive
+      .findRegisteredModelsArchiveTableHeaderButton('Model name')
+      .should(be.sortAscending);
+    registeredModelArchive.findRegisteredModelsArchiveTableHeaderButton('Model name').click();
+    registeredModelArchive
+      .findRegisteredModelsArchiveTableHeaderButton('Model name')
+      .should(be.sortDescending);
   });
 });
 

diff --git a/frontend/src/concepts/modelRegistry/types.ts b/frontend/src/concepts/modelRegistry/types.ts
@@ -86,7 +86,7 @@ export type ModelRegistryBase = {
   name: string;
   externalID?: string;
   description?: string;
-  createTimeSinceEpoch?: string;
+  createTimeSinceEpoch: string;
   lastUpdateTimeSinceEpoch: string;
   customProperties: ModelRegistryCustomProperties;
 };

diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx
@@ -19,7 +19,10 @@ import { SearchType } from '~/concepts/dashboard/DashboardSearchField';
 import { ModelVersion, RegisteredModel } from '~/concepts/modelRegistry/types';
 import SimpleSelect from '~/components/SimpleSelect';
 import EmptyModelRegistryState from '~/pages/modelRegistry/screens/components/EmptyModelRegistryState';
-import { filterModelVersions } from '~/pages/modelRegistry/screens/utils';
+import {
+  filterModelVersions,
+  sortModelVersionsByCreateTime,
+} from '~/pages/modelRegistry/screens/utils';
 import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext';
 import {
   modelVersionArchiveUrl,
@@ -74,7 +77,7 @@ const ModelVersionListView: React.FC<ModelVersionListViewProps> = ({
     <ModelVersionsTable
       refresh={refresh}
       clearFilters={() => setSearch('')}
-      modelVersions={filteredModelVersions}
+      modelVersions={sortModelVersionsByCreateTime(filteredModelVersions)}
       toolbarContent={
         <ToolbarContent>
           <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">

diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx
@@ -22,6 +22,7 @@ const ModelVersionsTable: React.FC<ModelVersionsTableProps> = ({
     data={modelVersions}
     columns={mvColumns}
     toolbarContent={toolbarContent}
+    defaultSortColumn={3}
     enablePagination
     emptyTableView={<DashboardEmptyTableView onClearFilters={clearFilters} />}
     rowRenderer={(mv) => (

diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx
@@ -22,6 +22,7 @@ const RegisteredModelTable: React.FC<RegisteredModelTableProps> = ({
     data={registeredModels}
     columns={rmColumns}
     toolbarContent={toolbarContent}
+    defaultSortColumn={2}
     enablePagination
     emptyTableView={<DashboardEmptyTableView onClearFilters={clearFilters} />}
     rowRenderer={(rm) => (

diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts
@@ -20,7 +20,7 @@ export const rmColumns: SortableData<RegisteredModel>[] = [
     sortable: (a: RegisteredModel, b: RegisteredModel): number => {
       const first = parseInt(a.lastUpdateTimeSinceEpoch);
       const second = parseInt(b.lastUpdateTimeSinceEpoch);
-      return new Date(first).getTime() - new Date(second).getTime();
+      return new Date(second).getTime() - new Date(first).getTime();
     },
   },
   {

diff --git a/.../src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx b/.../src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx
@@ -22,9 +22,9 @@ const RegisteredModelsArchiveTable: React.FC<RegisteredModelsArchiveTableProps>
     data={registeredModels}
     columns={rmColumns}
     toolbarContent={toolbarContent}
+    defaultSortColumn={2}
     enablePagination
     emptyTableView={<DashboardEmptyTableView onClearFilters={clearFilters} />}
-    defaultSortColumn={1}
     rowRenderer={(rm) => (
       <RegisteredModelTableRow key={rm.name} registeredModel={rm} isArchiveRow refresh={refresh} />
     )}

diff --git a/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts b/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts
@@ -16,6 +16,7 @@ import {
   mergeUpdatedProperty,
   mergeUpdatedLabels,
   filterRegisteredModels,
+  sortModelVersionsByCreateTime,
 } from '~/pages/modelRegistry/screens/utils';
 import { SearchType } from '~/concepts/dashboard/DashboardSearchField';
 
@@ -321,3 +322,27 @@ describe('filterRegisteredModels', () => {
     expect(filtered).toEqual(registeredModels);
   });
 });
+
+describe('sortModelVersionsByCreateTime', () => {
+  it('should return list of sorted modelVersions by create time', () => {
+    const modelVersions: ModelVersion[] = [
+      mockModelVersion({
+        name: 'model version 1',
+        author: 'Author 1',
+        id: '1',
+        createTimeSinceEpoch: '1725018764650',
+        lastUpdateTimeSinceEpoch: '1725030215299',
+      }),
+      mockModelVersion({
+        name: 'model version 1',
+        author: 'Author 1',
+        id: '1',
+        createTimeSinceEpoch: '1725028468207',
+        lastUpdateTimeSinceEpoch: '1725030142332',
+      }),
+    ];
+
+    const result = sortModelVersionsByCreateTime(modelVersions);
+    expect(result).toEqual([modelVersions[1], modelVersions[0]]);
+  });
+});
diff --git a/frontend/src/pages/modelRegistry/screens/components/ModelTimestamp.tsx b/frontend/src/pages/modelRegistry/screens/components/ModelTimestamp.tsx
@@ -19,7 +19,7 @@ const ModelTimestamp: React.FC<ModelTimestampProps> = ({ timeSinceEpoch }) => {
 
   return (
     <Timestamp
-      date={new Date(timeSinceEpoch)}
+      date={new Date(parseInt(timeSinceEpoch))}
       tooltip={{
         variant: TimestampTooltipVariant.default,
       }}

diff --git a/frontend/src/pages/modelRegistry/screens/utils.ts b/frontend/src/pages/modelRegistry/screens/utils.ts
@@ -104,6 +104,13 @@ export const filterModelVersions = (
     }
   });
 
+export const sortModelVersionsByCreateTime = (registeredModels: ModelVersion[]): ModelVersion[] =>
+  registeredModels.toSorted((a, b) => {
+    const first = parseInt(a.createTimeSinceEpoch);
+    const second = parseInt(b.createTimeSinceEpoch);
+    return new Date(second).getTime() - new Date(first).getTime();
+  });
+
 export const filterRegisteredModels = (
   unfilteredRegisteredModels: RegisteredModel[],
   search: string,