From 4b8a06801c1594b7fe8e94d54c077d7a09a4fc2d Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 24 Sep 2019 15:14:17 -0400 Subject: [PATCH 1/3] Add missing job detail fields and status icons --- .../src/components/DetailList/DetailList.jsx | 2 +- .../src/screens/Job/JobDetail/JobDetail.jsx | 88 ++++++++++++++++--- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/awx/ui_next/src/components/DetailList/DetailList.jsx b/awx/ui_next/src/components/DetailList/DetailList.jsx index 11d55474b3..f47cfa89bb 100644 --- a/awx/ui_next/src/components/DetailList/DetailList.jsx +++ b/awx/ui_next/src/components/DetailList/DetailList.jsx @@ -11,7 +11,7 @@ const DetailList = ({ children, stacked, ...props }) => ( export default styled(DetailList)` display: grid; grid-gap: 20px; - align-items: baseline; + align-items: flex-start; ${props => props.stacked ? ` diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx index 2705f1803c..5c217b8bc8 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx @@ -10,6 +10,7 @@ import { DetailList, Detail } from '@components/DetailList'; import { ChipGroup, Chip, CredentialChip } from '@components/Chip'; import { VariablesInput as _VariablesInput } from '@components/CodeMirrorInput'; import ErrorDetail from '@components/ErrorDetail'; +import { StatusIcon } from '@components/Sparkline'; import { toTitleCase } from '@util/strings'; import { Job } from '../../../types'; import { @@ -37,6 +38,14 @@ const VariablesInput = styled(_VariablesInput)` } `; +const StatusDetailValue = styled.div` + align-items: center; + display: inline-flex; + > div { + margin-right: 10px; + } +`; + const VERBOSITY = { 0: '0 (Normal)', 1: '1 (Verbose)', @@ -45,18 +54,50 @@ const VERBOSITY = { 4: '4 (Connection Debug)', }; +const getLaunchedByDetails = ({ summary_fields = {}, related = {} }) => { + const { + created_by: createdBy, + job_template: jobTemplate, + schedule, + } = summary_fields; + const { schedule: relatedSchedule } = related; + + if (!createdBy && !schedule) { + return null; + } + + let link; + let value; + + if (createdBy) { + link = `/users/${createdBy.id}`; + value = createdBy.username; + } else if (relatedSchedule && jobTemplate) { + link = `/templates/job_template/${jobTemplate.id}/schedules/${schedule.id}`; + value = schedule.name; + } else { + link = null; + value = schedule.name; + } + + return { link, value }; +}; + function JobDetail({ job, i18n, history }) { const { - job_template: jobTemplate, - project, - inventory, - instance_group: instanceGroup, credentials, + instance_group: instanceGroup, + inventory, + job_template: jobTemplate, labels, + project, } = job.summary_fields; const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false); const [errorMsg, setErrorMsg] = useState(); + const { value: launchedByValue, link: launchedByLink } = + getLaunchedByDetails(job) || {}; + const deleteJob = async () => { try { switch (job.type) { @@ -84,11 +125,20 @@ function JobDetail({ job, i18n, history }) { setIsDeleteModalOpen(false); } }; + return ( - {/* TODO: add status icon? */} - + {/* TODO: hookup status to websockets */} + + {job.status && } + {toTitleCase(job.status)} + + } + /> {jobTemplate && ( @@ -102,6 +152,18 @@ function JobDetail({ job, i18n, history }) { /> )} + + {launchedByLink ? ( + {launchedByValue} + ) : ( + launchedByValue + )} + + } + /> {inventory && ( )} - {/* TODO: show project status icon */} {project && ( {project.name}} + value={ + + {project.status && } + {project.name} + + } /> )} - + - - + + {instanceGroup && ( Date: Tue, 24 Sep 2019 15:16:59 -0400 Subject: [PATCH 2/3] Expand job detail tests to verify more fields * Update job detail tests to use large mock job data source * Move mock job data source into a shared file * Update OrgAccess snapshot due to DetailList style change --- .../screens/Job/JobDetail/JobDetail.test.jsx | 76 +++++++++++-------- .../screens/Job/JobOutput/JobOutput.test.jsx | 2 +- .../Job/{JobOutput => shared}/data.job.json | 0 .../OrganizationAccessItem.test.jsx.snap | 20 ++--- 4 files changed, 54 insertions(+), 44 deletions(-) rename awx/ui_next/src/screens/Job/{JobOutput => shared}/data.job.json (100%) diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx index 939616ff3f..879722bde9 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx @@ -4,64 +4,74 @@ import { mountWithContexts } from '@testUtils/enzymeHelpers'; import { sleep } from '@testUtils/testUtils'; import JobDetail from './JobDetail'; import { JobsAPI, ProjectUpdatesAPI } from '@api'; +import mockJobData from '../shared/data.job.json'; jest.mock('@api'); describe('', () => { - let job; - - beforeEach(() => { - job = { - name: 'Foo', - summary_fields: {}, - }; - }); - test('initially renders succesfully', () => { - mountWithContexts(); + mountWithContexts(); }); test('should display a Close button', () => { - const wrapper = mountWithContexts(); + const wrapper = mountWithContexts(); expect(wrapper.find('Button[aria-label="close"]').length).toBe(1); wrapper.unmount(); }); test('should display details', () => { - job.status = 'Successful'; - job.started = '2019-07-02T17:35:22.753817Z'; - job.finished = '2019-07-02T17:35:34.910800Z'; + const wrapper = mountWithContexts(); - const wrapper = mountWithContexts(); - const details = wrapper.find('Detail'); - - function assertDetail(detail, label, value) { - expect(detail.prop('label')).toEqual(label); - expect(detail.prop('value')).toEqual(value); + function assertDetail(label, value) { + expect(wrapper.find(`Detail[label="${label}"] dt`).text()).toBe(label); + expect(wrapper.find(`Detail[label="${label}"] dd`).text()).toBe(value); } - assertDetail(details.at(0), 'Status', 'Successful'); - assertDetail(details.at(1), 'Started', job.started); - assertDetail(details.at(2), 'Finished', job.finished); + assertDetail('Status', 'Successful'); + assertDetail('Started', mockJobData.started); + assertDetail('Finished', mockJobData.finished); + assertDetail('Template', mockJobData.summary_fields.job_template.name); + assertDetail('Job Type', 'Run'); + assertDetail('Launched By', mockJobData.summary_fields.created_by.username); + assertDetail('Inventory', mockJobData.summary_fields.inventory.name); + assertDetail('Project', mockJobData.summary_fields.project.name); + assertDetail('Revision', mockJobData.scm_revision); + assertDetail('Playbook', mockJobData.playbook); + assertDetail('Verbosity', '0 (Normal)'); + assertDetail('Environment', mockJobData.custom_virtualenv); + assertDetail('Execution Node', mockJobData.execution_node); + assertDetail( + 'Instance Group', + mockJobData.summary_fields.instance_group.name + ); + assertDetail('Job Slice', '0/1'); + assertDetail('Credentials', 'SSH: Demo Credential'); }); test('should display credentials', () => { - job.summary_fields.credentials = [ - { - id: 1, - name: 'Foo', - cloud: false, - kind: 'ssh', - }, - ]; - const wrapper = mountWithContexts(); + const wrapper = mountWithContexts(); const credentialChip = wrapper.find('CredentialChip'); expect(credentialChip.prop('credential')).toEqual( - job.summary_fields.credentials[0] + mockJobData.summary_fields.credentials[0] ); }); + + test('should display successful job status icon', () => { + const wrapper = mountWithContexts(); + const statusDetail = wrapper.find('Detail[label="Status"]'); + expect(statusDetail.find('StatusIcon__SuccessfulTop')).toHaveLength(1); + expect(statusDetail.find('StatusIcon__SuccessfulBottom')).toHaveLength(1); + }); + + test('should display successful project status icon', () => { + const wrapper = mountWithContexts(); + const statusDetail = wrapper.find('Detail[label="Project"]'); + expect(statusDetail.find('StatusIcon__SuccessfulTop')).toHaveLength(1); + expect(statusDetail.find('StatusIcon__SuccessfulBottom')).toHaveLength(1); + }); + test('should properly delete job', async () => { job = { name: 'Rage', diff --git a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.test.jsx b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.test.jsx index 7762efdb76..987a1ab88f 100644 --- a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.test.jsx +++ b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; import JobOutput from './JobOutput'; import { JobsAPI } from '@api'; -import mockJobData from './data.job.json'; +import mockJobData from '../shared/data.job.json'; import mockJobEventsData from './data.job_events.json'; jest.mock('@api'); diff --git a/awx/ui_next/src/screens/Job/JobOutput/data.job.json b/awx/ui_next/src/screens/Job/shared/data.job.json similarity index 100% rename from awx/ui_next/src/screens/Job/JobOutput/data.job.json rename to awx/ui_next/src/screens/Job/shared/data.job.json diff --git a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap index 8e8c16a6dd..043a609933 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap +++ b/awx/ui_next/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccessItem.test.jsx.snap @@ -321,9 +321,9 @@ exports[` initially renders succesfully 1`] = ` "componentStyle": ComponentStyle { "componentId": "DetailList-sc-12g7m4-0", "isStatic": false, - "lastClassName": "gmERnX", + "lastClassName": "eYaZBv", "rules": Array [ - "display:grid;grid-gap:20px;align-items:baseline;", + "display:grid;grid-gap:20px;align-items:flex-start;", [Function], ], }, @@ -341,15 +341,15 @@ exports[` initially renders succesfully 1`] = ` stacked={true} >
initially renders succesfully 1`] = ` "componentStyle": ComponentStyle { "componentId": "DetailList-sc-12g7m4-0", "isStatic": false, - "lastClassName": "gmERnX", + "lastClassName": "eYaZBv", "rules": Array [ - "display:grid;grid-gap:20px;align-items:baseline;", + "display:grid;grid-gap:20px;align-items:flex-start;", [Function], ], }, @@ -504,15 +504,15 @@ exports[` initially renders succesfully 1`] = ` stacked={true} >
Date: Thu, 26 Sep 2019 09:20:33 -0400 Subject: [PATCH 3/3] Add class to StatusIcon wrapper and fix merge conflicts --- .../src/components/Sparkline/StatusIcon.jsx | 9 +++--- .../src/screens/Job/JobDetail/JobDetail.jsx | 14 ++++---- .../screens/Job/JobDetail/JobDetail.test.jsx | 32 +++++-------------- .../screens/Job/JobOutput/HostEventModal.jsx | 2 +- 4 files changed, 20 insertions(+), 37 deletions(-) diff --git a/awx/ui_next/src/components/Sparkline/StatusIcon.jsx b/awx/ui_next/src/components/Sparkline/StatusIcon.jsx index 5f048f5cb5..2c9b7ae16b 100644 --- a/awx/ui_next/src/components/Sparkline/StatusIcon.jsx +++ b/awx/ui_next/src/components/Sparkline/StatusIcon.jsx @@ -75,11 +75,12 @@ const SkippedBottom = styled.div` const StatusIcon = ({ status, ...props }) => { return ( -
+
{status === 'running' && } - {(status === 'new' || status === 'pending' || status === 'waiting') && ( - - )} + {(status === 'new' || + status === 'pending' || + status === 'waiting' || + status === 'never updated') && } {(status === 'failed' || status === 'error' || status === 'canceled') && ( diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx index 5c217b8bc8..2fe5fdb50a 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.jsx @@ -41,7 +41,7 @@ const VariablesInput = styled(_VariablesInput)` const StatusDetailValue = styled.div` align-items: center; display: inline-flex; - > div { + .at-c-statusIcon { margin-right: 10px; } `; @@ -155,13 +155,11 @@ function JobDetail({ job, i18n, history }) { - {launchedByLink ? ( - {launchedByValue} - ) : ( - launchedByValue - )} - + launchedByLink ? ( + {launchedByValue} + ) : ( + launchedByValue + ) } /> {inventory && ( diff --git a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx index 879722bde9..0c592cc90f 100644 --- a/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx +++ b/awx/ui_next/src/screens/Job/JobDetail/JobDetail.test.jsx @@ -73,19 +73,8 @@ describe('', () => { }); test('should properly delete job', async () => { - job = { - name: 'Rage', - id: 1, - type: 'job', - summary_fields: { - job_template: { name: 'Spud' }, - }, - }; - const wrapper = mountWithContexts(); - wrapper - .find('button') - .at(0) - .simulate('click'); + const wrapper = mountWithContexts(); + wrapper.find('button[aria-label="Delete"]').simulate('click'); await sleep(1); wrapper.update(); const modal = wrapper.find('Modal'); @@ -93,17 +82,12 @@ describe('', () => { modal.find('button[aria-label="Delete"]').simulate('click'); expect(JobsAPI.destroy).toHaveBeenCalledTimes(1); }); - // The test below is skipped until react can be upgraded to at least 16.9.0. An upgrade to - // react - router will likely be necessary also. + /* + The test below is skipped until react can be upgraded to at least 16.9.0. An upgrade to + react - router will likely be necessary also. + See: https://github.com/ansible/awx/issues/4817 + */ test.skip('should display error modal when a job does not delete properly', async () => { - job = { - name: 'Angry', - id: 'a', - type: 'project_update', - summary_fields: { - job_template: { name: 'Peanut' }, - }, - }; ProjectUpdatesAPI.destroy.mockRejectedValue( new Error({ response: { @@ -116,7 +100,7 @@ describe('', () => { }, }) ); - const wrapper = mountWithContexts(); + const wrapper = mountWithContexts(); wrapper .find('button') diff --git a/awx/ui_next/src/screens/Job/JobOutput/HostEventModal.jsx b/awx/ui_next/src/screens/Job/JobOutput/HostEventModal.jsx index 08e3866983..45f6074d7d 100644 --- a/awx/ui_next/src/screens/Job/JobOutput/HostEventModal.jsx +++ b/awx/ui_next/src/screens/Job/JobOutput/HostEventModal.jsx @@ -32,7 +32,7 @@ const Modal = styled(PFModal)` const HostNameDetailValue = styled.div` align-items: center; display: inline-flex; - > div { + .at-c-statusIcon { margin-right: 10px; } `;