From c8cc1a2e46c1019947fe44b4624d5734105b874b Mon Sep 17 00:00:00 2001 From: zhou-yinyuan <147299494+zhou-yinyuan@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:04:01 +0800 Subject: [PATCH] ADM-999 [frontend][docs]: update readme and fix sonar issue (#1596) * ADM-999 [docs]: update readme * ADM-999 [frontend]: fix sonar issue * ADM-999 [frontend]: change the loading logic * ADM-999 [frontend]: fix e2e --- README.md | 25 ++++++++--- .../__tests__/context/configSlice.test.ts | 35 ++++++++++++++++ ...eControlConfigurationBranchEffect.test.tsx | 3 ++ ...rceControlConfigurationCrewEffect.test.tsx | 3 ++ ...rceControlConfigurationRepoEffect.test.tsx | 3 ++ .../fixtures/import-file/chart-step-data.ts | 10 ++--- ...ing-unhappy-path-config-file.template.json | 4 +- frontend/e2e/pages/metrics/metrics-step.ts | 2 +- .../SourceControlMetricSelection/index.tsx | 21 ++++++++-- frontend/src/context/Metrics/metricsSlice.ts | 41 ++++++++++--------- ...SourceControlConfigurationBranchEffect.tsx | 8 +++- ...etSourceControlConfigurationCrewEffect.tsx | 4 +- ...ControlConfigurationOrganizationEffect.tsx | 10 +++-- ...etSourceControlConfigurationRepoEffect.tsx | 8 +++- .../useVerifySourceControlTokenEffect.ts | 7 +++- 15 files changed, 138 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 99afa3ed4..1c01fdf67 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ According to your selected required data, you need to input account settings for | Pipeline change failure rate | Pipeline | | Pipeline mean time to recovery | Pipeline | -If only `Lead time for changes` is selected among the four DORA metrics - `Lead time for changes`, `Deployment frequency`, `Pipeline change failure rate`, and `Pipeline mean time to recovery`, you will see another option `None` in the pipeline tool configuration. If you choose the `None` option, when calculating `Lead time for changes`, only the `PR lead time` will be considered, and `pipeline lead time` will not be calculated. +If only `Lead time for changes` is selected among the four DORA metrics - `Lead time for changes`, `Deployment frequency`, `Pipeline change failure rate`, and `Pipeline mean time to recovery`, you will see another option `Other` in the pipeline tool configuration. If you choose the `Other` option, when calculating `Lead time for changes`, only the `PR lead time` will be considered, and `pipeline lead time` will not be calculated. ![Image 3-4](https://cdn.jsdelivr.net/gh/au-heartbeat/data-hosting@main/readme/3-4-1.png)\ Image 3-4，Project config @@ -200,10 +200,10 @@ _Image 3-5, create Jira token_ **The details for Pipeline:** -|Items| Description | -|---|-------------------------------------------------------------------------------------------------------------------------------------------------------| -|PipelineTool| The pipeline tool you team use, currently heartbeat only support buildkite. If only `Lead time for changes` is selected among the four DORA metrics, the `None` option will appear, indicating that when calculating `Lead time for changes`, only `PR lead time` will be considered, and `pipeline lead time` will not be calculated. | -|Token| Generate buildkite token with below link, https://buildkite.com/user/api-access-tokens | +| Items | Description | +|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| PipelineTool | The pipeline tool you team use, currently heartbeat only support buildkite. If only `Lead time for changes` is selected among the four DORA metrics, the `Other` option will appear, indicating that when calculating `Lead time for changes`, only `PR lead time` will be considered, and `pipeline lead time` will not be calculated. | +| Token | Generate buildkite token with below link, https://buildkite.com/user/api-access-tokens | ![Image 3-5](https://cdn.jsdelivr.net/gh/au-heartbeat/data-hosting@main/readme/select-none-option-in-the-pipeline-configuration.png) @@ -329,6 +329,21 @@ The data source of **crew setting** is Github code committer. Heartbeat will lis If builds were manually/scheduled triggered or could not find code committer from Github, Heartbeat will mark as "Unknown" in crew setting. + +#### 3.2.6 Source Control configuration + +![Image 3-19](https://cdn.jsdelivr.net/gh/au-heartbeat/data-hosting@main/readme/source-control-configuration-in-metrics-page.png)\ +_Image 3-19，Settings for Source control_ + +They are sharing the similar settings to pipeline configuration + +| Items | Description | +|--------------|------------------------------------------| +| Organization | The organization for your source control | +| Repo | The repo for your source control | +| Branches | Your selected branches | +| Crew setting | Your selected author from github | + ## 3.3 Export and import config info ### 3.3.1 Export Config Json File diff --git a/frontend/__tests__/context/configSlice.test.ts b/frontend/__tests__/context/configSlice.test.ts index 3ceb42d83..81887f7ff 100644 --- a/frontend/__tests__/context/configSlice.test.ts +++ b/frontend/__tests__/context/configSlice.test.ts @@ -346,6 +346,41 @@ describe('config reducer', () => { expect(config.sourceControl.verifiedResponse.repoList.children).toEqual(expectedSourceControlVerifiedRepoList1); expect(config2.sourceControl.verifiedResponse.repoList.children).toEqual(expectedSourceControlVerifiedRepoList2); }); + + it('should clear source control verified repo list', () => { + const initialState = { + ...initialConfigState, + sourceControl: { + ...initialConfigState.sourceControl, + verifiedResponse: { + repoList: { + children: [ + { + name: 'organization', + value: 'mock-org1', + children: [], + }, + { + name: 'organization', + value: 'mock-org2', + children: [], + }, + ], + name: 'root', + value: '-1', + }, + }, + }, + }; + const expectedSourceControlVerifiedRepoList: string[] = []; + const action = { + type: 'config/clearSourceControlVerifiedResponse', + }; + + const config = configReducer(initialState, action); + + expect(config.sourceControl.verifiedResponse.repoList.children).toEqual(expectedSourceControlVerifiedRepoList); + }); }); describe('select methods', () => { diff --git a/frontend/__tests__/hooks/useGetSourceControlConfigurationBranchEffect.test.tsx b/frontend/__tests__/hooks/useGetSourceControlConfigurationBranchEffect.test.tsx index a205803f4..f29375bf3 100644 --- a/frontend/__tests__/hooks/useGetSourceControlConfigurationBranchEffect.test.tsx +++ b/frontend/__tests__/hooks/useGetSourceControlConfigurationBranchEffect.test.tsx @@ -14,6 +14,9 @@ jest.mock('react-redux', () => ({ useDispatch: () => mockDispatch, useSelector: (selector: () => TSelected) => { const originalUseSelector = jest.requireActual('react-redux').useSelector; + if (selector.name === 'selectShouldGetSourceControlConfig') { + return true; + } return originalUseSelector(selector); }, })); diff --git a/frontend/__tests__/hooks/useGetSourceControlConfigurationCrewEffect.test.tsx b/frontend/__tests__/hooks/useGetSourceControlConfigurationCrewEffect.test.tsx index 5c77f6581..6d5b64e68 100644 --- a/frontend/__tests__/hooks/useGetSourceControlConfigurationCrewEffect.test.tsx +++ b/frontend/__tests__/hooks/useGetSourceControlConfigurationCrewEffect.test.tsx @@ -15,6 +15,9 @@ jest.mock('react-redux', () => ({ useDispatch: () => mockDispatch, useSelector: (selector: () => TSelected) => { const originalUseSelector = jest.requireActual('react-redux').useSelector; + if (selector.name === 'selectShouldGetSourceControlConfig') { + return true; + } return originalUseSelector(selector); }, })); diff --git a/frontend/__tests__/hooks/useGetSourceControlConfigurationRepoEffect.test.tsx b/frontend/__tests__/hooks/useGetSourceControlConfigurationRepoEffect.test.tsx index 71513b29f..c5d35dcb6 100644 --- a/frontend/__tests__/hooks/useGetSourceControlConfigurationRepoEffect.test.tsx +++ b/frontend/__tests__/hooks/useGetSourceControlConfigurationRepoEffect.test.tsx @@ -15,6 +15,9 @@ jest.mock('react-redux', () => ({ useDispatch: () => mockDispatch, useSelector: (selector: () => TSelected) => { const originalUseSelector = jest.requireActual('react-redux').useSelector; + if (selector.name === 'selectShouldGetSourceControlConfig') { + return true; + } return originalUseSelector(selector); }, })); diff --git a/frontend/e2e/fixtures/import-file/chart-step-data.ts b/frontend/e2e/fixtures/import-file/chart-step-data.ts index be0d1aa9d..e6256eafa 100644 --- a/frontend/e2e/fixtures/import-file/chart-step-data.ts +++ b/frontend/e2e/fixtures/import-file/chart-step-data.ts @@ -1,6 +1,6 @@ export const chartStepData = { unSelectBranch: 'main', - addNewBranch: ['ADM-963'], + addNewBranch: ['ADM-998'], errorDateRange: [ { startDate: '2024-09-07T00:00:00.000+08:00', @@ -19,12 +19,12 @@ export const chartStepData = { ], rightDateRange: [ { - startDate: '2024-06-03T00:00:00.000+08:00', - endDate: '2024-06-06T23:59:59.999+08:00', + startDate: '2024-08-12T00:00:00.000+08:00', + endDate: '2024-08-25T23:59:59.999+08:00', }, { - startDate: '2024-06-07T00:00:00.000+08:00', - endDate: '2024-06-07T23:59:59.999+08:00', + startDate: '2024-08-26T00:00:00.000+08:00', + endDate: '2024-09-02T23:59:59.999+08:00', }, ], }; diff --git a/frontend/e2e/fixtures/input-files/charting-unhappy-path-config-file.template.json b/frontend/e2e/fixtures/input-files/charting-unhappy-path-config-file.template.json index 651925a2a..f65cf43ba 100644 --- a/frontend/e2e/fixtures/input-files/charting-unhappy-path-config-file.template.json +++ b/frontend/e2e/fixtures/input-files/charting-unhappy-path-config-file.template.json @@ -1,8 +1,8 @@ { "projectName": "Heartbeat Metrics", "dateRange": { - "startDate": "2024-06-03T00:00:00.000+08:00", - "endDate": "2024-06-07T23:59:59.999+08:00" + "startDate": "2024-08-12T00:00:00.000+08:00", + "endDate": "2024-09-02T23:59:59.999+08:00" }, "calendarType": "CN", "metrics": [ diff --git a/frontend/e2e/pages/metrics/metrics-step.ts b/frontend/e2e/pages/metrics/metrics-step.ts index 5014e4d07..6f1599135 100644 --- a/frontend/e2e/pages/metrics/metrics-step.ts +++ b/frontend/e2e/pages/metrics/metrics-step.ts @@ -641,7 +641,7 @@ export class MetricsStep { await this.pipelineBranchSelect.click(); for (const branchName of branches) { await this.page.getByRole('combobox', { name: 'Branches' }).fill(branchName); - await this.page.getByRole('option', { name: branchName }).getByRole('checkbox').check(); + await this.page.getByRole('option', { name: branchName }).getByRole('checkbox').first().check(); } await this.page.keyboard.press('Escape'); } diff --git a/frontend/src/containers/MetricsStep/SouceControlConfiguration/SourceControlMetricSelection/index.tsx b/frontend/src/containers/MetricsStep/SouceControlConfiguration/SourceControlMetricSelection/index.tsx index cc42ee1eb..7c07bf2a9 100644 --- a/frontend/src/containers/MetricsStep/SouceControlConfiguration/SourceControlMetricSelection/index.tsx +++ b/frontend/src/containers/MetricsStep/SouceControlConfiguration/SourceControlMetricSelection/index.tsx @@ -10,14 +10,17 @@ import { selectSourceControlBranches, selectDateRange, } from '@src/context/config/configSlice'; +import { + selectSourceControlConfigurationSettings, + updateShouldGetSourceControlConfig, +} from '@src/context/Metrics/metricsSlice'; import { useGetSourceControlConfigurationBranchEffect } from '@src/hooks/useGetSourceControlConfigurationBranchEffect'; import { useGetSourceControlConfigurationRepoEffect } from '@src/hooks/useGetSourceControlConfigurationRepoEffect'; import { useGetSourceControlConfigurationCrewEffect } from '@src/hooks/useGetSourceControlConfigurationCrewEffect'; import { SourceControlBranch } from '@src/containers/MetricsStep/SouceControlConfiguration/SourceControlBranch'; import { SingleSelection } from '@src/containers/MetricsStep/DeploymentFrequencySettings/SingleSelection'; -import { selectSourceControlConfigurationSettings } from '@src/context/Metrics/metricsSlice'; +import { useAppDispatch, useAppSelector } from '@src/hooks'; import { Loading } from '@src/components/Loading'; -import { useAppSelector } from '@src/hooks'; import { store } from '@src/store'; import { useEffect } from 'react'; @@ -62,6 +65,7 @@ export const SourceControlMetricSelection = ({ isGetAllCrews, } = useGetSourceControlConfigurationCrewEffect(); const storeContext = store.getState(); + const dispatch = useAppDispatch(); const organizationNameOptions = selectSourceControlOrganizations(storeContext); const repoNameOptions = selectSourceControlRepos(storeContext, organization); const branchNameOptions = selectSourceControlBranches(storeContext, organization, repo); @@ -98,10 +102,15 @@ export const SourceControlMetricSelection = ({ useEffect(() => { if (!isGetAllCrews && organization && repo && selectedBranches) { - selectedBranches.forEach((it) => getSourceControlCrewInfo(organization, repo, it, dateRanges)); + Promise.all(selectedBranches.map((it) => getSourceControlCrewInfo(organization, repo, it, dateRanges))).then( + () => { + dispatch(updateShouldGetSourceControlConfig(false)); + }, + ); } }, [ dateRanges, + dispatch, getSourceControlBranchInfo, getSourceControlCrewInfo, getSourceControlRepoInfo, @@ -124,7 +133,11 @@ export const SourceControlMetricSelection = ({ const handleOnUpdateBranches = (id: number, label: string, value: string[]): void => { const branchNeedGetCrews = value.filter((it) => selectedBranches?.every((branch) => branch !== it)); onUpdateSourceControl(id, label, value); - branchNeedGetCrews.forEach((branch) => getSourceControlCrewInfo(organization, repo, branch, dateRanges)); + Promise.all( + branchNeedGetCrews.map((branch) => getSourceControlCrewInfo(organization, repo, branch, dateRanges)), + ).then(() => { + dispatch(updateShouldGetSourceControlConfig(false)); + }); }; useEffect(() => { diff --git a/frontend/src/context/Metrics/metricsSlice.ts b/frontend/src/context/Metrics/metricsSlice.ts index 6ee330d53..6da643fe2 100644 --- a/frontend/src/context/Metrics/metricsSlice.ts +++ b/frontend/src/context/Metrics/metricsSlice.ts @@ -403,26 +403,25 @@ export const metricsSlice = createSlice({ state.sourceControlConfigurationSettings = sourceControlConfigurationSettings.map((it) => { if (it.id !== updateId) { return it; + } + if (label === 'organization') { + return { + ...it, + organization: value, + repo: '', + branches: [], + }; + } else if (label === 'repo') { + return { + ...it, + repo: value, + branches: [], + }; } else { - if (label === 'organization') { - return { - ...it, - organization: value, - repo: '', - branches: [], - }; - } else if (label === 'repo') { - return { - ...it, - repo: value, - branches: [], - }; - } else { - return { - ...it, - branches: value, - }; - } + return { + ...it, + branches: value, + }; } }); }, @@ -494,6 +493,9 @@ export const metricsSlice = createSlice({ updateShouldGetPipelineConfig: (state, action) => { state.shouldGetPipeLineConfig = action.payload; }, + updateShouldGetSourceControlConfig: (state, action) => { + state.shouldGetSourceControlConfig = action.payload; + }, updateMetricsImportedData: (state, action) => { const { @@ -839,6 +841,7 @@ export const { updateAdvancedSettings, updateShouldGetBoardConfig, updateShouldGetPipelineConfig, + updateShouldGetSourceControlConfig, updateReworkTimesSettings, updateFirstTimeRoadMetricsBoardData, updateShouldRetryPipelineConfig, diff --git a/frontend/src/hooks/useGetSourceControlConfigurationBranchEffect.tsx b/frontend/src/hooks/useGetSourceControlConfigurationBranchEffect.tsx index 2f2e5cfbb..c6a1f706a 100644 --- a/frontend/src/hooks/useGetSourceControlConfigurationBranchEffect.tsx +++ b/frontend/src/hooks/useGetSourceControlConfigurationBranchEffect.tsx @@ -1,5 +1,8 @@ +import { + selectShouldGetSourceControlConfig, + updateSourceControlConfigurationSettingsFirstInto, +} from '@src/context/Metrics/metricsSlice'; import { selectSourceControl, updateSourceControlVerifiedResponse } from '@src/context/config/configSlice'; -import { updateSourceControlConfigurationSettingsFirstInto } from '@src/context/Metrics/metricsSlice'; import { sourceControlClient } from '@src/clients/sourceControl/SourceControlClient'; import { useAppDispatch, useAppSelector } from '@src/hooks/index'; import { SourceControlTypes } from '@src/constants/resources'; @@ -14,7 +17,8 @@ export interface IUseGetSourceControlConfigurationBranchInterface { export const useGetSourceControlConfigurationBranchEffect = (): IUseGetSourceControlConfigurationBranchInterface => { const dispatch = useAppDispatch(); const [isLoading, setIsLoading] = useState(false); - const [isGetBranch, setIsGetBranch] = useState(false); + const shouldGetSourceControlConfig = useAppSelector(selectShouldGetSourceControlConfig); + const [isGetBranch, setIsGetBranch] = useState(!shouldGetSourceControlConfig); const restoredSourceControlInfo = useAppSelector(selectSourceControl); function getEnumKeyByEnumValue(enumValue: string): SourceControlTypes { diff --git a/frontend/src/hooks/useGetSourceControlConfigurationCrewEffect.tsx b/frontend/src/hooks/useGetSourceControlConfigurationCrewEffect.tsx index 0fbc190ed..bdb243616 100644 --- a/frontend/src/hooks/useGetSourceControlConfigurationCrewEffect.tsx +++ b/frontend/src/hooks/useGetSourceControlConfigurationCrewEffect.tsx @@ -1,4 +1,5 @@ import { DateRange, selectSourceControl, updateSourceControlVerifiedResponse } from '@src/context/config/configSlice'; +import { selectShouldGetSourceControlConfig } from '@src/context/Metrics/metricsSlice'; import { sourceControlClient } from '@src/clients/sourceControl/SourceControlClient'; import { FULFILLED, SourceControlTypes } from '@src/constants/resources'; import { useAppDispatch, useAppSelector } from '@src/hooks/index'; @@ -18,7 +19,8 @@ export interface IUseGetSourceControlConfigurationCrewInterface { export const useGetSourceControlConfigurationCrewEffect = (): IUseGetSourceControlConfigurationCrewInterface => { const dispatch = useAppDispatch(); const [isLoading, setIsLoading] = useState(false); - const [isGetAllCrews, setIsGetAllCrews] = useState(false); + const shouldGetSourceControlConfig = useAppSelector(selectShouldGetSourceControlConfig); + const [isGetAllCrews, setIsGetAllCrews] = useState(!shouldGetSourceControlConfig); const restoredSourceControlInfo = useAppSelector(selectSourceControl); function getEnumKeyByEnumValue(enumValue: string): SourceControlTypes { diff --git a/frontend/src/hooks/useGetSourceControlConfigurationOrganizationEffect.tsx b/frontend/src/hooks/useGetSourceControlConfigurationOrganizationEffect.tsx index 41ba36313..9ea62dbb5 100644 --- a/frontend/src/hooks/useGetSourceControlConfigurationOrganizationEffect.tsx +++ b/frontend/src/hooks/useGetSourceControlConfigurationOrganizationEffect.tsx @@ -72,15 +72,17 @@ export const useGetSourceControlConfigurationOrganizationEffect = }, [dispatch, restoredSourceControlInfo.token, restoredSourceControlInfo.type]); useEffect(() => { - if (!apiTouchedRef.current && !isLoading) { + if (!apiTouchedRef.current && !isLoading && shouldGetSourceControlConfig) { apiTouchedRef.current = true; getSourceControlInfo(); } - }, [getSourceControlInfo, isLoading]); + }, [getSourceControlInfo, isLoading, shouldGetSourceControlConfig]); useEffect(() => { - dispatch(clearSourceControlVerifiedResponse()); - }, [dispatch, restoredSourceControlInfo.token]); + if (shouldGetSourceControlConfig) { + dispatch(clearSourceControlVerifiedResponse()); + } + }, [dispatch, restoredSourceControlInfo.token, shouldGetSourceControlConfig]); return { isLoading, diff --git a/frontend/src/hooks/useGetSourceControlConfigurationRepoEffect.tsx b/frontend/src/hooks/useGetSourceControlConfigurationRepoEffect.tsx index a807ac51f..19e77bb7f 100644 --- a/frontend/src/hooks/useGetSourceControlConfigurationRepoEffect.tsx +++ b/frontend/src/hooks/useGetSourceControlConfigurationRepoEffect.tsx @@ -1,5 +1,8 @@ +import { + selectShouldGetSourceControlConfig, + updateSourceControlConfigurationSettingsFirstInto, +} from '@src/context/Metrics/metricsSlice'; import { DateRange, selectSourceControl, updateSourceControlVerifiedResponse } from '@src/context/config/configSlice'; -import { updateSourceControlConfigurationSettingsFirstInto } from '@src/context/Metrics/metricsSlice'; import { sourceControlClient } from '@src/clients/sourceControl/SourceControlClient'; import { FULFILLED, SourceControlTypes } from '@src/constants/resources'; import { useAppDispatch, useAppSelector } from '@src/hooks/index'; @@ -14,7 +17,8 @@ export interface IUseGetSourceControlConfigurationRepoInterface { export const useGetSourceControlConfigurationRepoEffect = (): IUseGetSourceControlConfigurationRepoInterface => { const dispatch = useAppDispatch(); const [isLoading, setIsLoading] = useState(false); - const [isGetRepo, setIsGetRepo] = useState(false); + const shouldGetSourceControlConfig = useAppSelector(selectShouldGetSourceControlConfig); + const [isGetRepo, setIsGetRepo] = useState(!shouldGetSourceControlConfig); const restoredSourceControlInfo = useAppSelector(selectSourceControl); function getEnumKeyByEnumValue(enumValue: string): SourceControlTypes { diff --git a/frontend/src/hooks/useVerifySourceControlTokenEffect.ts b/frontend/src/hooks/useVerifySourceControlTokenEffect.ts index d127cd95a..d44b3e688 100644 --- a/frontend/src/hooks/useVerifySourceControlTokenEffect.ts +++ b/frontend/src/hooks/useVerifySourceControlTokenEffect.ts @@ -1,4 +1,8 @@ -import { initDeploymentFrequencySettings, updateShouldGetPipelineConfig } from '@src/context/Metrics/metricsSlice'; +import { + initDeploymentFrequencySettings, + updateShouldGetPipelineConfig, + updateShouldGetSourceControlConfig, +} from '@src/context/Metrics/metricsSlice'; import { SOURCE_CONTROL_ERROR_MESSAGE } from '@src/containers/ConfigStep/Form/literal'; import { SourceControlVerifyRequestDTO } from '@src/clients/sourceControl/dto/request'; import { sourceControlClient } from '@src/clients/sourceControl/SourceControlClient'; @@ -34,6 +38,7 @@ export const useVerifySourceControlTokenEffect = () => { const persistReduxData = (sourceControlConfig: ISourceControlData) => { dispatch(updateSourceControl(sourceControlConfig)); dispatch(updateShouldGetPipelineConfig(true)); + dispatch(updateShouldGetSourceControlConfig(true)); dispatch(initDeploymentFrequencySettings()); }; const resetFields = () => {