From f8e5e5f6913a02fde4a5f4440a305a99607c3220 Mon Sep 17 00:00:00 2001 From: Jeremy Davis Date: Wed, 29 Jan 2020 17:39:21 +0100 Subject: [PATCH] SONAR-12964 Add hotspots reviewed percent to hotspots page --- .../securityHotspots/SecurityHotspotsApp.tsx | 59 +- .../SecurityHotspotsAppRenderer.tsx | 9 + .../__tests__/SecurityHotspotsApp-test.tsx | 56 +- .../SecurityHotspotsAppRenderer-test.tsx | 2 + .../SecurityHotspotsApp-test.tsx.snap | 2 + .../SecurityHotspotsAppRenderer-test.tsx.snap | 2 + .../securityHotspots/components/FilterBar.tsx | 106 +++- .../components/__tests__/FilterBar-test.tsx | 8 + .../__snapshots__/FilterBar-test.tsx.snap | 575 +++++++++++++----- .../main/js/components/measure/Measure.tsx | 2 +- .../resources/org/sonar/l10n/core.properties | 2 + 11 files changed, 629 insertions(+), 194 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsApp.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsApp.tsx index 8af095d3ca7..13e16ce83b8 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsApp.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsApp.tsx @@ -20,13 +20,16 @@ import { Location } from 'history'; import * as React from 'react'; import { addNoFooterPageClass, removeNoFooterPageClass } from 'sonar-ui-common/helpers/pages'; +import { getMeasures } from '../../api/measures'; import { getSecurityHotspotList, getSecurityHotspots } from '../../api/security-hotspots'; import { withCurrentUser } from '../../components/hoc/withCurrentUser'; import { Router } from '../../components/hoc/withRouter'; +import { getLeakValue } from '../../components/measure/utils'; import { getBranchLikeQuery, isPullRequest, isSameBranchLike } from '../../helpers/branch-like'; import { getStandards } from '../../helpers/security-standard'; import { isLoggedIn } from '../../helpers/users'; import { BranchLike } from '../../types/branch-like'; +import { ComponentQualifier } from '../../types/component'; import { HotspotFilters, HotspotResolution, @@ -52,8 +55,10 @@ interface State { hotspotKeys?: string[]; hotspots: RawHotspot[]; hotspotsPageIndex: number; + hotspotsReviewedMeasure?: string; hotspotsTotal?: number; loading: boolean; + loadingMeasure: boolean; loadingMore: boolean; securityCategories: T.StandardSecurityCategories; selectedHotspotKey: string | undefined; @@ -69,6 +74,7 @@ export class SecurityHotspotsApp extends React.PureComponent { this.state = { loading: true, + loadingMeasure: false, loadingMore: false, hotspots: [], hotspotsPageIndex: 1, @@ -129,7 +135,11 @@ export class SecurityHotspotsApp extends React.PureComponent { }; fetchInitialData() { - return Promise.all([getStandards(), this.fetchSecurityHotspots()]) + return Promise.all([ + getStandards(), + this.fetchSecurityHotspots(), + this.fetchSecurityHotspotsReviewed() + ]) .then(([{ sonarsourceSecurity }, { hotspots, paging }]) => { if (!this.mounted) { return; @@ -146,6 +156,38 @@ export class SecurityHotspotsApp extends React.PureComponent { .catch(this.handleCallFailure); } + fetchSecurityHotspotsReviewed() { + const { branchLike, component } = this.props; + const { filters } = this.state; + + const reviewedHotspotsMetricKey = filters.sinceLeakPeriod + ? 'new_security_hotspots_reviewed' + : 'security_hotspots_reviewed'; + + this.setState({ loadingMeasure: true }); + return getMeasures({ + component: component.key, + metricKeys: reviewedHotspotsMetricKey, + ...getBranchLikeQuery(branchLike) + }) + .then(measures => { + if (!this.mounted) { + return; + } + const measure = measures && measures.length > 0 ? measures[0] : undefined; + const hotspotsReviewedMeasure = filters.sinceLeakPeriod + ? getLeakValue(measure) + : measure?.value; + + this.setState({ hotspotsReviewedMeasure, loadingMeasure: false }); + }) + .catch(() => { + if (this.mounted) { + this.setState({ loadingMeasure: false }); + } + }); + } + fetchSecurityHotspots(page = 1) { const { branchLike, component, location } = this.props; const { filters } = this.state; @@ -208,7 +250,12 @@ export class SecurityHotspotsApp extends React.PureComponent { handleChangeFilters = (changes: Partial) => { this.setState( ({ filters }) => ({ filters: { ...filters, ...changes } }), - this.reloadSecurityHotspotList + () => { + this.reloadSecurityHotspotList(); + if (changes.sinceLeakPeriod !== undefined) { + this.fetchSecurityHotspotsReviewed(); + } + } ); }; @@ -229,6 +276,7 @@ export class SecurityHotspotsApp extends React.PureComponent { } return null; }); + return this.fetchSecurityHotspotsReviewed(); }; handleShowAllHotspots = () => { @@ -259,12 +307,14 @@ export class SecurityHotspotsApp extends React.PureComponent { }; render() { - const { branchLike } = this.props; + const { branchLike, component } = this.props; const { hotspotKeys, hotspots, + hotspotsReviewedMeasure, hotspotsTotal, loading, + loadingMeasure, loadingMore, securityCategories, selectedHotspotKey, @@ -276,9 +326,12 @@ export class SecurityHotspotsApp extends React.PureComponent { branchLike={branchLike} filters={filters} hotspots={hotspots} + hotspotsReviewedMeasure={hotspotsReviewedMeasure} hotspotsTotal={hotspotsTotal} + isProject={component.qualifier === ComponentQualifier.Project} isStaticListOfHotspots={Boolean(hotspotKeys && hotspotKeys.length > 0)} loading={loading} + loadingMeasure={loadingMeasure} loadingMore={loadingMore} onChangeFilters={this.handleChangeFilters} onHotspotClick={this.handleHotspotClick} diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsAppRenderer.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsAppRenderer.tsx index 3e3b800bc96..898dcbe5074 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsAppRenderer.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/SecurityHotspotsAppRenderer.tsx @@ -42,9 +42,12 @@ export interface SecurityHotspotsAppRendererProps { branchLike?: BranchLike; filters: HotspotFilters; hotspots: RawHotspot[]; + hotspotsReviewedMeasure?: string; hotspotsTotal?: number; + isProject: boolean; isStaticListOfHotspots: boolean; loading: boolean; + loadingMeasure: boolean; loadingMore: boolean; onChangeFilters: (filters: Partial) => void; onHotspotClick: (key: string) => void; @@ -59,9 +62,12 @@ export default function SecurityHotspotsAppRenderer(props: SecurityHotspotsAppRe const { branchLike, hotspots, + hotspotsReviewedMeasure, hotspotsTotal, + isProject, isStaticListOfHotspots, loading, + loadingMeasure, loadingMore, securityCategories, selectedHotspotKey, @@ -72,7 +78,10 @@ export default function SecurityHotspotsAppRenderer(props: SecurityHotspotsAppRe
({ removeNoFooterPageClass: jest.fn() })); +jest.mock('../../../api/measures', () => ({ + getMeasures: jest.fn().mockResolvedValue([]) +})); + jest.mock('../../../api/security-hotspots', () => ({ getSecurityHotspots: jest.fn().mockResolvedValue({ hotspots: [], paging: { total: 0 } }), getSecurityHotspotList: jest.fn().mockResolvedValue({ hotspots: [], rules: [] }) @@ -70,10 +75,12 @@ it('should load data correctly', async () => { total: 1 } }); + (getMeasures as jest.Mock).mockResolvedValue([{ value: '86.6' }]); const wrapper = shallowRender(); expect(wrapper.state().loading).toBe(true); + expect(wrapper.state().loadingMeasure).toBe(true); expect(addNoFooterPageClass).toBeCalled(); expect(getStandards).toBeCalled(); @@ -82,6 +89,11 @@ it('should load data correctly', async () => { branch: branch.name }) ); + expect(getMeasures).toBeCalledWith( + expect.objectContaining({ + branch: branch.name + }) + ); await waitAndUpdate(wrapper); @@ -91,8 +103,8 @@ it('should load data correctly', async () => { expect(wrapper.state().securityCategories).toEqual({ cat1: { title: 'cat 1' } }); - - expect(wrapper.state()); + expect(wrapper.state().loadingMeasure).toBe(false); + expect(wrapper.state().hotspotsReviewedMeasure).toBe('86.6'); }); it('should load data correctly when hotspot key list is forced', async () => { @@ -155,11 +167,12 @@ it('should set "leakperiod" filter according to context (branchlike & location q }); it('should set "assigned to me" filter according to context (logged in & explicit location query)', () => { - expect(shallowRender().state().filters.assignedToMe).toBe(false); - expect( - shallowRender({ location: mockLocation({ query: { assignedToMe: 'true' } }) }).state().filters - .assignedToMe - ).toBe(false); + const wrapper = shallowRender(); + expect(wrapper.state().filters.assignedToMe).toBe(false); + + wrapper.setProps({ location: mockLocation({ query: { assignedToMe: 'true' } }) }); + expect(wrapper.state().filters.assignedToMe).toBe(false); + expect(shallowRender({ currentUser: mockLoggedInUser() }).state().filters.assignedToMe).toBe( false ); @@ -224,7 +237,9 @@ it('should handle hotspot update', async () => { status: HotspotStatus.REVIEWED, resolution: HotspotResolution.SAFE }); + expect(getMeasures).toBeCalled(); + await waitAndUpdate(wrapper); const previousState = wrapper.state(); wrapper.instance().handleHotspotUpdate({ key: 'unknown', @@ -251,8 +266,11 @@ it('should handle status filter change', async () => { await waitAndUpdate(wrapper); + expect(getMeasures).toBeCalledTimes(1); + // Set filter to SAFE: wrapper.instance().handleChangeFilters({ status: HotspotStatusFilter.SAFE }); + expect(getMeasures).toBeCalledTimes(1); expect(getSecurityHotspots).toBeCalledWith( expect.objectContaining({ status: HotspotStatus.REVIEWED, resolution: HotspotResolution.SAFE }) @@ -274,6 +292,30 @@ it('should handle status filter change', async () => { expect(wrapper.state().hotspots).toHaveLength(0); }); +it('should handle leakPeriod filter change', async () => { + const hotspots = [mockRawHotspot({ key: 'key1' })]; + const hotspots2 = [mockRawHotspot({ key: 'key2' })]; + (getSecurityHotspots as jest.Mock) + .mockResolvedValueOnce({ hotspots, paging: { total: 1 } }) + .mockResolvedValueOnce({ hotspots: hotspots2, paging: { total: 1 } }) + .mockResolvedValueOnce({ hotspots: [], paging: { total: 0 } }); + + const wrapper = shallowRender(); + + expect(getSecurityHotspots).toBeCalledWith( + expect.objectContaining({ status: HotspotStatus.TO_REVIEW, resolution: undefined }) + ); + + await waitAndUpdate(wrapper); + + expect(getMeasures).toBeCalledTimes(1); + + wrapper.instance().handleChangeFilters({ sinceLeakPeriod: true }); + + expect(getMeasures).toBeCalledTimes(2); + expect(getSecurityHotspots).toBeCalledWith(expect.objectContaining({ sinceLeakPeriod: true })); +}); + function shallowRender(props: Partial = {}) { return shallow( = {}) { status: HotspotStatusFilter.TO_REVIEW }} hotspots={[]} + isProject={true} isStaticListOfHotspots={true} loading={false} + loadingMeasure={false} loadingMore={false} onChangeFilters={jest.fn()} onHotspotClick={jest.fn()} diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsApp-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsApp-test.tsx.snap index 2cda8bd69a6..78f624205a3 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsApp-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsApp-test.tsx.snap @@ -18,8 +18,10 @@ exports[`should render correctly 1`] = ` } } hotspots={Array []} + isProject={true} isStaticListOfHotspots={false} loading={true} + loadingMeasure={true} loadingMore={false} onChangeFilters={[Function]} onHotspotClick={[Function]} diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsAppRenderer-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsAppRenderer-test.tsx.snap index 5af2220ac69..64e20d6e7c9 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsAppRenderer-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/securityHotspots/__tests__/__snapshots__/SecurityHotspotsAppRenderer-test.tsx.snap @@ -12,7 +12,9 @@ exports[`should render correctly 1`] = ` "status": "TO_REVIEW", } } + isProject={true} isStaticListOfHotspots={true} + loadingMeasure={false} onBranch={false} onChangeFilters={[MockFunction]} onShowAllHotspots={[MockFunction]} diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/FilterBar.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/FilterBar.tsx index b108b52b490..67151c4a16c 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/FilterBar.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/FilterBar.tsx @@ -18,17 +18,24 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; +import HelpTooltip from 'sonar-ui-common/components/controls/HelpTooltip'; import RadioToggle from 'sonar-ui-common/components/controls/RadioToggle'; import Select from 'sonar-ui-common/components/controls/Select'; +import DeferredSpinner from 'sonar-ui-common/components/ui/DeferredSpinner'; import { translate } from 'sonar-ui-common/helpers/l10n'; import { withCurrentUser } from '../../../components/hoc/withCurrentUser'; +import Measure from '../../../components/measure/Measure'; +import CoverageRating from '../../../components/ui/CoverageRating'; import { isLoggedIn } from '../../../helpers/users'; import { HotspotFilters, HotspotStatusFilter } from '../../../types/security-hotspots'; export interface FilterBarProps { currentUser: T.CurrentUser; filters: HotspotFilters; + hotspotsReviewedMeasure?: string; + isProject: boolean; isStaticListOfHotspots: boolean; + loadingMeasure: boolean; onBranch: boolean; onChangeFilters: (filters: Partial) => void; onShowAllHotspots: () => void; @@ -56,7 +63,15 @@ const assigneeFilterOptions = [ ]; export function FilterBar(props: FilterBarProps) { - const { currentUser, filters, isStaticListOfHotspots, onBranch } = props; + const { + currentUser, + filters, + hotspotsReviewedMeasure, + isProject, + isStaticListOfHotspots, + loadingMeasure, + onBranch + } = props; return (
@@ -65,46 +80,73 @@ export function FilterBar(props: FilterBarProps) { {translate('hotspot.filters.show_all')} ) : ( - <> -

{translate('hotspot.filters.title')}

+
+
+

{translate('hotspot.filters.title')}

- {isLoggedIn(currentUser) && ( - - props.onChangeFilters({ assignedToMe: value === AssigneeFilterOption.ME }) - } - options={assigneeFilterOptions} - value={filters.assignedToMe ? AssigneeFilterOption.ME : AssigneeFilterOption.ALL} - /> - )} + {isLoggedIn(currentUser) && ( + + props.onChangeFilters({ assignedToMe: value === AssigneeFilterOption.ME }) + } + options={assigneeFilterOptions} + value={filters.assignedToMe ? AssigneeFilterOption.ME : AssigneeFilterOption.ALL} + /> + )} - {translate('status')} - - props.onChangeFilters({ sinceLeakPeriod: option.value }) + onChange={(option: { value: HotspotStatusFilter }) => + props.onChangeFilters({ status: option.value }) } - options={periodOptions} + options={statusOptions} searchable={false} - value={filters.sinceLeakPeriod} + value={filters.status} /> + + {onBranch && ( + - + + - + +
+
+ + metric.security_hotspots_reviewed.name + + + + + +
+
+
+`; + +exports[`should render correctly: with hotspots reviewed measure 1`] = ` +
+
- status - - +