From 83b503b4593bcc888956ac41e329268a0d22b7aa Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Thu, 11 Jul 2024 12:37:57 -0400 Subject: [PATCH] Set experiment as Default on create run/schedule page --- .../cypress/pages/pipelines/createRunPage.ts | 63 +++++++++++++++--- .../cypress/pages/pipelines/experiments.ts | 60 ++++++++++++----- .../tests/mocked/pipelines/experiments.cy.ts | 64 +++++++++++-------- .../mocked/pipelines/pipelineCreateRuns.cy.ts | 44 +++++++------ .../pipelines/content/createRun/RunPage.tsx | 4 +- .../experiments/useDefaultExperiment.ts | 34 ++++++++++ 6 files changed, 198 insertions(+), 71 deletions(-) create mode 100644 frontend/src/pages/pipelines/global/experiments/useDefaultExperiment.ts diff --git a/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts b/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts index f8ab655e83..265939886c 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts @@ -1,12 +1,12 @@ /* eslint-disable camelcase */ -import type { - ExperimentKFv2, - PipelineKFv2, - PipelineRecurringRunKFv2, - PipelineRunKFv2, - PipelineVersionKFv2, +import { + type ExperimentKFv2, + type PipelineKFv2, + type PipelineRecurringRunKFv2, + type PipelineRunKFv2, + type PipelineVersionKFv2, } from '~/concepts/pipelines/kfTypes'; -import { buildMockExperiments, buildMockRunKF } from '~/__mocks__'; +import { buildMockRunKF } from '~/__mocks__'; import { buildMockPipelines } from '~/__mocks__/mockPipelinesProxy'; import { buildMockPipelineVersionsV2 } from '~/__mocks__/mockPipelineVersionsProxy'; import { Contextual } from '~/__tests__/cypress/cypress/pages/components/Contextual'; @@ -159,13 +159,58 @@ export class CreateRunPage { cy.findByTestId('pipeline-selector-table-list').find('td').contains(name).click(); } - mockGetExperiments(namespace: string, experiments?: ExperimentKFv2[]): Cypress.Chainable { + mockGetExperiments(namespace: string, experiments: ExperimentKFv2[]): Cypress.Chainable { return cy.interceptOdh( 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/experiments', { path: { namespace, serviceName: 'dspa' }, }, - buildMockExperiments(experiments), + (req) => { + // eslint-disable-next-line @typescript-eslint/naming-convention + const { filter, sort_by, page_size } = req.query; + let results = experiments; + if (sort_by) { + const fields = sort_by.toString().split(' '); + const sortField = fields[0]; + const sortDirection = fields[1]; + // more fields to be added + if (sortField === 'created_at') { + if (sortDirection === 'desc') { + results = results.toSorted( + (a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime(), + ); + } else { + results = results.toSorted( + (a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime(), + ); + } + } + } + if (filter) { + const { predicates } = JSON.parse(filter.toString()); + + if (predicates.length > 0) { + predicates.forEach((predicate: { key: string; string_value: string }) => { + // eslint-disable-next-line @typescript-eslint/naming-convention + const { key, string_value } = predicate; + if (key === 'storage_state') { + results = results.filter((experiment) => experiment.storage_state === string_value); + } + if (key === 'name') { + results = results.filter((experiment) => + experiment.display_name.includes(string_value), + ); + } + }); + } + } + + if (page_size) { + results = results.slice(0, Number(page_size)); + } + + req.reply({ experiments: results, total_size: results.length }); + }, ); } diff --git a/frontend/src/__tests__/cypress/cypress/pages/pipelines/experiments.ts b/frontend/src/__tests__/cypress/cypress/pages/pipelines/experiments.ts index 24d6c30827..d355e9a9bc 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/pipelines/experiments.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/pipelines/experiments.ts @@ -1,6 +1,6 @@ /* eslint-disable camelcase */ -import type { ExperimentKFv2 } from '~/concepts/pipelines/kfTypes'; +import { type ExperimentKFv2 } from '~/concepts/pipelines/kfTypes'; import { TableRow } from '~/__tests__/cypress/cypress/pages/components/table'; class ExperimentsTabs { @@ -30,29 +30,57 @@ class ExperimentsTabs { return new ExperimentsTable(() => cy.findByTestId('experiments-archived-tab-content')); } - mockGetExperiments( - namespace: string, - activeExperiments: ExperimentKFv2[], - archivedExperiments: ExperimentKFv2[] = [], - ) { + mockGetExperiments(namespace: string, experiments: ExperimentKFv2[]): Cypress.Chainable { return cy.interceptOdh( 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/experiments', { path: { namespace, serviceName: 'dspa' }, }, (req) => { - const { predicates } = JSON.parse(req.query.filter.toString()); - - if (predicates.length === 0) { - req.reply({ experiments: activeExperiments, total_size: activeExperiments.length }); - } else { - const [{ string_value: experimentState }] = predicates; - if (experimentState === 'ARCHIVED') { - req.reply({ experiments: archivedExperiments, total_size: archivedExperiments.length }); - } else { - req.reply({ experiments: activeExperiments, total_size: activeExperiments.length }); + // eslint-disable-next-line @typescript-eslint/naming-convention + const { filter, sort_by, page_size } = req.query; + let results = experiments; + if (sort_by) { + const fields = sort_by.toString().split(' '); + const sortField = fields[0]; + const sortDirection = fields[1]; + // more fields to be added + if (sortField === 'created_at') { + if (sortDirection === 'desc') { + results = results.toSorted( + (a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime(), + ); + } else { + results = results.toSorted( + (a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime(), + ); + } } } + if (filter) { + const { predicates } = JSON.parse(filter.toString()); + + if (predicates.length > 0) { + predicates.forEach((predicate: { key: string; string_value: string }) => { + // eslint-disable-next-line @typescript-eslint/naming-convention + const { key, string_value } = predicate; + if (key === 'storage_state') { + results = results.filter((experiment) => experiment.storage_state === string_value); + } + if (key === 'name') { + results = results.filter((experiment) => + experiment.display_name.includes(string_value), + ); + } + }); + } + } + + if (page_size) { + results = results.slice(0, Number(page_size)); + } + + req.reply({ experiments: results, total_size: results.length }); }, ); } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/experiments.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/experiments.cy.ts index cae6adfbb8..9db7f85102 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/experiments.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/experiments.cy.ts @@ -53,6 +53,11 @@ const mockExperiments = [ }), ]; +const mockArchivedExperiments = mockExperiments.map((experiment) => ({ + ...experiment, + storage_state: StorageStateKF.ARCHIVED, +})); + const mockActiveRuns = buildMockRunKF({ display_name: 'Test active run 4', run_id: 'run-4', @@ -109,10 +114,7 @@ describe('Experiments', () => { experimentsTabs.getActiveExperimentsTable().findFilterTextField().type('Test experiment 2'); // Mock experiments (filtered by typed experiment name) - experimentsTabs.mockGetExperiments( - projectName, - mockExperiments.filter((exp) => exp.display_name.includes('Test experiment 2')), - ); + experimentsTabs.mockGetExperiments(projectName, mockExperiments); // Verify only rows with the typed experiment name exist experimentsTabs.getActiveExperimentsTable().findRows().should('have.length', 1); @@ -125,6 +127,7 @@ describe('Experiments', () => { it('archive a single experiment', () => { const [experimentToArchive] = mockExperiments; + const [experimentArchived] = mockArchivedExperiments; const activeExperimentsTable = experimentsTabs.getActiveExperimentsTable(); @@ -134,14 +137,14 @@ describe('Experiments', () => { .findKebabAction('Archive') .click(); - experimentsTabs.mockGetExperiments( - projectName, - [mockExperiments[1], mockExperiments[2]], - [experimentToArchive], - ); - archiveExperimentModal.findConfirmInput().type(experimentToArchive.display_name); + experimentsTabs.mockGetExperiments(projectName, [ + mockExperiments[1], + mockExperiments[2], + experimentArchived, + ]); + archiveExperimentModal.findConfirmInput().type(experimentArchived.display_name); archiveExperimentModal.findSubmitButton().click(); - activeExperimentsTable.shouldRowNotBeVisible(experimentToArchive.display_name); + activeExperimentsTable.shouldRowNotBeVisible(experimentArchived.display_name); experimentsTabs.findArchivedTab().click(); experimentsTabs @@ -159,7 +162,7 @@ describe('Experiments', () => { }); activeExperimentsTable.findActionsKebab().findDropdownItem('Archive').click(); - experimentsTabs.mockGetExperiments(projectName, [], mockExperiments); + experimentsTabs.mockGetExperiments(projectName, mockArchivedExperiments); bulkArchiveExperimentModal.findConfirmInput().type('Archive 3 experiments'); bulkArchiveExperimentModal.findSubmitButton().click(); activeExperimentsTable.findEmptyState().should('exist'); @@ -178,12 +181,13 @@ describe('Experiments', () => { describe('Archived experiments', () => { beforeEach(() => { initIntercepts(); - experimentsTabs.mockGetExperiments(projectName, [], mockExperiments); + experimentsTabs.mockGetExperiments(projectName, mockArchivedExperiments); experimentsTabs.visit(projectName); experimentsTabs.findArchivedTab().click(); }); it('restore a single experiment', () => { - const [experimentToRestore] = mockExperiments; + const [experimentToRestore] = mockArchivedExperiments; + const [experimentRestored] = mockExperiments; const archivedExperimentsTable = experimentsTabs.getArchivedExperimentsTable(); @@ -196,11 +200,11 @@ describe('Experiments', () => { .findKebabAction('Restore') .click(); - experimentsTabs.mockGetExperiments( - projectName, - [experimentToRestore], - [mockExperiments[1], mockExperiments[2]], - ); + experimentsTabs.mockGetExperiments(projectName, [ + mockArchivedExperiments[1], + mockArchivedExperiments[2], + experimentRestored, + ]); restoreExperimentModal.findSubmitButton().click(); archivedExperimentsTable.shouldRowNotBeVisible(experimentToRestore.display_name); @@ -214,7 +218,7 @@ describe('Experiments', () => { it('restore multiple experiments', () => { const archivedExperimentsTable = experimentsTabs.getArchivedExperimentsTable(); - mockExperiments.forEach((archivedExperiment) => { + mockArchivedExperiments.forEach((archivedExperiment) => { archivedExperimentsTable.mockRestoreExperiment( archivedExperiment.experiment_id, projectName, @@ -225,7 +229,7 @@ describe('Experiments', () => { .click(); }); archivedExperimentsTable.findRestoreExperimentButton().click(); - experimentsTabs.mockGetExperiments(projectName, mockExperiments, []); + experimentsTabs.mockGetExperiments(projectName, mockExperiments); bulkRestoreExperimentModal.findSubmitButton().click(); archivedExperimentsTable.findEmptyState().should('exist'); @@ -318,7 +322,7 @@ describe('Experiments', () => { describe('Runs page for archived experiment', () => { const archivedExperimentsTable = experimentsTabs.getArchivedExperimentsTable(); - const mockExperiment = { ...mockExperiments[0], storage_state: StorageStateKF.ARCHIVED }; + const [mockArchivedExperiment] = mockArchivedExperiments; beforeEach(() => { initIntercepts(); @@ -328,10 +332,10 @@ describe('Runs page for archived experiment', () => { path: { namespace: projectName, serviceName: 'dspa', - experimentId: mockExperiment.experiment_id, + experimentId: mockArchivedExperiment.experiment_id, }, }, - mockExperiment, + mockArchivedExperiment, ); cy.interceptOdh( 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/runs', @@ -348,14 +352,20 @@ describe('Runs page for archived experiment', () => { }, { recurringRuns: [buildMockRecurringRunKF({ status: RecurringRunStatus.DISABLED })] }, ); - experimentsTabs.mockGetExperiments(projectName, [], mockExperiments); + experimentsTabs.mockGetExperiments(projectName, mockArchivedExperiments); experimentsTabs.visit(projectName); experimentsTabs.findArchivedTab().click(); - archivedExperimentsTable.getRowByName(mockExperiment.display_name).find().find('a').click(); + archivedExperimentsTable + .getRowByName(mockArchivedExperiment.display_name) + .find() + .find('a') + .click(); }); it('navigates to the runs page when clicking an experiment name', () => { - verifyRelativeURL(`/experiments/${projectName}/${mockExperiment.experiment_id}/runs`); + verifyRelativeURL( + `/experiments/${projectName}/${mockArchivedExperiment.experiment_id}/runs/archived`, + ); cy.findByLabelText('Breadcrumb').findByText(`Experiments - ${projectName}`); }); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelineCreateRuns.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelineCreateRuns.cy.ts index d4cc80d008..0bf16916ce 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelineCreateRuns.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelineCreateRuns.cy.ts @@ -1,6 +1,6 @@ /* eslint-disable camelcase */ import type { PipelineRecurringRunKFv2, PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; -import { InputDefinitionParameterType } from '~/concepts/pipelines/kfTypes'; +import { InputDefinitionParameterType, StorageStateKF } from '~/concepts/pipelines/kfTypes'; import { buildMockRunKF, buildMockPipelineV2, @@ -32,10 +32,17 @@ const mockExperiments = [ buildMockExperimentKF({ display_name: 'Test experiment 1', experiment_id: 'experiment-1', + created_at: '2024-01-30T15:46:33Z', }), buildMockExperimentKF({ display_name: 'Test experiment 2', - experiment_id: 'experiment-1', + experiment_id: 'experiment-2', + }), + buildMockExperimentKF({ + display_name: 'Default', + experiment_id: 'default', + storage_state: StorageStateKF.ARCHIVED, + created_at: '2024-01-29T15:46:33Z', }), ]; const initialMockRuns = [ @@ -133,6 +140,7 @@ describe('Pipeline create runs', () => { // Fill out the form without a schedule and submit createRunPage.fillName('New run'); createRunPage.fillDescription('New run description'); + createRunPage.findExperimentSelect().should('contain.text', 'Select an experiment'); createRunPage.findExperimentSelect().should('not.be.disabled').click(); createRunPage.selectExperimentByName('Test experiment 1'); createRunPage.findPipelineSelect().should('not.be.disabled').click(); @@ -486,22 +494,6 @@ describe('Pipeline create runs', () => { }); describe('Schedules', () => { - beforeEach(() => { - mockExperiments.forEach((experiment) => { - cy.interceptOdh( - 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/experiments/:experimentId', - { - path: { - namespace: projectName, - serviceName: 'dspa', - experimentId: experiment.experiment_id, - }, - }, - experiment, - ); - }); - }); - it('switches to scheduled runs from triggered', () => { visitLegacyRunsPage(); pipelineRunsGlobal.findSchedulesTab().click(); @@ -546,6 +538,7 @@ describe('Pipeline create runs', () => { // Mock experiments, pipelines & versions for form select dropdowns createSchedulePage.mockGetExperiments(projectName, mockExperiments); + createSchedulePage.mockGetExperiments(projectName, mockExperiments); createSchedulePage.mockGetPipelines(projectName, [mockPipeline]); createSchedulePage.mockGetPipelineVersions( projectName, @@ -563,6 +556,7 @@ describe('Pipeline create runs', () => { // Fill out the form with a schedule and submit createSchedulePage.fillName('New recurring run'); createSchedulePage.fillDescription('New recurring run description'); + createSchedulePage.findExperimentSelect().should('contain.text', 'Default'); createSchedulePage.findExperimentSelect().should('not.be.disabled').click(); createSchedulePage.selectExperimentByName('Test experiment 1'); createSchedulePage.findPipelineSelect().should('not.be.disabled').click(); @@ -817,4 +811,18 @@ const initIntercepts = () => { pipeline_version_id: mockPipelineVersion.pipeline_version_id, }), ); + + mockExperiments.forEach((experiment) => { + cy.interceptOdh( + 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/experiments/:experimentId', + { + path: { + namespace: projectName, + serviceName: 'dspa', + experimentId: experiment.experiment_id, + }, + }, + experiment, + ); + }); }; diff --git a/frontend/src/concepts/pipelines/content/createRun/RunPage.tsx b/frontend/src/concepts/pipelines/content/createRun/RunPage.tsx index b68706510f..4f157d06ac 100644 --- a/frontend/src/concepts/pipelines/content/createRun/RunPage.tsx +++ b/frontend/src/concepts/pipelines/content/createRun/RunPage.tsx @@ -30,6 +30,7 @@ import { useGetSearchParamValues } from '~/utilities/useGetSearchParamValues'; import { PipelineRunSearchParam } from '~/concepts/pipelines/content/types'; import { asEnumMember } from '~/utilities/utils'; import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; +import useDefaultExperiment from '~/pages/pipelines/global/experiments/useDefaultExperiment'; type RunPageProps = { cloneRun?: PipelineRunKFv2 | PipelineRecurringRunKFv2 | null; @@ -64,6 +65,7 @@ const RunPage: React.FC = ({ const isSchedule = runType === RunTypeOption.SCHEDULED; const isExperimentsAvailable = useIsAreaAvailable(SupportedArea.PIPELINE_EXPERIMENTS).status; + const [defaultExperiment] = useDefaultExperiment(); const jumpToSections = Object.values(CreateRunPageSections).filter( (section) => @@ -94,7 +96,7 @@ const RunPage: React.FC = ({ runType: runTypeData, pipeline: locationPipeline || contextPipeline, version: locationVersion || contextPipelineVersion, - experiment: locationExperiment || contextExperiment, + experiment: locationExperiment || contextExperiment || defaultExperiment, }); const onValueChange = React.useCallback( diff --git a/frontend/src/pages/pipelines/global/experiments/useDefaultExperiment.ts b/frontend/src/pages/pipelines/global/experiments/useDefaultExperiment.ts new file mode 100644 index 0000000000..4e3338678f --- /dev/null +++ b/frontend/src/pages/pipelines/global/experiments/useDefaultExperiment.ts @@ -0,0 +1,34 @@ +import React from 'react'; +import { usePipelinesAPI } from '~/concepts/pipelines/context'; +import { ExperimentKFv2, PipelinesFilterOp } from '~/concepts/pipelines/kfTypes'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; + +/** + * Fetch the first created experiment and check it's name to make sure it's default experiment + */ +const useDefaultExperiment = (): FetchState => { + const { api } = usePipelinesAPI(); + + const getDefaultExperiment = React.useCallback< + FetchStateCallbackPromise + >(async () => { + const response = await api.listExperiments( + {}, + { + filter: { + predicates: [ + // eslint-disable-next-line camelcase + { key: 'name', operation: PipelinesFilterOp.EQUALS, string_value: 'Default' }, + ], + }, + pageSize: 1, + }, + ); + + return response.experiments?.[0] || null; + }, [api]); + + return useFetchState(getDefaultExperiment, null); +}; + +export default useDefaultExperiment;