From 6c2204528ce508030dc904a53dc9cd100fe849ae Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Tue, 16 Jul 2024 22:54:29 -0400 Subject: [PATCH] [RHOAIENG-4702] Improve unique naming for runs and versions in pipelines --- .../cypress/pages/pipelines/createRunPage.ts | 4 +- .../mocked/pipelines/pipelineCreateRuns.cy.ts | 4 + .../tests/mocked/pipelines/pipelines.cy.ts | 190 +++++++++--------- .../mocked/pipelines/pipelinesList.cy.ts | 175 +++++++++++++--- .../src/concepts/k8s/NameDescriptionField.tsx | 24 ++- .../apiHooks/useAllPipelineVersions.ts | 74 ++++++- .../apiHooks/usePipelineRecurringRuns.ts | 62 +++++- .../pipelines/apiHooks/usePipelineRuns.ts | 62 +++++- .../pipelines/apiHooks/usePipelines.ts | 2 +- .../content/DuplicateNameHelperText.tsx | 31 +++ .../pipelines/content/createRun/RunForm.tsx | 7 + .../pipelines/content/createRun/utils.ts | 2 + .../content/import/PipelineImportModal.tsx | 10 +- .../import/PipelineVersionImportModal.tsx | 11 +- 14 files changed, 510 insertions(+), 148 deletions(-) create mode 100644 frontend/src/concepts/pipelines/content/DuplicateNameHelperText.tsx diff --git a/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts b/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts index f8ab655e83..74bf5b0306 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/pipelines/createRunPage.ts @@ -144,11 +144,11 @@ export class CreateRunPage { } fillName(value: string): void { - this.findNameInput().type(value); + this.findNameInput().clear().type(value); } fillDescription(value: string): void { - this.findDescriptionInput().type(value); + this.findDescriptionInput().clear().type(value); } selectExperimentByName(name: string): void { 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..4440f9a56c 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 @@ -131,6 +131,8 @@ describe('Pipeline create runs', () => { createRunPage.find(); // Fill out the form without a schedule and submit + createRunPage.fillName(initialMockRuns[0].display_name); + cy.findByTestId('duplicate-name-help-text').should('be.visible'); createRunPage.fillName('New run'); createRunPage.fillDescription('New run description'); createRunPage.findExperimentSelect().should('not.be.disabled').click(); @@ -561,6 +563,8 @@ describe('Pipeline create runs', () => { createSchedulePage.find(); // Fill out the form with a schedule and submit + createRunPage.fillName(initialMockRecurringRuns[0].display_name); + cy.findByTestId('duplicate-name-help-text').should('be.visible'); createSchedulePage.fillName('New recurring run'); createSchedulePage.fillDescription('New recurring run description'); createSchedulePage.findExperimentSelect().should('not.be.disabled').click(); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelines.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelines.cy.ts index 6f70b8c831..decdbe02a1 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelines.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelines.cy.ts @@ -364,8 +364,28 @@ describe('Pipelines', () => { }); it('View pipeline server', () => { - const visitPipelineProjects = () => pipelinesGlobal.visit(projectName); - viewPipelineServerDetailsTest(visitPipelineProjects); + initIntercepts({}); + cy.interceptK8s( + { + model: SecretModel, + ns: projectName, + }, + mockSecretK8sResource({ + s3Bucket: 'c2RzZA==', + namespace: projectName, + name: 'aws-connection-test', + }), + ); + pipelinesGlobal.visit(projectName); + + pipelinesGlobal.selectPipelineServerAction('View pipeline server configuration'); + viewPipelineServerModal.shouldHaveAccessKey('sdsd'); + viewPipelineServerModal.findPasswordHiddenButton().click(); + viewPipelineServerModal.shouldHaveSecretKey('sdsd'); + viewPipelineServerModal.shouldHaveEndPoint('https://s3.amazonaws.com'); + viewPipelineServerModal.shouldHaveBucketName('test-pipelines-bucket'); + + viewPipelineServerModal.findCloseButton().click(); }); it('renders the page with pipelines table data', () => { @@ -538,11 +558,8 @@ describe('Pipelines', () => { }; const uploadedMockPipeline = buildMockPipelineV2(uploadPipelineParams); - // Intercept upload/re-fetch of pipelines + // Intercept uploaded pipeline pipelineImportModal.mockUploadPipeline(uploadPipelineParams, projectName).as('uploadPipeline'); - pipelinesTable - .mockGetPipelines([initialMockPipeline, uploadedMockPipeline], projectName) - .as('refreshPipelines'); // Wait for the pipelines table to load pipelinesTable.find(); @@ -552,9 +569,15 @@ describe('Pipelines', () => { // Fill out the "Import pipeline" modal and submit pipelineImportModal.shouldBeOpen(); + pipelineImportModal.fillPipelineName(initialMockPipeline.display_name); + cy.findByTestId('duplicate-name-help-text').should('be.visible'); pipelineImportModal.fillPipelineName('New pipeline'); pipelineImportModal.fillPipelineDescription('New pipeline description'); pipelineImportModal.uploadPipelineYaml(pipelineYamlPath); + + pipelinesTable + .mockGetPipelines([initialMockPipeline, uploadedMockPipeline], projectName) + .as('refreshPipelines'); pipelineImportModal.submit(); // Wait for upload/fetch requests @@ -574,7 +597,7 @@ describe('Pipelines', () => { }); cy.wait('@refreshPipelines').then((interception) => { - expect(interception.request.query).to.eql({ sort_by: 'created_at desc', page_size: '10' }); + expect(interception.request.query).to.eql({ sort_by: 'created_at desc' }); }); // Verify the uploaded pipeline is in the table @@ -613,18 +636,10 @@ describe('Pipelines', () => { }; const createdMockPipeline = buildMockPipelineV2(createPipelineAndVersionParams.pipeline); - // Intercept upload/re-fetch of pipelines + // Intercept upload of pipelines pipelineImportModal .mockCreatePipelineAndVersion(createPipelineAndVersionParams, projectName) .as('createPipelineAndVersion'); - pipelinesTable - .mockGetPipelines([initialMockPipeline, createdMockPipeline], projectName) - .as('refreshPipelines'); - pipelinesTable.mockGetPipelineVersions( - [buildMockPipelineVersionV2(createPipelineAndVersionParams.pipeline_version)], - 'new-pipeline', - projectName, - ); // Wait for the pipelines table to load pipelinesTable.find(); @@ -637,6 +652,16 @@ describe('Pipelines', () => { pipelineImportModal.fillPipelineName('New pipeline'); pipelineImportModal.findImportPipelineRadio().check(); pipelineImportModal.findPipelineUrlInput().type('https://example.com/pipeline.yaml'); + + // Intercept re-fetch of pipelines + pipelinesTable + .mockGetPipelines([initialMockPipeline, createdMockPipeline], projectName) + .as('refreshPipelines'); + pipelinesTable.mockGetPipelineVersions( + [buildMockPipelineVersionV2(createPipelineAndVersionParams.pipeline_version)], + 'new-pipeline', + projectName, + ); pipelineImportModal.submit(); // Wait for upload/fetch requests @@ -664,17 +689,10 @@ describe('Pipelines', () => { const uploadedMockPipelineVersion = buildMockPipelineVersionV2(uploadVersionParams); - // Intercept upload/re-fetch of pipeline versions + // Intercept upload of pipeline version pipelineVersionImportModal .mockUploadVersion(uploadVersionParams, projectName) .as('uploadVersion'); - pipelinesTable - .mockGetPipelineVersions( - [initialMockPipelineVersion, uploadedMockPipelineVersion], - initialMockPipeline.pipeline_id, - projectName, - ) - .as('refreshVersions'); // Fill out the "Upload new version" modal and submit pipelineVersionImportModal.shouldBeOpen(); @@ -682,6 +700,15 @@ describe('Pipelines', () => { pipelineVersionImportModal.fillVersionName('New pipeline version'); pipelineVersionImportModal.fillVersionDescription('New pipeline version description'); pipelineVersionImportModal.uploadPipelineYaml(pipelineYamlPath); + + // Intercept re-fetch of pipeline versions + pipelinesTable + .mockGetPipelineVersions( + [initialMockPipelineVersion, uploadedMockPipelineVersion], + initialMockPipeline.pipeline_id, + projectName, + ) + .as('refreshVersions'); pipelineVersionImportModal.submit(); // Wait for upload/fetch requests @@ -735,17 +762,7 @@ describe('Pipelines', () => { // Open the "Upload new version" modal pipelinesGlobal.findUploadVersionButton().click(); - const uploadedMockPipelineVersion = buildMockPipelineVersionV2(createPipelineVersionParams); - - // Intercept upload/re-fetch of pipeline versions - pipelinesTable - .mockGetPipelineVersions( - [initialMockPipelineVersion, uploadedMockPipelineVersion], - initialMockPipeline.pipeline_id, - projectName, - ) - .as('refreshVersions'); - + // Intercept uploaded pipeline version pipelineVersionImportModal .mockCreatePipelineVersion(createPipelineVersionParams, projectName) .as('createVersion'); @@ -753,9 +770,21 @@ describe('Pipelines', () => { // Fill out the "Upload new version" modal and submit pipelineVersionImportModal.shouldBeOpen(); pipelineVersionImportModal.selectPipelineByName('Test pipeline'); + pipelineVersionImportModal.fillVersionName(initialMockPipelineVersion.display_name); + cy.findByTestId('duplicate-name-help-text').should('be.visible'); pipelineVersionImportModal.fillVersionName('New pipeline version'); pipelineVersionImportModal.findImportPipelineRadio().check(); pipelineVersionImportModal.findPipelineUrlInput().type('https://example.com/pipeline.yaml'); + + // Intercept re-fetch of pipeline versions + const uploadedMockPipelineVersion = buildMockPipelineVersionV2(createPipelineVersionParams); + pipelinesTable + .mockGetPipelineVersions( + [initialMockPipelineVersion, uploadedMockPipelineVersion], + initialMockPipeline.pipeline_id, + projectName, + ) + .as('refreshVersions'); pipelineVersionImportModal.submit(); // Wait for upload/fetch requests @@ -956,13 +985,24 @@ describe('Pipelines', () => { }); it('navigate to create run page from pipeline row', () => { - const visitPipelineProjects = () => pipelinesGlobal.visit(projectName); - runCreateRunPageNavTest(visitPipelineProjects); + initIntercepts({}); + pipelinesGlobal.visit(projectName); + + // Wait for the pipelines table to load + pipelinesTable.find(); + pipelinesTable + .getRowById(initialMockPipeline.pipeline_id) + .findKebabAction('Create run') + .click(); + verifyRelativeURL( + `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/runs/create`, + ); }); it('run and schedule dropdown action should be disabeld when pipeline has no versions', () => { initIntercepts({ hasNoPipelineVersions: true }); pipelinesGlobal.visit(projectName); + pipelinesTable .getRowById(initialMockPipeline.pipeline_id) .findKebabAction('Create schedule') @@ -974,8 +1014,18 @@ describe('Pipelines', () => { }); it('navigates to "Schedule run" page from pipeline row', () => { - const visitPipelineProjects = () => pipelinesGlobal.visit(projectName); - runScheduleRunPageNavTest(visitPipelineProjects); + initIntercepts({}); + pipelinesGlobal.visit(projectName); + + pipelinesTable.find(); + pipelinesTable + .getRowById(initialMockPipeline.pipeline_id) + .findKebabAction('Create schedule') + .click(); + + verifyRelativeURL( + `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/schedules/create`, + ); }); it('navigate to create run page from pipeline version row', () => { @@ -1136,7 +1186,7 @@ type HandlersProps = { nextPageToken?: string | undefined; }; -export const initIntercepts = ({ +const initIntercepts = ({ isEmpty = false, mockPipelines = [initialMockPipeline], hasNoPipelineVersions = false, @@ -1234,63 +1284,3 @@ const createDeletePipelineIntercept = (pipelineId: string) => }, mockSuccessGoogleRpcStatus({}), ); - -export const runCreateRunPageNavTest = (visitPipelineProjects: () => void): void => { - initIntercepts({}); - visitPipelineProjects(); - - // Wait for the pipelines table to load - pipelinesTable.find(); - pipelinesTable.getRowById(initialMockPipeline.pipeline_id).findKebabAction('Create run').click(); - verifyRelativeURL( - `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/runs/create`, - ); -}; - -export const runScheduleRunPageNavTest = (visitPipelineProjects: () => void): void => { - initIntercepts({}); - visitPipelineProjects(); - - pipelinesTable.find(); - pipelinesTable - .getRowById(initialMockPipeline.pipeline_id) - .findKebabAction('Create schedule') - .click(); - - verifyRelativeURL( - `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/schedules/create`, - ); -}; - -export const viewPipelineServerDetailsTest = (visitPipelineProjects: () => void): void => { - initIntercepts({}); - cy.interceptK8s( - { - model: SecretModel, - ns: projectName, - }, - mockSecretK8sResource({ - s3Bucket: 'c2RzZA==', - namespace: projectName, - name: 'aws-connection-test', - }), - ); - visitPipelineProjects(); - viewPipelineDetails(); -}; - -const viewPipelineDetails = ( - accessKey = 'sdsd', - secretKey = 'sdsd', - endpoint = 'https://s3.amazonaws.com', - bucketName = 'test-pipelines-bucket', -) => { - pipelinesGlobal.selectPipelineServerAction('View pipeline server configuration'); - viewPipelineServerModal.shouldHaveAccessKey(accessKey); - viewPipelineServerModal.findPasswordHiddenButton().click(); - viewPipelineServerModal.shouldHaveSecretKey(secretKey); - viewPipelineServerModal.shouldHaveEndPoint(endpoint); - viewPipelineServerModal.shouldHaveBucketName(bucketName); - - viewPipelineServerModal.findCloseButton().click(); -}; diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelinesList.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelinesList.cy.ts index 4509fe5e9d..f121ad95af 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelinesList.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/pipelines/pipelinesList.cy.ts @@ -1,5 +1,11 @@ /* eslint-disable camelcase */ -import { buildMockPipelineVersionV2 } from '~/__mocks__'; +import { + buildMockPipelineVersionV2, + buildMockPipelineVersionsV2, + mockProjectK8sResource, + mockRouteK8sResource, + mockSecretK8sResource, +} from '~/__mocks__'; import { mockDataSciencePipelineApplicationK8sResource } from '~/__mocks__/mockDataSciencePipelinesApplicationK8sResource'; import { mockK8sResourceList } from '~/__mocks__/mockK8sResourceList'; import { mock404Error } from '~/__mocks__/mockK8sStatus'; @@ -9,18 +15,19 @@ import { configurePipelineServerModal, pipelineVersionImportModal, PipelineSort, + pipelinesGlobal, + viewPipelineServerModal, } from '~/__tests__/cypress/cypress/pages/pipelines'; import { pipelinesSection } from '~/__tests__/cypress/cypress/pages/pipelines/pipelinesSection'; import { projectDetails } from '~/__tests__/cypress/cypress/pages/projects'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; -import { DataSciencePipelineApplicationModel } from '~/__tests__/cypress/cypress/utils/models'; -import type { PipelineKFv2 } from '~/concepts/pipelines/kfTypes'; import { - initIntercepts, - runCreateRunPageNavTest, - runScheduleRunPageNavTest, - viewPipelineServerDetailsTest, -} from './pipelines.cy'; + DataSciencePipelineApplicationModel, + ProjectModel, + RouteModel, + SecretModel, +} from '~/__tests__/cypress/cypress/utils/models'; +import type { PipelineKFv2 } from '~/concepts/pipelines/kfTypes'; const projectName = 'test-project-name'; const initialMockPipeline = buildMockPipelineV2({ display_name: 'Test pipeline' }); @@ -44,7 +51,7 @@ const mockPipelines: PipelineKFv2[] = [ describe('PipelinesList', () => { it('should show the configure pipeline server button when the server is not configured', () => { - initIntercepts({ isEmpty: true }); + initIntercepts({ isEmptyProject: true }); cy.interceptK8s( { model: DataSciencePipelineApplicationModel, @@ -63,7 +70,7 @@ describe('PipelinesList', () => { }); it('should verify that clicking on Configure pipeline server button will open a modal', () => { - initIntercepts({ isEmpty: true }); + initIntercepts({ isEmptyProject: true }); projectDetails.visitSection(projectName, 'pipelines-projects'); pipelinesSection.findCreatePipelineButton().should('be.enabled'); pipelinesSection.findCreatePipelineButton().click(); @@ -71,13 +78,32 @@ describe('PipelinesList', () => { }); it('should view pipeline server', () => { - const visitPipelineProjects = () => - projectDetails.visitSection(projectName, 'pipelines-projects'); - viewPipelineServerDetailsTest(visitPipelineProjects); + initIntercepts(); + cy.interceptK8s( + { + model: SecretModel, + ns: projectName, + }, + mockSecretK8sResource({ + s3Bucket: 'c2RzZA==', + namespace: projectName, + name: 'aws-connection-test', + }), + ); + projectDetails.visitSection(projectName, 'pipelines-projects'); + + pipelinesGlobal.selectPipelineServerAction('View pipeline server configuration'); + viewPipelineServerModal.shouldHaveAccessKey('sdsd'); + viewPipelineServerModal.findPasswordHiddenButton().click(); + viewPipelineServerModal.shouldHaveSecretKey('sdsd'); + viewPipelineServerModal.shouldHaveEndPoint('https://s3.amazonaws.com'); + viewPipelineServerModal.shouldHaveBucketName('test-pipelines-bucket'); + + viewPipelineServerModal.findCloseButton().click(); }); it('should disable the upload version button when the list is empty', () => { - initIntercepts({}); + initIntercepts(); cy.interceptK8sList( DataSciencePipelineApplicationModel, mockK8sResourceList([mockDataSciencePipelineApplicationK8sResource({})]), @@ -95,13 +121,11 @@ describe('PipelinesList', () => { ).as('pipelines'); projectDetails.visitSection(projectName, 'pipelines-projects'); - pipelinesSection.findImportPipelineSplitButton().should('be.enabled').click(); cy.wait('@pipelines').then((interception) => { expect(interception.request.query).to.eql({ sort_by: 'created_at desc', - page_size: '5', }); }); @@ -109,7 +133,7 @@ describe('PipelinesList', () => { }); it('should show the ability to delete the pipeline server kebab option', () => { - initIntercepts({}); + initIntercepts(); projectDetails.visitSection(projectName, 'pipelines-projects'); @@ -118,9 +142,9 @@ describe('PipelinesList', () => { }); it('should show the ability to upload new version when clicking the pipeline server kebab option', () => { - initIntercepts({}); - + initIntercepts(); projectDetails.visitSection(projectName, 'pipelines-projects'); + pipelinesTable.find(); const pipelineRow = pipelinesTable.getRowById(initialMockPipeline.pipeline_id); pipelineRow.findKebabAction('Upload new version').should('be.visible').click(); @@ -128,7 +152,7 @@ describe('PipelinesList', () => { }); it('should navigate to details page when clicking on the version name', () => { - initIntercepts({}); + initIntercepts(); projectDetails.visitSection(projectName, 'pipelines-projects'); pipelinesTable.find(); @@ -144,7 +168,7 @@ describe('PipelinesList', () => { }); it('clicking on upload a new pipeline version should show a modal', () => { - initIntercepts({}); + initIntercepts(); cy.intercept( { pathname: `/api/service/pipelines/${projectName}/dspa/apis/v2beta1/pipelines`, @@ -173,19 +197,114 @@ describe('PipelinesList', () => { }); it('navigates to create run page from pipeline row', () => { - const visitPipelineProjects = () => - projectDetails.visitSection(projectName, 'pipelines-projects'); - runCreateRunPageNavTest(visitPipelineProjects); + initIntercepts(); + projectDetails.visitSection(projectName, 'pipelines-projects'); + + // Wait for the pipelines table to load + pipelinesTable.find(); + pipelinesTable + .getRowById(initialMockPipeline.pipeline_id) + .findKebabAction('Create run') + .click(); + verifyRelativeURL( + `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/runs/create`, + ); }); it('navigates to "Schedule run" page from pipeline row', () => { - const visitPipelineProjects = () => - projectDetails.visitSection(projectName, 'pipelines-projects'); - runScheduleRunPageNavTest(visitPipelineProjects); + initIntercepts(); + projectDetails.visitSection(projectName, 'pipelines-projects'); + + pipelinesTable.find(); + pipelinesTable + .getRowById(initialMockPipeline.pipeline_id) + .findKebabAction('Create schedule') + .click(); + + verifyRelativeURL( + `/pipelines/${projectName}/${initialMockPipeline.pipeline_id}/${initialMockPipelineVersion.pipeline_version_id}/schedules/create`, + ); }); }); const pipelineTableSetup = () => { - initIntercepts({ mockPipelines }); + initIntercepts(); projectDetails.visitSection(projectName, 'pipelines-projects'); }; + +export const initIntercepts = ( + { isEmptyProject }: { isEmptyProject?: boolean } = { isEmptyProject: false }, +): void => { + cy.interceptK8sList( + DataSciencePipelineApplicationModel, + mockK8sResourceList( + isEmptyProject + ? [] + : [mockDataSciencePipelineApplicationK8sResource({ namespace: projectName })], + ), + ); + cy.interceptK8s( + DataSciencePipelineApplicationModel, + mockDataSciencePipelineApplicationK8sResource({ + namespace: projectName, + dspaSecretName: 'aws-connection-test', + }), + ); + cy.interceptK8s( + RouteModel, + mockRouteK8sResource({ + notebookName: 'ds-pipeline-dspa', + namespace: projectName, + }), + ); + cy.interceptK8sList( + ProjectModel, + mockK8sResourceList([ + mockProjectK8sResource({ k8sName: projectName }), + mockProjectK8sResource({ k8sName: `${projectName}-2`, displayName: 'Test Project 2' }), + ]), + ); + + cy.interceptOdh( + 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/pipelines', + { + path: { namespace: projectName, serviceName: 'dspa' }, + }, + buildMockPipelines([initialMockPipeline]), + ).as('getPipelines'); + + cy.interceptOdh( + 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/pipelines/:pipelineId/versions', + { + path: { + namespace: projectName, + serviceName: 'dspa', + pipelineId: initialMockPipeline.pipeline_id, + }, + }, + buildMockPipelineVersionsV2([initialMockPipelineVersion]), + ); + cy.interceptOdh( + 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/pipelines/:pipelineId', + { + path: { + namespace: projectName, + serviceName: 'dspa', + pipelineId: initialMockPipeline.pipeline_id, + }, + }, + initialMockPipeline, + ); + cy.interceptOdh( + 'GET /api/service/pipelines/:namespace/:serviceName/apis/v2beta1/pipelines/:pipelineId/versions/:pipelineVersionId', + { + path: { + namespace: projectName, + serviceName: 'dspa', + pipelineId: initialMockPipeline.pipeline_id, + pipelineVersionId: initialMockPipelineVersion.pipeline_version_id, + }, + }, + initialMockPipelineVersion, + ); +}; diff --git a/frontend/src/concepts/k8s/NameDescriptionField.tsx b/frontend/src/concepts/k8s/NameDescriptionField.tsx index df983f1cd2..16e89293fe 100644 --- a/frontend/src/concepts/k8s/NameDescriptionField.tsx +++ b/frontend/src/concepts/k8s/NameDescriptionField.tsx @@ -13,6 +13,7 @@ import { import { ExclamationCircleIcon, HelpIcon } from '@patternfly/react-icons'; import { NameDescType } from '~/pages/projects/types'; import { isValidK8sName, translateDisplayNameForK8s } from '~/concepts/k8s/utils'; +import { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText'; type NameDescriptionFieldProps = { nameFieldId: string; @@ -23,6 +24,7 @@ type NameDescriptionFieldProps = { showK8sName?: boolean; disableK8sName?: boolean; maxLength?: number; + hasDuplicateName?: boolean; }; const NameDescriptionField: React.FC = ({ @@ -34,7 +36,10 @@ const NameDescriptionField: React.FC = ({ showK8sName, disableK8sName, maxLength, + hasDuplicateName, }) => { + const { name } = data; + const hasMaxLengthError = maxLength && name.length > maxLength; const autoSelectNameRef = React.useRef(null); React.useEffect(() => { @@ -45,11 +50,11 @@ const NameDescriptionField: React.FC = ({ const k8sName = React.useMemo(() => { if (showK8sName) { - return translateDisplayNameForK8s(data.name); + return translateDisplayNameForK8s(name); } return ''; - }, [showK8sName, data.name]); + }, [showK8sName, name]); return ( @@ -61,15 +66,20 @@ const NameDescriptionField: React.FC = ({ id={nameFieldId} data-testid={nameFieldId} name={nameFieldId} - value={data.name} - onChange={(e, name) => setData({ ...data, name })} - maxLength={maxLength} + value={name} + onChange={(_e, value) => setData({ ...data, name: value })} + validated={hasMaxLengthError ? 'error' : 'default'} /> - {maxLength && ( + + {hasMaxLengthError && ( - {`Cannot exceed ${maxLength} characters`} + + Cannot exceed {maxLength} characters. + )} + + {hasDuplicateName && } {showK8sName && ( diff --git a/frontend/src/concepts/pipelines/apiHooks/useAllPipelineVersions.ts b/frontend/src/concepts/pipelines/apiHooks/useAllPipelineVersions.ts index 060b40bbdf..a31791e492 100644 --- a/frontend/src/concepts/pipelines/apiHooks/useAllPipelineVersions.ts +++ b/frontend/src/concepts/pipelines/apiHooks/useAllPipelineVersions.ts @@ -2,10 +2,41 @@ import React from 'react'; import { PipelineVersionKFv2 } from '~/concepts/pipelines/kfTypes'; import { usePipelinesAPI } from '~/concepts/pipelines/context'; import usePipelineQuery from '~/concepts/pipelines/apiHooks/usePipelineQuery'; -import { PipelineListPaged, PipelineOptions } from '~/concepts/pipelines/types'; +import { + ListPipelineVersions, + PipelineListPaged, + PipelineOptions, + PipelineParams, +} from '~/concepts/pipelines/types'; import { FetchState, NotReadyError } from '~/utilities/useFetchState'; import { useAllPipelines } from '~/concepts/pipelines/apiHooks/usePipelines'; import { useDeepCompareMemoize } from '~/utilities/useDeepCompareMemoize'; +import { K8sAPIOptions } from '~/k8sTypes'; + +/** + * Recursively fetch each pipeline version page when a next_page_token exists. + */ +async function getAllVersions( + opts: K8sAPIOptions, + pipelineId: string, + params: PipelineParams | undefined, + listPipelineVersions: ListPipelineVersions, +): Promise { + const result = await listPipelineVersions(opts, pipelineId, params); + let allVersions = result.pipeline_versions ?? []; + + if (result.next_page_token) { + const nextVersions = await getAllVersions( + opts, + pipelineId, + { pageSize: result.pipeline_versions?.length, pageToken: result.next_page_token }, + listPipelineVersions, + ); + allVersions = allVersions.concat(nextVersions); + } + + return allVersions; +} /** * Fetch all pipelines, then use those pipeline IDs to accumulate a list of pipeline versions. @@ -26,15 +57,15 @@ export const useAllPipelineVersions = ( } const pipelineVersionRequests = pipelineIds.map((pipelineId) => - api.listPipelineVersions(opts, pipelineId, params), + getAllVersions(opts, pipelineId, params, api.listPipelineVersions), ); const results = await Promise.all(pipelineVersionRequests); return results.reduce( - (acc: { total_size: number; items: PipelineVersionKFv2[] }, result) => { + (acc: { total_size: number; items: PipelineVersionKFv2[] }, versions) => { // eslint-disable-next-line camelcase - acc.total_size += result.total_size || 0; - acc.items = acc.items.concat(result.pipeline_versions || []); + acc.total_size += versions.length || 0; + acc.items = acc.items.concat(versions); return acc; }, @@ -48,3 +79,36 @@ export const useAllPipelineVersions = ( refreshRate, ); }; + +/** + * Fetch all version pages for a single pipeline. + */ +export const useAllVersionsByPipelineId = ( + pipelineId: string, + options?: PipelineOptions, + refreshRate?: number, +): FetchState> => { + const { api } = usePipelinesAPI(); + + return usePipelineQuery( + React.useCallback( + async (opts, params) => { + if (!pipelineId) { + return Promise.reject(new NotReadyError('No pipeline ID.')); + } + + const allVersions = await getAllVersions( + opts, + pipelineId, + params, + api.listPipelineVersions, + ); + + return { items: allVersions }; + }, + [api.listPipelineVersions, pipelineId], + ), + options, + refreshRate, + ); +}; diff --git a/frontend/src/concepts/pipelines/apiHooks/usePipelineRecurringRuns.ts b/frontend/src/concepts/pipelines/apiHooks/usePipelineRecurringRuns.ts index ec217767ac..14d6c0ee63 100644 --- a/frontend/src/concepts/pipelines/apiHooks/usePipelineRecurringRuns.ts +++ b/frontend/src/concepts/pipelines/apiHooks/usePipelineRecurringRuns.ts @@ -3,7 +3,67 @@ import { PipelineRecurringRunKFv2 } from '~/concepts/pipelines/kfTypes'; import { FetchState } from '~/utilities/useFetchState'; import { usePipelinesAPI } from '~/concepts/pipelines/context'; import usePipelineQuery from '~/concepts/pipelines/apiHooks/usePipelineQuery'; -import { PipelineListPaged, PipelineRunOptions } from '~/concepts/pipelines/types'; +import { + ListPipelineRecurringRuns, + PipelineListPaged, + PipelineRunOptions, + PipelineRunParams, +} from '~/concepts/pipelines/types'; +import { K8sAPIOptions } from '~/k8sTypes'; + +/** + * Recursively fetch each recurring run page when a next_page_token exists. + */ +async function getAllRecurringRuns( + opts: K8sAPIOptions, + params: PipelineRunParams | undefined, + listPipelineRecurringRuns: ListPipelineRecurringRuns, +): Promise { + const result = await listPipelineRecurringRuns(opts, params); + let allRecurringRuns = result.recurringRuns ?? []; + + if (result.next_page_token) { + const nextRecurringRuns = await getAllRecurringRuns( + opts, + { ...params, pageToken: result.next_page_token }, + listPipelineRecurringRuns, + ); + allRecurringRuns = allRecurringRuns.concat(nextRecurringRuns); + } + + return allRecurringRuns; +} + +/** + * Fetch all recurring run pages. + */ +export const useAllPipelineRecurringRuns = ( + options?: PipelineRunOptions, +): FetchState> => { + const { api } = usePipelinesAPI(); + const experimentId = options?.experimentId; + const pipelineVersionId = options?.pipelineVersionId; + + return usePipelineQuery( + React.useCallback( + async (opts, params) => { + const allRecurringRuns = await getAllRecurringRuns( + opts, + { + ...params, + ...(experimentId && { experimentId }), + ...(pipelineVersionId && { pipelineVersionId }), + }, + api.listPipelineRecurringRuns, + ); + + return { items: allRecurringRuns, totalSize: allRecurringRuns.length }; + }, + [api, experimentId, pipelineVersionId], + ), + options, + ); +}; const usePipelineRecurringRuns = ( options?: PipelineRunOptions, diff --git a/frontend/src/concepts/pipelines/apiHooks/usePipelineRuns.ts b/frontend/src/concepts/pipelines/apiHooks/usePipelineRuns.ts index 8753d7be29..292edc6286 100644 --- a/frontend/src/concepts/pipelines/apiHooks/usePipelineRuns.ts +++ b/frontend/src/concepts/pipelines/apiHooks/usePipelineRuns.ts @@ -3,7 +3,67 @@ import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; import { FetchState } from '~/utilities/useFetchState'; import { usePipelinesAPI } from '~/concepts/pipelines/context'; import usePipelineQuery from '~/concepts/pipelines/apiHooks/usePipelineQuery'; -import { PipelineListPaged, PipelineRunOptions } from '~/concepts/pipelines/types'; +import { + ListPipelineRuns, + PipelineListPaged, + PipelineRunOptions, + PipelineRunParams, +} from '~/concepts/pipelines/types'; +import { K8sAPIOptions } from '~/k8sTypes'; + +/** + * Recursively fetch each active run page when a next_page_token exists. + */ +async function getAllActiveRuns( + opts: K8sAPIOptions, + params: PipelineRunParams | undefined, + listPipelineActiveRuns: ListPipelineRuns, +): Promise { + const result = await listPipelineActiveRuns(opts, params); + let allActiveRuns = result.runs ?? []; + + if (result.next_page_token) { + const nextActiveRuns = await getAllActiveRuns( + opts, + { ...params, pageToken: result.next_page_token }, + listPipelineActiveRuns, + ); + allActiveRuns = allActiveRuns.concat(nextActiveRuns); + } + + return allActiveRuns; +} + +/** + * Fetch all active run pages. + */ +export const useAllPipelineActiveRuns = ( + options?: PipelineRunOptions, +): FetchState> => { + const { api } = usePipelinesAPI(); + const experimentId = options?.experimentId; + const pipelineVersionId = options?.pipelineVersionId; + + return usePipelineQuery( + React.useCallback( + async (opts, params) => { + const allRuns = await getAllActiveRuns( + opts, + { + ...params, + ...(experimentId && { experimentId }), + ...(pipelineVersionId && { pipelineVersionId }), + }, + api.listPipelineActiveRuns, + ); + + return { items: allRuns, totalSize: allRuns.length }; + }, + [api, experimentId, pipelineVersionId], + ), + options, + ); +}; export const usePipelineActiveRuns = ( options?: PipelineRunOptions, diff --git a/frontend/src/concepts/pipelines/apiHooks/usePipelines.ts b/frontend/src/concepts/pipelines/apiHooks/usePipelines.ts index c9446956c2..1afa6f1653 100644 --- a/frontend/src/concepts/pipelines/apiHooks/usePipelines.ts +++ b/frontend/src/concepts/pipelines/apiHooks/usePipelines.ts @@ -60,7 +60,7 @@ async function getAllPipelines( if (result.next_page_token) { const nextPipelines = await getAllPipelines( opts, - { pageToken: result.next_page_token }, + { ...params, pageToken: result.next_page_token }, listPipelines, ); allPipelines = allPipelines.concat(nextPipelines); diff --git a/frontend/src/concepts/pipelines/content/DuplicateNameHelperText.tsx b/frontend/src/concepts/pipelines/content/DuplicateNameHelperText.tsx new file mode 100644 index 0000000000..727d3c0df8 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/DuplicateNameHelperText.tsx @@ -0,0 +1,31 @@ +import { HelperText, HelperTextItem, HelperTextItemProps, Icon } from '@patternfly/react-core'; +import { InfoCircleIcon } from '@patternfly/react-icons'; +import React from 'react'; + +interface DuplicateNameHelperTextProps { + name: string; + isError?: boolean; +} + +export const DuplicateNameHelperText: React.FC = ({ + name, + isError, +}) => { + const helperTextItemProps: HelperTextItemProps = isError + ? { variant: 'error', hasIcon: true } + : { + icon: ( + + + + ), + }; + + return ( + + + {name} already exists. Try a different name. + + + ); +}; diff --git a/frontend/src/concepts/pipelines/content/createRun/RunForm.tsx b/frontend/src/concepts/pipelines/content/createRun/RunForm.tsx index 2ca75e042d..b39ee77c42 100644 --- a/frontend/src/concepts/pipelines/content/createRun/RunForm.tsx +++ b/frontend/src/concepts/pipelines/content/createRun/RunForm.tsx @@ -9,6 +9,8 @@ import { PipelineVersionKFv2, RuntimeConfigParameters } from '~/concepts/pipelin import ProjectAndExperimentSection from '~/concepts/pipelines/content/createRun/contentSections/ProjectAndExperimentSection'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import { useLatestPipelineVersion } from '~/concepts/pipelines/apiHooks/useLatestPipelineVersion'; +import { useAllPipelineActiveRuns } from '~/concepts/pipelines/apiHooks/usePipelineRuns'; +import { useAllPipelineRecurringRuns } from '~/concepts/pipelines/apiHooks/usePipelineRecurringRuns'; import PipelineSection from './contentSections/PipelineSection'; import { RunTypeSection } from './contentSections/RunTypeSection'; import { CreateRunPageSections, RUN_NAME_CHARACTER_LIMIT, runPageSectionTitles } from './const'; @@ -27,6 +29,8 @@ const RunForm: React.FC = ({ data, onValueChange, isCloned }) => { const selectedVersion = data.version || latestVersion; const paramsRef = React.useRef(data.params); const isSchedule = data.runType.type === RunTypeOption.SCHEDULED; + const [{ items: activeRuns }] = useAllPipelineActiveRuns(); + const [{ items: recurringRuns }] = useAllPipelineRecurringRuns(); const updateInputParams = React.useCallback( (version: PipelineVersionKFv2 | undefined) => @@ -75,6 +79,9 @@ const RunForm: React.FC = ({ data, onValueChange, isCloned }) => { data={data.nameDesc} setData={(nameDesc) => onValueChange('nameDesc', nameDesc)} maxLength={RUN_NAME_CHARACTER_LIMIT} + hasDuplicateName={(isSchedule ? recurringRuns : activeRuns).some( + (run) => run.display_name === data.nameDesc.name, + )} /> {isSchedule && data.runType.type === RunTypeOption.SCHEDULED && ( diff --git a/frontend/src/concepts/pipelines/content/createRun/utils.ts b/frontend/src/concepts/pipelines/content/createRun/utils.ts index 10f88ffabd..4951c6cfda 100644 --- a/frontend/src/concepts/pipelines/content/createRun/utils.ts +++ b/frontend/src/concepts/pipelines/content/createRun/utils.ts @@ -8,6 +8,7 @@ import { import { ParametersKF, PipelineVersionKFv2 } from '~/concepts/pipelines/kfTypes'; import { getCorePipelineSpec } from '~/concepts/pipelines/getCorePipelineSpec'; import { convertToDate } from '~/utilities/time'; +import { RUN_NAME_CHARACTER_LIMIT } from './const'; const runTypeSafeData = (runType: RunFormData['runType']): boolean => runType.type !== RunTypeOption.SCHEDULED || @@ -45,6 +46,7 @@ export const isFilledRunFormData = (formData: RunFormData): formData is SafeRunF return ( !!formData.nameDesc.name && + formData.nameDesc.name.length <= RUN_NAME_CHARACTER_LIMIT && !!formData.pipeline && !!formData.version && hasRequiredInputParams && diff --git a/frontend/src/concepts/pipelines/content/import/PipelineImportModal.tsx b/frontend/src/concepts/pipelines/content/import/PipelineImportModal.tsx index f366553447..bec7f27145 100644 --- a/frontend/src/concepts/pipelines/content/import/PipelineImportModal.tsx +++ b/frontend/src/concepts/pipelines/content/import/PipelineImportModal.tsx @@ -14,6 +14,8 @@ import { usePipelinesAPI } from '~/concepts/pipelines/context'; import { usePipelineImportModalData } from '~/concepts/pipelines/content/import/useImportModalData'; import { PipelineKFv2 } from '~/concepts/pipelines/kfTypes'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { useAllPipelines } from '~/concepts/pipelines/apiHooks/usePipelines'; +import { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText'; import PipelineUploadRadio from './PipelineUploadRadio'; import { PipelineUploadOption } from './utils'; @@ -28,11 +30,14 @@ const PipelineImportModal: React.FC = ({ isOpen, onClo const [error, setError] = React.useState(); const [{ name, description, fileContents, pipelineUrl, uploadOption }, setData, resetData] = usePipelineImportModalData(); + const [{ items: pipelines }] = useAllPipelines(); + const hasDuplicateNameError = pipelines.some((pipeline) => pipeline.display_name === name); const isImportButtonDisabled = !apiAvailable || importing || !name || + hasDuplicateNameError || (uploadOption === PipelineUploadOption.URL_IMPORT ? !pipelineUrl : !fileContents); const onBeforeClose = (pipeline?: PipelineKFv2) => { @@ -121,8 +126,11 @@ const PipelineImportModal: React.FC = ({ isOpen, onClo data-testid="pipeline-name" name="pipeline-name" value={name} - onChange={(e, value) => setData('name', value)} + onChange={(_e, value) => setData('name', value)} + validated={hasDuplicateNameError ? 'error' : 'default'} /> + + {hasDuplicateNameError && } diff --git a/frontend/src/concepts/pipelines/content/import/PipelineVersionImportModal.tsx b/frontend/src/concepts/pipelines/content/import/PipelineVersionImportModal.tsx index fc361f54a4..58addb87c2 100644 --- a/frontend/src/concepts/pipelines/content/import/PipelineVersionImportModal.tsx +++ b/frontend/src/concepts/pipelines/content/import/PipelineVersionImportModal.tsx @@ -15,6 +15,8 @@ import { usePipelineVersionImportModalData } from '~/concepts/pipelines/content/ import { PipelineKFv2, PipelineVersionKFv2 } from '~/concepts/pipelines/kfTypes'; import PipelineSelector from '~/concepts/pipelines/content/pipelineSelector/PipelineSelector'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { useAllVersionsByPipelineId } from '~/concepts/pipelines/apiHooks/useAllPipelineVersions'; +import { DuplicateNameHelperText } from '~/concepts/pipelines/content/DuplicateNameHelperText'; import { PipelineUploadOption, generatePipelineVersionName } from './utils'; import PipelineUploadRadio from './PipelineUploadRadio'; @@ -35,6 +37,8 @@ const PipelineVersionImportModal: React.FC = ({ setData, resetData, ] = usePipelineVersionImportModalData(existingPipeline); + const [{ items: versions }] = useAllVersionsByPipelineId(pipeline?.pipeline_id ?? ''); + const hasDuplicateName = versions.some((version) => version.display_name === name); const pipelineId = pipeline?.pipeline_id || ''; const pipelineName = pipeline?.display_name || ''; @@ -43,6 +47,7 @@ const PipelineVersionImportModal: React.FC = ({ !apiAvailable || importing || !name || + hasDuplicateName || !pipeline || (uploadOption === PipelineUploadOption.URL_IMPORT ? !pipelineUrl : !fileContents); @@ -136,8 +141,10 @@ const PipelineVersionImportModal: React.FC = ({ id="pipeline-version-name" name="pipeline-version-name" value={name} - onChange={(e, value) => setData('name', value)} + onChange={(_e, value) => setData('name', value)} /> + + {hasDuplicateName && } @@ -149,7 +156,7 @@ const PipelineVersionImportModal: React.FC = ({ data-testid="pipeline-version-description" name="pipeline-version-description" value={description} - onChange={(e, value) => setData('description', value)} + onChange={(_e, value) => setData('description', value)} />