From fb1c76910b51357ef255b637daeb6223e5bc7578 Mon Sep 17 00:00:00 2001 From: Philippe Perrin Date: Wed, 11 May 2022 16:52:26 +0200 Subject: [PATCH] SONAR-16303 Display rule description header and tabs for issues --- .../issues/components/IssueRuleHeader.tsx | 47 +++++++ .../apps/issues/components/IssueTabViewer.tsx | 122 ++++++++++++++++++ .../js/apps/issues/components/IssuesApp.tsx | 116 +++++++++++------ .../components/__tests__/IssuesApp-test.tsx | 21 ++- .../components/HotspotViewerTabs.tsx | 2 +- .../HotspotViewerTabs-test.tsx.snap | 20 +-- server/sonar-web/src/main/js/types/types.ts | 3 +- .../resources/org/sonar/l10n/core.properties | 5 + 8 files changed, 283 insertions(+), 53 deletions(-) create mode 100644 server/sonar-web/src/main/js/apps/issues/components/IssueRuleHeader.tsx create mode 100644 server/sonar-web/src/main/js/apps/issues/components/IssueTabViewer.tsx diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssueRuleHeader.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssueRuleHeader.tsx new file mode 100644 index 00000000000..74584d257d8 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/components/IssueRuleHeader.tsx @@ -0,0 +1,47 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import * as React from 'react'; +import { Link } from 'react-router'; +import { getRuleUrl } from '../../../helpers/urls'; +import { Issue, RuleDetails } from '../../../types/types'; + +interface IssueRuleHeaderProps { + ruleDetails: RuleDetails; + issue: Issue; +} + +export default function IssueRuleHeader(props: IssueRuleHeaderProps) { + const { + ruleDetails: { name, key }, + issue: { message } + } = props; + + return ( + <> +

{message}

+
+ {name} + + {key} + +
+ + ); +} diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssueTabViewer.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssueTabViewer.tsx new file mode 100644 index 00000000000..f0db6bbddbb --- /dev/null +++ b/server/sonar-web/src/main/js/apps/issues/components/IssueTabViewer.tsx @@ -0,0 +1,122 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import classNames from 'classnames'; +import * as React from 'react'; +import BoxedTabs from '../../../components/controls/BoxedTabs'; +import { translate } from '../../../helpers/l10n'; +import { sanitizeString } from '../../../helpers/sanitize'; +import { RuleDescriptionSections, RuleDetails } from '../../../types/types'; + +interface Props { + codeTabContent: React.ReactNode; + ruleDetails: RuleDetails; +} + +interface State { + currentTabKey: TabKeys; + tabs: Tab[]; +} + +interface Tab { + key: TabKeys; + label: React.ReactNode; + content: string; +} + +export enum TabKeys { + Code = 'code', + WhyIsThisAnIssue = 'why', + HowToFixIt = 'how', + Resources = 'resources' +} + +export default class IssueViewerTabs extends React.PureComponent { + constructor(props: Props) { + super(props); + const tabs = this.computeTabs(); + this.state = { + currentTabKey: tabs[0].key, + tabs + }; + } + + componentDidUpdate(prevProps: Props) { + if (prevProps.ruleDetails !== this.props.ruleDetails) { + const tabs = this.computeTabs(); + this.setState({ + currentTabKey: tabs[0].key, + tabs + }); + } + } + + handleSelectTabs = (currentTabKey: TabKeys) => { + this.setState({ currentTabKey }); + }; + + computeTabs() { + const { ruleDetails } = this.props; + + return [ + { + key: TabKeys.Code, + label: translate('issue.tabs.code'), + content: '' + }, + { + key: TabKeys.WhyIsThisAnIssue, + label: translate('issue.tabs.why'), + content: + ruleDetails.descriptionSections?.find( + section => section.key === RuleDescriptionSections.DEFAULT + )?.content ?? '' + } + ]; + } + + render() { + const { codeTabContent } = this.props; + const { tabs, currentTabKey } = this.state; + + return ( + <> + +
+
+ {codeTabContent} +
+ {tabs.slice(1).map(tab => ( +
+ ))} +
+ + ); + } +} diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx index cf030276b8a..e3c5384f700 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/IssuesApp.tsx @@ -23,6 +23,7 @@ import * as React from 'react'; import { Helmet } from 'react-helmet-async'; import { FormattedMessage } from 'react-intl'; import { searchIssues } from '../../../api/issues'; +import { getRuleDetails } from '../../../api/rules'; import A11ySkipTarget from '../../../components/a11y/A11ySkipTarget'; import EmptySearch from '../../../components/common/EmptySearch'; import FiltersHeader from '../../../components/common/FiltersHeader'; @@ -63,7 +64,7 @@ import { ReferencedRule } from '../../../types/issues'; import { SecurityStandard } from '../../../types/security'; -import { Component, Dict, Issue, Paging, RawQuery } from '../../../types/types'; +import { Component, Dict, Issue, Paging, RawQuery, RuleDetails } from '../../../types/types'; import { CurrentUser, UserBase } from '../../../types/users'; import * as actions from '../actions'; import ConciseIssuesList from '../conciseIssuesList/ConciseIssuesList'; @@ -87,8 +88,10 @@ import { STANDARDS } from '../utils'; import BulkChangeModal, { MAX_PAGE_SIZE } from './BulkChangeModal'; +import IssueRuleHeader from './IssueRuleHeader'; import IssuesList from './IssuesList'; import IssuesSourceViewer from './IssuesSourceViewer'; +import IssueTabViewer from './IssueTabViewer'; import MyIssuesFilter from './MyIssuesFilter'; import NoIssues from './NoIssues'; import NoMyIssues from './NoMyIssues'; @@ -112,6 +115,7 @@ export interface State { facets: Dict; issues: Issue[]; loading: boolean; + loadingRule: boolean; loadingFacets: Dict; loadingMore: boolean; locationsNavigator: boolean; @@ -119,6 +123,7 @@ export interface State { openFacets: Dict; openIssue?: Issue; openPopup?: { issue: string; name: string }; + openRuleDetails?: RuleDetails; paging?: Paging; query: Query; referencedComponentsById: Dict; @@ -148,6 +153,7 @@ export default class App extends React.PureComponent { issues: [], loading: true, loadingFacets: {}, + loadingRule: false, loadingMore: false, locationsNavigator: false, myIssues: areMyIssuesSelected(props.location.query), @@ -229,6 +235,9 @@ export default class App extends React.PureComponent { selectedLocationIndex: undefined }); } + if (this.state.openIssue && this.state.openIssue.key !== prevState.openIssue?.key) { + this.loadRule(); + } } componentWillUnmount() { @@ -328,6 +337,20 @@ export default class App extends React.PureComponent { } }; + async loadRule() { + const { openIssue } = this.state; + if (openIssue === undefined) { + return; + } + this.setState({ loadingRule: true }); + const openRuleDetails = await getRuleDetails({ key: openIssue.rule }) + .then(response => response.rule) + .catch(() => undefined); + if (this.mounted) { + this.setState({ loadingRule: false, openRuleDetails }); + } + } + selectPreviousIssue = () => { const { issues } = this.state; const selectedIndex = this.getSelectedIndex(); @@ -1086,44 +1109,63 @@ export default class App extends React.PureComponent { } renderPage() { - const { cannotShowOpenIssue, checkAll, issues, loading, openIssue, paging } = this.state; + const { + cannotShowOpenIssue, + openRuleDetails, + checkAll, + issues, + loading, + openIssue, + paging, + loadingRule + } = this.state; return (
- {openIssue ? ( - - ) : ( - - {checkAll && paging && paging.total > MAX_PAGE_SIZE && ( - - {MAX_PAGE_SIZE} }} - /> - - )} - {cannotShowOpenIssue && (!paging || paging.total > 0) && ( - - {translateWithParameters( - 'issues.cannot_open_issue_max_initial_X_fetched', - MAX_INITAL_FETCH - )} - - )} - {this.renderList()} - - )} + + {openIssue && openRuleDetails ? ( + <> + + + } + ruleDetails={openRuleDetails} + /> + + ) : ( + + {checkAll && paging && paging.total > MAX_PAGE_SIZE && ( + + {MAX_PAGE_SIZE} }} + /> + + )} + {cannotShowOpenIssue && (!paging || paging.total > 0) && ( + + {translateWithParameters( + 'issues.cannot_open_issue_max_initial_X_fetched', + MAX_INITAL_FETCH + )} + + )} + {this.renderList()} + + )} +
); } diff --git a/server/sonar-web/src/main/js/apps/issues/components/__tests__/IssuesApp-test.tsx b/server/sonar-web/src/main/js/apps/issues/components/__tests__/IssuesApp-test.tsx index 69bd17793da..b1f3ebc1418 100644 --- a/server/sonar-web/src/main/js/apps/issues/components/__tests__/IssuesApp-test.tsx +++ b/server/sonar-web/src/main/js/apps/issues/components/__tests__/IssuesApp-test.tsx @@ -20,6 +20,7 @@ import { shallow } from 'enzyme'; import * as React from 'react'; import { searchIssues } from '../../../../api/issues'; +import { getRuleDetails } from '../../../../api/rules'; import handleRequiredAuthentication from '../../../../helpers/handleRequiredAuthentication'; import { KeyboardCodes, KeyboardKeys } from '../../../../helpers/keycodes'; import { mockPullRequest } from '../../../../helpers/mocks/branch-like'; @@ -54,6 +55,7 @@ import { import BulkChangeModal from '../BulkChangeModal'; import App from '../IssuesApp'; import IssuesSourceViewer from '../IssuesSourceViewer'; +import IssueViewerTabs from '../IssueTabViewer'; jest.mock('../../../../helpers/pages', () => ({ addSideBarClass: jest.fn(), @@ -68,6 +70,10 @@ jest.mock('../../../../api/issues', () => ({ searchIssues: jest.fn().mockResolvedValue({ facets: [], issues: [] }) })); +jest.mock('../../../../api/rules', () => ({ + getRuleDetails: jest.fn() +})); + const RAW_ISSUES = [ mockRawIssue(false, { key: 'foo' }), mockRawIssue(false, { key: 'bar' }), @@ -96,10 +102,12 @@ beforeEach(() => { rules: [], users: [] }); + + (getRuleDetails as jest.Mock).mockResolvedValue({ rule: { test: 'test' } }); }); afterEach(() => { - jest.clearAllMocks(); + // jest.clearAllMocks(); (searchIssues as jest.Mock).mockReset(); }); @@ -195,11 +203,17 @@ it('should open standard facets for vulnerabilities and hotspots', () => { it('should switch to source view if an issue is selected', async () => { const wrapper = shallowRender(); await waitAndUpdate(wrapper); - expect(wrapper.find(IssuesSourceViewer).exists()).toBe(false); + expect(wrapper.find(IssueViewerTabs).exists()).toBe(false); wrapper.setProps({ location: mockLocation({ query: { open: 'third' } }) }); await waitAndUpdate(wrapper); - expect(wrapper.find(IssuesSourceViewer).exists()).toBe(true); + expect( + wrapper + .find(IssueViewerTabs) + .dive() + .find(IssuesSourceViewer) + .exists() + ).toBe(true); }); it('should correctly bind key events for issue navigation', async () => { @@ -505,6 +519,7 @@ it('should update the open issue when it is changed', async () => { await waitAndUpdate(wrapper); const issue = wrapper.state().issues[0]; + wrapper.setProps({ location: mockLocation({ query: { open: issue.key } }) }); await waitAndUpdate(wrapper); diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotViewerTabs.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotViewerTabs.tsx index 4d335618427..4adab7cdcb5 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotViewerTabs.tsx +++ b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotViewerTabs.tsx @@ -176,7 +176,7 @@ export default class HotspotViewerTabs extends React.PureComponent hidden: currentTab.key !== tab.key })} // eslint-disable-next-line react/no-danger - dangerouslySetInnerHTML={{ __html: sanitizeString(currentTab.content) }} + dangerouslySetInnerHTML={{ __html: sanitizeString(tab.content) }} /> ))}
diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap b/server/sonar-web/src/main/js/apps/security-hotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap index ae95c4feab2..8fe81cb3664 100644 --- a/server/sonar-web/src/main/js/apps/security-hotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap +++ b/server/sonar-web/src/main/js/apps/security-hotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap @@ -44,7 +44,7 @@ exports[`should render correctly: fix 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "

This a strong message about fixing !

", + "__html": "

This a strong message about risk !

", } } key="risk" @@ -53,7 +53,7 @@ exports[`should render correctly: fix 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "

This a strong message about fixing !

", + "__html": "

This a strong message about vulnerability !

", } } key="vulnerability" @@ -115,7 +115,7 @@ exports[`should render correctly: risk 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about risk !

", } } key="risk" @@ -124,7 +124,7 @@ exports[`should render correctly: risk 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about vulnerability !

", } } key="vulnerability" @@ -133,7 +133,7 @@ exports[`should render correctly: risk 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about fixing !

", } } key="fix" @@ -186,7 +186,7 @@ exports[`should render correctly: vulnerability 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "

This a strong message about vulnerability !

", + "__html": "

This a strong message about risk !

", } } key="risk" @@ -204,7 +204,7 @@ exports[`should render correctly: vulnerability 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "

This a strong message about vulnerability !

", + "__html": "

This a strong message about fixing !

", } } key="fix" @@ -257,7 +257,7 @@ exports[`should render correctly: with comments or changelog element 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about risk !

", } } key="risk" @@ -266,7 +266,7 @@ exports[`should render correctly: with comments or changelog element 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about vulnerability !

", } } key="vulnerability" @@ -275,7 +275,7 @@ exports[`should render correctly: with comments or changelog element 1`] = ` className="markdown big-padded hidden" dangerouslySetInnerHTML={ Object { - "__html": "", + "__html": "

This a strong message about fixing !

", } } key="fix" diff --git a/server/sonar-web/src/main/js/types/types.ts b/server/sonar-web/src/main/js/types/types.ts index 80a7c7eaa28..890237e2040 100644 --- a/server/sonar-web/src/main/js/types/types.ts +++ b/server/sonar-web/src/main/js/types/types.ts @@ -573,7 +573,6 @@ export enum RuleDescriptionSections { HOW_TO_FIX = 'how_to_fix', RESOURCES = 'resources' } - export interface RuleDescriptionSection { key: RuleDescriptionSections; content: string; @@ -603,8 +602,8 @@ export interface RuleDetails extends Rule { defaultDebtRemFnType?: string; defaultRemFnBaseEffort?: string; defaultRemFnType?: string; - effortToFixDescription?: string; descriptionSections?: RuleDescriptionSection[]; + effortToFixDescription?: string; htmlDesc?: string; htmlNote?: string; internalKey?: string; diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties index 2c67289ef8b..6adedd92aae 100644 --- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties +++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties @@ -851,6 +851,11 @@ issue.transition.resolveasreviewed=Resolve as Reviewed issue.transition.resolveasreviewed.description=There is no Vulnerability in the code issue.transition.resetastoreview=Reset as To Review issue.transition.resetastoreview.description=The Security Hotspot should be analyzed again +issue.tabs.code=Where is the issue? +issue.tabs.why=Why is this an issue? +issue.tabs.how=How to fix it? +issue.tabs.resources=Resources + vulnerability.transition.resetastoreview=Reset as To Review vulnerability.transition.resetastoreview.description=The vulnerability can't be fixed as is and needs more details. The security hotspot needs to be reviewed again vulnerability.transition.resolveasreviewed=Resolve as Reviewed -- 2.39.5