Skip to content

Commit

Permalink
Adm-936 [frontend]: When the first pipeline has no steps, it will not…
Browse files Browse the repository at this point in the history
… crash when adding the pipeline (#1434)

* ADM-936 fix: remove the crash logic of pipeline

* ADM-936 test: add e2e test

* ADM-936 fix: fix eslint

* ADM-936 test: add test

* ADM-936 test: add unit test

* ADM-936 fix: fix e2e test
  • Loading branch information
Leiqiuhong authored May 9, 2024
1 parent 07b1c57 commit 081a690
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe('PipelineMetricSelection', () => {
});

await waitFor(() => {
expect(mockHandleClickRemoveButton).toHaveBeenCalledTimes(2);
expect(mockHandleClickRemoveButton).toHaveBeenCalledTimes(0);
});
});

Expand Down
6 changes: 6 additions & 0 deletions frontend/__tests__/context/metricsSlice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,12 @@ describe('saveMetricsSetting reducer', () => {
expect(savedPipelineCrews.pipelineCrews).toBe(crews);
});

it('should return empty array given crews is undefined', () => {
const savedPipelineCrews = saveMetricsSettingReducer(initState, savePipelineCrews(undefined));

expect(savedPipelineCrews.pipelineCrews).toEqual([]);
});

it('should update ShouldRetryPipelineConfig', async () => {
store.dispatch(updateShouldRetryPipelineConfig(true));
expect(selectShouldRetryPipelineConfig(store.getState())).toEqual(true);
Expand Down
35 changes: 35 additions & 0 deletions frontend/__tests__/utils/Util.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import {
sortDateRanges,
sortDisabledOptions,
transformToCleanedBuildKiteEmoji,
updateResponseCrews,
} from '@src/utils/util';
import { CleanedBuildKiteEmoji, OriginBuildKiteEmoji } from '@src/constants/emojis/emoji';
import { CYCLE_TIME_SETTINGS_TYPES, METRICS_CONSTANTS } from '@src/constants/resources';
import { ICycleTimeSetting, IPipelineConfig } from '@src/context/Metrics/metricsSlice';
import { IPipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { BoardInfoResponse } from '@src/hooks/useGetBoardInfo';
import { EMPTY_STRING } from '@src/constants/commons';
import { PIPELINE_TOOL_TYPES } from '../fixtures';
Expand Down Expand Up @@ -570,3 +572,36 @@ describe('combineBoardInfo function', () => {
expect(combineBoardData).toStrictEqual(expectResults);
});
});

describe('updateResponseCrews function', () => {
const mockData = {
id: '0',
name: 'pipelineName',
orgId: '',
orgName: 'orgName',
repository: '',
steps: [] as string[],
branches: [] as string[],
crews: ['a', 'b', 'c'],
} as IPipeline;
it('should update crews when pipelineName and org both matched', () => {
const expectData = [
{
...mockData,
crews: [],
},
];
const result = updateResponseCrews('orgName', 'pipelineName', [mockData]);
expect(result).toEqual(expectData);
});

it('should not update crews when pipelineName or org not matched', () => {
const expectData = [
{
...mockData,
},
];
const result = updateResponseCrews('xxx', 'xxx', [mockData]);
expect(result).toEqual(expectData);
});
});
22 changes: 18 additions & 4 deletions frontend/e2e/pages/metrics/metrics-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,13 +582,27 @@ export class MetricsStep {
.getByLabel('Open')
.nth(1)
.click();
await expect(this.page.getByRole('option', { name: firstPipelineConfig.pipelineName })).not.toBeEnabled();
}

async RemoveFirstNewPipeline() {
const pipelineList = this.pipelineSettingSection.getByText('Organization *Pipeline Name *Remove');
async addNewPipelineAndSelectOrgAndName() {
await this.pipelineNewPipelineButton.click();
await this.pipelineSettingSection.getByText('Organization *Remove').getByLabel('Open').click();
await this.page.getByRole('option', { name: 'Thoughtworks-Heartbeat' }).click();
await this.pipelineSettingSection
.getByText('Organization *Pipeline Name *Remove')
.getByLabel('Open')
.nth(1)
.click();
await this.page.getByRole('option', { name: 'Heartbeat-E2E' }).click();
}

async checkPipelineLength(length: number) {
const pipelineLength = await this.pipelineSettingSection.getByText('Organization *').count();
expect(pipelineLength).toEqual(length);
}

await pipelineList.nth(0).getByRole('button', { name: 'remove' }).click();
async removePipeline(index: number) {
await this.pipelineSettingSection.getByText('Remove').nth(index).click();
}

async checkPipelineFillNoStep(pipelineSettings: typeof metricsStepData.deployment) {
Expand Down
5 changes: 4 additions & 1 deletion frontend/e2e/specs/side-path/unhappy-path.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ test('unhappy path when import file', async ({ homePage, configStep, metricsStep
await configStep.goToMetrics();

await metricsStep.checkBoardNoCard();
await metricsStep.addNewPipelineAndSelectOrgAndName();
await metricsStep.checkPipelineLength(2);
await metricsStep.removePipeline(1);
await metricsStep.checkPipelineFillNoStep(importUnhappyPathProjectFromFile.deployment);
await metricsStep.goToPreviousStep();
await configStep.typeInDateRange(dateRange);
Expand All @@ -72,7 +75,7 @@ test('unhappy path when import file', async ({ homePage, configStep, metricsStep
await metricsStep.selectCrews(modifiedCorrectProjectFromFile.crews);
await metricsStep.deselectBranch(modifiedCorrectProjectFromFile.deletedBranch);
await metricsStep.addNewPipelineAndSelectSamePipeline(importUnhappyPathProjectFromFile.deployment);
await metricsStep.RemoveFirstNewPipeline();
await metricsStep.removePipeline(1);
await metricsStep.selectDoneHeartbeatState(ModifiedhbStateData[6]);
await metricsStep.validateNextButtonNotClickable();
await metricsStep.selectDoneHeartbeatState(hbStateData[6]);
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/clients/pipeline/dto/response.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { pipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { IPipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';

export interface IPipelineInfoResponseDTO {
pipelineList: pipeline[];
pipelineList: IPipeline[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,24 +110,20 @@ export const PipelineMetricSelection = ({
);
setLoadingCompletedNumber((value) => Math.max(value - 1, 0));
getSteps(params, organizationId, buildId, pipelineType, token).then((res) => {
if (res && !res.haveStep) {
isShowRemoveButton && handleRemoveClick();
} else {
const steps = res?.response ?? [];
const branches = res?.branches ?? [];
const pipelineCrews = res?.pipelineCrews ?? [];
dispatch(
updatePipelineToolVerifyResponseSteps({
organization,
pipelineName: _pipelineName,
steps,
branches,
pipelineCrews,
}),
);
res?.haveStep && dispatch(updatePipelineStep({ steps, id, type, branches, pipelineCrews }));
dispatch(updateShouldGetPipelineConfig(false));
}
const steps = res?.response ?? [];
const branches = res?.branches ?? [];
const pipelineCrews = res?.pipelineCrews ?? [];
dispatch(
updatePipelineToolVerifyResponseSteps({
organization,
pipelineName: _pipelineName,
steps,
branches,
pipelineCrews,
}),
);
res?.haveStep && dispatch(updatePipelineStep({ steps, id, type, branches, pipelineCrews }));
dispatch(updateShouldGetPipelineConfig(false));
res && setIsShowNoStepWarning(!res.haveStep);
});
};
Expand Down
10 changes: 5 additions & 5 deletions frontend/src/context/Metrics/metricsSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
METRICS_CONSTANTS,
} from '@src/constants/resources';
import { convertCycleTimeSettings, getSortedAndDeduplicationBoardingMapping } from '@src/utils/util';
import { pipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { IPipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import _, { omit, uniqWith, isEqual, intersection, concat } from 'lodash';
import { createSlice } from '@reduxjs/toolkit';
import camelCase from 'lodash.camelcase';
Expand Down Expand Up @@ -309,7 +309,7 @@ export const metricsSlice = createSlice({
state.users = action.payload;
},
savePipelineCrews: (state, action) => {
state.pipelineCrews = action.payload;
state.pipelineCrews = action.payload || [];
},
updateCycleTimeSettings: (state, action) => {
state.cycleTimeSettings = action.payload;
Expand Down Expand Up @@ -477,11 +477,11 @@ export const metricsSlice = createSlice({
if (pipelineCrews) {
state.pipelineCrews = setPipelineCrews(isProjectCreated, pipelineCrews, state.pipelineCrews);
}
const orgNames: Array<string> = _.uniq(pipelineList.map((item: pipeline) => item.orgName));
const orgNames: Array<string> = _.uniq(pipelineList.map((item: IPipeline) => item.orgName));
const filteredPipelineNames = (organization: string) =>
pipelineList
.filter((pipeline: pipeline) => pipeline.orgName.toLowerCase() === organization.toLowerCase())
.map((item: pipeline) => item.name);
.filter((pipeline: IPipeline) => pipeline.orgName.toLowerCase() === organization.toLowerCase())
.map((item: IPipeline) => item.name);

const uniqueResponse = (res: IPipelineConfig[]) => {
let itemsOmitId = uniqWith(
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/context/config/configSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { initialPipelineToolState, IPipelineToolState } from '@src/context/config/pipelineTool/pipelineToolSlice';
import { initialSourceControlState, ISourceControl } from '@src/context/config/sourceControl/sourceControlSlice';
import { IBoardState, initialBoardState } from '@src/context/config/board/boardSlice';
import { pipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { IPipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { uniqPipelineListCrews, updateResponseCrews } from '@src/utils/util';
import { SortType } from '@src/containers/ConfigStep/DateRangePicker/types';
import { createSlice } from '@reduxjs/toolkit';
Expand Down Expand Up @@ -171,7 +171,7 @@ export const configSlice = createSlice({
},
updatePipelineToolVerifyResponse: (state, action) => {
const { pipelineList } = action.payload;
state.pipelineTool.verifiedResponse.pipelineList = pipelineList.map((pipeline: pipeline) => ({
state.pipelineTool.verifiedResponse.pipelineList = pipelineList.map((pipeline: IPipeline) => ({
...pipeline,
steps: [],
}));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export interface IPipelineToolVerifyResponse {
pipelineList: pipeline[];
pipelineList: IPipeline[];
}

export interface pipeline {
export interface IPipeline {
id: string;
name: string;
orgId: string;
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/utils/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { CYCLE_TIME_LIST, CYCLE_TIME_SETTINGS_TYPES, METRICS_CONSTANTS } from '@
import { CleanedBuildKiteEmoji, OriginBuildKiteEmoji } from '@src/constants/emojis/emoji';
import { ICycleTimeSetting, IPipelineConfig } from '@src/context/Metrics/metricsSlice';
import { ITargetFieldType } from '@src/components/Common/MultiAutoComplete/styles';
import { pipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { IPipeline } from '@src/context/config/pipelineTool/verifyResponseSlice';
import { includes, isEqual, sortBy, uniq, uniqBy } from 'lodash';
import { BoardInfoResponse } from '@src/hooks/useGetBoardInfo';
import { DATE_FORMAT_TEMPLATE } from '@src/constants/template';
Expand Down Expand Up @@ -159,7 +159,7 @@ export const convertCycleTimeSettings = (
return cycleTimeSettings?.map(({ status, value }: ICycleTimeSetting) => ({ [status]: value }));
};

export const updateResponseCrews = (organization: string, pipelineName: string, pipelineList: pipeline[]) => {
export const updateResponseCrews = (organization: string, pipelineName: string, pipelineList: IPipeline[]) => {
return pipelineList.map((pipeline) =>
pipeline.name === pipelineName && pipeline.orgName === organization
? {
Expand All @@ -170,7 +170,7 @@ export const updateResponseCrews = (organization: string, pipelineName: string,
);
};

export const uniqPipelineListCrews = (pipelineList: pipeline[]) =>
export const uniqPipelineListCrews = (pipelineList: IPipeline[]) =>
uniq(pipelineList.flatMap(({ crews }) => crews)).filter((crew) => crew !== undefined);

export function existBlockState(cycleTimeSettings: ICycleTimeSetting[]) {
Expand Down

0 comments on commit 081a690

Please sign in to comment.