From 52851c57d8216c8abce09937a5f09f0bd9cf9841 Mon Sep 17 00:00:00 2001 From: Michael Abashian Date: Wed, 26 Jun 2019 11:40:15 -0400 Subject: [PATCH] Display details about network errors in alert modal and content error components (#288) Display details about network errors in alert modal and content error components --- src/App.jsx | 14 ++- src/components/AlertModal/AlertModal.jsx | 16 ++-- src/components/ContentError/ContentError.jsx | 40 +++++--- .../ContentError/ContentError.test.jsx | 10 +- src/components/ErrorDetail/ErrorDetail.jsx | 95 +++++++++++++++++++ .../ErrorDetail/ErrorDetail.test.jsx | 19 ++++ src/components/ErrorDetail/index.js | 1 + src/components/LaunchButton/LaunchButton.jsx | 10 +- .../LaunchButton/LaunchButton.test.jsx | 11 ++- .../PaginatedDataList/PaginatedDataList.jsx | 10 +- src/screens/Job/Job.jsx | 14 +-- src/screens/Job/JobList/JobList.jsx | 24 ++--- src/screens/Organization/Organization.jsx | 16 ++-- .../OrganizationAccess/OrganizationAccess.jsx | 30 +++--- .../OrganizationAccess.test.jsx | 2 +- .../OrganizationAccess.test.jsx.snap | 9 +- .../OrganizationDetail/OrganizationDetail.jsx | 10 +- .../OrganizationList/OrganizationList.jsx | 24 ++--- .../OrganizationList.test.jsx | 28 +++++- .../OrganizationNotifications.jsx | 22 +++-- .../OrganizationNotifications.test.jsx.snap | 8 +- .../OrganizationTeams/OrganizationTeams.jsx | 12 +-- .../JobTemplateDetail/JobTemplateDetail.jsx | 8 +- src/screens/Template/Template.jsx | 18 ++-- .../Template/TemplateList/TemplateList.jsx | 24 ++--- .../TemplateList/TemplatesList.test.jsx | 10 +- 26 files changed, 341 insertions(+), 144 deletions(-) create mode 100644 src/components/ErrorDetail/ErrorDetail.jsx create mode 100644 src/components/ErrorDetail/ErrorDetail.test.jsx create mode 100644 src/components/ErrorDetail/index.js diff --git a/src/App.jsx b/src/App.jsx index 1e1053d982..db0bee52f3 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -17,6 +17,7 @@ import AlertModal from '@components/AlertModal'; import NavExpandableGroup from '@components/NavExpandableGroup'; import BrandLogo from '@components/BrandLogo'; import PageHeaderToolbar from '@components/PageHeaderToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import { ConfigProvider } from '@contexts/Config'; const PageHeader = styled(PFPageHeader)` @@ -48,7 +49,7 @@ class App extends Component { version: null, isAboutModalOpen: false, isNavOpen, - hasConfigError: false, + configError: null }; this.handleLogout = this.handleLogout.bind(this); @@ -81,7 +82,9 @@ class App extends Component { } handleConfigErrorClose () { - this.setState({ hasConfigError: false }); + this.setState({ + configError: null + }); } async loadConfig () { @@ -92,7 +95,7 @@ class App extends Component { this.setState({ ansible_version, custom_virtualenvs, version, me }); } catch (err) { - this.setState({ hasConfigError: true }); + this.setState({ configError: err }); } } @@ -104,7 +107,7 @@ class App extends Component { isNavOpen, me, version, - hasConfigError, + configError } = this.state; const { i18n, @@ -170,12 +173,13 @@ class App extends Component { onClose={this.handleAboutClose} /> {i18n._(t`Failed to retrieve configuration.`)} + ); diff --git a/src/components/AlertModal/AlertModal.jsx b/src/components/AlertModal/AlertModal.jsx index 264e9cf355..bdfd4e9761 100644 --- a/src/components/AlertModal/AlertModal.jsx +++ b/src/components/AlertModal/AlertModal.jsx @@ -20,9 +20,13 @@ const getIcon = (variant) => { return icon; }; -export default ({ variant, children, ...props }) => ( - - {children} - {getIcon(variant)} - -); +export default ({ variant, children, ...props }) => { + const { isOpen = null } = props; + props.isOpen = Boolean(isOpen); + return ( + + {children} + {getIcon(variant)} + + ); +}; diff --git a/src/components/ContentError/ContentError.jsx b/src/components/ContentError/ContentError.jsx index b5721cc988..c9988c3b72 100644 --- a/src/components/ContentError/ContentError.jsx +++ b/src/components/ContentError/ContentError.jsx @@ -1,26 +1,40 @@ import React from 'react'; import { t } from '@lingui/macro'; +import styled from 'styled-components'; import { withI18n } from '@lingui/react'; import { Title, - EmptyState, + EmptyState as PFEmptyState, EmptyStateIcon, EmptyStateBody } from '@patternfly/react-core'; import { ExclamationTriangleIcon } from '@patternfly/react-icons'; -// TODO: Pass actual error as prop and display expandable details for network errors. -const ContentError = ({ i18n }) => ( - - - - {i18n._(t`Something went wrong...`)} - - - {i18n._(t`There was an error loading this content. Please reload the page.`)} - - -); +import ErrorDetail from '@components/ErrorDetail'; + +const EmptyState = styled(PFEmptyState)` + width: var(--pf-c-empty-state--m-lg--MaxWidth); +`; + +class ContentError extends React.Component { + render () { + const { error, i18n } = this.props; + return ( + + + + {i18n._(t`Something went wrong...`)} + + + {i18n._(t`There was an error loading this content. Please reload the page.`)} + + {error && ( + + )} + + ); + } +} export { ContentError as _ContentError }; export default withI18n()(ContentError); diff --git a/src/components/ContentError/ContentError.test.jsx b/src/components/ContentError/ContentError.test.jsx index db0d32c3aa..484bbd91d8 100644 --- a/src/components/ContentError/ContentError.test.jsx +++ b/src/components/ContentError/ContentError.test.jsx @@ -5,7 +5,15 @@ import ContentError from './ContentError'; describe('ContentError', () => { test('renders the expected content', () => { - const wrapper = mountWithContexts(); + const wrapper = mountWithContexts(); expect(wrapper).toHaveLength(1); }); }); diff --git a/src/components/ErrorDetail/ErrorDetail.jsx b/src/components/ErrorDetail/ErrorDetail.jsx new file mode 100644 index 0000000000..8c11aac176 --- /dev/null +++ b/src/components/ErrorDetail/ErrorDetail.jsx @@ -0,0 +1,95 @@ +import React, { Component, Fragment } from 'react'; +import PropTypes from 'prop-types'; +import styled from 'styled-components'; +import { withI18n } from '@lingui/react'; +import { t } from '@lingui/macro'; + +import { + Card as PFCard, + CardBody as PFCardBody, + Expandable as PFExpandable +} from '@patternfly/react-core'; + +const Card = styled(PFCard)` + background-color: var(--pf-global--BackgroundColor--200); + overflow-wrap: break-word; +`; + +const CardBody = styled(PFCardBody)` + max-height: 200px; + overflow: scroll; +`; + +const Expandable = styled(PFExpandable)` + text-align: left; +`; + +class ErrorDetail extends Component { + constructor (props) { + super(props); + + this.state = { + isExpanded: false + }; + + this.handleToggle = this.handleToggle.bind(this); + this.renderNetworkError = this.renderNetworkError.bind(this); + this.renderStack = this.renderStack.bind(this); + } + + handleToggle () { + const { isExpanded } = this.state; + this.setState({ isExpanded: !isExpanded }); + } + + renderNetworkError () { + const { error } = this.props; + const { response } = error; + + const message = typeof response.data === 'string' + ? response.data + : response.data.detail; + + return ( + + + {response.config.method.toUpperCase()} + {' '} + {response.config.url} + {' '} + + {response.status} + + + {message} + + ); + } + + renderStack () { + const { error } = this.props; + return ({error.stack}); + } + + render () { + const { isExpanded } = this.state; + const { error, i18n } = this.props; + + return ( + + + {Object.prototype.hasOwnProperty.call(error, 'response') + ? this.renderNetworkError() + : this.renderStack() + } + + + ); + } +} + +ErrorDetail.propTypes = { + error: PropTypes.instanceOf(Error).isRequired +}; + +export default withI18n()(ErrorDetail); diff --git a/src/components/ErrorDetail/ErrorDetail.test.jsx b/src/components/ErrorDetail/ErrorDetail.test.jsx new file mode 100644 index 0000000000..3e7452f5ba --- /dev/null +++ b/src/components/ErrorDetail/ErrorDetail.test.jsx @@ -0,0 +1,19 @@ +import React from 'react'; +import { mountWithContexts } from '@testUtils/enzymeHelpers'; + +import ErrorDetail from './ErrorDetail'; + +describe('ErrorDetail', () => { + test('renders the expected content', () => { + const wrapper = mountWithContexts(); + expect(wrapper).toHaveLength(1); + }); +}); diff --git a/src/components/ErrorDetail/index.js b/src/components/ErrorDetail/index.js new file mode 100644 index 0000000000..0f380db7ab --- /dev/null +++ b/src/components/ErrorDetail/index.js @@ -0,0 +1 @@ +export { default } from './ErrorDetail'; diff --git a/src/components/LaunchButton/LaunchButton.jsx b/src/components/LaunchButton/LaunchButton.jsx index 33153285e9..79c590160d 100644 --- a/src/components/LaunchButton/LaunchButton.jsx +++ b/src/components/LaunchButton/LaunchButton.jsx @@ -8,6 +8,7 @@ import { withI18n } from '@lingui/react'; import { t } from '@lingui/macro'; import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; import { JobTemplatesAPI } from '@api'; const StyledLaunchButton = styled(Button)` @@ -28,7 +29,7 @@ class LaunchButton extends React.Component { super(props); this.state = { - launchError: false, + launchError: null, promptError: false }; @@ -38,7 +39,7 @@ class LaunchButton extends React.Component { } handleLaunchErrorClose () { - this.setState({ launchError: false }); + this.setState({ launchError: null }); } handlePromptErrorClose () { @@ -55,8 +56,8 @@ class LaunchButton extends React.Component { } else { this.setState({ promptError: true }); } - } catch (error) { - this.setState({ launchError: true }); + } catch (err) { + this.setState({ launchError: err }); } } @@ -89,6 +90,7 @@ class LaunchButton extends React.Component { onClose={this.handleLaunchErrorClose} > {i18n._(t`Failed to launch job.`)} + { done(); }); test('displays error modal after unsuccessful launch', async (done) => { - JobTemplatesAPI.launch.mockRejectedValue({}); + JobTemplatesAPI.launch.mockRejectedValue(new Error({ + response: { + config: { + method: 'post', + url: '/api/v2/job_templates/1/launch' + }, + data: 'An error occurred', + status: 403 + } + })); const wrapper = mountWithContexts(); const launchButton = wrapper.find('LaunchButton__StyledLaunchButton'); launchButton.simulate('click'); diff --git a/src/components/PaginatedDataList/PaginatedDataList.jsx b/src/components/PaginatedDataList/PaginatedDataList.jsx index ab42df512a..dbb0d4af50 100644 --- a/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -72,7 +72,7 @@ class PaginatedDataList extends React.Component { render () { const [orderBy, sortOrder] = this.getSortOrder(); const { - hasContentError, + contentError, hasContentLoading, emptyStateControls, items, @@ -100,8 +100,8 @@ class PaginatedDataList extends React.Component { let Content; if (hasContentLoading && items.length <= 0) { Content = (); - } else if (hasContentError) { - Content = (); + } else if (contentError) { + Content = (); } else if (items.length <= 0) { Content = (); } else { @@ -174,12 +174,12 @@ PaginatedDataList.propTypes = { showPageSizeOptions: PropTypes.bool, renderToolbar: PropTypes.func, hasContentLoading: PropTypes.bool, - hasContentError: PropTypes.bool, + contentError: PropTypes.shape(), }; PaginatedDataList.defaultProps = { hasContentLoading: false, - hasContentError: false, + contentError: null, toolbarColumns: [], itemName: 'item', itemNamePlural: '', diff --git a/src/screens/Job/Job.jsx b/src/screens/Job/Job.jsx index a64be2a894..c83a9477a3 100644 --- a/src/screens/Job/Job.jsx +++ b/src/screens/Job/Job.jsx @@ -19,7 +19,7 @@ class Job extends Component { this.state = { job: null, - hasContentError: false, + contentError: null, hasContentLoading: true, isInitialized: false }; @@ -46,13 +46,13 @@ class Job extends Component { } = this.props; const id = parseInt(match.params.id, 10); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data } = await JobsAPI.readDetail(id); setBreadcrumb(data); this.setState({ job: data }); - } catch (error) { - this.setState({ hasContentError: true }); + } catch (err) { + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -67,7 +67,7 @@ class Job extends Component { const { job, - hasContentError, + contentError, hasContentLoading, isInitialized } = this.state; @@ -103,11 +103,11 @@ class Job extends Component { cardHeader = null; } - if (!hasContentLoading && hasContentError) { + if (!hasContentLoading && contentError) { return ( - + ); diff --git a/src/screens/Job/JobList/JobList.jsx b/src/screens/Job/JobList/JobList.jsx index 07736823a6..7b8faef76c 100644 --- a/src/screens/Job/JobList/JobList.jsx +++ b/src/screens/Job/JobList/JobList.jsx @@ -11,6 +11,7 @@ import { import { UnifiedJobsAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarDeleteButton } from '@components/PaginatedDataList'; @@ -31,8 +32,8 @@ class JobList extends Component { this.state = { hasContentLoading: true, - hasContentError: false, - hasDeletionError: false, + contentError: null, + deletionError: null, selected: [], jobs: [], itemCount: 0, @@ -56,7 +57,7 @@ class JobList extends Component { } handleDeleteErrorClose () { - this.setState({ hasDeletionError: false }); + this.setState({ deletionError: null }); } handleSelectAll (isSelected) { @@ -76,11 +77,11 @@ class JobList extends Component { async handleDelete () { const { selected } = this.state; - this.setState({ hasContentLoading: true, hasDeletionError: false }); + this.setState({ hasContentLoading: true }); try { await Promise.all(selected.map(({ id }) => UnifiedJobsAPI.destroy(id))); } catch (err) { - this.setState({ hasDeletionError: true }); + this.setState({ deletionError: err }); } finally { await this.loadJobs(); } @@ -90,7 +91,7 @@ class JobList extends Component { const { location } = this.props; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data: { count, results } } = await UnifiedJobsAPI.read(params); this.setState({ @@ -99,7 +100,7 @@ class JobList extends Component { selected: [], }); } catch (err) { - this.setState({ hasContentError: true }); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -107,9 +108,9 @@ class JobList extends Component { render () { const { - hasContentError, + contentError, hasContentLoading, - hasDeletionError, + deletionError, jobs, itemCount, selected, @@ -125,7 +126,7 @@ class JobList extends Component { {i18n._(t`Failed to delete one or more jobs.`)} + ); diff --git a/src/screens/Organization/Organization.jsx b/src/screens/Organization/Organization.jsx index 920e66126d..108c040cf4 100644 --- a/src/screens/Organization/Organization.jsx +++ b/src/screens/Organization/Organization.jsx @@ -21,7 +21,7 @@ class Organization extends Component { this.state = { organization: null, hasContentLoading: true, - hasContentError: false, + contentError: null, isInitialized: false, isNotifAdmin: false, isAuditorOfThisOrg: false, @@ -50,7 +50,7 @@ class Organization extends Component { } = this.props; const id = parseInt(match.params.id, 10); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const [{ data }, notifAdminRes, auditorRes, adminRes] = await Promise.all([ OrganizationsAPI.readDetail(id), @@ -66,7 +66,7 @@ class Organization extends Component { isAdminOfThisOrg: adminRes.data.results.length > 0 }); } catch (err) { - this.setState(({ hasContentError: true })); + this.setState(({ contentError: err })); } finally { this.setState({ hasContentLoading: false }); } @@ -79,13 +79,13 @@ class Organization extends Component { } = this.props; const id = parseInt(match.params.id, 10); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data } = await OrganizationsAPI.readDetail(id); setBreadcrumb(data); this.setState({ organization: data }); } catch (err) { - this.setState(({ hasContentError: true })); + this.setState(({ contentError: err })); } finally { this.setState({ hasContentLoading: false }); } @@ -102,7 +102,7 @@ class Organization extends Component { const { organization, - hasContentError, + contentError, hasContentLoading, isInitialized, isNotifAdmin, @@ -162,11 +162,11 @@ class Organization extends Component { cardHeader = null; } - if (!hasContentLoading && hasContentError) { + if (!hasContentLoading && contentError) { return ( - + ); diff --git a/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx b/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx index b46b36cbc4..26079c34a6 100644 --- a/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx +++ b/src/screens/Organization/OrganizationAccess/OrganizationAccess.jsx @@ -7,6 +7,7 @@ import { OrganizationsAPI, TeamsAPI, UsersAPI } from '@api'; import AddResourceRole from '@components/AddRole/AddResourceRole'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarAddButton } from '@components/PaginatedDataList'; import { getQSConfig, @@ -33,9 +34,9 @@ class OrganizationAccess extends React.Component { super(props); this.state = { accessRecords: [], - hasContentError: false, + contentError: null, hasContentLoading: true, - hasDeletionError: false, + deletionError: null, deletionRecord: null, deletionRole: null, isAddModalOpen: false, @@ -70,7 +71,7 @@ class OrganizationAccess extends React.Component { const { organization, location } = this.props; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data: { @@ -79,8 +80,8 @@ class OrganizationAccess extends React.Component { } } = await OrganizationsAPI.readAccessList(organization.id, params); this.setState({ itemCount, accessRecords }); - } catch (error) { - this.setState({ hasContentError: true }); + } catch (err) { + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -96,7 +97,7 @@ class OrganizationAccess extends React.Component { handleDeleteErrorClose () { this.setState({ - hasDeletionError: false, + deletionError: null, deletionRecord: null, deletionRole: null }); @@ -123,10 +124,10 @@ class OrganizationAccess extends React.Component { deletionRole: null, deletionRecord: null }); - } catch (error) { + } catch (err) { this.setState({ hasContentLoading: false, - hasDeletionError: true + deletionError: err }); } } @@ -148,21 +149,21 @@ class OrganizationAccess extends React.Component { const { organization, i18n } = this.props; const { accessRecords, - hasContentError, + contentError, hasContentLoading, deletionRole, deletionRecord, - hasDeletionError, + deletionError, itemCount, - isAddModalOpen, + isAddModalOpen } = this.state; const canEdit = organization.summary_fields.user_capabilities.edit; - const isDeleteModalOpen = !hasContentLoading && !hasDeletionError && deletionRole; + const isDeleteModalOpen = !hasContentLoading && !deletionError && deletionRole; return ( )} {i18n._(t`Failed to delete role`)} + ); diff --git a/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx b/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx index 549aba75b4..8634c1030d 100644 --- a/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx +++ b/src/screens/Organization/OrganizationAccess/OrganizationAccess.test.jsx @@ -84,7 +84,7 @@ describe('', () => { await waitForElement(wrapper, 'OrganizationAccessItem', el => el.length === 2); expect(wrapper.find('PaginatedDataList').prop('items')).toEqual(data.results); expect(wrapper.find('OrganizationAccess').state('hasContentLoading')).toBe(false); - expect(wrapper.find('OrganizationAccess').state('hasContentError')).toBe(false); + expect(wrapper.find('OrganizationAccess').state('contentError')).toBe(null); done(); }); diff --git a/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap b/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap index d53e73084e..321c03c4eb 100644 --- a/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap +++ b/src/screens/Organization/OrganizationAccess/__snapshots__/OrganizationAccess.test.jsx.snap @@ -34,7 +34,7 @@ exports[` initially renders succesfully 1`] = ` } > initially renders succesfully 1`] = ` withHash={true} > initially renders succesfully 1`] = ` > initially renders succesfully 1`] = ` <_default - isOpen={false} + isOpen={null} onClose={[Function]} title="Error!" variant="danger" diff --git a/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx b/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx index 22ab73a71a..31b30c03bb 100644 --- a/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx +++ b/src/screens/Organization/OrganizationDetail/OrganizationDetail.jsx @@ -20,7 +20,7 @@ class OrganizationDetail extends Component { super(props); this.state = { - hasContentError: false, + contentError: null, hasContentLoading: true, instanceGroups: [], }; @@ -39,7 +39,7 @@ class OrganizationDetail extends Component { const { data: { results = [] } } = await OrganizationsAPI.readInstanceGroups(id); this.setState({ instanceGroups: [...results] }); } catch (err) { - this.setState({ hasContentError: true }); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -48,7 +48,7 @@ class OrganizationDetail extends Component { render () { const { hasContentLoading, - hasContentError, + contentError, instanceGroups, } = this.state; @@ -70,8 +70,8 @@ class OrganizationDetail extends Component { return (); } - if (hasContentError) { - return (); + if (contentError) { + return (); } return ( diff --git a/src/screens/Organization/OrganizationList/OrganizationList.jsx b/src/screens/Organization/OrganizationList/OrganizationList.jsx index b5adfd0044..e0eaab93bb 100644 --- a/src/screens/Organization/OrganizationList/OrganizationList.jsx +++ b/src/screens/Organization/OrganizationList/OrganizationList.jsx @@ -11,6 +11,7 @@ import { import { OrganizationsAPI } from '@api'; import AlertModal from '@components/AlertModal'; import DataListToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarAddButton, ToolbarDeleteButton, @@ -31,8 +32,8 @@ class OrganizationsList extends Component { this.state = { hasContentLoading: true, - hasContentError: false, - hasDeletionError: false, + contentError: null, + deletionError: null, organizations: [], selected: [], itemCount: 0, @@ -75,17 +76,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(); } @@ -108,7 +109,7 @@ class OrganizationsList extends Component { optionsPromise, ]); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const [{ data: { count, results } }, { data: { actions } }] = await promises; this.setState({ @@ -118,7 +119,7 @@ class OrganizationsList extends Component { selected: [], }); } catch (err) { - this.setState(({ hasContentError: true })); + this.setState(({ contentError: err })); } finally { this.setState({ hasContentLoading: false }); } @@ -131,9 +132,9 @@ class OrganizationsList extends Component { const { actions, itemCount, - hasContentError, + contentError, hasContentLoading, - hasDeletionError, + deletionError, selected, organizations, } = this.state; @@ -147,7 +148,7 @@ class OrganizationsList extends Component { {i18n._(t`Failed to delete one or more organizations.`)} + ); diff --git a/src/screens/Organization/OrganizationList/OrganizationList.test.jsx b/src/screens/Organization/OrganizationList/OrganizationList.test.jsx index 603e70e439..00739b7c12 100644 --- a/src/screens/Organization/OrganizationList/OrganizationList.test.jsx +++ b/src/screens/Organization/OrganizationList/OrganizationList.test.jsx @@ -60,6 +60,20 @@ const mockAPIOrgsList = { describe('', () => { let wrapper; + beforeEach(() => { + OrganizationsAPI.read = () => Promise.resolve({ + data: { + count: 0, + results: [] + } + }); + OrganizationsAPI.readOptions = () => Promise.resolve({ + data: { + actions: [] + } + }); + }); + test('initially renders succesfully', () => { mountWithContexts(); }); @@ -122,8 +136,17 @@ describe('', () => { expect(fetchOrgs).toBeCalled(); }); - test('error is shown when org not successfully deleted from api', async () => { - OrganizationsAPI.destroy = () => Promise.reject(); + test('error is shown when org not successfully deleted from api', async (done) => { + OrganizationsAPI.destroy.mockRejectedValue(new Error({ + response: { + config: { + method: 'delete', + url: '/api/v2/organizations/1' + }, + data: 'An error occurred' + } + })); + wrapper = mountWithContexts(); wrapper.find('OrganizationsList').setState({ organizations: mockAPIOrgsList.data.results, @@ -133,5 +156,6 @@ describe('', () => { }); wrapper.find('ToolbarDeleteButton').prop('onDelete')(); await waitForElement(wrapper, 'Modal', (el) => el.props().isOpen === true && el.props().title === 'Error!'); + done(); }); }); diff --git a/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx b/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx index b6e76ef55e..99e09ae5c5 100644 --- a/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx +++ b/src/screens/Organization/OrganizationNotifications/OrganizationNotifications.jsx @@ -6,6 +6,7 @@ import { t } from '@lingui/macro'; import { OrganizationsAPI } from '@api'; import AlertModal from '@components/AlertModal'; +import ErrorDetail from '@components/ErrorDetail'; import NotificationListItem from '@components/NotificationsList/NotificationListItem'; import PaginatedDataList from '@components/PaginatedDataList'; import { getQSConfig, parseNamespacedQueryString } from '@util/qs'; @@ -26,9 +27,9 @@ class OrganizationNotifications extends Component { constructor (props) { super(props); this.state = { - hasContentError: false, + contentError: null, hasContentLoading: true, - toggleError: false, + toggleError: null, toggleLoading: false, itemCount: 0, notifications: [], @@ -55,7 +56,7 @@ class OrganizationNotifications extends Component { const { id, location } = this.props; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data: { @@ -85,8 +86,8 @@ class OrganizationNotifications extends Component { successTemplateIds: successTemplates.results.map(s => s.id), errorTemplateIds: errorTemplates.results.map(e => e.id), }); - } catch { - this.setState({ hasContentError: true }); + } catch (err) { + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -125,20 +126,20 @@ class OrganizationNotifications extends Component { ); this.setState(stateUpdateFunction); } catch (err) { - this.setState({ toggleError: true }); + this.setState({ toggleError: err }); } finally { this.setState({ toggleLoading: false }); } } handleNotificationErrorClose () { - this.setState({ toggleError: false }); + this.setState({ toggleError: null }); } render () { const { canToggleNotifications, i18n } = this.props; const { - hasContentError, + contentError, hasContentLoading, toggleError, toggleLoading, @@ -151,7 +152,7 @@ class OrganizationNotifications extends Component { return ( {i18n._(t`Failed to toggle notification.`)} + ); diff --git a/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap b/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap index 54295cb524..1e0c33d06d 100644 --- a/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/src/screens/Organization/OrganizationNotifications/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -39,7 +39,7 @@ exports[` initially renders succesfully 1`] = ` } > initially renders succesfully 1`] = ` withHash={true} > initially renders succesfully 1`] = ` > initially renders succesfully 1`] = ` <_default - isOpen={false} + isOpen={null} onClose={[Function]} title="Error!" variant="danger" diff --git a/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx b/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx index 2710f4ce53..44c01139e7 100644 --- a/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx +++ b/src/screens/Organization/OrganizationTeams/OrganizationTeams.jsx @@ -19,7 +19,7 @@ class OrganizationTeams extends React.Component { this.loadOrganizationTeamsList = this.loadOrganizationTeamsList.bind(this); this.state = { - hasContentError: false, + contentError: null, hasContentLoading: true, itemCount: 0, teams: [], @@ -41,7 +41,7 @@ class OrganizationTeams extends React.Component { const { id, location } = this.props; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - this.setState({ hasContentLoading: true, hasContentError: false }); + this.setState({ hasContentLoading: true, contentError: null }); try { const { data: { count = 0, results = [] }, @@ -50,18 +50,18 @@ class OrganizationTeams extends React.Component { itemCount: count, teams: results, }); - } catch { - this.setState({ hasContentError: true }); + } catch (err) { + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } } render () { - const { hasContentError, hasContentLoading, teams, itemCount } = this.state; + const { contentError, hasContentLoading, teams, itemCount } = this.state; return ( ); + return (); } if (hasContentLoading) { diff --git a/src/screens/Template/Template.jsx b/src/screens/Template/Template.jsx index 37d421574b..11e7e6186d 100644 --- a/src/screens/Template/Template.jsx +++ b/src/screens/Template/Template.jsx @@ -24,7 +24,7 @@ class Template extends Component { super(props); this.state = { - hasContentError: false, + contentError: null, hasContentLoading: true, template: null, }; @@ -46,15 +46,13 @@ class Template extends Component { const { setBreadcrumb, match } = this.props; const { id } = match.params; - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data } = await JobTemplatesAPI.readDetail(id); setBreadcrumb(data); - this.setState({ - template: data, - }); - } catch { - this.setState({ hasContentError: true }); + this.setState({ template: data }); + } catch (err) { + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -68,7 +66,7 @@ class Template extends Component { match, } = this.props; const { - hasContentError, + contentError, hasContentLoading, template } = this.state; @@ -98,11 +96,11 @@ class Template extends Component { cardHeader = null; } - if (!hasContentLoading && hasContentError) { + if (!hasContentLoading && contentError) { return ( - + ); diff --git a/src/screens/Template/TemplateList/TemplateList.jsx b/src/screens/Template/TemplateList/TemplateList.jsx index cd5c92d159..58c5d623dd 100644 --- a/src/screens/Template/TemplateList/TemplateList.jsx +++ b/src/screens/Template/TemplateList/TemplateList.jsx @@ -15,6 +15,7 @@ import { } from '@api'; import AlertModal from '@components/AlertModal'; import DatalistToolbar from '@components/DataListToolbar'; +import ErrorDetail from '@components/ErrorDetail'; import PaginatedDataList, { ToolbarDeleteButton } from '@components/PaginatedDataList'; @@ -37,8 +38,8 @@ class TemplatesList extends Component { this.state = { hasContentLoading: true, - hasContentError: false, - hasDeletionError: false, + contentError: null, + deletionError: null, selected: [], templates: [], itemCount: 0, @@ -62,7 +63,7 @@ class TemplatesList extends Component { } handleDeleteErrorClose () { - this.setState({ hasDeletionError: false }); + this.setState({ deletionError: null }); } handleSelectAll (isSelected) { @@ -83,7 +84,7 @@ class TemplatesList extends Component { async handleTemplateDelete () { const { selected } = this.state; - this.setState({ hasContentLoading: true, hasDeletionError: false }); + this.setState({ hasContentLoading: true }); try { await Promise.all(selected.map(({ type, id }) => { let deletePromise; @@ -95,7 +96,7 @@ class TemplatesList extends Component { return deletePromise; })); } catch (err) { - this.setState({ hasDeletionError: true }); + this.setState({ deletionError: err }); } finally { await this.loadTemplates(); } @@ -105,7 +106,7 @@ class TemplatesList extends Component { const { location } = this.props; const params = parseNamespacedQueryString(QS_CONFIG, location.search); - this.setState({ hasContentError: false, hasContentLoading: true }); + this.setState({ contentError: null, hasContentLoading: true }); try { const { data: { count, results } } = await UnifiedJobTemplatesAPI.read(params); this.setState({ @@ -114,7 +115,7 @@ class TemplatesList extends Component { selected: [], }); } catch (err) { - this.setState({ hasContentError: true }); + this.setState({ contentError: err }); } finally { this.setState({ hasContentLoading: false }); } @@ -122,9 +123,9 @@ class TemplatesList extends Component { render () { const { - hasContentError, + contentError, hasContentLoading, - hasDeletionError, + deletionError, templates, itemCount, selected, @@ -139,7 +140,7 @@ class TemplatesList extends Component { {i18n._(t`Failed to delete one or more template.`)} + ); diff --git a/src/screens/Template/TemplateList/TemplatesList.test.jsx b/src/screens/Template/TemplateList/TemplatesList.test.jsx index 0230102f9a..f4b6d4f0ae 100644 --- a/src/screens/Template/TemplateList/TemplatesList.test.jsx +++ b/src/screens/Template/TemplateList/TemplatesList.test.jsx @@ -151,7 +151,15 @@ describe('', () => { }); test('error is shown when template not successfully deleted from api', async () => { - JobTemplatesAPI.destroy = () => Promise.reject(); + JobTemplatesAPI.destroy.mockRejectedValue(new Error({ + response: { + config: { + method: 'delete', + url: '/api/v2/job_templates/1' + }, + data: 'An error occurred' + } + })); const wrapper = mountWithContexts(); wrapper.find('TemplatesList').setState({ templates: mockTemplates,