From 091cb24d9d65820ccb308d5edb2096028264258c Mon Sep 17 00:00:00 2001
From: Mathieu Suen <mathieu.suen@sonarsource.com>
Date: Wed, 5 Feb 2020 16:52:59 +0100
Subject: [PATCH] SONAR-12718: Always select first tab when switching hotspot.

---
 .../components/HotspotViewerTabs.tsx          | 162 +++++++++++-------
 .../__tests__/HotspotViewerTabs-test.tsx      |  75 ++++----
 .../HotspotViewerTabs-test.tsx.snap           | 159 -----------------
 3 files changed, 140 insertions(+), 256 deletions(-)

diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotViewerTabs.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotViewerTabs.tsx
index 4f04d247d9a..961c8829c64 100644
--- a/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotViewerTabs.tsx
+++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/HotspotViewerTabs.tsx
@@ -20,89 +20,123 @@
 import { sanitize } from 'dompurify';
 import * as React from 'react';
 import BoxedTabs from 'sonar-ui-common/components/controls/BoxedTabs';
+import Tab from 'sonar-ui-common/components/controls/Tabs';
 import { translate } from 'sonar-ui-common/helpers/l10n';
 import { Hotspot } from '../../../types/security-hotspots';
 import { getHotspotReviewHistory } from '../utils';
 import HotspotViewerReviewHistoryTab from './HotspotViewerReviewHistoryTab';
 
