From dcadbc8c5080ae97ebb3ac1b27bc7fd4c6bab3cc Mon Sep 17 00:00:00 2001 From: ppadti Date: Fri, 30 Aug 2024 21:40:35 +0530 Subject: [PATCH] Update MR tables default sort state --- frontend/src/__mocks__/mockModelVersion.ts | 4 ++- .../modelRegistry/registeredModelArchive.ts | 4 +++ .../mocked/modelRegistry/modelRegistry.cy.ts | 11 ++++++-- .../mocked/modelRegistry/modelVersions.cy.ts | 14 ++++++++--- .../registeredModelArchive.cy.ts | 20 +++++++++++++++ frontend/src/concepts/modelRegistry/types.ts | 2 +- .../ModelVersions/ModelVersionListView.tsx | 7 ++++-- .../ModelVersions/ModelVersionsTable.tsx | 1 + .../RegisteredModels/RegisteredModelTable.tsx | 1 + .../RegisteredModelsTableColumns.ts | 2 +- .../RegisteredModelsArchiveTable.tsx | 2 +- .../screens/__tests__/utils.spec.ts | 25 +++++++++++++++++++ .../screens/components/ModelTimestamp.tsx | 2 +- .../src/pages/modelRegistry/screens/utils.ts | 7 ++++++ 14 files changed, 90 insertions(+), 12 deletions(-) diff --git a/frontend/src/__mocks__/mockModelVersion.ts b/frontend/src/__mocks__/mockModelVersion.ts index 3593a127e6..e0fe281142 100644 --- 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 index 9fad8a4ee7..fcba9d4102 100644 --- 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 index 6f39297340..912d675f5c 100644 --- 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 index 87a996a948..78bb89178d 100644 --- 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/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts index fa72797c28..3b9cc02efc 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registeredModelArchive.cy.ts +++ b/frontend/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 index 142809ce35..3d25428b9c 100644 --- 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 index 8a6240aa19..10402e20b6 100644 --- 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 = ({ setSearch('')} - modelVersions={filteredModelVersions} + modelVersions={sortModelVersionsByCreateTime(filteredModelVersions)} toolbarContent={ } breakpoint="xl"> diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions/ModelVersionsTable.tsx index f04f6943d2..e959d4630b 100644 --- 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 = ({ data={modelVersions} columns={mvColumns} toolbarContent={toolbarContent} + defaultSortColumn={3} enablePagination emptyTableView={} rowRenderer={(mv) => ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelTable.tsx index b6ffe98cf6..593479dad6 100644 --- 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 = ({ data={registeredModels} columns={rmColumns} toolbarContent={toolbarContent} + defaultSortColumn={2} enablePagination emptyTableView={} rowRenderer={(rm) => ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/RegisteredModelsTableColumns.ts index b367f9ad74..de801e5160 100644 --- 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[] = [ 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/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx index eeec2cd6e1..43f0de93eb 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModelsArchive/RegisteredModelsArchiveTable.tsx @@ -22,9 +22,9 @@ const RegisteredModelsArchiveTable: React.FC data={registeredModels} columns={rmColumns} toolbarContent={toolbarContent} + defaultSortColumn={2} enablePagination emptyTableView={} - defaultSortColumn={1} rowRenderer={(rm) => ( )} diff --git a/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts b/frontend/src/pages/modelRegistry/screens/__tests__/utils.spec.ts index 88aea0175c..fb5925e116 100644 --- 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 index 999b3e7a02..7fbebbea6f 100644 --- 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 = ({ timeSinceEpoch }) => { return ( + 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,