From 5549dac17d66c39a7b2ffd2d39f719d6ac820e69 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 22 Aug 2019 11:22:37 -0400 Subject: [PATCH] Hook up delete on jobs list. Add more comprehensive error handling on delete in organization and template lists. --- awx/ui_next/src/api/index.js | 15 +++ awx/ui_next/src/api/models/AdHocCommands.js | 10 ++ .../src/api/models/InventoryUpdates.js | 10 ++ awx/ui_next/src/api/models/ProjectUpdates.js | 10 ++ awx/ui_next/src/api/models/SystemJobs.js | 10 ++ awx/ui_next/src/api/models/WorkflowJobs.js | 10 ++ .../src/screens/Job/JobList/JobList.jsx | 58 ++++++++-- .../src/screens/Job/JobList/JobList.test.jsx | 100 ++++++++++++++++-- .../OrganizationList/OrganizationList.jsx | 14 +-- .../Template/TemplateList/TemplateList.jsx | 14 +-- .../TemplateList/TemplatesList.test.jsx | 4 +- 11 files changed, 222 insertions(+), 33 deletions(-) create mode 100644 awx/ui_next/src/api/models/AdHocCommands.js create mode 100644 awx/ui_next/src/api/models/InventoryUpdates.js create mode 100644 awx/ui_next/src/api/models/ProjectUpdates.js create mode 100644 awx/ui_next/src/api/models/SystemJobs.js create mode 100644 awx/ui_next/src/api/models/WorkflowJobs.js diff --git a/awx/ui_next/src/api/index.js b/awx/ui_next/src/api/index.js index 58f628e75d..128b9aa706 100644 --- a/awx/ui_next/src/api/index.js +++ b/awx/ui_next/src/api/index.js @@ -1,49 +1,64 @@ +import AdHocCommands from './models/AdHocCommands'; import Config from './models/Config'; import InstanceGroups from './models/InstanceGroups'; import Inventories from './models/Inventories'; +import InventoryUpdates from './models/InventoryUpdates'; import JobTemplates from './models/JobTemplates'; import Jobs from './models/Jobs'; import Labels from './models/Labels'; import Me from './models/Me'; import Organizations from './models/Organizations'; import Projects from './models/Projects'; +import ProjectUpdates from './models/ProjectUpdates'; import Root from './models/Root'; +import SystemJobs from './models/SystemJobs'; import Teams from './models/Teams'; import UnifiedJobTemplates from './models/UnifiedJobTemplates'; import UnifiedJobs from './models/UnifiedJobs'; import Users from './models/Users'; +import WorkflowJobs from './models/WorkflowJobs'; import WorkflowJobTemplates from './models/WorkflowJobTemplates'; +const AdHocCommandsAPI = new AdHocCommands(); const ConfigAPI = new Config(); const InstanceGroupsAPI = new InstanceGroups(); const InventoriesAPI = new Inventories(); +const InventoryUpdatesAPI = new InventoryUpdates(); const JobTemplatesAPI = new JobTemplates(); const JobsAPI = new Jobs(); const LabelsAPI = new Labels(); const MeAPI = new Me(); const OrganizationsAPI = new Organizations(); const ProjectsAPI = new Projects(); +const ProjectUpdatesAPI = new ProjectUpdates(); const RootAPI = new Root(); +const SystemJobsAPI = new SystemJobs(); const TeamsAPI = new Teams(); const UnifiedJobTemplatesAPI = new UnifiedJobTemplates(); const UnifiedJobsAPI = new UnifiedJobs(); const UsersAPI = new Users(); +const WorkflowJobsAPI = new WorkflowJobs(); const WorkflowJobTemplatesAPI = new WorkflowJobTemplates(); export { + AdHocCommandsAPI, ConfigAPI, InstanceGroupsAPI, InventoriesAPI, + InventoryUpdatesAPI, JobTemplatesAPI, JobsAPI, LabelsAPI, MeAPI, OrganizationsAPI, ProjectsAPI, + ProjectUpdatesAPI, RootAPI, + SystemJobsAPI, TeamsAPI, UnifiedJobTemplatesAPI, UnifiedJobsAPI, UsersAPI, + WorkflowJobsAPI, WorkflowJobTemplatesAPI, }; diff --git a/awx/ui_next/src/api/models/AdHocCommands.js b/awx/ui_next/src/api/models/AdHocCommands.js new file mode 100644 index 0000000000..1bfd78e9cb --- /dev/null +++ b/awx/ui_next/src/api/models/AdHocCommands.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class AdHocCommands extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/ad_hoc_commands/'; + } +} + +export default AdHocCommands; diff --git a/awx/ui_next/src/api/models/InventoryUpdates.js b/awx/ui_next/src/api/models/InventoryUpdates.js new file mode 100644 index 0000000000..0b30042e2c --- /dev/null +++ b/awx/ui_next/src/api/models/InventoryUpdates.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class InventoryUpdates extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/inventory_updates/'; + } +} + +export default InventoryUpdates; diff --git a/awx/ui_next/src/api/models/ProjectUpdates.js b/awx/ui_next/src/api/models/ProjectUpdates.js new file mode 100644 index 0000000000..46d0633f0d --- /dev/null +++ b/awx/ui_next/src/api/models/ProjectUpdates.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class ProjectUpdates extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/project_updates/'; + } +} + +export default ProjectUpdates; diff --git a/awx/ui_next/src/api/models/SystemJobs.js b/awx/ui_next/src/api/models/SystemJobs.js new file mode 100644 index 0000000000..d7b6ec1750 --- /dev/null +++ b/awx/ui_next/src/api/models/SystemJobs.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class SystemJobs extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/system_jobs/'; + } +} + +export default SystemJobs; diff --git a/awx/ui_next/src/api/models/WorkflowJobs.js b/awx/ui_next/src/api/models/WorkflowJobs.js new file mode 100644 index 0000000000..dc484b1bce --- /dev/null +++ b/awx/ui_next/src/api/models/WorkflowJobs.js @@ -0,0 +1,10 @@ +import Base from '../Base'; + +class WorkflowJobs extends Base { + constructor(http) { + super(http); + this.baseUrl = '/api/v2/workflow_jobs/'; + } +} + +export default WorkflowJobs; diff --git a/awx/ui_next/src/screens/Job/JobList/JobList.jsx b/awx/ui_next/src/screens/Job/JobList/JobList.jsx index 6a4c44067d..528a800a52 100644 --- a/awx/ui_next/src/screens/Job/JobList/JobList.jsx +++ b/awx/ui_next/src/screens/Job/JobList/JobList.jsx @@ -4,9 +4,18 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; -import { UnifiedJobsAPI } from '@api'; +import { + AdHocCommandsAPI, + InventoryUpdatesAPI, + JobsAPI, + ProjectUpdatesAPI, + SystemJobsAPI, + UnifiedJobsAPI, + WorkflowJobsAPI, +} from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarDeleteButton, } from '@components/PaginatedDataList'; @@ -27,8 +36,8 @@ class JobList extends Component { this.state = { hasContentLoading: true, + deletionError: null, contentError: null, - deletionError: false, selected: [], jobs: [], itemCount: 0, @@ -36,7 +45,7 @@ class JobList extends Component { this.loadJobs = this.loadJobs.bind(this); this.handleSelectAll = this.handleSelectAll.bind(this); this.handleSelect = this.handleSelect.bind(this); - this.handleDelete = this.handleDelete.bind(this); + this.handleJobDelete = this.handleJobDelete.bind(this); this.handleDeleteErrorClose = this.handleDeleteErrorClose.bind(this); } @@ -52,7 +61,7 @@ class JobList extends Component { } handleDeleteErrorClose() { - this.setState({ deletionError: false }); + this.setState({ deletionError: null }); } handleSelectAll(isSelected) { @@ -70,13 +79,41 @@ class JobList extends Component { } } - async handleDelete() { - const { selected } = this.state; - this.setState({ hasContentLoading: true, deletionError: false }); + async handleJobDelete() { + const { selected, itemCount } = this.state; + this.setState({ hasContentLoading: true }); try { - await Promise.all(selected.map(({ id }) => UnifiedJobsAPI.destroy(id))); + await Promise.all( + selected.map(({ type, id }) => { + let deletePromise; + switch (type) { + case 'job': + deletePromise = JobsAPI.destroy(id); + break; + case 'ad_hoc_command': + deletePromise = AdHocCommandsAPI.destroy(id); + break; + case 'system_job': + deletePromise = SystemJobsAPI.destroy(id); + break; + case 'project_update': + deletePromise = ProjectUpdatesAPI.destroy(id); + break; + case 'inventory_update': + deletePromise = InventoryUpdatesAPI.destroy(id); + break; + case 'workflow_job': + deletePromise = WorkflowJobsAPI.destroy(id); + break; + default: + break; + } + return deletePromise; + }) + ); + this.setState({ itemCount: itemCount - selected.length }); } catch (err) { - this.setState({ deletionError: true }); + this.setState({ deletionError: err }); } finally { await this.loadJobs(); } @@ -150,7 +187,7 @@ class JobList extends Component { additionalControls={[ , @@ -176,6 +213,7 @@ class JobList extends Component { onClose={this.handleDeleteErrorClose} > {i18n._(t`Failed to delete one or more jobs.`)} + ); diff --git a/awx/ui_next/src/screens/Job/JobList/JobList.test.jsx b/awx/ui_next/src/screens/Job/JobList/JobList.test.jsx index e4880d2870..7c1654dcd4 100644 --- a/awx/ui_next/src/screens/Job/JobList/JobList.test.jsx +++ b/awx/ui_next/src/screens/Job/JobList/JobList.test.jsx @@ -1,7 +1,15 @@ import React from 'react'; import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers'; -import { UnifiedJobsAPI } from '@api'; +import { + AdHocCommandsAPI, + InventoryUpdatesAPI, + JobsAPI, + ProjectUpdatesAPI, + SystemJobsAPI, + UnifiedJobsAPI, + WorkflowJobsAPI, +} from '@api'; import JobList from './JobList'; jest.mock('@api'); @@ -11,7 +19,7 @@ const mockResults = [ id: 1, url: '/api/v2/project_updates/1', name: 'job 1', - type: 'project update', + type: 'project_update', summary_fields: { user_capabilities: { delete: true, @@ -31,9 +39,42 @@ const mockResults = [ }, { id: 3, - url: '/api/v2/jobs/3', + url: '/api/v2/inventory_updates/3', name: 'job 3', - type: 'job', + type: 'inventory_update', + summary_fields: { + user_capabilities: { + delete: true, + }, + }, + }, + { + id: 4, + url: '/api/v2/workflow_jobs/4', + name: 'job 4', + type: 'workflow_job', + summary_fields: { + user_capabilities: { + delete: true, + }, + }, + }, + { + id: 5, + url: '/api/v2/system_jobs/5', + name: 'job 5', + type: 'system_job', + summary_fields: { + user_capabilities: { + delete: true, + }, + }, + }, + { + id: 6, + url: '/api/v2/ad_hoc_commands/6', + name: 'job 6', + type: 'ad_hoc_command', summary_fields: { user_capabilities: { delete: true, @@ -52,7 +93,7 @@ describe('', () => { await waitForElement( wrapper, 'JobList', - el => el.state('jobs').length === 3 + el => el.state('jobs').length === 6 ); done(); @@ -61,7 +102,7 @@ describe('', () => { test('select makes expected state updates', async done => { const [mockItem] = mockResults; const wrapper = mountWithContexts(); - await waitForElement(wrapper, 'JobListItem', el => el.length === 3); + await waitForElement(wrapper, 'JobListItem', el => el.length === 6); wrapper .find('JobListItem') @@ -79,20 +120,59 @@ describe('', () => { }); test('select-all-delete makes expected state updates and api calls', async done => { + AdHocCommandsAPI.destroy = jest.fn(); + InventoryUpdatesAPI.destroy = jest.fn(); + JobsAPI.destroy = jest.fn(); + ProjectUpdatesAPI.destroy = jest.fn(); + SystemJobsAPI.destroy = jest.fn(); + WorkflowJobsAPI.destroy = jest.fn(); const wrapper = mountWithContexts(); - await waitForElement(wrapper, 'JobListItem', el => el.length === 3); + await waitForElement(wrapper, 'JobListItem', el => el.length === 6); wrapper.find('DataListToolbar').prop('onSelectAll')(true); - expect(wrapper.find('JobList').state('selected').length).toEqual(3); + expect(wrapper.find('JobList').state('selected').length).toEqual(6); wrapper.find('DataListToolbar').prop('onSelectAll')(false); expect(wrapper.find('JobList').state('selected').length).toEqual(0); wrapper.find('DataListToolbar').prop('onSelectAll')(true); - expect(wrapper.find('JobList').state('selected').length).toEqual(3); + expect(wrapper.find('JobList').state('selected').length).toEqual(6); wrapper.find('ToolbarDeleteButton').prop('onDelete')(); - expect(UnifiedJobsAPI.destroy).toHaveBeenCalledTimes(3); + expect(AdHocCommandsAPI.destroy).toHaveBeenCalledTimes(1); + expect(InventoryUpdatesAPI.destroy).toHaveBeenCalledTimes(1); + expect(JobsAPI.destroy).toHaveBeenCalledTimes(1); + expect(ProjectUpdatesAPI.destroy).toHaveBeenCalledTimes(1); + expect(SystemJobsAPI.destroy).toHaveBeenCalledTimes(1); + expect(WorkflowJobsAPI.destroy).toHaveBeenCalledTimes(1); + + done(); + }); + + test('error is shown when job not successfully deleted from api', async done => { + JobsAPI.destroy.mockRejectedValue( + new Error({ + response: { + config: { + method: 'delete', + url: '/api/v2/jobs/2', + }, + data: 'An error occurred', + }, + }) + ); + const wrapper = mountWithContexts(); + wrapper.find('JobList').setState({ + jobs: mockResults, + itemCount: 6, + selected: mockResults.slice(1, 2), + }); + wrapper.find('ToolbarDeleteButton').prop('onDelete')(); + await waitForElement( + wrapper, + 'Modal', + el => el.props().isOpen === true && el.props().title === 'Error!' + ); done(); }); diff --git a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx index 4f943126f5..76a6089755 100644 --- a/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/awx/ui_next/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -7,6 +7,7 @@ import { Card, PageSection, PageSectionVariants } from '@patternfly/react-core'; import { OrganizationsAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarAddButton, ToolbarDeleteButton, @@ -28,7 +29,7 @@ class OrganizationsList extends Component { this.state = { hasContentLoading: true, contentError: null, - hasDeletionError: false, + deletionError: null, organizations: [], selected: [], itemCount: 0, @@ -71,17 +72,17 @@ class OrganizationsList extends Component { } handleDeleteErrorClose() { - this.setState({ hasDeletionError: false }); + this.setState({ deletionError: null }); } async handleOrgDelete() { const { selected } = this.state; - this.setState({ hasContentLoading: true, hasDeletionError: false }); + this.setState({ hasContentLoading: true }); try { await Promise.all(selected.map(org => OrganizationsAPI.destroy(org.id))); } catch (err) { - this.setState({ hasDeletionError: true }); + this.setState({ deletionError: err }); } finally { await this.loadOrganizations(); } @@ -134,7 +135,7 @@ class OrganizationsList extends Component { itemCount, contentError, hasContentLoading, - hasDeletionError, + deletionError, selected, organizations, } = this.state; @@ -212,12 +213,13 @@ class OrganizationsList extends Component { {i18n._(t`Failed to delete one or more organizations.`)} + ); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx index 05d86493ae..ebd6d7838d 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplateList.jsx @@ -15,6 +15,7 @@ import { import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarDeleteButton, ToolbarAddButton, @@ -39,7 +40,7 @@ class TemplatesList extends Component { this.state = { hasContentLoading: true, contentError: null, - hasDeletionError: false, + deletionError: null, selected: [], templates: [], itemCount: 0, @@ -66,7 +67,7 @@ class TemplatesList extends Component { } handleDeleteErrorClose() { - this.setState({ hasDeletionError: false }); + this.setState({ deletionError: null }); } handleSelectAll(isSelected) { @@ -92,7 +93,7 @@ class TemplatesList extends Component { async handleTemplateDelete() { const { selected, itemCount } = this.state; - this.setState({ hasContentLoading: true, hasDeletionError: false }); + this.setState({ hasContentLoading: true }); try { await Promise.all( selected.map(({ type, id }) => { @@ -107,7 +108,7 @@ class TemplatesList extends Component { ); this.setState({ itemCount: itemCount - selected.length }); } catch (err) { - this.setState({ hasDeletionError: true }); + this.setState({ deletionError: err }); } finally { await this.loadTemplates(); } @@ -159,7 +160,7 @@ class TemplatesList extends Component { const { contentError, hasContentLoading, - hasDeletionError, + deletionError, templates, itemCount, selected, @@ -287,12 +288,13 @@ class TemplatesList extends Component { /> {i18n._(t`Failed to delete one or more template.`)} + ); diff --git a/awx/ui_next/src/screens/Template/TemplateList/TemplatesList.test.jsx b/awx/ui_next/src/screens/Template/TemplateList/TemplatesList.test.jsx index 436ed8c877..b94c8080f4 100644 --- a/awx/ui_next/src/screens/Template/TemplateList/TemplatesList.test.jsx +++ b/awx/ui_next/src/screens/Template/TemplateList/TemplatesList.test.jsx @@ -199,7 +199,7 @@ describe('', () => { expect(WorkflowJobTemplatesAPI.destroy).toHaveBeenCalledTimes(1); }); - test('error is shown when template not successfully deleted from api', async () => { + test('error is shown when template not successfully deleted from api', async done => { JobTemplatesAPI.destroy.mockRejectedValue( new Error({ response: { @@ -225,5 +225,7 @@ describe('', () => { 'Modal', el => el.props().isOpen === true && el.props().title === 'Error!' ); + + done(); }); });