From 3d98d98d3c1296f85cacdd06b97ce8bd8686591f Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Thu, 8 Aug 2019 11:53:47 -0400
Subject: [PATCH] Moves tooltip and link logic out to the sparkline from the
 job status icon

---
 .../components/Sparkline/JobStatusIcon.jsx    | 56 ++++-----------
 .../Sparkline/JobStatusIcon.test.jsx          | 71 +++++--------------
 .../src/components/Sparkline/Sparkline.jsx    | 17 ++---
 .../components/Sparkline/Sparkline.test.jsx   |  6 +-
 4 files changed, 45 insertions(+), 105 deletions(-)

diff --git a/awx/ui_next/src/components/Sparkline/JobStatusIcon.jsx b/awx/ui_next/src/components/Sparkline/JobStatusIcon.jsx
index 3a1360a6dd..8a462f72ed 100644
--- a/awx/ui_next/src/components/Sparkline/JobStatusIcon.jsx
+++ b/awx/ui_next/src/components/Sparkline/JobStatusIcon.jsx
@@ -1,8 +1,6 @@
-import React, { Fragment } from 'react';
-import { node, number, shape, string } from 'prop-types';
-import { Link } from 'react-router-dom';
+import React from 'react';
+import { string } from 'prop-types';
 import styled, { keyframes } from 'styled-components';
-import { Tooltip } from '@patternfly/react-core';
 
 const Pulse = keyframes`
   from {
@@ -58,59 +56,31 @@ const FailedBottom = styled.div`
   background-color: #d9534f;
 `;
 
-const JobStatusIcon = ({ job, link, tooltip, ...props }) => {
-  let Icon = (
-    <Fragment>
-      {job.status === 'running' && <RunningJob />}
-      {(job.status === 'new' ||
-        job.status === 'pending' ||
-        job.status === 'waiting') && <WaitingJob />}
-      {(job.status === 'failed' ||
-        job.status === 'error' ||
-        job.status === 'canceled') && (
+const JobStatusIcon = ({ status, ...props }) => {
+  return (
+    <div {...props}>
+      {status === 'running' && <RunningJob />}
+      {(status === 'new' || status === 'pending' || status === 'waiting') && (
+        <WaitingJob />
+      )}
+      {(status === 'failed' || status === 'error' || status === 'canceled') && (
         <FinishedJob>
           <FailedTop />
           <FailedBottom />
         </FinishedJob>
       )}
-      {job.status === 'successful' && (
+      {status === 'successful' && (
         <FinishedJob>
           <SuccessfulTop />
           <SuccessfulBottom />
         </FinishedJob>
       )}
-    </Fragment>
+    </div>
   );
-
-  if (link) {
-    Icon = <Link to={link}>{Icon}</Link>;
-  }
-
-  if (tooltip) {
-    return (
-      <div {...props}>
-        <Tooltip position="top" content={tooltip}>
-          {Icon}
-        </Tooltip>
-      </div>
-    );
-  }
-
-  return <div {...props}>{Icon}</div>;
 };
 
 JobStatusIcon.propTypes = {
-  job: shape({
-    id: number.isRequired,
-    status: string.isRequired,
-  }).isRequired,
-  link: string,
-  tooltip: node,
-};
-
-JobStatusIcon.defaultProps = {
-  link: null,
-  tooltip: null,
+  status: string.isRequired,
 };
 
 export default JobStatusIcon;
diff --git a/awx/ui_next/src/components/Sparkline/JobStatusIcon.test.jsx b/awx/ui_next/src/components/Sparkline/JobStatusIcon.test.jsx
index ab915db6d5..204d305394 100644
--- a/awx/ui_next/src/components/Sparkline/JobStatusIcon.test.jsx
+++ b/awx/ui_next/src/components/Sparkline/JobStatusIcon.test.jsx
@@ -1,63 +1,28 @@
 import React from 'react';
 import { mount } from 'enzyme';
-import { mountWithContexts } from '../../../testUtils/enzymeHelpers';
-
 import JobStatusIcon from './JobStatusIcon';
 
 describe('JobStatusIcon', () => {
-  const job = {
-    id: 1,
-    status: 'successful',
-  };
-
-  test('renders the expected content', () => {
-    const wrapper = mount(<JobStatusIcon job={job} />);
+  test('renders the successful job', () => {
+    const wrapper = mount(<JobStatusIcon status="successful" />);
     expect(wrapper).toHaveLength(1);
-    expect(wrapper.find('Tooltip')).toHaveLength(0);
-    expect(wrapper.find('Link')).toHaveLength(0);
-  });
-  test('renders with tooltip if tooltip passed', () => {
-    const wrapper = mount(<JobStatusIcon job={job} tooltip="Foo Bar" />);
-    expect(wrapper).toHaveLength(1);
-    expect(wrapper.find('Tooltip')).toHaveLength(1);
-    expect(wrapper.find('Link')).toHaveLength(0);
-  });
-  test('renders with link if link passed', () => {
-    const wrapper = mountWithContexts(
-      <JobStatusIcon job={job} link="/jobs/playbook/1" />
-    );
-    expect(wrapper).toHaveLength(1);
-    expect(wrapper.find('Tooltip')).toHaveLength(0);
-    expect(wrapper.find('Link')).toHaveLength(1);
-  });
-  test('renders running job', () => {
-    const runningJob = {
-      id: 2,
-      status: 'running',
-    };
-    const wrapper = mount(<JobStatusIcon job={runningJob} />);
-    expect(wrapper.find('JobStatusIcon__RunningJob')).toHaveLength(1);
-  });
-  test('renders waiting job', () => {
-    const waitingJob = {
-      id: 3,
-      status: 'waiting',
-    };
-    const wrapper = mount(<JobStatusIcon job={waitingJob} />);
-    expect(wrapper.find('JobStatusIcon__WaitingJob')).toHaveLength(1);
-  });
-  test('renders failed job', () => {
-    const failedJob = {
-      id: 4,
-      status: 'failed',
-    };
-    const wrapper = mount(<JobStatusIcon job={failedJob} />);
-    expect(wrapper.find('JobStatusIcon__FailedTop')).toHaveLength(1);
-    expect(wrapper.find('JobStatusIcon__FailedBottom')).toHaveLength(1);
-  });
-  test('renders successful job', () => {
-    const wrapper = mount(<JobStatusIcon job={job} />);
     expect(wrapper.find('JobStatusIcon__SuccessfulTop')).toHaveLength(1);
     expect(wrapper.find('JobStatusIcon__SuccessfulBottom')).toHaveLength(1);
   });
+  test('renders running job', () => {
+    const wrapper = mount(<JobStatusIcon status="running" />);
+    expect(wrapper).toHaveLength(1);
+    expect(wrapper.find('JobStatusIcon__RunningJob')).toHaveLength(1);
+  });
+  test('renders waiting job', () => {
+    const wrapper = mount(<JobStatusIcon status="waiting" />);
+    expect(wrapper).toHaveLength(1);
+    expect(wrapper.find('JobStatusIcon__WaitingJob')).toHaveLength(1);
+  });
+  test('renders failed job', () => {
+    const wrapper = mount(<JobStatusIcon status="failed" />);
+    expect(wrapper).toHaveLength(1);
+    expect(wrapper.find('JobStatusIcon__FailedTop')).toHaveLength(1);
+    expect(wrapper.find('JobStatusIcon__FailedBottom')).toHaveLength(1);
+  });
 });
diff --git a/awx/ui_next/src/components/Sparkline/Sparkline.jsx b/awx/ui_next/src/components/Sparkline/Sparkline.jsx
index 0456b4eaa7..5291eeca85 100644
--- a/awx/ui_next/src/components/Sparkline/Sparkline.jsx
+++ b/awx/ui_next/src/components/Sparkline/Sparkline.jsx
@@ -1,13 +1,15 @@
 import React, { Fragment } from 'react';
 import { arrayOf, object } from 'prop-types';
 import { withI18n } from '@lingui/react';
-import { JobStatusIcon as _JobStatusIcon } from '@components/Sparkline';
+import { Link as _Link } from 'react-router-dom';
+import { JobStatusIcon } from '@components/Sparkline';
+import { Tooltip } from '@patternfly/react-core';
 import styled from 'styled-components';
 import { t } from '@lingui/macro';
 import { JOB_TYPE_URL_SEGMENTS } from '../../constants';
 
 /* eslint-disable react/jsx-pascal-case */
-const JobStatusIcon = styled(props => <_JobStatusIcon {...props} />)`
+const Link = styled(props => <_Link {...props} />)`
   margin-right: 5px;
 `;
 /* eslint-enable react/jsx-pascal-case */
