From ecbe1b914367dc4d2a6b197ff6d23edab540b2c7 Mon Sep 17 00:00:00 2001 From: Jeremy Davis Date: Tue, 28 Apr 2020 17:59:46 +0200 Subject: [PATCH] SONAR-12914 security hotspots autoscroll to category --- .../__snapshots__/IssueLabel-test.tsx.snap | 2 + .../security-hotspots/SecurityHotspotsApp.tsx | 12 ++- .../SecurityHotspotsAppRenderer.tsx | 89 ++++++++++--------- .../__tests__/SecurityHotspotsApp-test.tsx | 19 ++++ .../SecurityHotspotsAppRenderer-test.tsx | 47 ++++++++++ .../SecurityHotspotsAppRenderer-test.tsx.snap | 29 ++++++ .../components/HotspotCategory.tsx | 2 +- .../HotspotCategory-test.tsx.snap | 4 + server/sonar-web/src/main/js/helpers/urls.ts | 12 ++- 9 files changed, 172 insertions(+), 44 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/overview/components/__tests__/__snapshots__/IssueLabel-test.tsx.snap b/server/sonar-web/src/main/js/apps/overview/components/__tests__/__snapshots__/IssueLabel-test.tsx.snap index ad55fcd971b..39ca9c58551 100644 --- a/server/sonar-web/src/main/js/apps/overview/components/__tests__/__snapshots__/IssueLabel-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/overview/components/__tests__/__snapshots__/IssueLabel-test.tsx.snap @@ -124,6 +124,7 @@ exports[`should render correctly for hotspots 1`] = ` "query": Object { "assignedToMe": undefined, "branch": undefined, + "category": undefined, "hotspots": undefined, "id": "my-project", "pullRequest": "1001", @@ -157,6 +158,7 @@ exports[`should render correctly for hotspots 2`] = ` "query": Object { "assignedToMe": undefined, "branch": undefined, + "category": undefined, "hotspots": undefined, "id": "my-project", "pullRequest": "1001", diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsApp.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsApp.tsx index 5367fb99884..2ed7abdd10d 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsApp.tsx +++ b/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsApp.tsx @@ -144,12 +144,22 @@ export class SecurityHotspotsApp extends React.PureComponent { return; } + const requestedCategory = this.props.location.query.category; + + let selectedHotspot; + if (hotspots.length > 0) { + const hotspotForCategory = requestedCategory + ? hotspots.find(h => h.securityCategory === requestedCategory) + : undefined; + selectedHotspot = hotspotForCategory ?? hotspots[0]; + } + this.setState({ hotspots, hotspotsTotal: paging.total, loading: false, securityCategories: sonarsourceSecurity, - selectedHotspot: hotspots.length > 0 ? hotspots[0] : undefined + selectedHotspot }); }) .catch(this.handleCallFailure); diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsAppRenderer.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsAppRenderer.tsx index c9d8b687f84..43e4dfdcf65 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsAppRenderer.tsx +++ b/server/sonar-web/src/main/js/apps/security-hotspots/SecurityHotspotsAppRenderer.tsx @@ -21,6 +21,7 @@ import * as React from 'react'; import { Helmet } from 'react-helmet-async'; import DeferredSpinner from 'sonar-ui-common/components/ui/DeferredSpinner'; import { translate } from 'sonar-ui-common/helpers/l10n'; +import { scrollToElement } from 'sonar-ui-common/helpers/scrolling'; import A11ySkipTarget from '../../app/components/a11y/A11ySkipTarget'; import Suggestions from '../../app/components/embed-docs-modal/Suggestions'; import ScreenPositionHelper from '../../components/common/ScreenPositionHelper'; @@ -69,6 +70,17 @@ export default function SecurityHotspotsAppRenderer(props: SecurityHotspotsAppRe filters } = props; + const scrollableRef = React.useRef(null); + + React.useEffect(() => { + const parent = scrollableRef.current; + const element = + selectedHotspot && document.querySelector(`[data-hotspot-key="${selectedHotspot.key}"]`); + if (parent && element) { + scrollToElement(element, { parent, smooth: true, topOffset: 150, bottomOffset: 400 }); + } + }, [selectedHotspot]); + return (
- {loading ? ( - - ) : ( - <> - {hotspots.length === 0 || !selectedHotspot ? ( - - ) : ( -
-
- -
-
- -
+ {loading && } + + {!loading && + (hotspots.length === 0 || !selectedHotspot ? ( + + ) : ( +
+
+ +
+
+
- )} - - )} +
+ ))}
)} diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsApp-test.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsApp-test.tsx index 8692f0fc345..4921949f20b 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsApp-test.tsx +++ b/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsApp-test.tsx @@ -107,6 +107,25 @@ it('should load data correctly', async () => { expect(wrapper.state().hotspotsReviewedMeasure).toBe('86.6'); }); +it('should handle category request', async () => { + const hotspots = [mockRawHotspot(), mockRawHotspot({ securityCategory: 'log-injection' })]; + (getSecurityHotspots as jest.Mock).mockResolvedValue({ + hotspots, + paging: { + total: 1 + } + }); + (getMeasures as jest.Mock).mockResolvedValue([{ value: '86.6' }]); + + const wrapper = shallowRender({ + location: mockLocation({ query: { category: hotspots[1].securityCategory } }) + }); + + await waitAndUpdate(wrapper); + + expect(wrapper.state().selectedHotspot).toBe(hotspots[1]); +}); + it('should load data correctly when hotspot key list is forced', async () => { const hotspots = [ mockRawHotspot({ key: 'test1' }), diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsAppRenderer-test.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsAppRenderer-test.tsx index 5cdbc37d471..af6beba4db4 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsAppRenderer-test.tsx +++ b/server/sonar-web/src/main/js/apps/security-hotspots/__tests__/SecurityHotspotsAppRenderer-test.tsx @@ -19,6 +19,7 @@ */ import { shallow } from 'enzyme'; import * as React from 'react'; +import { scrollToElement } from 'sonar-ui-common/helpers/scrolling'; import ScreenPositionHelper from '../../../components/common/ScreenPositionHelper'; import { mockRawHotspot } from '../../../helpers/mocks/security-hotspots'; import { mockComponent } from '../../../helpers/testMocks'; @@ -28,6 +29,14 @@ import SecurityHotspotsAppRenderer, { SecurityHotspotsAppRendererProps } from '../SecurityHotspotsAppRenderer'; +jest.mock('sonar-ui-common/helpers/scrolling', () => ({ + scrollToElement: jest.fn() +})); + +beforeEach(() => { + jest.clearAllMocks(); +}); + it('should render correctly', () => { expect(shallowRender()).toMatchSnapshot(); expect( @@ -42,6 +51,11 @@ it('should render correctly', () => { .find(ScreenPositionHelper) .dive() ).toMatchSnapshot('no hotspots'); + expect( + shallowRender({ loading: true }) + .find(ScreenPositionHelper) + .dive() + ).toMatchSnapshot('loading'); }); it('should render correctly with hotspots', () => { @@ -70,6 +84,39 @@ it('should properly propagate the "show all" call', () => { expect(onShowAllHotspots).toHaveBeenCalled(); }); +describe('side effect', () => { + const fakeElement = document.createElement('span'); + const fakeParent = document.createElement('div'); + + beforeEach(() => { + jest.spyOn(React, 'useEffect').mockImplementationOnce(f => f()); + jest.spyOn(document, 'querySelector').mockImplementationOnce(() => fakeElement); + jest.spyOn(React, 'useRef').mockImplementationOnce(() => ({ current: fakeParent })); + }); + + it('should trigger scrolling', () => { + shallowRender({ selectedHotspot: mockRawHotspot() }); + + expect(scrollToElement).toBeCalledWith( + fakeElement, + expect.objectContaining({ parent: fakeParent }) + ); + }); + + it('should not trigger scrolling if no selected hotspot', () => { + shallowRender(); + expect(scrollToElement).not.toBeCalled(); + }); + + it('should not trigger scrolling if no parent', () => { + const mockUseRef = jest.spyOn(React, 'useRef'); + mockUseRef.mockReset(); + mockUseRef.mockImplementationOnce(() => ({ current: null })); + shallowRender({ selectedHotspot: mockRawHotspot() }); + expect(scrollToElement).not.toBeCalled(); + }); +}); + function shallowRender(props: Partial = {}) { return shallow( `; +exports[`should render correctly: loading 1`] = ` +
+
+ + + + +
+
+`; + exports[`should render correctly: no hotspots 1`] = `
{hotspots.map(h => ( -
  • +