From 590a9d5d2cd511bde4f7763478fb7f6b8d0e2faa Mon Sep 17 00:00:00 2001 From: Wouter Admiraal Date: Mon, 6 Apr 2020 17:02:48 +0200 Subject: [PATCH] SONAR-13195 PR overview not showing all failing conditions --- .../js/apps/overview/__tests__/utils-test.ts | 2 +- .../pullRequests/PullRequestOverview.tsx | 59 +++++++++++++++---- .../__tests__/PullRequestOverview-test.tsx | 20 +++++-- .../src/main/js/types/quality-gates.ts | 4 +- 4 files changed, 66 insertions(+), 19 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/overview/__tests__/utils-test.ts b/server/sonar-web/src/main/js/apps/overview/__tests__/utils-test.ts index 927b36415e2..7e06a7b7bdf 100644 --- a/server/sonar-web/src/main/js/apps/overview/__tests__/utils-test.ts +++ b/server/sonar-web/src/main/js/apps/overview/__tests__/utils-test.ts @@ -62,7 +62,7 @@ function mockMeasure() { }), mockQualityGateStatusCondition({ metric: MetricKey.new_duplicated_lines_density, - level: 'WARNING', + level: 'OK', warning: '5' }) ] diff --git a/server/sonar-web/src/main/js/apps/overview/pullRequests/PullRequestOverview.tsx b/server/sonar-web/src/main/js/apps/overview/pullRequests/PullRequestOverview.tsx index 18862261bce..4be5783d3af 100644 --- a/server/sonar-web/src/main/js/apps/overview/pullRequests/PullRequestOverview.tsx +++ b/server/sonar-web/src/main/js/apps/overview/pullRequests/PullRequestOverview.tsx @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as classNames from 'classnames'; +import { differenceBy, uniq } from 'lodash'; import * as React from 'react'; import { connect } from 'react-redux'; import HelpTooltip from 'sonar-ui-common/components/controls/HelpTooltip'; @@ -48,7 +49,7 @@ interface StateProps { } interface DispatchProps { - fetchBranchStatus: (branchLike: BranchLike, projectKey: string) => Promise; + fetchBranchStatus: (branchLike: BranchLike, projectKey: string) => void; } interface OwnProps { @@ -73,29 +74,65 @@ export class PullRequestOverview extends React.PureComponent { componentDidMount() { this.mounted = true; - this.fetchBranchData(); + if (this.props.conditions === undefined) { + this.fetchBranchStatusData(); + } else { + this.fetchBranchData(); + } + } + + componentDidUpdate(prevProps: Props) { + if (this.conditionsHaveChanged(prevProps)) { + this.fetchBranchData(); + } } componentWillUnmount() { this.mounted = false; } - fetchBranchData = () => { + conditionsHaveChanged = (prevProps: Props) => { + const prevConditions = prevProps.conditions ?? []; + const newConditions = this.props.conditions ?? []; + const diff = differenceBy( + prevConditions.filter(c => c.level === 'ERROR'), + newConditions.filter(c => c.level === 'ERROR'), + c => c.metric + ); + + return ( + (prevProps.conditions === undefined && this.props.conditions !== undefined) || diff.length > 0 + ); + }; + + fetchBranchStatusData = () => { const { branchLike, component: { key } } = this.props; + this.props.fetchBranchStatus(branchLike, key); + }; + + fetchBranchData = () => { + const { + branchLike, + component: { key }, + conditions + } = this.props; this.setState({ loading: true }); - Promise.all([ - getMeasuresAndMeta(key, PR_METRICS, { - additionalFields: 'metrics', - ...getBranchLikeQuery(branchLike) - }), - this.props.fetchBranchStatus(branchLike, key) - ]).then( - ([{ component, metrics }]) => { + const metricKeys = + conditions !== undefined + ? // Also load metrics that apply to failing QG conditions. + uniq([...PR_METRICS, ...conditions.filter(c => c.level !== 'OK').map(c => c.metric)]) + : PR_METRICS; + + getMeasuresAndMeta(key, metricKeys, { + additionalFields: 'metrics', + ...getBranchLikeQuery(branchLike) + }).then( + ({ component, metrics }) => { if (this.mounted && component.measures) { this.setState({ loading: false, diff --git a/server/sonar-web/src/main/js/apps/overview/pullRequests/__tests__/PullRequestOverview-test.tsx b/server/sonar-web/src/main/js/apps/overview/pullRequests/__tests__/PullRequestOverview-test.tsx index 1c2c1cb34d5..ae06e66200a 100644 --- a/server/sonar-web/src/main/js/apps/overview/pullRequests/__tests__/PullRequestOverview-test.tsx +++ b/server/sonar-web/src/main/js/apps/overview/pullRequests/__tests__/PullRequestOverview-test.tsx @@ -24,6 +24,7 @@ import { getMeasuresAndMeta } from '../../../../api/measures'; import { mockPullRequest } from '../../../../helpers/mocks/branch-like'; import { mockQualityGateStatusCondition } from '../../../../helpers/mocks/quality-gates'; import { mockComponent } from '../../../../helpers/testMocks'; +import { PR_METRICS } from '../../utils'; import { PullRequestOverview } from '../PullRequestOverview'; jest.mock('../../../../api/measures', () => { @@ -103,13 +104,22 @@ it('should render correctly for a failed QG', async () => { expect(wrapper).toMatchSnapshot(); }); -it('should correctly handle a WS failure', async () => { - (getMeasuresAndMeta as jest.Mock).mockRejectedValueOnce({}); - const fetchBranchStatus = jest.fn().mockRejectedValue({}); - const wrapper = shallowRender({ fetchBranchStatus }); +it('should correctly fetch all required metrics for a passing QG', async () => { + const wrapper = shallowRender({ conditions: [] }); + await waitAndUpdate(wrapper); + expect(getMeasuresAndMeta).toBeCalledWith('my-project', PR_METRICS, expect.any(Object)); +}); +it('should correctly fetch all required metrics for a failing QG', async () => { + const wrapper = shallowRender({ + conditions: [mockQualityGateStatusCondition({ level: 'ERROR', metric: 'foo' })] + }); await waitAndUpdate(wrapper); - expect(wrapper.type()).toBeNull(); + expect(getMeasuresAndMeta).toBeCalledWith( + 'my-project', + [...PR_METRICS, 'foo'], + expect.any(Object) + ); }); function shallowRender(props: Partial = {}) { diff --git a/server/sonar-web/src/main/js/types/quality-gates.ts b/server/sonar-web/src/main/js/types/quality-gates.ts index e3dbe2130c3..84b14f1bf45 100644 --- a/server/sonar-web/src/main/js/types/quality-gates.ts +++ b/server/sonar-web/src/main/js/types/quality-gates.ts @@ -44,7 +44,7 @@ export interface QualityGateApplicationStatusCondition { metric: string; periodIndex?: number; onLeak?: boolean; - status: string; + status: T.Status; value: string; warningThreshold?: string; } @@ -67,7 +67,7 @@ export interface QualityGateStatus { export interface QualityGateStatusCondition { actual?: string; error?: string; - level: string; + level: T.Status; metric: string; op: string; period?: number; -- 2.39.5