From b501b1b4a27b26f4dbbca99995980edb6449c2c1 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 9 Jan 2020 17:42:08 +0100 Subject: [PATCH] SONAR-12719 Handle access rights --- .../components/HotspotActionsForm.tsx | 100 ++++++---- .../components/HotspotActionsFormRenderer.tsx | 64 ++++++- .../components/HotspotAssigneeSelect.tsx | 3 +- .../__tests__/HotspotActionsForm-test.tsx | 89 +++++---- .../HotspotActionsFormRenderer-test.tsx | 56 +++++- .../__tests__/HotspotAssigneeSelect-test.tsx | 10 +- .../HotspotActions-test.tsx.snap | 3 + .../HotspotActionsForm-test.tsx.snap | 100 +++++++++- .../HotspotActionsFormRenderer-test.tsx.snap | 173 ++++++++++++++++-- .../HotspotSnippetContainer-test.tsx.snap | 1 + ...spotSnippetContainerRenderer-test.tsx.snap | 1 + .../HotspotViewerRenderer-test.tsx.snap | 13 ++ ...otspotViewerReviewHistoryTab-test.tsx.snap | 1 + .../HotspotViewerTabs-test.tsx.snap | 7 + .../js/helpers/mocks/security-hotspots.ts | 1 + .../src/main/js/types/security-hotspots.ts | 1 + 16 files changed, 527 insertions(+), 96 deletions(-) diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotActionsForm.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotActionsForm.tsx index 1787fa9e1a2..86dd3113604 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotActionsForm.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotActionsForm.tsx @@ -18,11 +18,14 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import * as React from 'react'; -import { assignSecurityHotspot, setSecurityHotspotStatus } from '../../../api/security-hotspots'; +import { + assignSecurityHotspot, + commentSecurityHotspot, + setSecurityHotspotStatus +} from '../../../api/security-hotspots'; import { Hotspot, HotspotResolution, - HotspotSetStatusRequest, HotspotStatus, HotspotStatusOption, HotspotUpdateFields @@ -42,11 +45,22 @@ interface State { } export default class HotspotActionsForm extends React.Component { - state: State = { - comment: '', - selectedOption: HotspotStatusOption.FIXED, - submitting: false - }; + constructor(props: Props) { + super(props); + + let selectedOption = HotspotStatusOption.FIXED; + if (props.hotspot.status === HotspotStatus.TO_REVIEW) { + selectedOption = HotspotStatusOption.ADDITIONAL_REVIEW; + } else if (props.hotspot.resolution) { + selectedOption = HotspotStatusOption[props.hotspot.resolution]; + } + + this.state = { + comment: '', + selectedOption, + submitting: false + }; + } handleSelectOption = (selectedOption: HotspotStatusOption) => { this.setState({ selectedOption }); @@ -71,40 +85,62 @@ export default class HotspotActionsForm extends React.Component { ? HotspotStatus.TO_REVIEW : HotspotStatus.REVIEWED; - const data: HotspotSetStatusRequest = { status }; - - // If reassigning, ignore comment for status update. It will be sent with the reassignment below - if (comment && !(selectedOption === HotspotStatusOption.ADDITIONAL_REVIEW && selectedUser)) { - data.comment = comment; - } - - if (selectedOption !== HotspotStatusOption.ADDITIONAL_REVIEW) { - data.resolution = HotspotResolution[selectedOption]; - } + const resolution = + selectedOption !== HotspotStatusOption.ADDITIONAL_REVIEW + ? HotspotResolution[selectedOption] + : undefined; this.setState({ submitting: true }); - return setSecurityHotspotStatus(hotspot.key, data) - .then(() => { - if (selectedOption === HotspotStatusOption.ADDITIONAL_REVIEW && selectedUser) { - return this.assignHotspot(selectedUser, comment); - } - return null; - }) + /* + * updateAssignee depends on updateStatus, hence these are chained rather than + * run in parallel. The comment should also appear last in the changelog. + */ + return Promise.resolve() + .then(() => this.updateStatus(hotspot, status, resolution)) + .then(() => this.updateAssignee(hotspot, selectedOption, selectedUser)) + .then(() => this.addComment(hotspot, comment)) .then(() => { - this.props.onSubmit({ status, resolution: data.resolution }); + this.props.onSubmit({ status, resolution }); + // No need to set "submitting", we are closing the window }) .catch(() => { this.setState({ submitting: false }); }); }; - assignHotspot = (assignee: T.UserActive, comment: string) => { - const { hotspot } = this.props; + updateStatus = (hotspot: Hotspot, status: HotspotStatus, resolution?: HotspotResolution) => { + if ( + hotspot.canChangeStatus && + (status !== hotspot.status || resolution !== hotspot.resolution) + ) { + return setSecurityHotspotStatus(hotspot.key, { status, resolution }); + } + + return Promise.resolve(); + }; - return assignSecurityHotspot(hotspot.key, { - assignee: assignee.login, - comment - }); + updateAssignee = ( + hotspot: Hotspot, + selectedOption: HotspotStatusOption, + selectedUser?: T.UserActive + ) => { + if ( + selectedOption === HotspotStatusOption.ADDITIONAL_REVIEW && + selectedUser && + selectedUser.login !== hotspot.assignee + ) { + return assignSecurityHotspot(hotspot.key, { + assignee: selectedUser.login + }); + } + return Promise.resolve(); + }; + + addComment = (hotspot: Hotspot, comment: string) => { + if (comment.length > 0) { + return commentSecurityHotspot(hotspot.key, comment); + } + return Promise.resolve(); }; render() { @@ -114,7 +150,7 @@ export default class HotspotActionsForm extends React.Component { return ( void; onChangeComment: (comment: string) => void; onSelectOption: (option: HotspotStatusOption) => void; @@ -38,23 +44,28 @@ export interface HotspotActionsFormRendererProps { } export default function HotspotActionsFormRenderer(props: HotspotActionsFormRendererProps) { - const { comment, hotspotStatus, selectedOption, submitting } = props; + const { comment, hotspot, selectedOption, submitting } = props; + + const disableStatusChange = !hotspot.canChangeStatus; return (

{translate('hotspots.form.title')}

{renderOption({ + disabled: disableStatusChange, option: HotspotStatusOption.FIXED, selectedOption, onClick: props.onSelectOption })} {renderOption({ + disabled: disableStatusChange, option: HotspotStatusOption.SAFE, selectedOption, onClick: props.onSelectOption })} {renderOption({ + disabled: disableStatusChange, option: HotspotStatusOption.ADDITIONAL_REVIEW, selectedOption, onClick: props.onSelectOption @@ -86,26 +97,61 @@ export default function HotspotActionsFormRenderer(props: HotspotActionsFormRend
{submitting && } - - {translate('hotspots.form.submit', hotspotStatus)} + + {translate('hotspots.form.submit', hotspot.status)}
); } +const noop = () => {}; + +function changes(params: { + comment: string; + hotspot: Hotspot; + selectedOption: HotspotStatusOption; + selectedUser?: T.UserActive; +}) { + const { comment, hotspot, selectedOption, selectedUser } = params; + + const status = + selectedOption === HotspotStatusOption.ADDITIONAL_REVIEW + ? HotspotStatus.TO_REVIEW + : HotspotStatus.REVIEWED; + + const resolution = + selectedOption !== HotspotStatusOption.ADDITIONAL_REVIEW + ? HotspotResolution[selectedOption] + : undefined; + + return ( + comment.length > 0 || + selectedUser || + status !== hotspot.status || + resolution !== hotspot.resolution + ); +} + function renderOption(params: { + disabled: boolean; option: HotspotStatusOption; onClick: (option: HotspotStatusOption) => void; selectedOption: HotspotStatusOption; }) { - const { onClick, option, selectedOption } = params; + const { disabled, onClick, option, selectedOption } = params; return (
- -

{translate('hotspots.status_option', option)}

+ +

+ {translate('hotspots.status_option', option)} +

-
+
{translate('hotspots.status_option', option, 'description')}
diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotAssigneeSelect.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotAssigneeSelect.tsx index e2d16a1edf8..45f272a4e6d 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotAssigneeSelect.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotAssigneeSelect.tsx @@ -25,7 +25,7 @@ import { isUserActive } from '../../../helpers/users'; import HotspotAssigneeSelectRenderer from './HotspotAssigneeSelectRenderer'; interface Props { - onSelect: (user: T.UserActive) => void; + onSelect: (user?: T.UserActive) => void; } interface State { @@ -58,6 +58,7 @@ export default class HotspotAssigneeSelect extends React.PureComponent { if (query.length < 2) { this.setState({ open: false, query }); + this.props.onSelect(undefined); return Promise.resolve([]); } diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsForm-test.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsForm-test.tsx index f5370b21e78..11fb8aa26ef 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsForm-test.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsForm-test.tsx @@ -19,8 +19,12 @@ */ import { shallow } from 'enzyme'; import * as React from 'react'; -import { waitAndUpdate } from 'sonar-ui-common/helpers/testUtils'; -import { assignSecurityHotspot, setSecurityHotspotStatus } from '../../../../api/security-hotspots'; +import { mockEvent, waitAndUpdate } from 'sonar-ui-common/helpers/testUtils'; +import { + assignSecurityHotspot, + commentSecurityHotspot, + setSecurityHotspotStatus +} from '../../../../api/security-hotspots'; import { mockHotspot } from '../../../../helpers/mocks/security-hotspots'; import { mockLoggedInUser } from '../../../../helpers/testMocks'; import { @@ -32,6 +36,7 @@ import HotspotActionsForm from '../HotspotActionsForm'; jest.mock('../../../../api/security-hotspots', () => ({ assignSecurityHotspot: jest.fn().mockResolvedValue(undefined), + commentSecurityHotspot: jest.fn().mockResolvedValue(undefined), setSecurityHotspotStatus: jest.fn().mockResolvedValue(undefined) })); @@ -52,41 +57,55 @@ it('should handle comment change', () => { expect(wrapper.state().comment).toBe('new comment'); }); -it('should handle submit', async () => { - const onSubmit = jest.fn(); - const wrapper = shallowRender({ onSubmit }); - wrapper.setState({ selectedOption: HotspotStatusOption.ADDITIONAL_REVIEW }); - await waitAndUpdate(wrapper); +describe('submit', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); - const preventDefault = jest.fn(); - const promise = wrapper.instance().handleSubmit({ preventDefault } as any); - expect(preventDefault).toBeCalled(); + it('should be handled for additional review', async () => { + const onSubmit = jest.fn(); + const wrapper = shallowRender({ onSubmit }); + wrapper.setState({ selectedOption: HotspotStatusOption.ADDITIONAL_REVIEW }); - expect(wrapper.state().submitting).toBe(true); - await promise; - expect(setSecurityHotspotStatus).toBeCalledWith('key', { - status: HotspotStatus.TO_REVIEW + const promise = wrapper.instance().handleSubmit(mockEvent()); + + expect(wrapper.state().submitting).toBe(true); + await promise; + expect(setSecurityHotspotStatus).toBeCalledWith('key', { + status: HotspotStatus.TO_REVIEW + }); + expect(onSubmit).toBeCalled(); }); - expect(onSubmit).toBeCalled(); - // SAFE - wrapper.setState({ comment: 'commentsafe', selectedOption: HotspotStatusOption.SAFE }); - await waitAndUpdate(wrapper); - await wrapper.instance().handleSubmit({ preventDefault } as any); - expect(setSecurityHotspotStatus).toBeCalledWith('key', { - comment: 'commentsafe', - status: HotspotStatus.REVIEWED, - resolution: HotspotResolution.SAFE + it('should be handled for SAFE', async () => { + const wrapper = shallowRender(); + wrapper.setState({ comment: 'commentsafe', selectedOption: HotspotStatusOption.SAFE }); + await wrapper.instance().handleSubmit(mockEvent()); + expect(setSecurityHotspotStatus).toBeCalledWith('key', { + status: HotspotStatus.REVIEWED, + resolution: HotspotResolution.SAFE + }); + expect(commentSecurityHotspot).toBeCalledWith('key', 'commentsafe'); }); - // FIXED - wrapper.setState({ comment: 'commentFixed', selectedOption: HotspotStatusOption.FIXED }); - await waitAndUpdate(wrapper); - await wrapper.instance().handleSubmit({ preventDefault } as any); - expect(setSecurityHotspotStatus).toBeCalledWith('key', { - comment: 'commentFixed', - status: HotspotStatus.REVIEWED, - resolution: HotspotResolution.FIXED + it('should be handled for FIXED', async () => { + const wrapper = shallowRender({ + hotspot: mockHotspot({ key: 'key', status: HotspotStatus.TO_REVIEW }) + }); + wrapper.setState({ comment: 'commentfixed', selectedOption: HotspotStatusOption.FIXED }); + await wrapper.instance().handleSubmit(mockEvent()); + expect(setSecurityHotspotStatus).toBeCalledWith('key', { + status: HotspotStatus.REVIEWED, + resolution: HotspotResolution.FIXED + }); + expect(commentSecurityHotspot).toBeCalledWith('key', 'commentfixed'); + }); + + it('should ignore no change', async () => { + const wrapper = shallowRender(); + wrapper.setState({ selectedOption: HotspotStatusOption.FIXED }); + await wrapper.instance().handleSubmit(mockEvent()); + expect(setSecurityHotspotStatus).not.toBeCalled(); }); }); @@ -110,9 +129,9 @@ it('should handle assignment', async () => { status: HotspotStatus.TO_REVIEW }); expect(assignSecurityHotspot).toBeCalledWith('key', { - assignee: 'userLogin', - comment: 'assignment comment' + assignee: 'userLogin' }); + expect(commentSecurityHotspot).toBeCalledWith('key', 'assignment comment'); expect(onSubmit).toBeCalled(); }); @@ -120,9 +139,11 @@ it('should handle submit failure', async () => { const onSubmit = jest.fn(); (setSecurityHotspotStatus as jest.Mock).mockRejectedValueOnce('failure'); const wrapper = shallowRender({ onSubmit }); + wrapper.setState({ selectedOption: HotspotStatusOption.ADDITIONAL_REVIEW }); const promise = wrapper.instance().handleSubmit({ preventDefault: jest.fn() } as any); expect(wrapper.state().submitting).toBe(true); - await promise.catch(() => {}); + await promise; + await waitAndUpdate(wrapper); expect(wrapper.state().submitting).toBe(false); expect(onSubmit).not.toBeCalled(); }); diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsFormRenderer-test.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsFormRenderer-test.tsx index ea8d4cd4cb6..77dc7596bd0 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsFormRenderer-test.tsx +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotActionsFormRenderer-test.tsx @@ -19,8 +19,14 @@ */ import { shallow } from 'enzyme'; import * as React from 'react'; +import { SubmitButton } from 'sonar-ui-common/components/controls/buttons'; +import { mockHotspot } from '../../../../helpers/mocks/security-hotspots'; import { mockLoggedInUser } from '../../../../helpers/testMocks'; -import { HotspotStatus, HotspotStatusOption } from '../../../../types/security-hotspots'; +import { + HotspotResolution, + HotspotStatus, + HotspotStatusOption +} from '../../../../types/security-hotspots'; import HotspotActionsForm from '../HotspotActionsForm'; import HotspotActionsFormRenderer, { HotspotActionsFormRendererProps @@ -38,13 +44,59 @@ it('should render correctly', () => { selectedUser: mockLoggedInUser() }) ).toMatchSnapshot('user selected'); + expect(shallowRender({ hotspot: mockHotspot({ canChangeStatus: false }) })).toMatchSnapshot( + 'restricted access' + ); +}); + +it('should enable the submit button if anything has changed', () => { + const hotspot = mockHotspot({ + status: HotspotStatus.REVIEWED, + resolution: HotspotResolution.SAFE + }); + const selectedOption = HotspotStatusOption.SAFE; + expect( + shallowRender({ comment: '', hotspot, selectedOption, selectedUser: undefined }) + .find(SubmitButton) + .props().disabled + ).toBe(true); + expect( + shallowRender({ comment: 'some comment', hotspot, selectedOption, selectedUser: undefined }) + .find(SubmitButton) + .props().disabled + ).toBe(false); + expect( + shallowRender({ comment: '', hotspot, selectedOption, selectedUser: mockLoggedInUser() }) + .find(SubmitButton) + .props().disabled + ).toBe(false); + expect( + shallowRender({ + comment: '', + hotspot, + selectedOption: HotspotStatusOption.FIXED, + selectedUser: undefined + }) + .find(SubmitButton) + .props().disabled + ).toBe(false); + expect( + shallowRender({ + comment: '', + hotspot, + selectedOption: HotspotStatusOption.ADDITIONAL_REVIEW, + selectedUser: undefined + }) + .find(SubmitButton) + .props().disabled + ).toBe(false); }); function shallowRender(props: Partial = {}) { return shallow( { const users = [mockUser({ login: '1' }), mockUser({ login: '2' }), mockUser({ login: '3' })]; (searchUsers as jest.Mock).mockResolvedValueOnce({ users }); - const wrapper = shallowRender(); + const onSelect = jest.fn(); + + const wrapper = shallowRender({ onSelect }); wrapper.instance().handleSearch('j'); expect(searchUsers).not.toBeCalled(); @@ -90,6 +92,12 @@ it('should handle search', async () => { expect(wrapper.state().loading).toBe(false); expect(wrapper.state().open).toBe(true); expect(wrapper.state().suggestedUsers).toHaveLength(3); + + jest.clearAllMocks(); + + await wrapper.instance().handleSearch(''); + expect(searchUsers).not.toBeCalled(); + expect(onSelect).toBeCalledWith(undefined); }); function shallowRender(props?: Partial) { diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActions-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActions-test.tsx.snap index 657718ecd2c..e0d10a202a4 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActions-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActions-test.tsx.snap @@ -35,6 +35,7 @@ exports[`should open when clicked 1`] = ` "login": "author", "name": "John Doe", }, + "canChangeStatus": true, "changelog": Array [], "comment": Array [], "component": Object { @@ -174,6 +175,7 @@ exports[`should register an eventlistener: Dropdown open 1`] = ` "login": "author", "name": "John Doe", }, + "canChangeStatus": true, "changelog": Array [], "comment": Array [], "component": Object { @@ -298,6 +300,7 @@ exports[`should register an eventlistener: Dropdown still open 1`] = ` "login": "author", "name": "John Doe", }, + "canChangeStatus": true, "changelog": Array [], "comment": Array [], "component": Object { diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsForm-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsForm-test.tsx.snap index 721aace521d..9ad46c82552 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsForm-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsForm-test.tsx.snap @@ -3,7 +3,105 @@ exports[`should render correctly 1`] = ` This a strong message about fixing !

", + "key": "squid:S2077", + "name": "That rule", + "riskDescription": "

This a strong message about risk !

", + "securityCategory": "sql-injection", + "vulnerabilityDescription": "

This a strong message about vulnerability !

", + "vulnerabilityProbability": "HIGH", + }, + "status": "REVIEWED", + "textRange": Object { + "endLine": 142, + "endOffset": 83, + "startLine": 142, + "startOffset": 26, + }, + "updateDate": "2013-05-13T17:55:42+0200", + "users": Array [ + Object { + "active": true, + "local": true, + "login": "assignee", + "name": "John Doe", + }, + Object { + "active": true, + "local": true, + "login": "author", + "name": "John Doe", + }, + ], + } + } onAssign={[Function]} onChangeComment={[Function]} onSelectOption={[Function]} diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsFormRenderer-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsFormRenderer-test.tsx.snap index 9264fff96e1..25f5ec0e7a5 100644 --- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsFormRenderer-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotActionsFormRenderer-test.tsx.snap @@ -16,10 +16,13 @@ exports[`should render correctly 1`] = ` > -

+

hotspots.status_option.FIXED

@@ -34,10 +37,13 @@ exports[`should render correctly 1`] = ` > -

+

hotspots.status_option.SAFE

@@ -52,10 +58,13 @@ exports[`should render correctly 1`] = ` > -

+

hotspots.status_option.ADDITIONAL_REVIEW

@@ -90,7 +99,7 @@ exports[`should render correctly 1`] = ` - hotspots.form.submit.TO_REVIEW + hotspots.form.submit.REVIEWED
@@ -112,10 +121,13 @@ exports[`should render correctly: Submitting 1`] = ` > -

+

hotspots.status_option.FIXED

@@ -130,10 +142,13 @@ exports[`should render correctly: Submitting 1`] = ` > -

+

hotspots.status_option.SAFE

@@ -148,10 +163,13 @@ exports[`should render correctly: Submitting 1`] = ` > -

+

hotspots.status_option.ADDITIONAL_REVIEW

@@ -189,7 +207,112 @@ exports[`should render correctly: Submitting 1`] = ` - hotspots.form.submit.TO_REVIEW + hotspots.form.submit.REVIEWED + + + +`; + +exports[`should render correctly: restricted access 1`] = ` +
+

+ hotspots.form.title +

+
+
+ +

+ hotspots.status_option.FIXED +

+ +
+ hotspots.status_option.FIXED.description +
+
+
+ +

+ hotspots.status_option.SAFE +

+ +
+ hotspots.status_option.SAFE.description +
+
+
+ +

+ hotspots.status_option.ADDITIONAL_REVIEW +

+ +
+ hotspots.status_option.ADDITIONAL_REVIEW.description +
+
+
+
+ +