@@ -30,12 +32,11 @@ const Sparkline = ({ i18n, jobs }) => {
   );
 
   return jobs.map(job => (
-    <JobStatusIcon
-      key={job.id}
-      job={job}
-      link={`/jobs/${JOB_TYPE_URL_SEGMENTS[job.type]}/${job.id}`}
-      tooltip={generateTooltip(job)}
-    />
+    <Tooltip position="top" content={generateTooltip(job)} key={job.id}>
+      <Link to={`/jobs/${JOB_TYPE_URL_SEGMENTS[job.type]}/${job.id}`}>
+        <JobStatusIcon status={job.status} />
+      </Link>
+    </Tooltip>
   ));
 };
 
diff --git a/awx/ui_next/src/components/Sparkline/Sparkline.test.jsx b/awx/ui_next/src/components/Sparkline/Sparkline.test.jsx
index 427709577b..e39003a841 100644
--- a/awx/ui_next/src/components/Sparkline/Sparkline.test.jsx
+++ b/awx/ui_next/src/components/Sparkline/Sparkline.test.jsx
@@ -9,18 +9,22 @@ describe('Sparkline', () => {
     const wrapper = mount(<Sparkline />);
     expect(wrapper).toHaveLength(1);
   });
-  test('renders an icon for each job', () => {
+  test('renders an icon with tooltips and links for each job', () => {
     const jobs = [
       {
         id: 1,
         status: 'successful',
+        finished: '2019-08-08T15:27:57.320120Z',
       },
       {
         id: 2,
         status: 'failed',
+        finished: '2019-08-09T15:27:57.320120Z',
       },
     ];
     const wrapper = mountWithContexts(<Sparkline jobs={jobs} />);
     expect(wrapper.find('JobStatusIcon')).toHaveLength(2);
+    expect(wrapper.find('Tooltip')).toHaveLength(2);
+    expect(wrapper.find('Link')).toHaveLength(2);
   });
 });