-export interface HotspotViewerTabsProps {
+interface Props {
   hotspot: Hotspot;
   onUpdateHotspot: () => void;
 }
 
-export enum Tabs {
+interface State {
+  currentTab: Tab;
+  tabs: Tab[];
+}
+
+interface Tab {
+  key: TabKeys;
+  label: React.ReactNode;
+  content: React.ReactNode;
+}
+
+export enum TabKeys {
   RiskDescription = 'risk',
   VulnerabilityDescription = 'vulnerability',
   FixRecommendation = 'fix',
   ReviewHistory = 'review'
 }
 
-export default function HotspotViewerTabs(props: HotspotViewerTabsProps) {
-  const { hotspot } = props;
-  const [currentTabKey, setCurrentTabKey] = React.useState(Tabs.RiskDescription);
-  const hotspotReviewHistory = React.useMemo(() => getHotspotReviewHistory(hotspot), [hotspot]);
+export default class HotspotViewerTabs extends React.PureComponent<Props, State> {
+  constructor(props: Props) {
+    super(props);
+    const tabs = this.computeTabs();
+    this.state = {
+      currentTab: tabs[0],
+      tabs
+    };
+  }
 
-  const tabs = [
-    {
-      key: Tabs.RiskDescription,
-      label: translate('hotspots.tabs.risk_description'),
-      content: hotspot.rule.riskDescription || ''
-    },
-    {
-      key: Tabs.VulnerabilityDescription,
-      label: translate('hotspots.tabs.vulnerability_description'),
-      content: hotspot.rule.vulnerabilityDescription || ''
-    },
-    {
-      key: Tabs.FixRecommendation,
-      label: translate('hotspots.tabs.fix_recommendations'),
-      content: hotspot.rule.fixRecommendations || ''
-    },
-    {
-      key: Tabs.ReviewHistory,
-      label: (
-        <>
-          <span>{translate('hotspots.tabs.review_history')}</span>
-          {hotspotReviewHistory.functionalCount > 0 && (
-            <span className="counter-badge spacer-left">
-              {hotspotReviewHistory.functionalCount}
-            </span>
-          )}
-        </>
-      ),
-      content: hotspotReviewHistory.history.length > 0 && (
-        <HotspotViewerReviewHistoryTab
-          history={hotspotReviewHistory.history}
-          hotspot={hotspot}
-          onUpdateHotspot={props.onUpdateHotspot}
-        />
-      )
+  componentDidUpdate(prevProps: Props) {
+    if (this.props.hotspot.key !== prevProps.hotspot.key) {
+      const tabs = this.computeTabs();
+      this.setState({
+        currentTab: tabs[0],
+        tabs
+      });
     }
-  ].filter(tab => Boolean(tab.content));
-
-  if (tabs.length === 0) {
-    return null;
   }
 
-  const currentTab = tabs.find(tab => tab.key === currentTabKey) || tabs[0];
+  handleSelectTabs = (tabKey: TabKeys) => {
+    const { tabs } = this.state;
+    const currentTab = tabs.find(tab => tab.key === tabKey)!;
+    this.setState({ currentTab });
+  };
 
-  return (
-    <>
-      <BoxedTabs
-        onSelect={tabKey => setCurrentTabKey(tabKey)}
-        selected={currentTabKey}
-        tabs={tabs}
-      />
-      <div className="bordered huge-spacer-bottom">
-        {typeof currentTab.content === 'string' ? (
-          <div
-            className="markdown big-padded"
-            dangerouslySetInnerHTML={{ __html: sanitize(currentTab.content) }}
+  computeTabs() {
+    const { hotspot } = this.props;
+    const hotspotReviewHistory = getHotspotReviewHistory(hotspot);
+    return [
+      {
+        key: TabKeys.RiskDescription,
+        label: translate('hotspots.tabs.risk_description'),
+        content: hotspot.rule.riskDescription || ''
+      },
+      {
+        key: TabKeys.VulnerabilityDescription,
+        label: translate('hotspots.tabs.vulnerability_description'),
+        content: hotspot.rule.vulnerabilityDescription || ''
+      },
+      {
+        key: TabKeys.FixRecommendation,
+        label: translate('hotspots.tabs.fix_recommendations'),
+        content: hotspot.rule.fixRecommendations || ''
+      },
+      {
+        key: TabKeys.ReviewHistory,
+        label: (
+          <>
+            <span>{translate('hotspots.tabs.review_history')}</span>
+            {hotspotReviewHistory.functionalCount > 0 && (
+              <span className="counter-badge spacer-left">
+                {hotspotReviewHistory.functionalCount}
+              </span>
+            )}
+          </>
+        ),
+        content: hotspotReviewHistory.history.length > 0 && (
+          <HotspotViewerReviewHistoryTab
+            history={hotspotReviewHistory.history}
+            hotspot={hotspot}
+            onUpdateHotspot={this.props.onUpdateHotspot}
           />
-        ) : (
-          <>{currentTab.content}</>
-        )}
-      </div>
-    </>
-  );
+        )
+      }
+    ].filter(tab => Boolean(tab.content));
+  }
+
+  render() {
+    const { tabs, currentTab } = this.state;
+    if (tabs.length === 0) {
+      return null;
+    }
+
+    return (
+      <>
+        <BoxedTabs onSelect={this.handleSelectTabs} selected={currentTab.key} tabs={tabs} />
+        <div className="bordered huge-spacer-bottom">
+          {typeof currentTab.content === 'string' ? (
+            <div
+              className="markdown big-padded"
+              dangerouslySetInnerHTML={{ __html: sanitize(currentTab.content) }}
+            />
+          ) : (
+            <>{currentTab.content}</>
+          )}
+        </div>
+      </>
+    );
+  }
 }
diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotViewerTabs-test.tsx b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotViewerTabs-test.tsx
index 57a5bf08252..24a09807b6e 100644
--- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotViewerTabs-test.tsx
+++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/HotspotViewerTabs-test.tsx
@@ -23,34 +23,22 @@ import BoxedTabs from 'sonar-ui-common/components/controls/BoxedTabs';
 import { mockHotspot, mockHotspotRule } from '../../../../helpers/mocks/security-hotspots';
 import { mockUser } from '../../../../helpers/testMocks';
 import HotspotViewerReviewHistoryTab from '../HotspotViewerReviewHistoryTab';
-import HotspotViewerTabs, { HotspotViewerTabsProps, Tabs } from '../HotspotViewerTabs';
+import HotspotViewerTabs, { TabKeys } from '../HotspotViewerTabs';
 
 it('should render correctly', () => {
   const wrapper = shallowRender();
   expect(wrapper).toMatchSnapshot('risk');
 
-  const onSelect = wrapper.find(BoxedTabs).prop('onSelect') as (tab: Tabs) => void;
+  const onSelect = wrapper.find(BoxedTabs).prop('onSelect') as (tab: TabKeys) => void;
 
-  if (!onSelect) {
-    fail('onSelect should be defined');
-  } else {
-    onSelect(Tabs.VulnerabilityDescription);
-    expect(wrapper).toMatchSnapshot('vulnerability');
+  onSelect(TabKeys.VulnerabilityDescription);
+  expect(wrapper).toMatchSnapshot('vulnerability');
 
-    onSelect(Tabs.FixRecommendation);
-    expect(wrapper).toMatchSnapshot('fix');
+  onSelect(TabKeys.FixRecommendation);
+  expect(wrapper).toMatchSnapshot('fix');
 
-    onSelect(Tabs.ReviewHistory);
-    expect(wrapper).toMatchSnapshot('review');
-  }
-
-  expect(
-    shallowRender({
-      hotspot: mockHotspot({
-        rule: mockHotspotRule({ riskDescription: undefined })
-      })
-    })
-  ).toMatchSnapshot('empty tab');
+  onSelect(TabKeys.ReviewHistory);
+  expect(wrapper).toMatchSnapshot('review');
 
   expect(
     shallowRender({
@@ -84,26 +72,47 @@ it('should render correctly', () => {
   ).toMatchSnapshot('with comments or changelog element');
 });
 
+it('should filter empty tab', () => {
+  const count = shallowRender({
+    hotspot: mockHotspot({
+      rule: mockHotspotRule()
+    })
+  }).state().tabs.length;
+
+  expect(
+    shallowRender({
+      hotspot: mockHotspot({
+        rule: mockHotspotRule({ riskDescription: undefined })
+      })
+    }).state().tabs.length
+  ).toBe(count - 1);
+});
+
 it('should propagate onHotspotUpdate correctly', () => {
   const onUpdateHotspot = jest.fn();
   const wrapper = shallowRender({ onUpdateHotspot });
+  const onSelect = wrapper.find(BoxedTabs).prop('onSelect') as (tab: TabKeys) => void;
+
+  onSelect(TabKeys.ReviewHistory);
+  wrapper
+    .find(HotspotViewerReviewHistoryTab)
+    .props()
+    .onUpdateHotspot();
+  expect(onUpdateHotspot).toHaveBeenCalled();
+});
 
-  const onSelect = wrapper.find(BoxedTabs).prop('onSelect') as (tab: Tabs) => void;
+it('should select first tab on hotspot update', () => {
+  const wrapper = shallowRender();
+  const onSelect = wrapper.find(BoxedTabs).prop('onSelect') as (tab: TabKeys) => void;
 
-  if (!onSelect) {
-    fail('onSelect should be defined');
-  } else {
-    onSelect(Tabs.ReviewHistory);
-    wrapper
-      .find(HotspotViewerReviewHistoryTab)
-      .props()
-      .onUpdateHotspot();
-    expect(onUpdateHotspot).toHaveBeenCalled();
-  }
+  onSelect(TabKeys.ReviewHistory);
+  expect(wrapper.state().currentTab.key).toBe(TabKeys.ReviewHistory);
+  wrapper.setProps({ hotspot: mockHotspot({ key: 'new_key' }) });
+  expect(wrapper.state().currentTab.key).toBe(TabKeys.RiskDescription);
 });
 
-function shallowRender(props?: Partial<HotspotViewerTabsProps>) {
-  return shallow(
+function shallowRender(props?: Partial<HotspotViewerTabs['props']>) {
+  return shallow<HotspotViewerTabs>(
     <HotspotViewerTabs hotspot={mockHotspot()} onUpdateHotspot={jest.fn()} {...props} />
   );
 }
diff --git a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap
index dc66125bd8e..76d3667a4ff 100644
--- a/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap
+++ b/server/sonar-web/src/main/js/apps/securityHotspots/components/__tests__/__snapshots__/HotspotViewerTabs-test.tsx.snap
@@ -1,164 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`should render correctly: empty tab 1`] = `
-<Fragment>
-  <BoxedTabs
-    onSelect={[Function]}
-    selected="risk"
-    tabs={
-      Array [
-        Object {
-          "content": "<p>This a <strong>strong</strong> message about vulnerability !</p>",
-          "key": "vulnerability",
-          "label": "hotspots.tabs.vulnerability_description",
-        },
-        Object {
-          "content": "<p>This a <strong>strong</strong> message about fixing !</p>",
-          "key": "fix",
-          "label": "hotspots.tabs.fix_recommendations",
-        },
-        Object {
-          "content": <HotspotViewerReviewHistoryTab
-            history={
-              Array [
-                Object {
-                  "date": "2013-05-13T17:55:41+0200",
-                  "type": 0,
-                  "user": Object {
-                    "active": true,
-                    "local": true,
-                    "login": "author",
-                    "name": "John Doe",
-                  },
-                },
-              ]
-            }
-            hotspot={
-              Object {
-                "assignee": "assignee",
-                "assigneeUser": Object {
-                  "active": true,
-                  "local": true,
-                  "login": "assignee",
-                  "name": "John Doe",
-                },
-                "author": "author",
-                "authorUser": Object {
-                  "active": true,
-                  "local": true,
-                  "login": "author",
-                  "name": "John Doe",
-                },
-                "canChangeStatus": true,
-                "changelog": Array [],
-                "comment": Array [],
-                "component": Object {
-                  "breadcrumbs": Array [],
-                  "key": "my-project",
-                  "name": "MyProject",
-                  "organization": "foo",
-                  "qualifier": "FIL",
-                  "qualityGate": Object {
-                    "isDefault": true,
-                    "key": "30",
-                    "name": "Sonar way",
-                  },
-                  "qualityProfiles": Array [
-                    Object {
-                      "deleted": false,
-                      "key": "my-qp",
-                      "language": "ts",
-                      "name": "Sonar way",
-                    },
-                  ],
-                  "tags": Array [],
-                },
-                "creationDate": "2013-05-13T17:55:41+0200",
-                "key": "01fc972e-2a3c-433e-bcae-0bd7f88f5123",
-                "line": 142,
-                "message": "'3' is a magic number.",
-                "project": Object {
-                  "breadcrumbs": Array [],
-                  "key": "my-project",
-                  "name": "MyProject",
-                  "organization": "foo",
-                  "qualifier": "TRK",
-                  "qualityGate": Object {
-                    "isDefault": true,
-                    "key": "30",
-                    "name": "Sonar way",
-                  },
-                  "qualityProfiles": Array [
-                    Object {
-                      "deleted": false,
-                      "key": "my-qp",
-                      "language": "ts",
-                      "name": "Sonar way",
-                    },
-                  ],
-                  "tags": Array [],
-                },
-                "resolution": "FIXED",
-                "rule": Object {
-                  "fixRecommendations": "<p>This a <strong>strong</strong> message about fixing !</p>",
-                  "key": "squid:S2077",
-                  "name": "That rule",
-                  "riskDescription": undefined,
-                  "securityCategory": "sql-injection",
-                  "vulnerabilityDescription": "<p>This a <strong>strong</strong> message about vulnerability !</p>",
-                  "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",
-                  },
-                ],
-              }
-            }
-            onUpdateHotspot={[MockFunction]}
-          />,
-          "key": "review",
-          "label": <React.Fragment>
-            <span>
-              hotspots.tabs.review_history
-            </span>
-          </React.Fragment>,
-        },
-      ]
-    }
-  />
-  <div
-    className="bordered huge-spacer-bottom"
-  >
-    <div
-      className="markdown big-padded"
-      dangerouslySetInnerHTML={
-        Object {
-          "__html": "<p>This a <strong>strong</strong> message about vulnerability !</p>",
-        }
-      }
-    />
-  </div>
-</Fragment>
-`;
-
 exports[`should render correctly: fix 1`] = `
 <Fragment>
   <BoxedTabs
-- 
2.39.5