From af3419c2dda8e9e09c0caff34604a76461e036d3 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 8 Apr 2019 11:46:11 -0400 Subject: [PATCH 01/20] update eslint to allow short circuit ternary chains --- .eslintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc b/.eslintrc index 2131031f6b..463676d62b 100644 --- a/.eslintrc +++ b/.eslintrc @@ -48,6 +48,7 @@ "object-curly-newline": "off", "space-before-function-paren": ["error", "always"], "no-trailing-spaces": ["error"], + "no-unused-expressions": ["error", { "allowShortCircuit": true }], "react/prefer-stateless-function": "off", "react/prop-types": "off", "jsx-a11y/label-has-for": "off", From e20cf72dd6578b9006e72079b61005de28998ba0 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 8 Apr 2019 12:17:34 -0400 Subject: [PATCH 02/20] add AlertModal component and update styling of delete confirmations --- src/app.scss | 91 ++++++++++++++++--- src/components/AlertModal.jsx | 28 ++++++ .../components/OrganizationAccessList.jsx | 18 ++-- .../screens/OrganizationsList.jsx | 60 +++--------- 4 files changed, 129 insertions(+), 68 deletions(-) create mode 100644 src/components/AlertModal.jsx diff --git a/src/app.scss b/src/app.scss index 1137b3ffd5..08b66f24c0 100644 --- a/src/app.scss +++ b/src/app.scss @@ -6,6 +6,7 @@ // // masthead overrides // + .pf-c-page__header-brand { max-width: 255px; } @@ -114,8 +115,9 @@ // // switch overrides -// // https://github.com/patternfly/patternfly-next/issues/915 +// + .pf-c-switch { .pf-c-switch__label::before { display: none; @@ -145,6 +147,7 @@ .awx-c-modal.pf-c-modal-box { margin: 0; + padding: 24px; width: 600px; .pf-c-modal-box__body { @@ -231,13 +234,21 @@ border-bottom: 1px solid #d7d7d7; } +.at-c-listCardBody { + --pf-c-card__footer--PaddingX: 0; + --pf-c-card__footer--PaddingY: 0; + --pf-c-card__body--PaddingX: 0; + --pf-c-card__body--PaddingY: 0; +} + .awx-c-card { position: relative; } - - +// // PF Alert notification component overrides +// + .pf-c-alert__title { --pf-c-alert__title--PaddingTop: 20px; --pf-c-alert__title--PaddingRight: 20px; @@ -272,17 +283,6 @@ margin-bottom: 10px; } -.orgListAlert-actionBtn{ - margin:0 10px; -} -.orgListDetete-progressBar{ - padding-right: 32px; -} -.orgListDelete-progressBar-noShow{ - display: none; - padding-right: 32px; -} - .awx-c-form-action-group { float: right; display: block; @@ -293,3 +293,66 @@ } } +// +// AlertModal styles +// + +.at-c-alertModal.pf-c-modal-box { + border: 0; + border-left: 56px solid black; + + .at-c-alertModal__icon { + position: absolute; + font-size: 23px; + top: 28px; + left: -39px; + } +} + +.at-c-alertModal--warning.pf-c-modal-box { + border-color: var(--pf-global--warning-color--100); + + .pf-c-title { + color: var(--pf-global--warning-color--200); + } + + .at-c-alertModal__icon { + color: var(--pf-global--warning-color--200); + } +} + +.at-c-alertModal--danger.pf-c-modal-box { + border-color: var(--pf-global--danger-color--100); + + .pf-c-title { + color: var(--pf-global--danger-color--200); + } + + .at-c-alertModal__icon { + color: white; + } +} + +.at-c-alertModal--info.pf-c-modal-box { + border-color: var(--pf-global--info-color--100); + + .pf-c-title { + color: var(--pf-global--info-color--200); + } + + .at-c-alertModal__icon { + color: var(--pf-global--info-color--200); + } +} + +.at-c-alertModal--success.pf-c-modal-box { + border-color: var(--pf-global--success-color--100); + + .pf-c-title { + color: var(--pf-global--success-color--200); + } + + .at-c-alertModal__icon { + color: var(--pf-global--success-color--200); + } +} diff --git a/src/components/AlertModal.jsx b/src/components/AlertModal.jsx new file mode 100644 index 0000000000..264e9cf355 --- /dev/null +++ b/src/components/AlertModal.jsx @@ -0,0 +1,28 @@ +import React from 'react'; + +import { + Modal +} from '@patternfly/react-core'; + +import { ExclamationTriangleIcon, ExclamationCircleIcon, InfoCircleIcon, CheckCircleIcon } from '@patternfly/react-icons'; + +const getIcon = (variant) => { + let icon; + if (variant === 'warning') { + icon = (); + } else if (variant === 'danger') { + icon = (); + } if (variant === 'info') { + icon = (); + } if (variant === 'success') { + icon = (); + } + return icon; +}; + +export default ({ variant, children, ...props }) => ( + + {children} + {getIcon(variant)} + +); diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index abc289ed96..7e31033025 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { DataList, DataListItem, DataListCell, Text, - TextContent, TextVariants, Chip, Alert, AlertActionCloseButton, Button + TextContent, TextVariants, Chip, Button } from '@patternfly/react-core'; import { I18n, i18nMark } from '@lingui/react'; @@ -13,6 +13,7 @@ import { Link } from 'react-router-dom'; +import AlertModal from '../../../components/AlertModal'; import Pagination from '../../../components/Pagination'; import DataListToolbar from '../../../components/DataListToolbar'; @@ -333,17 +334,18 @@ class OrganizationAccessList extends React.Component { showExpandCollapse /> {showWarning && ( - } + isOpen={showWarning} + onClose={this.hideWarning} + actions={[ + , + + ]} > {warningMsg} - - - - - + )} {results.map(result => ( diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 8b38d8fe95..2f50887c79 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -5,26 +5,26 @@ import React, { import { withRouter } from 'react-router-dom'; + import { I18n, i18nMark } from '@lingui/react'; import { Trans, t } from '@lingui/macro'; + import { Card, EmptyState, EmptyStateIcon, EmptyStateBody, - Modal, PageSection, PageSectionVariants, Title, - Button, - Progress, - ProgressVariant + Button } from '@patternfly/react-core'; import { CubesIcon } from '@patternfly/react-icons'; import DataListToolbar from '../../../components/DataListToolbar'; import OrganizationListItem from '../components/OrganizationListItem'; import Pagination from '../../../components/Pagination'; +import AlertModal from '../../../components/AlertModal'; import { encodeQueryString, @@ -61,8 +61,6 @@ class OrganizationsList extends Component { selected: [], isModalOpen: false, orgsToDelete: [], - orgsDeleted: [], - deleteSuccess: false, }; @@ -145,10 +143,7 @@ class OrganizationsList extends Component { handleClearOrgsToDelete () { this.setState({ isModalOpen: false, - orgsDeleted: [], - deleteSuccess: false, - orgsToDelete: [], - deleteStarted: false + orgsToDelete: [] }); this.onSelectAll(); } @@ -175,23 +170,13 @@ class OrganizationsList extends Component { } async handleOrgDelete (event) { - const { orgsToDelete, orgsDeleted } = this.state; + const { orgsToDelete } = this.state; const { api } = this.props; - this.setState({ deleteStarted: true }); orgsToDelete.forEach(async (org) => { - try { - const res = await api.destroyOrganization(org.id); - this.setState({ - orgsDeleted: orgsDeleted.concat(res) - }); - } catch { - this.setState({ deleteSuccess: false }); - } finally { - this.setState({ deleteSuccess: true }); + await api.destroyOrganization(org.id); const queryParams = this.getQueryParams(); this.fetchOrganizations(queryParams); - } }); event.preventDefault(); } @@ -261,12 +246,9 @@ class OrganizationsList extends Component { const { count, error, - deleteSuccess, - deleteStarted, loading, noInitialResults, orgsToDelete, - orgsDeleted, page, pageCount, page_size, @@ -283,13 +265,15 @@ class OrganizationsList extends Component { { isModalOpen && ( - Delete, + + ]} > {warningMsg}
@@ -301,24 +285,8 @@ class OrganizationsList extends Component {
))} -
- -

-
- {orgsDeleted.length - ? - : ( - - - - - )} -
- + )} {noInitialResults && ( From aea4a04c66b49635eed07b2fe37ea50df03ed7a8 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 8 Apr 2019 12:19:13 -0400 Subject: [PATCH 03/20] add RootDialog and Network contexts, update app bootstrapping --- src/App.jsx | 165 +++++++------- src/RootProvider.jsx | 44 ++++ src/context.jsx | 4 - src/contexts/Config.jsx | 89 ++++++++ src/contexts/Network.jsx | 84 ++++++++ src/contexts/RootDialog.jsx | 53 +++++ src/index.jsx | 420 ++++++++++++++++-------------------- 7 files changed, 544 insertions(+), 315 deletions(-) create mode 100644 src/RootProvider.jsx delete mode 100644 src/context.jsx create mode 100644 src/contexts/Config.jsx create mode 100644 src/contexts/Network.jsx create mode 100644 src/contexts/RootDialog.jsx diff --git a/src/App.jsx b/src/App.jsx index e4fdd54243..7e4699e33d 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -6,13 +6,20 @@ import { Page, PageHeader, PageSidebar, + Button } from '@patternfly/react-core'; +import { I18n } from '@lingui/react'; +import { t } from '@lingui/macro'; + +import { RootDialog } from './contexts/RootDialog'; +import { withNetwork } from './contexts/Network'; + +import AlertModal from './components/AlertModal'; import About from './components/About'; import NavExpandableGroup from './components/NavExpandableGroup'; import TowerLogo from './components/TowerLogo'; import PageHeaderToolbar from './components/PageHeaderToolbar'; -import { ConfigContext } from './context'; class App extends Component { constructor (props) { @@ -23,29 +30,24 @@ class App extends Component { && window.innerWidth >= parseInt(global_breakpoint_md.value, 10); this.state = { - ansible_version: null, - custom_virtualenvs: null, isAboutModalOpen: false, - isNavOpen, - version: null, + isNavOpen }; - this.fetchConfig = this.fetchConfig.bind(this); this.onLogout = this.onLogout.bind(this); this.onAboutModalClose = this.onAboutModalClose.bind(this); this.onAboutModalOpen = this.onAboutModalOpen.bind(this); this.onNavToggle = this.onNavToggle.bind(this); } - componentDidMount () { - this.fetchConfig(); - } - async onLogout () { - const { api } = this.props; - - await api.logout(); - window.location.replace('/#/login'); + const { api, handleHttpError } = this.props; + try { + await api.logout(); + window.location.replace('/#/login'); + } catch (err) { + handleHttpError(err); + } } onAboutModalOpen () { @@ -60,24 +62,12 @@ class App extends Component { this.setState(({ isNavOpen }) => ({ isNavOpen: !isNavOpen })); } - async fetchConfig () { - const { api } = this.props; - - try { - const { data: { ansible_version, custom_virtualenvs, version } } = await api.getConfig(); - this.setState({ ansible_version, custom_virtualenvs, version }); - } catch (err) { - this.setState({ ansible_version: null, custom_virtualenvs: null, version: null }); - } - } - render () { const { ansible_version, - custom_virtualenvs, isAboutModalOpen, isNavOpen, - version, + version } = this.state; const { @@ -86,63 +76,76 @@ class App extends Component { navLabel = '', } = this.props; - const config = { - ansible_version, - custom_virtualenvs, - version, - }; - return ( - - } - toolbar={( - + {({ i18n }) => ( + + {({ title, bodyText, variant = 'info', clearRootDialogMessage }) => ( + + {(title || bodyText) && ( + {i18n._(t`Close`)} + ]} + > + {bodyText} + + )} + } + toolbar={( + + )} + /> + )} + sidebar={( + + + {routeGroups.map(({ groupId, groupTitle, routes }) => ( + + ))} + + + )} + /> + )} + > + {render && render({ routeGroups })} + + - )} - /> - )} - sidebar={( - - - {routeGroups.map(({ groupId, groupTitle, routes }) => ( - - ))} - - - )} - /> - )} - > - - {render && render({ routeGroups })} - - - - + + )} + + )} + ); } } -export default App; +export default withNetwork(App); diff --git a/src/RootProvider.jsx b/src/RootProvider.jsx new file mode 100644 index 0000000000..7a868e63a3 --- /dev/null +++ b/src/RootProvider.jsx @@ -0,0 +1,44 @@ +import React, { Component } from 'react'; +import { + I18nProvider, +} from '@lingui/react'; + +import { NetworkProvider } from './contexts/Network'; +import { RootDialogProvider } from './contexts/RootDialog'; +import { ConfigProvider } from './contexts/Config'; + +import ja from '../build/locales/ja/messages'; +import en from '../build/locales/en/messages'; + +export function getLanguage (nav) { + const language = (nav.languages && nav.languages[0]) || nav.language || nav.userLanguage; + const languageWithoutRegionCode = language.toLowerCase().split(/[_-]+/)[0]; + + return languageWithoutRegionCode; +} + +class RootProvider extends Component { + render () { + const { children } = this.props; + + const catalogs = { en, ja }; + const language = getLanguage(navigator); + + return ( + + + + + {children} + + + + + ); + } +} + +export default RootProvider; diff --git a/src/context.jsx b/src/context.jsx deleted file mode 100644 index ab413dd313..0000000000 --- a/src/context.jsx +++ /dev/null @@ -1,4 +0,0 @@ -import React from 'react'; - -// eslint-disable-next-line import/prefer-default-export -export const ConfigContext = React.createContext({}); diff --git a/src/contexts/Config.jsx b/src/contexts/Config.jsx new file mode 100644 index 0000000000..6129f80413 --- /dev/null +++ b/src/contexts/Config.jsx @@ -0,0 +1,89 @@ + +import React, { Component } from 'react'; + +import { withNetwork } from './Network'; + +const ConfigContext = React.createContext({}); + +class provider extends Component { + constructor (props) { + super(props); + + this.state = { + value: { + ansible_version: null, + custom_virtualenvs: null, + version: null, + custom_logo: null, + custom_login_info: null + } + }; + + this.fetchConfig = this.fetchConfig.bind(this); + } + + componentDidMount () { + this.fetchConfig(); + } + + async fetchConfig () { + const { api, handleHttpError } = this.props; + + try { + const { + data: { + ansible_version, + custom_virtualenvs, + version + } + } = await api.getConfig(); + const { + data: { + custom_logo, + custom_login_info + } + } = await api.getRoot(); + this.setState({ + value: { + ansible_version, + custom_virtualenvs, + version, + custom_logo, + custom_login_info + } + }); + } catch (err) { + handleHttpError(err) || this.setState({ + value: { + ansible_version: null, + custom_virtualenvs: null, + version: null, + custom_logo: null, + custom_login_info: null + } + }); + } + } + + render () { + const { + value + } = this.state; + + const { children } = this.props; + + return ( + + {children} + + ); + } +} + +export const ConfigProvider = withNetwork(provider); + +export const Config = ({ children }) => ( + + {value => children(value)} + +); diff --git a/src/contexts/Network.jsx b/src/contexts/Network.jsx new file mode 100644 index 0000000000..c00d5b2895 --- /dev/null +++ b/src/contexts/Network.jsx @@ -0,0 +1,84 @@ + +import axios from 'axios'; +import React, { Component } from 'react'; + +import { withRouter } from 'react-router-dom'; + +import { withRootDialog } from './RootDialog'; + +import APIClient from '../api'; + +const NetworkContext = React.createContext({}); + +class prov extends Component { + constructor (props) { + super(props); + + this.state = { + value: { + api: new APIClient(axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken' })), + handleHttpError: err => { + if (err.response.status === 401) { + this.handle401(); + } else if (err.response.status === 404) { + this.handle404(); + } + return (err.response.status === 401 || err.response.status === 404); + } + } + }; + } + + handle401 () { + const { handle401, history, setRootDialogMessage } = this.props; + if (handle401) { + handle401(); + return; + } + history.replace('/login'); + setRootDialogMessage({ + bodyText: 'You have been logged out.', + }); + } + + handle404 () { + const { handle404, history, setRootDialogMessage } = this.props; + if (handle404) { + handle404(); + return; + } + history.replace('/home'); + setRootDialogMessage({ + title: '404', + bodyText: 'Cannot find resource.', + variant: 'warning' + }); + } + + render () { + const { + children + } = this.props; + + const { + value + } = this.state; + + return ( + + {children} + + ); + } +} + +export const NetworkProvider = withRootDialog(withRouter(prov)); + +export function withNetwork (Child) { + return (props) => ( + + {context => } + + ); +} + diff --git a/src/contexts/RootDialog.jsx b/src/contexts/RootDialog.jsx new file mode 100644 index 0000000000..089c59087f --- /dev/null +++ b/src/contexts/RootDialog.jsx @@ -0,0 +1,53 @@ +import React, { Component } from 'react'; + +const RootDialogContext = React.createContext({}); + +export class RootDialogProvider extends Component { + constructor (props) { + super(props); + + this.state = { + value: { + title: null, + setRootDialogMessage: ({ title, bodyText, variant }) => { + const { value } = this.state; + this.setState({ value: { ...value, title, bodyText, variant } }); + }, + clearRootDialogMessage: () => { + const { value } = this.state; + this.setState({ value: { ...value, title: null, bodyText: null, variant: null } }); + } + } + }; + } + + render () { + const { + children + } = this.props; + + const { + value + } = this.state; + + return ( + + {children} + + ); + } +} + +export const RootDialog = ({ children }) => ( + + {value => children(value)} + +); + +export function withRootDialog (Child) { + return (props) => ( + + {context => } + + ); +} diff --git a/src/index.jsx b/src/index.jsx index d3dd6407e6..9f542327a5 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -1,15 +1,13 @@ -import axios from 'axios'; import React from 'react'; import ReactDOM from 'react-dom'; import { HashRouter, - Redirect, Route, Switch, + Redirect } from 'react-router-dom'; import { - I18n, - I18nProvider, + I18n } from '@lingui/react'; import { t } from '@lingui/macro'; @@ -19,10 +17,13 @@ import './components/Pagination/styles.scss'; import './components/DataListToolbar/styles.scss'; import './components/SelectedList/styles.scss'; -import APIClient from './api'; +import { Config } from './contexts/Config'; -import App from './App'; import Background from './components/Background'; + +import RootProvider from './RootProvider'; +import App from './App'; + import Applications from './pages/Applications'; import Credentials from './pages/Credentials'; import CredentialTypes from './pages/CredentialTypes'; @@ -46,242 +47,201 @@ import License from './pages/License'; import Teams from './pages/Teams'; import Templates from './pages/Templates'; import Users from './pages/Users'; -import ja from '../build/locales/ja/messages'; -import en from '../build/locales/en/messages'; - -// -// Initialize http -// - -const http = axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken' }); - -// -// Derive the language and region from global user agent data. Example: es-US -// see: https://developer.mozilla.org/en-US/docs/Web/API/Navigator -// - -export function getLanguage (nav) { - const language = (nav.languages && nav.languages[0]) || nav.language || nav.userLanguage; - const languageWithoutRegionCode = language.toLowerCase().split(/[_-]+/)[0]; - - return languageWithoutRegionCode; -} - -// -// Function Main -// - -export async function main (render, api) { - const catalogs = { en, ja }; - const language = getLanguage(navigator); +// eslint-disable-next-line import/prefer-default-export +export async function main (render) { const el = document.getElementById('app'); - const { data: { custom_logo, custom_login_info } } = await api.getRoot(); - - const defaultRedirect = () => (); - const loginRoutes = ( - - ( - - )} - /> - - - ); return render( - + {({ i18n }) => ( - {!api.isAuthenticated() ? loginRoutes : ( - - - - ( - ( - routeGroups - .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) - .map(({ component: PageComponent, path }) => ( - ( - - )} - /> - )) - )} - /> - )} - /> - - )} + + ( + + {({ custom_logo, custom_login_info }) => ( + + )} + + )} + /> + } /> + ( + ( + routeGroups + .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) + .map(({ component: PageComponent, path }) => ( + ( + + )} + /> + )) + )} + /> + )} + /> + )} - + , el ); } -main(ReactDOM.render, new APIClient(http)); +main(ReactDOM.render); From 722ae932ab595e7641b14fb72b0086eae5ece50e Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 8 Apr 2019 12:20:44 -0400 Subject: [PATCH 04/20] update login modal to grab error from RootDialog --- src/app.scss | 8 ++++++++ src/pages/Login.jsx | 31 +++++++++++++++++-------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/app.scss b/src/app.scss index 08b66f24c0..20df8aa24b 100644 --- a/src/app.scss +++ b/src/app.scss @@ -356,3 +356,11 @@ color: var(--pf-global--success-color--200); } } + +// +// LoginModal overrides +// + +.pf-m-error p.pf-c-form__helper-text { + color: var(--pf-global--danger-color--100); +} diff --git a/src/pages/Login.jsx b/src/pages/Login.jsx index 4759f285c7..2a216ed505 100644 --- a/src/pages/Login.jsx +++ b/src/pages/Login.jsx @@ -1,5 +1,5 @@ import React, { Component } from 'react'; -import { Redirect } from 'react-router-dom'; +import { Redirect, withRouter } from 'react-router-dom'; import { I18n } from '@lingui/react'; import { t } from '@lingui/macro'; import { @@ -7,6 +7,9 @@ import { LoginPage, } from '@patternfly/react-core'; +import { withRootDialog } from '../contexts/RootDialog'; +import { withNetwork } from '../contexts/Network'; + import towerLogo from '../../images/tower-logo-header.svg'; class AWXLogin extends Component { @@ -17,7 +20,8 @@ class AWXLogin extends Component { username: '', password: '', isInputValid: true, - isLoading: false + isLoading: false, + isAuthenticated: false }; this.onChangeUsername = this.onChangeUsername.bind(this); @@ -35,7 +39,7 @@ class AWXLogin extends Component { async onLoginButtonClick (event) { const { username, password, isLoading } = this.state; - const { api } = this.props; + const { api, handleHttpError, clearRootDialogMessage } = this.props; event.preventDefault(); @@ -43,25 +47,23 @@ class AWXLogin extends Component { return; } + clearRootDialogMessage(); this.setState({ isLoading: true }); try { await api.login(username, password); + this.setState({ isAuthenticated: true, isLoading: false }); } catch (error) { - if (error.response && error.response.status === 401) { - this.setState({ isInputValid: false }); - } - } finally { - this.setState({ isLoading: false }); + handleHttpError(error) || this.setState({ isInputValid: false, isLoading: false }); } } render () { - const { username, password, isInputValid } = this.state; - const { api, alt, loginInfo, logo } = this.props; + const { username, password, isInputValid, isAuthenticated } = this.state; + const { alt, loginInfo, logo, bodyText: errorMessage } = this.props; const logoSrc = logo ? `data:image/jpeg;${logo}` : towerLogo; - if (api.isAuthenticated()) { + if (isAuthenticated) { return (); } @@ -75,10 +77,11 @@ class AWXLogin extends Component { textContent={loginInfo} > Date: Mon, 8 Apr 2019 12:34:02 -0400 Subject: [PATCH 05/20] update api calls to utilized network context --- src/components/Lookup/Lookup.jsx | 8 +- .../NotificationsList/Notifications.list.jsx | 74 +++++++++++-------- src/pages/Organizations/Organizations.jsx | 46 +++++++----- .../components/InstanceGroupsLookup.jsx | 7 +- .../components/OrganizationAccessList.jsx | 18 ++--- .../components/OrganizationForm.jsx | 13 ++-- .../screens/Organization/Organization.jsx | 32 ++++---- .../Organization/OrganizationAccess.jsx | 5 +- .../Organization/OrganizationDetail.jsx | 9 ++- .../screens/Organization/OrganizationEdit.jsx | 19 ++--- .../OrganizationNotifications.jsx | 4 +- .../Organization/OrganizationTeams.jsx | 10 +-- .../Organizations/screens/OrganizationAdd.jsx | 13 ++-- .../screens/OrganizationsList.jsx | 31 +++++--- 14 files changed, 163 insertions(+), 126 deletions(-) diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index c65eee0935..ec189fb1b2 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -15,6 +15,8 @@ import { import { I18n } from '@lingui/react'; import { Trans, t } from '@lingui/macro'; +import { withNetwork } from '../../contexts/Network'; + import CheckboxListItem from '../ListItem'; import DataListToolbar from '../DataListToolbar'; import SelectedList from '../SelectedList'; @@ -67,7 +69,7 @@ class Lookup extends React.Component { } async getData () { - const { getItems } = this.props; + const { getItems, handleHttpError } = this.props; const { page, page_size, sortedColumnKey, sortOrder } = this.state; this.setState({ error: false }); @@ -92,7 +94,7 @@ class Lookup extends React.Component { this.setState(stateToUpdate); } catch (err) { - this.setState({ error: true }); + handleHttpError(err) || this.setState({ error: true }); } } @@ -273,4 +275,4 @@ Lookup.defaultProps = { name: null, }; -export default Lookup; +export default withNetwork(Lookup); diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index 80ed1c2407..c3c44381ca 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -8,6 +8,8 @@ import { CubesIcon } from '@patternfly/react-icons'; import { I18n, i18nMark } from '@lingui/react'; import { Trans, t } from '@lingui/macro'; +import { withNetwork } from '../../contexts/Network'; + import DataListToolbar from '../DataListToolbar'; import NotificationListItem from './NotificationListItem'; import Pagination from '../Pagination'; @@ -117,58 +119,71 @@ class Notifications extends Component { } async createError (id, isCurrentlyOn) { - const { onCreateError, match } = this.props; + const { onCreateError, match, handleHttpError } = this.props; const postParams = { id }; + let errorHandled; if (isCurrentlyOn) { postParams.disassociate = true; } try { await onCreateError(match.params.id, postParams); } catch (err) { - this.setState({ error: true }); + errorHandled = handleHttpError(err); + if (!errorHandled) { + this.setState({ error: true }); + } } finally { - if (isCurrentlyOn) { - // Remove it from state - this.setState((prevState) => ({ - errorTemplateIds: prevState.errorTemplateIds.filter((templateId) => templateId !== id) - })); - } else { - // Add it to state - this.setState(prevState => ({ - errorTemplateIds: [...prevState.errorTemplateIds, id] - })); + if (!errorHandled) { + if (isCurrentlyOn) { + // Remove it from state + this.setState((prevState) => ({ + errorTemplateIds: prevState.errorTemplateIds.filter((templateId) => templateId !== id) + })); + } else { + // Add it to state + this.setState(prevState => ({ + errorTemplateIds: [...prevState.errorTemplateIds, id] + })); + } } } } async createSuccess (id, isCurrentlyOn) { - const { onCreateSuccess, match } = this.props; + const { onCreateSuccess, match, handleHttpError } = this.props; const postParams = { id }; + let errorHandled; if (isCurrentlyOn) { postParams.disassociate = true; } try { await onCreateSuccess(match.params.id, postParams); } catch (err) { - this.setState({ error: true }); + errorHandled = handleHttpError(err); + if (!errorHandled) { + this.setState({ error: true }); + } } finally { - if (isCurrentlyOn) { - // Remove it from state - this.setState((prevState) => ({ - successTemplateIds: prevState.successTemplateIds.filter((templateId) => templateId !== id) - })); - } else { - // Add it to state - this.setState(prevState => ({ - successTemplateIds: [...prevState.successTemplateIds, id] - })); + if (!errorHandled) { + if (isCurrentlyOn) { + // Remove it from state + this.setState((prevState) => ({ + successTemplateIds: prevState.successTemplateIds + .filter((templateId) => templateId !== id) + })); + } else { + // Add it to state + this.setState(prevState => ({ + successTemplateIds: [...prevState.successTemplateIds, id] + })); + } } } } async readNotifications (queryParams) { const { noInitialResults } = this.state; - const { onReadNotifications, onReadSuccess, onReadError, match } = this.props; + const { onReadNotifications, onReadSuccess, onReadError, match, handleHttpError } = this.props; const { page, page_size, order_by } = queryParams; let sortOrder = 'ascending'; @@ -233,12 +248,11 @@ class Notifications extends Component { } this.setState({ successTemplateIds, - errorTemplateIds + errorTemplateIds, + loading: false }); } catch (err) { - this.setState({ error: true }); - } finally { - this.setState({ loading: false }); + handleHttpError(err) || this.setState({ error: true, loading: false }); } } @@ -329,4 +343,4 @@ Notifications.propTypes = { onCreateSuccess: PropTypes.func.isRequired, }; -export default Notifications; +export default withNetwork(Notifications); diff --git a/src/pages/Organizations/Organizations.jsx b/src/pages/Organizations/Organizations.jsx index 7b5d41377e..28ac5864ab 100644 --- a/src/pages/Organizations/Organizations.jsx +++ b/src/pages/Organizations/Organizations.jsx @@ -1,11 +1,15 @@ import React, { Component, Fragment } from 'react'; -import { Route, Switch } from 'react-router-dom'; +import { Route, withRouter, Switch } from 'react-router-dom'; import { i18nMark } from '@lingui/react'; +import { NetworkProvider } from '../../contexts/Network'; +import { withRootDialog } from '../../contexts/RootDialog'; + +import Breadcrumbs from '../../components/Breadcrumbs/Breadcrumbs'; + import OrganizationsList from './screens/OrganizationsList'; import OrganizationAdd from './screens/OrganizationAdd'; import Organization from './screens/Organization/Organization'; -import Breadcrumbs from '../../components/Breadcrumbs/Breadcrumbs'; class Organizations extends Component { state = { @@ -13,7 +17,7 @@ class Organizations extends Component { '/organizations': i18nMark('Organizations'), '/organizations/add': i18nMark('Create New Organization') } - } + }; setBreadcrumbConfig = (organization) => { if (!organization) { @@ -35,7 +39,7 @@ class Organizations extends Component { } render () { - const { match, api, history, location } = this.props; + const { match, history, location, setRootDialogMessage } = this.props; const { breadcrumbConfig } = this.state; return ( @@ -47,28 +51,34 @@ class Organizations extends Component { ( - + )} /> ( - + render={({ match: newRouteMatch }) => ( + { + history.replace('/organizations'); + setRootDialogMessage({ + title: '404', + bodyText: `Cannot find organization with ID ${newRouteMatch.params.id}.`, + variant: 'warning' + }); + }} + > + + )} /> ( - + )} /> @@ -77,4 +87,4 @@ class Organizations extends Component { } } -export default Organizations; +export default withRootDialog(withRouter(Organizations)); diff --git a/src/pages/Organizations/components/InstanceGroupsLookup.jsx b/src/pages/Organizations/components/InstanceGroupsLookup.jsx index ac48d25972..703fd2420c 100644 --- a/src/pages/Organizations/components/InstanceGroupsLookup.jsx +++ b/src/pages/Organizations/components/InstanceGroupsLookup.jsx @@ -7,6 +7,8 @@ import { t } from '@lingui/macro'; import Lookup from '../../../components/Lookup'; +import { withNetwork } from '../../../contexts/Network'; + const INSTANCE_GROUPS_LOOKUP_COLUMNS = [ { name: i18nMark('Name'), key: 'name', isSortable: true }, { name: i18nMark('Modified'), key: 'modified', isSortable: false, isNumeric: true }, @@ -69,9 +71,6 @@ class InstanceGroupsLookup extends React.Component { } InstanceGroupsLookup.propTypes = { - api: PropTypes.shape({ - getInstanceGroups: PropTypes.func, - }).isRequired, value: PropTypes.arrayOf(PropTypes.object).isRequired, tooltip: PropTypes.string, onChange: PropTypes.func.isRequired, @@ -81,4 +80,4 @@ InstanceGroupsLookup.defaultProps = { tooltip: '', }; -export default InstanceGroupsLookup; +export default withNetwork(InstanceGroupsLookup); diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index 7e31033025..983bba1e01 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -13,6 +13,8 @@ import { Link } from 'react-router-dom'; +import { withNetwork } from '../../../contexts/Network'; + import AlertModal from '../../../components/AlertModal'; import Pagination from '../../../components/Pagination'; import DataListToolbar from '../../../components/DataListToolbar'; @@ -238,20 +240,16 @@ class OrganizationAccessList extends React.Component { } async removeAccessRole (roleId, resourceId, type) { - const { removeRole } = this.props; + const { removeRole, handleHttpError } = this.props; const url = `/api/v2/${type}/${resourceId}/roles/`; try { await removeRole(url, roleId); + const queryParams = this.getQueryParams(); + await this.fetchOrgAccessList(queryParams); + this.setState({ showWarning: false }); } catch (error) { - this.setState({ error }); + handleHttpError(error) || this.setState({ error }); } - const queryParams = this.getQueryParams(); - try { - this.fetchOrgAccessList(queryParams); - } catch (error) { - this.setState({ error }); - } - this.setState({ showWarning: false }); } handleWarning (roleName, roleId, resourceName, resourceId, type) { @@ -432,4 +430,4 @@ OrganizationAccessList.propTypes = { removeRole: PropTypes.func.isRequired, }; -export default OrganizationAccessList; +export default withNetwork(OrganizationAccessList); diff --git a/src/pages/Organizations/components/OrganizationForm.jsx b/src/pages/Organizations/components/OrganizationForm.jsx index 4000bf1a1c..37994f1bd6 100644 --- a/src/pages/Organizations/components/OrganizationForm.jsx +++ b/src/pages/Organizations/components/OrganizationForm.jsx @@ -9,7 +9,8 @@ import { FormGroup, } from '@patternfly/react-core'; -import { ConfigContext } from '../../../context'; +import { Config } from '../../../contexts/Config'; +import { withNetwork } from '../../../contexts/Network'; import FormRow from '../../../components/FormRow'; import FormField from '../../../components/FormField'; import FormActionGroup from '../../../components/FormActionGroup'; @@ -81,7 +82,7 @@ class OrganizationForm extends Component { } render () { - const { api, organization, handleCancel } = this.props; + const { organization, handleCancel } = this.props; const { instanceGroups, formIsValid, error } = this.state; const defaultVenv = '/venv/ansible/'; @@ -112,7 +113,7 @@ class OrganizationForm extends Component { type="text" label={i18n._(t`Description`)} /> - + {({ custom_virtualenvs }) => ( custom_virtualenvs && custom_virtualenvs.length > 1 && ( ) )} - + ( @@ -137,7 +138,6 @@ class Organization extends Component { path="/organizations/:id/details" render={() => ( @@ -148,7 +148,6 @@ class Organization extends Component { path="/organizations/:id/access" render={() => ( ( )} /> @@ -168,7 +169,6 @@ class Organization extends Component { path="/organizations/:id/notifications" render={() => ( { await api.associateInstanceGroup(instanceGroupsUrl, id); })); - } catch (err) { - this.setState({ error: err }); - } finally { this.handleSuccess(response.id); + } catch (err) { + handleHttpError(err) || this.setState({ error: err }); } } catch (err) { this.setState({ error: err }); @@ -58,7 +59,6 @@ class OrganizationAdd extends React.Component { } render () { - const { api } = this.props; const { error } = this.state; return ( @@ -82,7 +82,6 @@ class OrganizationAdd extends React.Component { @@ -105,4 +104,4 @@ OrganizationAdd.contextTypes = { }; export { OrganizationAdd as _OrganizationAdd }; -export default withRouter(OrganizationAdd); +export default withNetwork(withRouter(OrganizationAdd)); diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 2f50887c79..e1bfe618b4 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -21,6 +21,9 @@ import { } from '@patternfly/react-core'; import { CubesIcon } from '@patternfly/react-icons'; + +import { withNetwork } from '../../../contexts/Network'; + import DataListToolbar from '../../../components/DataListToolbar'; import OrganizationListItem from '../components/OrganizationListItem'; import Pagination from '../../../components/Pagination'; @@ -170,14 +173,21 @@ class OrganizationsList extends Component { } async handleOrgDelete (event) { - const { orgsToDelete } = this.state; + const { orgsToDelete, handleHttpError } = this.state; const { api } = this.props; + let errorHandled; - orgsToDelete.forEach(async (org) => { - await api.destroyOrganization(org.id); + try { + await Promise.all(orgsToDelete.map(async (org) => api.destroyOrganization(org.id))); + this.handleClearOrgsToDelete(); + } catch (err) { + errorHandled = handleHttpError(err); + } finally { + if (!errorHandled) { const queryParams = this.getQueryParams(); this.fetchOrganizations(queryParams); - }); + } + } event.preventDefault(); } @@ -192,7 +202,7 @@ class OrganizationsList extends Component { } async fetchOrganizations (queryParams) { - const { api } = this.props; + const { api, handleHttpError } = this.props; const { page, page_size, order_by } = queryParams; let sortOrder = 'ascending'; @@ -220,6 +230,7 @@ class OrganizationsList extends Component { sortedColumnKey, results, selected: [], + loading: false }; // This is in place to track whether or not the initial request @@ -233,9 +244,7 @@ class OrganizationsList extends Component { this.setState(stateToUpdate); this.updateUrl(queryParams); } catch (err) { - this.setState({ error: true }); - } finally { - this.setState({ loading: false }); + handleHttpError(err) || this.setState({ error: true, loading: false }); } } @@ -271,8 +280,8 @@ class OrganizationsList extends Component { isOpen={isModalOpen} onClose={this.handleClearOrgsToDelete} actions={[ - , - + , + ]} > {warningMsg} @@ -350,4 +359,4 @@ class OrganizationsList extends Component { } } -export default withRouter(OrganizationsList); +export default withNetwork(withRouter(OrganizationsList)); From 81267c7212b239d839c62e89ae0360116141fc16 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Mon, 8 Apr 2019 14:10:06 -0400 Subject: [PATCH 06/20] update language and stylign for all warning/danger modals --- src/contexts/Network.jsx | 8 +++-- src/pages/Organizations/Organizations.jsx | 9 ++++- .../components/OrganizationAccessList.jsx | 35 ++++++++++++++----- .../screens/OrganizationsList.jsx | 2 +- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/contexts/Network.jsx b/src/contexts/Network.jsx index c00d5b2895..b5b46286c2 100644 --- a/src/contexts/Network.jsx +++ b/src/contexts/Network.jsx @@ -4,6 +4,8 @@ import React, { Component } from 'react'; import { withRouter } from 'react-router-dom'; +import { i18nMark } from '@lingui/react'; + import { withRootDialog } from './RootDialog'; import APIClient from '../api'; @@ -37,7 +39,7 @@ class prov extends Component { } history.replace('/login'); setRootDialogMessage({ - bodyText: 'You have been logged out.', + bodyText: i18nMark('You have been logged out.') }); } @@ -49,8 +51,8 @@ class prov extends Component { } history.replace('/home'); setRootDialogMessage({ - title: '404', - bodyText: 'Cannot find resource.', + title: i18nMark('404'), + bodyText: i18nMark('Cannot find resource.'), variant: 'warning' }); } diff --git a/src/pages/Organizations/Organizations.jsx b/src/pages/Organizations/Organizations.jsx index 28ac5864ab..4baa46d978 100644 --- a/src/pages/Organizations/Organizations.jsx +++ b/src/pages/Organizations/Organizations.jsx @@ -1,6 +1,7 @@ import React, { Component, Fragment } from 'react'; import { Route, withRouter, Switch } from 'react-router-dom'; import { i18nMark } from '@lingui/react'; +import { Trans } from '@lingui/macro'; import { NetworkProvider } from '../../contexts/Network'; import { withRootDialog } from '../../contexts/RootDialog'; @@ -62,7 +63,13 @@ class Organizations extends Component { history.replace('/organizations'); setRootDialogMessage({ title: '404', - bodyText: `Cannot find organization with ID ${newRouteMatch.params.id}.`, + bodyText: ( + + Cannot find organization with ID + {` ${newRouteMatch.params.id}`} + . + + ), variant: 'warning' }); }} diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index 983bba1e01..bd070e7289 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -7,7 +7,7 @@ import { } from '@patternfly/react-core'; import { I18n, i18nMark } from '@lingui/react'; -import { t } from '@lingui/macro'; +import { t, Trans } from '@lingui/macro'; import { Link @@ -257,16 +257,33 @@ class OrganizationAccessList extends React.Component { let warningMsg; if (type === 'users') { - warningTitle = i18nMark('User Access Removal'); - warningMsg = i18nMark(`Please confirm that you would like to remove ${roleName} - access from ${resourceName}.`); + warningTitle = i18nMark('Remove User Access'); + warningMsg = ( + + Are you sure you want to remove + {` ${roleName} `} + access from + {` ${resourceName}`} + ? + + ); } if (type === 'teams') { - warningTitle = i18nMark('Team Access Removal'); - warningMsg = i18nMark(`Please confirm that you would like to remove ${roleName} - access from the team ${resourceName}. This will affect all - members of the team. If you would like to only remove access - for this particular user, please remove them from the team.`); + warningTitle = i18nMark('Remove Team Access'); + warningMsg = ( + + Are you sure you want to remove + {` ${roleName} `} + access from + {` ${resourceName}`} + ? Doing so affects all members of the team. +
+
+ If you + only + want to remove access for this particular user, please remove them from the team. + + ); } this.setState({ diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index e1bfe618b4..1698ee1474 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -153,7 +153,7 @@ class OrganizationsList extends Component { handleOpenOrgDeleteModal () { const { results, selected } = this.state; - const warningTitle = i18nMark('Delete Organization'); + const warningTitle = i18nMark(`Delete Organization${selected.length > 1 ? 's' : ''}`); const warningMsg = i18nMark('Are you sure you want to delete:'); const orgsToDelete = []; From ad0e409448644c22494f52623ad964caedaa6caa Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 9 Apr 2019 10:39:47 -0400 Subject: [PATCH 07/20] do 404 modal redirect on unknown routes --- src/components/NotifyAndRedirect.jsx | 42 +++++++++++++++++++ src/index.jsx | 28 ++++++++----- .../screens/Organization/Organization.jsx | 2 + 3 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 src/components/NotifyAndRedirect.jsx diff --git a/src/components/NotifyAndRedirect.jsx b/src/components/NotifyAndRedirect.jsx new file mode 100644 index 0000000000..11e286dae8 --- /dev/null +++ b/src/components/NotifyAndRedirect.jsx @@ -0,0 +1,42 @@ +import React, { Component } from 'react'; + +import { Redirect, withRouter } from 'react-router-dom'; + +import { Trans } from '@lingui/macro'; + +import { withRootDialog } from '../contexts/RootDialog'; + +class NotifyAndRedirect extends Component { + constructor (props) { + super(props); + + const { setRootDialogMessage, location } = this.props; + setRootDialogMessage({ + title: '404', + bodyText: ( + + Cannot find route + {` ${location.pathname}`} + . + + ), + variant: 'warning' + }); + } + + render () { + const { to, push, from, exact, strict, sensitive } = this.props; + return ( + + ); + } +} + +export default withRootDialog(withRouter(NotifyAndRedirect)); diff --git a/src/index.jsx b/src/index.jsx index 9f542327a5..59b857cd49 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -20,6 +20,7 @@ import './components/SelectedList/styles.scss'; import { Config } from './contexts/Config'; import Background from './components/Background'; +import NotifyAndRedirect from './components/NotifyAndRedirect'; import RootProvider from './RootProvider'; import App from './App'; @@ -220,17 +221,22 @@ export async function main (render) { }, ]} render={({ routeGroups }) => ( - routeGroups - .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) - .map(({ component: PageComponent, path }) => ( - ( - - )} - /> - )) + + {routeGroups + .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) + .map(({ component: PageComponent, path }) => ( + ( + + )} + /> + )) + .concat([ + + ])} + )} /> )} diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index e58d8903b8..2ba53ece98 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -17,6 +17,7 @@ import { withNetwork } from '../../../../contexts/Network'; import Tabs from '../../../../components/Tabs/Tabs'; import Tab from '../../../../components/Tabs/Tab'; +import NotifyAndRedirect from '../../../../components/NotifyAndRedirect'; import OrganizationAccess from './OrganizationAccess'; import OrganizationDetail from './OrganizationDetail'; @@ -175,6 +176,7 @@ class Organization extends Component { /> )} /> + {organization && } {error ? 'error!' : ''} {loading ? 'loading...' : ''} From 344713f93866cec8c3c367b9080701a9d9e55dee Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Wed, 10 Apr 2019 11:16:20 -0400 Subject: [PATCH 08/20] fix unit tests for network handling --- __tests__/App.test.jsx | 96 +++++++----- __tests__/RootProvider.test.jsx | 10 ++ __tests__/components/Lookup.test.jsx | 66 ++++++-- .../components/NotificationList.test.jsx | 93 +++++------ .../components/NotifyAndRedirect.test.jsx | 25 +++ __tests__/index.test.jsx | 48 +----- __tests__/pages/Login.test.jsx | 14 +- .../OrganizationAccessList.test.jsx | 35 +++-- .../components/OrganizationForm.test.jsx | 144 +++++++++++------- .../Organization/OrganizationEdit.test.jsx | 46 ++++-- .../OrganizationNotifications.test.jsx | 8 +- .../screens/OrganizationAdd.test.jsx | 72 +++++---- .../screens/OrganizationsList.test.jsx | 18 ++- src/App.jsx | 132 ++++++++-------- src/components/Lookup/Lookup.jsx | 1 + .../NotificationsList/Notifications.list.jsx | 1 + src/components/NotifyAndRedirect.jsx | 2 + src/contexts/Config.jsx | 6 +- src/contexts/Network.jsx | 11 +- src/index.jsx | 4 +- src/pages/Login.jsx | 1 + src/pages/Organizations/Organizations.jsx | 1 + .../components/OrganizationAccessList.jsx | 5 +- .../components/OrganizationForm.jsx | 2 + .../OrganizationNotifications.jsx | 1 + .../screens/OrganizationsList.jsx | 1 + 26 files changed, 499 insertions(+), 344 deletions(-) create mode 100644 __tests__/RootProvider.test.jsx create mode 100644 __tests__/components/NotifyAndRedirect.test.jsx diff --git a/__tests__/App.test.jsx b/__tests__/App.test.jsx index d98025eebd..9712833be4 100644 --- a/__tests__/App.test.jsx +++ b/__tests__/App.test.jsx @@ -2,38 +2,47 @@ import React from 'react'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import { mount, shallow } from 'enzyme'; +import { mount } from 'enzyme'; import { asyncFlush } from '../jest.setup'; -import App from '../src/App'; +import { ConfigProvider } from '../src/contexts/Config'; +import { NetworkProvider } from '../src/contexts/Network'; + +import App, { _App } from '../src/App'; + +const networkProviderValue = { api: {}, handleHttpError: () => {} }; describe('', () => { test('expected content is rendered', () => { const appWrapper = mount( - ( - routeGroups.map(({ groupId }) => (
)) - )} - /> + + + ( + routeGroups.map(({ groupId }) => (
)) + )} + /> + + ); @@ -58,20 +67,20 @@ describe('', () => { const ansible_version = '111'; const version = '222'; - const getConfig = jest.fn(() => Promise.resolve({ data: { ansible_version, version } })); - const api = { getConfig }; + const config = { ansible_version, version }; const wrapper = mount( - + + + + + ); - await asyncFlush(); - expect(getConfig).toHaveBeenCalledTimes(1); - // open about modal const aboutDropdown = 'Dropdown QuestionCircleIcon'; const aboutButton = 'DropdownItem li button'; @@ -97,7 +106,17 @@ describe('', () => { }); test('onNavToggle sets state.isNavOpen to opposite', () => { - const appWrapper = shallow(); + const appWrapper = mount( + + + + + + + + + + ).find('App'); const { onNavToggle } = appWrapper.instance(); [true, false, true, false, true].forEach(expected => { @@ -108,13 +127,22 @@ describe('', () => { test('onLogout makes expected call to api client', async (done) => { const logout = jest.fn(() => Promise.resolve()); - const api = { logout }; - const appWrapper = shallow(); + const appWrapper = mount( + + + + + <_App api={{ logout }} handleHttpError={() => {}} /> + + + + + ).find('App'); appWrapper.instance().onLogout(); await asyncFlush(); - expect(api.logout).toHaveBeenCalledTimes(1); + expect(logout).toHaveBeenCalledTimes(1); done(); }); diff --git a/__tests__/RootProvider.test.jsx b/__tests__/RootProvider.test.jsx new file mode 100644 index 0000000000..b09fd033a3 --- /dev/null +++ b/__tests__/RootProvider.test.jsx @@ -0,0 +1,10 @@ +import { getLanguage } from '../src/RootProvider'; + +describe('RootProvider.jsx', () => { + test('getLanguage returns the expected language code', () => { + expect(getLanguage({ languages: ['es-US'] })).toEqual('es'); + expect(getLanguage({ languages: ['es-US'], language: 'fr-FR', userLanguage: 'en-US' })).toEqual('es'); + expect(getLanguage({ language: 'fr-FR', userLanguage: 'en-US' })).toEqual('fr'); + expect(getLanguage({ userLanguage: 'en-US' })).toEqual('en'); + }); +}); diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index 468793d8eb..7d08ee3c83 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -2,6 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; import { I18nProvider } from '@lingui/react'; import Lookup from '../../src/components/Lookup'; +import { _Lookup } from '../../src/components/Lookup/Lookup'; let mockData = [{ name: 'foo', id: 1, isChecked: false }]; const mockColumns = [ @@ -11,7 +12,7 @@ describe('', () => { test('initially renders succesfully', () => { mount( - ', () => { getItems={() => { }} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ); }); + test('API response is formatted properly', (done) => { const wrapper = mount( - ', () => { getItems={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ).find('Lookup'); @@ -43,12 +47,13 @@ describe('', () => { done(); }); }); + test('Opens modal when search icon is clicked', () => { - const spy = jest.spyOn(Lookup.prototype, 'handleModalToggle'); + const spy = jest.spyOn(_Lookup.prototype, 'handleModalToggle'); const mockSelected = [{ name: 'foo', id: 1 }]; const wrapper = mount( - ', () => { getItems={() => { }} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ).find('Lookup'); @@ -71,12 +77,13 @@ describe('', () => { }]); expect(wrapper.state('isModalOpen')).toEqual(true); }); + test('calls "toggleSelected" when a user changes a checkbox', (done) => { - const spy = jest.spyOn(Lookup.prototype, 'toggleSelected'); + const spy = jest.spyOn(_Lookup.prototype, 'toggleSelected'); const mockSelected = [{ name: 'foo', id: 1 }]; const wrapper = mount( - ', () => { getItems={() => ({ data: { results: [{ name: 'test instance', id: 1 }] } })} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ); @@ -96,12 +104,13 @@ describe('', () => { done(); }); }); + test('calls "toggleSelected" when remove icon is clicked', () => { - const spy = jest.spyOn(Lookup.prototype, 'toggleSelected'); + const spy = jest.spyOn(_Lookup.prototype, 'toggleSelected'); mockData = [{ name: 'foo', id: 1 }, { name: 'bar', id: 2 }]; const wrapper = mount( - ', () => { getItems={() => { }} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ); @@ -117,6 +127,7 @@ describe('', () => { removeIcon.simulate('click'); expect(spy).toHaveBeenCalled(); }); + test('renders chips from prop value', () => { mockData = [{ name: 'foo', id: 0 }, { name: 'bar', id: 1 }]; const wrapper = mount( @@ -129,12 +140,14 @@ describe('', () => { getItems={() => { }} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ).find('Lookup'); const chip = wrapper.find('li.pf-c-chip'); expect(chip).toHaveLength(2); }); + test('toggleSelected successfully adds/removes row from lookupSelectedItems state', () => { mockData = []; const wrapper = mount( @@ -146,6 +159,7 @@ describe('', () => { getItems={() => { }} columns={mockColumns} sortedColumnKey="name" + handleHttpError={() => {}} /> ).find('Lookup'); @@ -163,6 +177,7 @@ describe('', () => { }); expect(wrapper.state('lookupSelectedItems')).toEqual([]); }); + test('saveModal calls callback with selected items', () => { mockData = []; const onLookupSaveFn = jest.fn(); @@ -174,6 +189,7 @@ describe('', () => { value={mockData} onLookupSave={onLookupSaveFn} getItems={() => { }} + handleHttpError={() => {}} /> ).find('Lookup'); @@ -191,11 +207,12 @@ describe('', () => { name: 'foo' }], 'fooBar'); }); + test('onSort sets state and calls getData ', () => { - const spy = jest.spyOn(Lookup.prototype, 'getData'); + const spy = jest.spyOn(_Lookup.prototype, 'getData'); const wrapper = mount( - { }} value={mockData} @@ -203,6 +220,7 @@ describe('', () => { columns={mockColumns} sortedColumnKey="name" getItems={() => { }} + handleHttpError={() => {}} /> ).find('Lookup'); @@ -211,11 +229,12 @@ describe('', () => { expect(wrapper.state('sortOrder')).toEqual('descending'); expect(spy).toHaveBeenCalled(); }); - test('onSetPage sets state and calls getData ', () => { - const spy = jest.spyOn(Lookup.prototype, 'getData'); + + test('onSearch calls getData (through calling onSort)', () => { + const spy = jest.spyOn(_Lookup.prototype, 'getData'); const wrapper = mount( - { }} value={mockData} @@ -223,6 +242,27 @@ describe('', () => { columns={mockColumns} sortedColumnKey="name" getItems={() => { }} + handleHttpError={() => {}} + /> + + ).find('Lookup'); + wrapper.instance().onSearch(); + expect(spy).toHaveBeenCalled(); + }); + + test('onSetPage sets state and calls getData ', () => { + const spy = jest.spyOn(_Lookup.prototype, 'getData'); + const wrapper = mount( + + <_Lookup + lookup_header="Foo Bar" + onLookupSave={() => { }} + value={mockData} + selected={[]} + columns={mockColumns} + sortedColumnKey="name" + getItems={() => { }} + handleHttpError={() => {}} /> ).find('Lookup'); diff --git a/__tests__/components/NotificationList.test.jsx b/__tests__/components/NotificationList.test.jsx index a743c3d81d..1990dfab8e 100644 --- a/__tests__/components/NotificationList.test.jsx +++ b/__tests__/components/NotificationList.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import Notifications from '../../src/components/NotificationsList/Notifications.list'; +import Notifications, { _Notifications } from '../../src/components/NotificationsList/Notifications.list'; describe('', () => { test('initially renders succesfully', () => { @@ -17,50 +17,52 @@ describe('', () => { onReadSuccess={jest.fn()} onCreateError={jest.fn()} onCreateSuccess={jest.fn()} + handleHttpError={() => {}} /> ); }); + test('fetches notifications on mount', () => { - const spy = jest.spyOn(Notifications.prototype, 'readNotifications'); + const spy = jest.spyOn(_Notifications.prototype, 'readNotifications'); mount( - - - - - + + <_Notifications + match={{ path: '/organizations/:id/?tab=notifications', url: '/organizations/:id/?tab=notifications' }} + location={{ search: '', pathname: '/organizations/:id/?tab=notifications' }} + onReadError={jest.fn()} + onReadNotifications={jest.fn()} + onReadSuccess={jest.fn()} + onCreateError={jest.fn()} + onCreateSuccess={jest.fn()} + handleHttpError={() => {}} + /> + ); expect(spy).toHaveBeenCalled(); }); + test('toggle success calls post', () => { - const spy = jest.spyOn(Notifications.prototype, 'createSuccess'); + const spy = jest.spyOn(_Notifications.prototype, 'createSuccess'); const wrapper = mount( - - - - - + + <_Notifications + match={{ path: '/organizations/:id/?tab=notifications', url: '/organizations/:id/?tab=notifications' }} + location={{ search: '', pathname: '/organizations/:id/?tab=notifications' }} + onReadError={jest.fn()} + onReadNotifications={jest.fn()} + onReadSuccess={jest.fn()} + onCreateError={jest.fn()} + onCreateSuccess={jest.fn()} + handleHttpError={() => {}} + /> + ).find('Notifications'); wrapper.instance().toggleNotification(1, true, 'success'); expect(spy).toHaveBeenCalledWith(1, true); }); + test('post success makes request and updates state properly', async () => { const createSuccess = jest.fn(); const wrapper = mount( @@ -74,6 +76,7 @@ describe('', () => { onReadSuccess={jest.fn()} onCreateError={jest.fn()} onCreateSuccess={createSuccess} + handleHttpError={() => {}} /> @@ -86,26 +89,27 @@ describe('', () => { expect(createSuccess).toHaveBeenCalledWith(1, { id: 44 }); expect(wrapper.state('successTemplateIds')).toContain(44); }); + test('toggle error calls post', () => { - const spy = jest.spyOn(Notifications.prototype, 'createError'); + const spy = jest.spyOn(_Notifications.prototype, 'createError'); const wrapper = mount( - - - - - + + <_Notifications + match={{ path: '/organizations/:id/?tab=notifications', url: '/organizations/:id/?tab=notifications' }} + location={{ search: '', pathname: '/organizations/:id/?tab=notifications' }} + onReadError={jest.fn()} + onReadNotifications={jest.fn()} + onReadSuccess={jest.fn()} + onCreateError={jest.fn()} + onCreateSuccess={jest.fn()} + handleHttpError={() => {}} + /> + ).find('Notifications'); wrapper.instance().toggleNotification(1, true, 'error'); expect(spy).toHaveBeenCalledWith(1, true); }); + test('post error makes request and updates state properly', async () => { const createError = jest.fn(); const wrapper = mount( @@ -119,6 +123,7 @@ describe('', () => { onReadSuccess={jest.fn()} onCreateError={createError} onCreateSuccess={jest.fn()} + handleHttpError={() => {}} /> @@ -131,6 +136,7 @@ describe('', () => { expect(createError).toHaveBeenCalledWith(1, { id: 44 }); expect(wrapper.state('errorTemplateIds')).toContain(44); }); + test('fetchNotifications', async () => { const mockQueryParams = { page: 44, @@ -171,6 +177,7 @@ describe('', () => { onReadError={readError} onCreateError={jest.fn()} onCreateSuccess={jest.fn()} + handleHttpError={() => {}} /> diff --git a/__tests__/components/NotifyAndRedirect.test.jsx b/__tests__/components/NotifyAndRedirect.test.jsx new file mode 100644 index 0000000000..4e5cb8f830 --- /dev/null +++ b/__tests__/components/NotifyAndRedirect.test.jsx @@ -0,0 +1,25 @@ +import React from 'react'; +import { mount } from 'enzyme'; + +import { MemoryRouter } from 'react-router-dom'; +import { I18nProvider } from '@lingui/react'; + +import { _NotifyAndRedirect } from '../../src/components/NotifyAndRedirect'; + +describe('', () => { + test('initially renders succesfully and calls setRootDialogMessage', () => { + const setRootDialogMessage = jest.fn(); + mount( + + + <_NotifyAndRedirect + to="foo" + setRootDialogMessage={setRootDialogMessage} + location={{ pathname: 'foo' }} + /> + + + ); + expect(setRootDialogMessage).toHaveBeenCalled(); + }); +}); diff --git a/__tests__/index.test.jsx b/__tests__/index.test.jsx index 4414a46cbd..d7ce697841 100644 --- a/__tests__/index.test.jsx +++ b/__tests__/index.test.jsx @@ -1,51 +1,11 @@ import { mount } from 'enzyme'; -import { main, getLanguage } from '../src/index'; +import { main } from '../src/index'; const render = template => mount(template); -const data = { custom_logo: 'foo', custom_login_info: '' }; describe('index.jsx', () => { - test('login loads when unauthenticated', async (done) => { - const isAuthenticated = () => false; - const getRoot = jest.fn(() => Promise.resolve({ data })); - - const api = { getRoot, isAuthenticated }; - const wrapper = await main(render, api); - - expect(api.getRoot).toHaveBeenCalled(); - expect(wrapper.find('App')).toHaveLength(0); - expect(wrapper.find('Login')).toHaveLength(1); - - const { src } = wrapper.find('Login Brand img').props(); - expect(src).toContain(data.custom_logo); - - done(); - }); - - test('app loads when authenticated', async (done) => { - const isAuthenticated = () => true; - const getRoot = jest.fn(() => Promise.resolve({ data })); - - const api = { getRoot, isAuthenticated }; - const wrapper = await main(render, api); - - expect(api.getRoot).toHaveBeenCalled(); - expect(wrapper.find('App')).toHaveLength(1); - expect(wrapper.find('Login')).toHaveLength(0); - - wrapper.find('header a').simulate('click'); - wrapper.update(); - - expect(wrapper.find('App')).toHaveLength(1); - expect(wrapper.find('Login')).toHaveLength(0); - - done(); - }); - - test('getLanguage returns the expected language code', () => { - expect(getLanguage({ languages: ['es-US'] })).toEqual('es'); - expect(getLanguage({ languages: ['es-US'], language: 'fr-FR', userLanguage: 'en-US' })).toEqual('es'); - expect(getLanguage({ language: 'fr-FR', userLanguage: 'en-US' })).toEqual('fr'); - expect(getLanguage({ userLanguage: 'en-US' })).toEqual('en'); + test('index.jsx loads without issue', () => { + const wrapper = main(render); + expect(wrapper.find('RootProvider')).toHaveLength(1); }); }); diff --git a/__tests__/pages/Login.test.jsx b/__tests__/pages/Login.test.jsx index 49bcffce5c..29a707fd59 100644 --- a/__tests__/pages/Login.test.jsx +++ b/__tests__/pages/Login.test.jsx @@ -1,9 +1,9 @@ import React from 'react'; import { MemoryRouter } from 'react-router-dom'; -import { mount, shallow } from 'enzyme'; +import { mount } from 'enzyme'; import { I18nProvider } from '@lingui/react'; import { asyncFlush } from '../../jest.setup'; -import AWXLogin from '../../src/pages/Login'; +import { _AWXLogin } from '../../src/pages/Login'; import APIClient from '../../src/api'; describe('', () => { @@ -32,7 +32,7 @@ describe('', () => { loginWrapper = mount( - + <_AWXLogin api={api} clearRootDialogMessage={() => {}} handleHttpError={() => {}} /> ); @@ -61,7 +61,7 @@ describe('', () => { loginWrapper = mount( - + <_AWXLogin api={api} logo="images/foo.jpg" alt="Foo Application" /> ); @@ -75,7 +75,7 @@ describe('', () => { loginWrapper = mount( - + <_AWXLogin api={api} /> ); @@ -166,9 +166,7 @@ describe('', () => { }); test('render Redirect to / when already authenticated', () => { - api.isAuthenticated = jest.fn(); - api.isAuthenticated.mockReturnValue(true); - loginWrapper = shallow(); + awxLogin.setState({ isAuthenticated: true }); const redirectElem = loginWrapper.find('Redirect'); expect(redirectElem.length).toBe(1); expect(redirectElem.props().to).toBe('/'); diff --git a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx index 588ff8df7a..36e1690116 100644 --- a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx @@ -3,7 +3,7 @@ import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import OrganizationAccessList from '../../../../src/pages/Organizations/components/OrganizationAccessList'; +import OrganizationAccessList, { _OrganizationAccessList } from '../../../../src/pages/Organizations/components/OrganizationAccessList'; const mockData = [ { @@ -66,16 +66,17 @@ describe('', () => { }); test('onExpand and onCompact methods called when user clicks on Expand and Compact icons respectively', async (done) => { - const onExpand = jest.spyOn(OrganizationAccessList.prototype, 'onExpand'); - const onCompact = jest.spyOn(OrganizationAccessList.prototype, 'onCompact'); + const onExpand = jest.spyOn(_OrganizationAccessList.prototype, 'onExpand'); + const onCompact = jest.spyOn(_OrganizationAccessList.prototype, 'onCompact'); const wrapper = mount( - ({ data: { count: 1, results: mockData } })} removeRole={() => {}} + handleHttpError={() => {}} /> @@ -94,15 +95,16 @@ describe('', () => { }); test('onSort being passed properly to DataListToolbar component', async (done) => { - const onSort = jest.spyOn(OrganizationAccessList.prototype, 'onSort'); + const onSort = jest.spyOn(_OrganizationAccessList.prototype, 'onSort'); const wrapper = mount( - ({ data: { count: 1, results: mockData } })} removeRole={() => {}} + handleHttpError={() => {}} /> @@ -141,17 +143,18 @@ describe('', () => { }); test('test handleWarning, confirmDelete, and removeRole methods for Alert component', (done) => { - const handleWarning = jest.spyOn(OrganizationAccessList.prototype, 'handleWarning'); - const confirmDelete = jest.spyOn(OrganizationAccessList.prototype, 'confirmDelete'); - const removeRole = jest.spyOn(OrganizationAccessList.prototype, 'removeAccessRole'); + const handleWarning = jest.spyOn(_OrganizationAccessList.prototype, 'handleWarning'); + const confirmDelete = jest.spyOn(_OrganizationAccessList.prototype, 'confirmDelete'); + const removeRole = jest.spyOn(_OrganizationAccessList.prototype, 'removeAccessRole'); const wrapper = mount( - ({ data: { count: 1, results: mockData } })} removeRole={() => {}} + handleHttpError={() => {}} /> @@ -164,19 +167,19 @@ describe('', () => { const rendered = wrapper.update().find('ChipButton'); rendered.find('button[aria-label="close"]').simulate('click'); expect(handleWarning).toHaveBeenCalled(); - const alert = wrapper.update().find('Alert'); - alert.find('button[aria-label="confirm-delete"]').simulate('click'); + const alertModal = wrapper.update().find('Modal'); + alertModal.find('button[aria-label="Confirm delete"]').simulate('click'); expect(confirmDelete).toHaveBeenCalled(); expect(removeRole).toHaveBeenCalled(); done(); }); }); - test('state is set appropriately when a user tries deleting a role', (done) => { + test.only('state is set appropriately when a user tries deleting a role', (done) => { const wrapper = mount( - ({ data: { count: 1, results: mockData } })} @@ -200,8 +203,8 @@ describe('', () => { ]; const rendered = wrapper.update().find('ChipButton'); rendered.find('button[aria-label="close"]').simulate('click'); - const alert = wrapper.update().find('Alert'); - alert.find('button[aria-label="confirm-delete"]').simulate('click'); + const alertModal = wrapper.update().find('Modal'); + alertModal.find('button[aria-label="Confirm delete"]').simulate('click'); expect(wrapper.state().warningTitle).not.toBe(null); expect(wrapper.state().warningMsg).not.toBe(null); expected.forEach(criteria => { diff --git a/__tests__/pages/Organizations/components/OrganizationForm.test.jsx b/__tests__/pages/Organizations/components/OrganizationForm.test.jsx index 8ef06dcdfd..5399e2e8e5 100644 --- a/__tests__/pages/Organizations/components/OrganizationForm.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationForm.test.jsx @@ -2,12 +2,21 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; +<<<<<<< HEAD import { ConfigContext } from '../../../../src/context'; import OrganizationForm from '../../../../src/pages/Organizations/components/OrganizationForm'; import { sleep } from '../../../testUtils'; +======= +import { ConfigProvider } from '../../../../src/contexts/Config'; +import { NetworkProvider } from '../../../../src/contexts/Network'; +import OrganizationForm, { _OrganizationForm } from '../../../../src/pages/Organizations/components/OrganizationForm'; + +const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); +>>>>>>> fix unit tests for network handling describe('', () => { let api; + let networkProviderValue; const mockData = { id: 1, @@ -23,6 +32,11 @@ describe('', () => { api = { getInstanceGroups: jest.fn(), }; + + networkProviderValue = { + api, + handleHttpError: () => {} + }; }); test('should request related instance groups from api', () => { @@ -34,16 +48,18 @@ describe('', () => { Promise.resolve({ data: { results: mockInstanceGroups } }) )); mount( - - - - - + + + + <_OrganizationForm + api={api} + organization={mockData} + handleSubmit={jest.fn()} + handleCancel={jest.fn()} + /> + + + ).find('OrganizationForm'); expect(api.getOrganizationInstanceGroups).toHaveBeenCalledTimes(1); @@ -58,16 +74,18 @@ describe('', () => { Promise.resolve({ data: { results: mockInstanceGroups } }) )); const wrapper = mount( - - - - - + + + + <_OrganizationForm + organization={mockData} + api={api} + handleSubmit={jest.fn()} + handleCancel={jest.fn()} + /> + + + ).find('OrganizationForm'); await wrapper.instance().componentDidMount(); @@ -78,12 +96,13 @@ describe('', () => { const wrapper = mount( - + + + ).find('OrganizationForm'); @@ -109,12 +128,13 @@ describe('', () => { const wrapper = mount( - + + + ).find('OrganizationForm'); @@ -137,14 +157,15 @@ describe('', () => { const wrapper = mount( - - - + + + + + ); @@ -157,16 +178,18 @@ describe('', () => { const wrapper = mount( - + + + ).find('OrganizationForm'); - expect(wrapper.prop('handleSubmit')).not.toHaveBeenCalled(); + expect(handleSubmit).not.toHaveBeenCalled(); wrapper.find('button[aria-label="Save"]').simulate('click'); await sleep(1); expect(handleSubmit).toHaveBeenCalledWith({ @@ -196,12 +219,14 @@ describe('', () => { const wrapper = mount( - + + <_OrganizationForm + organization={mockData} + api={api} + handleSubmit={handleSubmit} + handleCancel={jest.fn()} + /> + ).find('OrganizationForm'); @@ -223,12 +248,13 @@ describe('', () => { const wrapper = mount( - + + + ); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx index 3351dd8095..7aee6e5975 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx @@ -2,12 +2,16 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import OrganizationEdit, { _OrganizationEdit } from '../../../../../src/pages/Organizations/screens/Organization/OrganizationEdit'; + +import { NetworkProvider } from '../../../../../src/contexts/Network'; + +import { _OrganizationEdit } from '../../../../../src/pages/Organizations/screens/Organization/OrganizationEdit'; const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); describe('', () => { let api; + let networkProviderValue; const mockData = { name: 'Foo', @@ -26,16 +30,24 @@ describe('', () => { associateInstanceGroup: jest.fn(), disassociate: jest.fn(), }; + + networkProviderValue = { + api, + handleHttpError: () => {} + }; }); test('handleSubmit should call api update', () => { const wrapper = mount( - + + <_OrganizationEdit + organization={mockData} + api={api} + handleHttpError={() => {}} + /> + ); @@ -57,10 +69,13 @@ describe('', () => { const wrapper = mount( - + + <_OrganizationEdit + organization={mockData} + api={api} + handleHttpError={() => {}} + /> + ); @@ -94,11 +109,14 @@ describe('', () => { const wrapper = mount( - <_OrganizationEdit - history={history} - organization={mockData} - api={api} - /> + + <_OrganizationEdit + organization={mockData} + api={api} + handleHttpError={() => {}} + history={history} + /> + ); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx index d8e6723cb1..3061c7e9b8 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx @@ -1,13 +1,13 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; -import OrganizationNotifications from '../../../../../src/pages/Organizations/screens/Organization/OrganizationNotifications'; +import { _OrganizationNotifications } from '../../../../../src/pages/Organizations/screens/Organization/OrganizationNotifications'; describe('', () => { test('initially renders succesfully', () => { mount( - ', () => { createOrganizationNotificationSuccess: jest.fn(), createOrganizationNotificationError: jest.fn() }} + handleHttpError={() => {}} /> ); @@ -30,7 +31,7 @@ describe('', () => { const createOrganizationNotificationError = jest.fn(); const wrapper = mount( - ', () => { createOrganizationNotificationSuccess, createOrganizationNotificationError }} + handleHttpError={() => {}} /> ).find('OrganizationNotifications'); diff --git a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx index 607ae9acd9..5f7765966e 100644 --- a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx +++ b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx @@ -2,13 +2,17 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import { ConfigContext } from '../../../../src/context'; -import OrganizationAdd, { _OrganizationAdd } from '../../../../src/pages/Organizations/screens/OrganizationAdd'; + +import { ConfigProvider } from '../../../../src/contexts/Config'; +import { NetworkProvider } from '../../../../src/contexts/Network'; + +import { _OrganizationAdd } from '../../../../src/pages/Organizations/screens/OrganizationAdd'; const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); describe('', () => { let api; + let networkProviderValue; beforeEach(() => { api = { @@ -17,15 +21,22 @@ describe('', () => { associateInstanceGroup: jest.fn(), disassociate: jest.fn(), }; + + networkProviderValue = { + api, + handleHttpError: () => {} + }; }); test('handleSubmit should post to api', () => { const wrapper = mount( - + + + <_OrganizationAdd api={api} /> + + ); @@ -47,10 +58,11 @@ describe('', () => { const wrapper = mount( - <_OrganizationAdd - history={history} - api={api} - /> + + + <_OrganizationAdd api={api} history={history} /> + + ); @@ -68,10 +80,11 @@ describe('', () => { const wrapper = mount( - <_OrganizationAdd - history={history} - api={api} - /> + + + <_OrganizationAdd api={api} history={history} /> + + ); @@ -103,10 +116,11 @@ describe('', () => { const wrapper = mount( - <_OrganizationAdd - history={history} - api={api} - /> + + + <_OrganizationAdd api={api} history={history} /> + + ); @@ -121,9 +135,11 @@ describe('', () => { const wrapper = mount( - + + + <_OrganizationAdd api={api} /> + + ); @@ -156,9 +172,11 @@ describe('', () => { const wrapper = mount( - - - + + + <_OrganizationAdd api={api} /> + + ).find('OrganizationAdd').find('AnsibleSelect'); @@ -173,9 +191,11 @@ describe('', () => { const wrapper = mount( - - - + + + <_OrganizationAdd api={api} /> + + ).find('OrganizationAdd').find('AnsibleSelect'); diff --git a/__tests__/pages/Organizations/screens/OrganizationsList.test.jsx b/__tests__/pages/Organizations/screens/OrganizationsList.test.jsx index e874199da5..a3c737c126 100644 --- a/__tests__/pages/Organizations/screens/OrganizationsList.test.jsx +++ b/__tests__/pages/Organizations/screens/OrganizationsList.test.jsx @@ -2,7 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -import OrganizationsList from '../../../../src/pages/Organizations/screens/OrganizationsList'; +import { _OrganizationsList } from '../../../../src/pages/Organizations/screens/OrganizationsList'; const mockAPIOrgsList = { data: { @@ -48,25 +48,28 @@ describe('', () => { mount( - {}} /> ); }); - test.only('Modal closes when close button is clicked.', async (done) => { + // TODO: these tests were not correct. will work to clean these tests up after PR + test.skip('Modal closes when close button is clicked.', async (done) => { const handleClearOrgsToDelete = jest.fn(); const wrapper = mount( - {}} /> @@ -91,21 +94,22 @@ describe('', () => { }); }); - test.only('Orgs to delete length is 0 when all orgs are selected and Delete button is called.', async (done) => { + // TODO: these tests were not correct. will work to clean these tests up after PR + test.skip('Orgs to delete length is 0 when all orgs are selected and Delete button is called.', async (done) => { const handleClearOrgsToDelete = jest.fn(); const handleOrgDelete = jest.fn(); const fetchOrganizations = jest.fn(); const wrapper = mount( - {}} /> diff --git a/src/App.jsx b/src/App.jsx index 7e4699e33d..8b68aea0f6 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -14,6 +14,7 @@ import { t } from '@lingui/macro'; import { RootDialog } from './contexts/RootDialog'; import { withNetwork } from './contexts/Network'; +import { Config } from './contexts/Config'; import AlertModal from './components/AlertModal'; import About from './components/About'; @@ -64,10 +65,8 @@ class App extends Component { render () { const { - ansible_version, isAboutModalOpen, - isNavOpen, - version + isNavOpen } = this.state; const { @@ -77,75 +76,80 @@ class App extends Component { } = this.props; return ( - - {({ i18n }) => ( - - {({ title, bodyText, variant = 'info', clearRootDialogMessage }) => ( - - {(title || bodyText) && ( - {i18n._(t`Close`)} - ]} - > - {bodyText} - - )} - } - toolbar={( - + {({ ansible_version, version }) => ( + + {({ i18n }) => ( + + {({ title, bodyText, variant = 'info', clearRootDialogMessage }) => ( + + {(title || bodyText) && ( + {i18n._(t`Close`)} + ]} + > + {bodyText} + + )} + } + toolbar={( + + )} /> )} - /> - )} - sidebar={( - - - {routeGroups.map(({ groupId, groupTitle, routes }) => ( - - ))} - - + sidebar={( + + + {routeGroups.map(({ groupId, groupTitle, routes }) => ( + + ))} + + + )} + /> )} + > + {render && render({ routeGroups })} + + - )} - > - {render && render({ routeGroups })} - - - + + )} + )} - + )} - + ); } } +export { App as _App }; export default withNetwork(App); diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index ec189fb1b2..f4f77cae6b 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -275,4 +275,5 @@ Lookup.defaultProps = { name: null, }; +export { Lookup as _Lookup }; export default withNetwork(Lookup); diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index c3c44381ca..ac20a8363f 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -343,4 +343,5 @@ Notifications.propTypes = { onCreateSuccess: PropTypes.func.isRequired, }; +export { Notifications as _Notifications }; export default withNetwork(Notifications); diff --git a/src/components/NotifyAndRedirect.jsx b/src/components/NotifyAndRedirect.jsx index 11e286dae8..1741c8813a 100644 --- a/src/components/NotifyAndRedirect.jsx +++ b/src/components/NotifyAndRedirect.jsx @@ -26,6 +26,7 @@ class NotifyAndRedirect extends Component { render () { const { to, push, from, exact, strict, sensitive } = this.props; + return ( + {children} ); diff --git a/src/contexts/Network.jsx b/src/contexts/Network.jsx index b5b46286c2..cf3d17d773 100644 --- a/src/contexts/Network.jsx +++ b/src/contexts/Network.jsx @@ -59,21 +59,20 @@ class prov extends Component { render () { const { - children - } = this.props; - - const { - value + value: stateValue } = this.state; + const { value: propsValue, children } = this.props; + return ( - + {children} ); } } +export { NetworkProvider as _NetworkProvider }; export const NetworkProvider = withRootDialog(withRouter(prov)); export function withNetwork (Child) { diff --git a/src/index.jsx b/src/index.jsx index 59b857cd49..55747eb036 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -50,7 +50,7 @@ import Templates from './pages/Templates'; import Users from './pages/Users'; // eslint-disable-next-line import/prefer-default-export -export async function main (render) { +export function main (render) { const el = document.getElementById('app'); return render( @@ -246,7 +246,7 @@ export async function main (render) { )} - , el + , el || document.createElement('div') ); } diff --git a/src/pages/Login.jsx b/src/pages/Login.jsx index 2a216ed505..d3357d77bf 100644 --- a/src/pages/Login.jsx +++ b/src/pages/Login.jsx @@ -97,4 +97,5 @@ class AWXLogin extends Component { } } +export { AWXLogin as _AWXLogin }; export default withNetwork(withRootDialog(withRouter(AWXLogin))); diff --git a/src/pages/Organizations/Organizations.jsx b/src/pages/Organizations/Organizations.jsx index 4baa46d978..d5dd9dfdb0 100644 --- a/src/pages/Organizations/Organizations.jsx +++ b/src/pages/Organizations/Organizations.jsx @@ -94,4 +94,5 @@ class Organizations extends Component { } } +export { Organizations as _Organizations }; export default withRootDialog(withRouter(Organizations)); diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index bd070e7289..2345e4e6d0 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -355,8 +355,8 @@ class OrganizationAccessList extends React.Component { isOpen={showWarning} onClose={this.hideWarning} actions={[ - , - + , + ]} > {warningMsg} @@ -447,4 +447,5 @@ OrganizationAccessList.propTypes = { removeRole: PropTypes.func.isRequired, }; +export { OrganizationAccessList as _OrganizationAccessList }; export default withNetwork(OrganizationAccessList); diff --git a/src/pages/Organizations/components/OrganizationForm.jsx b/src/pages/Organizations/components/OrganizationForm.jsx index 37994f1bd6..d956bb8c47 100644 --- a/src/pages/Organizations/components/OrganizationForm.jsx +++ b/src/pages/Organizations/components/OrganizationForm.jsx @@ -28,6 +28,7 @@ class OrganizationForm extends Component { this.state = { instanceGroups: [], + initialInstanceGroups: [], formIsValid: true, }; } @@ -174,4 +175,5 @@ OrganizationForm.contextTypes = { custom_virtualenvs: PropTypes.arrayOf(PropTypes.string) }; +export { OrganizationForm as _OrganizationForm }; export default withNetwork(withRouter(OrganizationForm)); diff --git a/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx b/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx index 895fc27d1b..da658632c8 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx @@ -62,4 +62,5 @@ class OrganizationNotifications extends Component { } } +export { OrganizationNotifications as _OrganizationNotifications }; export default withNetwork(OrganizationNotifications); diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 1698ee1474..0417c63144 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -359,4 +359,5 @@ class OrganizationsList extends Component { } } +export { OrganizationsList as _OrganizationsList }; export default withNetwork(withRouter(OrganizationsList)); From abc37334492441043f074b2630fb13ec337f7023 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 11:06:39 -0400 Subject: [PATCH 09/20] fix test after rebase of org teams update --- .../components/OrganizationForm.test.jsx | 7 ---- .../Organization/OrganizationTeams.test.jsx | 41 ++++++++++++------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/__tests__/pages/Organizations/components/OrganizationForm.test.jsx b/__tests__/pages/Organizations/components/OrganizationForm.test.jsx index 5399e2e8e5..7078ea504e 100644 --- a/__tests__/pages/Organizations/components/OrganizationForm.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationForm.test.jsx @@ -2,18 +2,11 @@ import React from 'react'; import { mount } from 'enzyme'; import { MemoryRouter } from 'react-router-dom'; import { I18nProvider } from '@lingui/react'; -<<<<<<< HEAD -import { ConfigContext } from '../../../../src/context'; -import OrganizationForm from '../../../../src/pages/Organizations/components/OrganizationForm'; import { sleep } from '../../../testUtils'; -======= import { ConfigProvider } from '../../../../src/contexts/Config'; import { NetworkProvider } from '../../../../src/contexts/Network'; import OrganizationForm, { _OrganizationForm } from '../../../../src/pages/Organizations/components/OrganizationForm'; -const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms)); ->>>>>>> fix unit tests for network handling - describe('', () => { let api; let networkProviderValue; diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationTeams.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationTeams.test.jsx index d9f8a1c7c1..44effdca37 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationTeams.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationTeams.test.jsx @@ -6,6 +6,7 @@ import { createMemoryHistory } from 'history'; import { sleep } from '../../../../testUtils'; import OrganizationTeams, { _OrganizationTeams } from '../../../../../src/pages/Organizations/screens/Organization/OrganizationTeams'; import OrganizationTeamsList from '../../../../../src/pages/Organizations/components/OrganizationTeamsList'; +import { NetworkProvider } from '../../../../../src/contexts/Network'; const listData = { data: { @@ -30,6 +31,7 @@ describe('', () => { api={{ readOrganizationTeamsList: jest.fn(), }} + handleHttpError={() => {}} /> ); }); @@ -39,11 +41,14 @@ describe('', () => { mount( - + {} }} + > + + ).find('OrganizationTeams'); @@ -59,11 +64,14 @@ describe('', () => { const wrapper = mount( - + {} }} + > + + ); @@ -100,11 +108,14 @@ describe('', () => { const wrapper = mount( - + {} }} + > + + ); From e36320174cf7d216aa9919fe3da0df005232dd6d Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 11:58:15 -0400 Subject: [PATCH 10/20] Add contrib docs based on context changes and move qs to util dir --- CONTRIBUTING.md | 46 +++++++++++++++++++ __tests__/{ => util}/qs.test.js | 2 +- .../NotificationsList/Notifications.list.jsx | 2 +- src/{ => util}/qs.js | 0 4 files changed, 48 insertions(+), 2 deletions(-) rename __tests__/{ => util}/qs.test.js (93%) rename src/{ => util}/qs.js (100%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56bf691d3a..2f8575bc2f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,8 @@ Have questions about this document or anything not covered here? Feel free to re * [Build the user interface](#build-the-user-interface) * [Accessing the AWX web interface](#accessing-the-awx-web-interface) * [Working with React](#working-with-react) + * [App structure](#app-structure) + * [Naming files](#naming-files) * [Class constructors vs Class properties](#class-constructors-vs-class-properties) * [Binding](#binding) * [Typechecking with PropTypes](#typechecking-with-proptypes) @@ -56,6 +58,50 @@ Run the following to build the AWX UI: You can now log into the AWX web interface at [https://127.0.0.1:3001](https://127.0.0.1:3001). ## Working with React + +### App structure + +All source code lives in the `/src` directory and all tests live in the `/__tests__` directory (mimicing the internal structure of `/src`). + +Inside these folders, the internal structure is: +- **/components** - All generic components that are meant to be used in multiple contexts throughout awx. Things like buttons, tabs go here. +- **/contexts** - Components which utilize react's context api. +- **/pages** - Based on the various routes of awx. + - **/components** - Components that are meant to be used specifically by a particular route, but might be sharable across pages of that route. For example, a form component which is used on both add and edit screens. + - **/screens** - Individual pages of the route, such as add, edit, list, related lists, etc. +- **/util** - Stateless helper functions that aren't tied to react. + +#### Bootstrapping the application (root src/ files) + +In the root of `/src`, there are a few files which are used to initialize the react app. These are + +- **index.jsx** + - Connects react app to root dom node. + - Sets up root route structure, navigation grouping and login modal + - Calls base context providers + - Imports .scss styles. +- **app.jsx** + - Sets standard page layout, about modal, and root dialog modal. +- **RootProvider.jsx** + - Sets up all context providers. + - Initializes i18n. + +### Naming files + +Ideally, files should be named the same as the component they export, and tests with `.test` appended. In other words, `` would be defined in `FooBar.jsx`, and its tests would be defined in `FooBar.test.jsx`. + +#### Naming components that use the context api + +**File naming** - Since contexts export both consumer and provider (and potentially in withContext function form), the file can be simplified to be named after the consumer export. In other words, the file containing the `Network` context components would be named `Network.jsx`. + +**Component naming and conventions** - In order to provide a consistent interface with react-router and lingui, as well as make their usage easier and less verbose, context components follow these conventions: +- Providers are wrapped in a component in the `FooProvider` format. + - The value prop of the provider should be pulled from state. This is recommended by the react docs, [here](here). + - The provider should also be able to accept its value by prop for testing. + - Any sort of code related to grabbing data to put on the context should be done in this component. +- Consumers are wrapped in a component in the `Foo` format. +- If it makes sense, consumers can be exported as a function in the `withFoo()` format. If a component is wrapped in this function, its context values are available on the component as props. + ### Class constructors vs Class properties It is good practice to use constructor-bound instance methods rather than methods as class properties. Methods as arrow functions provide lexical scope and are bound to the Component class instance instead of the class itself. This makes it so we cannot easily test a Component's methods without invoking an instance of the Component and calling the method directly within our tests. diff --git a/__tests__/qs.test.js b/__tests__/util/qs.test.js similarity index 93% rename from __tests__/qs.test.js rename to __tests__/util/qs.test.js index 2016f49b45..01178e4b17 100644 --- a/__tests__/qs.test.js +++ b/__tests__/util/qs.test.js @@ -1,4 +1,4 @@ -import { encodeQueryString, parseQueryString } from '../src/qs'; +import { encodeQueryString, parseQueryString } from '../../src/util/qs'; describe('qs (qs.js)', () => { test('encodeQueryString returns the expected queryString', () => { diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index ac20a8363f..cd3ea675b7 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -14,7 +14,7 @@ import DataListToolbar from '../DataListToolbar'; import NotificationListItem from './NotificationListItem'; import Pagination from '../Pagination'; -import { parseQueryString } from '../../qs'; +import { parseQueryString } from '../../util/qs'; class Notifications extends Component { columns = [ diff --git a/src/qs.js b/src/util/qs.js similarity index 100% rename from src/qs.js rename to src/util/qs.js From bab7095d67816a53eec32270bb17c06d9ea32eea Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 12:00:39 -0400 Subject: [PATCH 11/20] Update qs path imports to include util --- src/pages/Organizations/components/OrganizationAccessList.jsx | 2 +- src/pages/Organizations/components/OrganizationTeamsList.jsx | 2 +- .../Organizations/screens/Organization/OrganizationTeams.jsx | 2 +- src/pages/Organizations/screens/OrganizationsList.jsx | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index 2345e4e6d0..09034eb111 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -21,7 +21,7 @@ import DataListToolbar from '../../../components/DataListToolbar'; import { parseQueryString, -} from '../../../qs'; +} from '../../../util/qs'; const userRolesWrapperStyle = { display: 'flex', diff --git a/src/pages/Organizations/components/OrganizationTeamsList.jsx b/src/pages/Organizations/components/OrganizationTeamsList.jsx index 5f30e4b7e1..8fb02519c6 100644 --- a/src/pages/Organizations/components/OrganizationTeamsList.jsx +++ b/src/pages/Organizations/components/OrganizationTeamsList.jsx @@ -20,7 +20,7 @@ import { withRouter, Link } from 'react-router-dom'; import Pagination from '../../../components/Pagination'; import DataListToolbar from '../../../components/DataListToolbar'; -import { encodeQueryString } from '../../../qs'; +import { encodeQueryString } from '../../../util/qs'; const detailWrapperStyle = { display: 'grid', diff --git a/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx b/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx index cbca34ff6c..3cfbdca636 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationTeams.jsx @@ -2,7 +2,7 @@ import React, { Fragment } from 'react'; import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; import OrganizationTeamsList from '../../components/OrganizationTeamsList'; -import { parseQueryString } from '../../../../qs'; +import { parseQueryString } from '../../../../util/qs'; import { withNetwork } from '../../../../contexts/Network'; const DEFAULT_QUERY_PARAMS = { diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 0417c63144..c2064fc923 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -32,7 +32,7 @@ import AlertModal from '../../../components/AlertModal'; import { encodeQueryString, parseQueryString, -} from '../../../qs'; +} from '../../../util/qs'; class OrganizationsList extends Component { columns = [ From b17fb8a596403b3627b606e7fd3e9410e9e4f521 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 12:36:19 -0400 Subject: [PATCH 12/20] Add handleHttpError prop to stop error from org detail test --- .../screens/Organization/OrganizationDetail.test.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx index e3b303d952..5e66115d03 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx @@ -36,6 +36,7 @@ describe('', () => { api={{ getOrganizationInstanceGroups }} + handleHttpError={() => {}} organization={mockDetails} /> @@ -63,6 +64,7 @@ describe('', () => { params: { id: '1' } }} organization={mockDetails} + handleHttpError={() => {}} api={{ getOrganizationInstanceGroups }} From a808462a3d06c2207fdbdf266cdb6027bc30d88c Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 12:40:27 -0400 Subject: [PATCH 13/20] remove test.only causing org access tests to skip --- .../Organizations/components/OrganizationAccessList.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx index 36e1690116..6c064e7eb3 100644 --- a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx @@ -175,7 +175,7 @@ describe('', () => { }); }); - test.only('state is set appropriately when a user tries deleting a role', (done) => { + test('state is set appropriately when a user tries deleting a role', (done) => { const wrapper = mount( From 85b9b4f896855f415a4082bf47cad07d4522b8ab Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 16:18:08 -0400 Subject: [PATCH 14/20] add missing link to react docs about context --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2f8575bc2f..dac39198fd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,7 +96,7 @@ Ideally, files should be named the same as the component they export, and tests **Component naming and conventions** - In order to provide a consistent interface with react-router and lingui, as well as make their usage easier and less verbose, context components follow these conventions: - Providers are wrapped in a component in the `FooProvider` format. - - The value prop of the provider should be pulled from state. This is recommended by the react docs, [here](here). + - The value prop of the provider should be pulled from state. This is recommended by the react docs, [here](https://reactjs.org/docs/context.html#caveats). - The provider should also be able to accept its value by prop for testing. - Any sort of code related to grabbing data to put on the context should be done in this component. - Consumers are wrapped in a component in the `Foo` format. From 64aecb85fa789198308dd4e9454814157193b076 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 17:07:46 -0400 Subject: [PATCH 15/20] move router setup to RootProvider --- CONTRIBUTING.md | 2 +- __tests__/index.test.jsx | 8 +- src/RootProvider.jsx | 30 +-- src/index.jsx | 385 +++++++++++++++++++-------------------- 4 files changed, 217 insertions(+), 208 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dac39198fd..40ca6c517f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -84,7 +84,7 @@ In the root of `/src`, there are a few files which are used to initialize the re - Sets standard page layout, about modal, and root dialog modal. - **RootProvider.jsx** - Sets up all context providers. - - Initializes i18n. + - Initializes i18n and router ### Naming files diff --git a/__tests__/index.test.jsx b/__tests__/index.test.jsx index d7ce697841..cf8f051938 100644 --- a/__tests__/index.test.jsx +++ b/__tests__/index.test.jsx @@ -1,7 +1,13 @@ +import React from 'react'; import { mount } from 'enzyme'; +import { MemoryRouter } from 'react-router-dom'; import { main } from '../src/index'; -const render = template => mount(template); +const render = template => mount( + + {template} + +); describe('index.jsx', () => { test('index.jsx loads without issue', () => { diff --git a/src/RootProvider.jsx b/src/RootProvider.jsx index 7a868e63a3..1f334c3b4c 100644 --- a/src/RootProvider.jsx +++ b/src/RootProvider.jsx @@ -3,6 +3,10 @@ import { I18nProvider, } from '@lingui/react'; +import { + HashRouter +} from 'react-router-dom'; + import { NetworkProvider } from './contexts/Network'; import { RootDialogProvider } from './contexts/RootDialog'; import { ConfigProvider } from './contexts/Config'; @@ -25,18 +29,20 @@ class RootProvider extends Component { const language = getLanguage(navigator); return ( - - - - - {children} - - - - + + + + + + {children} + + + + + ); } } diff --git a/src/index.jsx b/src/index.jsx index 55747eb036..099ab7a584 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -1,7 +1,6 @@ import React from 'react'; import ReactDOM from 'react-dom'; import { - HashRouter, Route, Switch, Redirect @@ -54,199 +53,197 @@ export function main (render) { const el = document.getElementById('app'); return render( - - - - {({ i18n }) => ( - - - ( - - {({ custom_logo, custom_login_info }) => ( - - )} - - )} - /> - } /> - ( - ( - - {routeGroups - .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) - .map(({ component: PageComponent, path }) => ( - ( - - )} - /> - )) - .concat([ - - ])} - - )} - /> - )} - /> - - - )} - - - , el || document.createElement('div') + + + {({ i18n }) => ( + + + ( + + {({ custom_logo, custom_login_info }) => ( + + )} + + )} + /> + } /> + ( + ( + + {routeGroups + .reduce((allRoutes, { routes }) => allRoutes.concat(routes), []) + .map(({ component: PageComponent, path }) => ( + ( + + )} + /> + )) + .concat([ + + ])} + + )} + /> + )} + /> + + + )} + + , el || document.createElement('div') ); } From 09a950570e87a9620c63737f634516f650d24bc3 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 17:10:32 -0400 Subject: [PATCH 16/20] Update provider export syntax --- src/contexts/Config.jsx | 4 ++-- src/contexts/Network.jsx | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/contexts/Config.jsx b/src/contexts/Config.jsx index 23b96ddf8f..3c71ccf21d 100644 --- a/src/contexts/Config.jsx +++ b/src/contexts/Config.jsx @@ -5,7 +5,7 @@ import { withNetwork } from './Network'; const ConfigContext = React.createContext({}); -class provider extends Component { +class Provider extends Component { constructor (props) { super(props); @@ -80,7 +80,7 @@ class provider extends Component { } } -export const ConfigProvider = withNetwork(provider); +export const ConfigProvider = withNetwork(Provider); export const Config = ({ children }) => ( diff --git a/src/contexts/Network.jsx b/src/contexts/Network.jsx index cf3d17d773..10f12ccb1e 100644 --- a/src/contexts/Network.jsx +++ b/src/contexts/Network.jsx @@ -12,7 +12,7 @@ import APIClient from '../api'; const NetworkContext = React.createContext({}); -class prov extends Component { +class Provider extends Component { constructor (props) { super(props); @@ -72,8 +72,8 @@ class prov extends Component { } } -export { NetworkProvider as _NetworkProvider }; -export const NetworkProvider = withRootDialog(withRouter(prov)); +export { Provider as _NetworkProvider }; +export const NetworkProvider = withRootDialog(withRouter(Provider)); export function withNetwork (Child) { return (props) => ( From 83e6255ba45e4892eae8066f9427c4d26a5905a6 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Thu, 11 Apr 2019 17:16:42 -0400 Subject: [PATCH 17/20] update NotifyAndRedirect to function component --- src/components/NotifyAndRedirect.jsx | 66 ++++++++++++++-------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/components/NotifyAndRedirect.jsx b/src/components/NotifyAndRedirect.jsx index 1741c8813a..0bb030440a 100644 --- a/src/components/NotifyAndRedirect.jsx +++ b/src/components/NotifyAndRedirect.jsx @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React from 'react'; import { Redirect, withRouter } from 'react-router-dom'; @@ -6,39 +6,39 @@ import { Trans } from '@lingui/macro'; import { withRootDialog } from '../contexts/RootDialog'; -class NotifyAndRedirect extends Component { - constructor (props) { - super(props); +const NotifyAndRedirect = ({ + to, + push, + from, + exact, + strict, + sensitive, + setRootDialogMessage, + location +}) => { + setRootDialogMessage({ + title: '404', + bodyText: ( + + Cannot find route + {` ${location.pathname}`} + . + + ), + variant: 'warning' + }); - const { setRootDialogMessage, location } = this.props; - setRootDialogMessage({ - title: '404', - bodyText: ( - - Cannot find route - {` ${location.pathname}`} - . - - ), - variant: 'warning' - }); - } - - render () { - const { to, push, from, exact, strict, sensitive } = this.props; - - return ( - - ); - } -} + return ( + + ); +}; export { NotifyAndRedirect as _NotifyAndRedirect }; export default withRootDialog(withRouter(NotifyAndRedirect)); From b9e0b2e0ade87ab7d4f71f52280b9a3fcf0e1a18 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 12 Apr 2019 14:35:18 -0400 Subject: [PATCH 18/20] update to correct grab handleHttpError from props instead of state --- src/pages/Organizations/screens/OrganizationsList.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index c2064fc923..daeeace8e1 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -173,8 +173,8 @@ class OrganizationsList extends Component { } async handleOrgDelete (event) { - const { orgsToDelete, handleHttpError } = this.state; - const { api } = this.props; + const { orgsToDelete } = this.state; + const { api, handleHttpError } = this.props; let errorHandled; try { From 63894bf82217c42cb3393b5fd55c2d57413cfc14 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 12 Apr 2019 14:35:35 -0400 Subject: [PATCH 19/20] update Promise.all map functions to not be async --- .../screens/Organization/OrganizationEdit.jsx | 8 ++------ src/pages/Organizations/screens/OrganizationAdd.jsx | 5 ++--- src/pages/Organizations/screens/OrganizationsList.jsx | 2 +- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx index 807f0ea58a..2b5fe68ca1 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx @@ -45,12 +45,8 @@ class OrganizationEdit extends Component { const url = organization.related.instance_groups; try { - await Promise.all(groupsToAssociate.map(async id => { - await api.associateInstanceGroup(url, id); - })); - await Promise.all(groupsToDisassociate.map(async id => { - await api.disassociate(url, id); - })); + await Promise.all(groupsToAssociate.map(id => api.associateInstanceGroup(url, id))); + await Promise.all(groupsToDisassociate.map(id => api.disassociate(url, id))); } catch (err) { handleHttpError(err) || this.setState({ error: err }); } diff --git a/src/pages/Organizations/screens/OrganizationAdd.jsx b/src/pages/Organizations/screens/OrganizationAdd.jsx index b957ede9d0..219e5da82d 100644 --- a/src/pages/Organizations/screens/OrganizationAdd.jsx +++ b/src/pages/Organizations/screens/OrganizationAdd.jsx @@ -36,9 +36,8 @@ class OrganizationAdd extends React.Component { const { data: response } = await api.createOrganization(values); const instanceGroupsUrl = response.related.instance_groups; try { - await Promise.all(groupsToAssociate.map(async id => { - await api.associateInstanceGroup(instanceGroupsUrl, id); - })); + await Promise.all(groupsToAssociate.map(id => api + .associateInstanceGroup(instanceGroupsUrl, id))); this.handleSuccess(response.id); } catch (err) { handleHttpError(err) || this.setState({ error: err }); diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index daeeace8e1..96bf1e83ee 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -178,7 +178,7 @@ class OrganizationsList extends Component { let errorHandled; try { - await Promise.all(orgsToDelete.map(async (org) => api.destroyOrganization(org.id))); + await Promise.all(orgsToDelete.map((org) => api.destroyOrganization(org.id))); this.handleClearOrgsToDelete(); } catch (err) { errorHandled = handleHttpError(err); From 526b640329046914d39e23f152db8c262239c634 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Fri, 12 Apr 2019 15:23:45 -0400 Subject: [PATCH 20/20] fix translation marked org and org access list strings --- .../components/OrganizationAccessList.jsx | 4 +- .../screens/OrganizationsList.jsx | 138 +++++++++--------- 2 files changed, 71 insertions(+), 71 deletions(-) diff --git a/src/pages/Organizations/components/OrganizationAccessList.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx index 09034eb111..c30c31943c 100644 --- a/src/pages/Organizations/components/OrganizationAccessList.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -355,8 +355,8 @@ class OrganizationAccessList extends React.Component { isOpen={showWarning} onClose={this.hideWarning} actions={[ - , - + , + ]} > {warningMsg} diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 96bf1e83ee..a3746ebaab 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -153,7 +153,7 @@ class OrganizationsList extends Component { handleOpenOrgDeleteModal () { const { results, selected } = this.state; - const warningTitle = i18nMark(`Delete Organization${selected.length > 1 ? 's' : ''}`); + const warningTitle = selected.length > 1 ? i18nMark('Delete Organization') : i18nMark('Delete Organizations'); const warningMsg = i18nMark('Are you sure you want to delete:'); const orgsToDelete = []; @@ -271,60 +271,60 @@ class OrganizationsList extends Component { } = this.state; const { match } = this.props; return ( - - - { isModalOpen && ( - Delete, - - ]} - > - {warningMsg} -
- {orgsToDelete.map((org) => ( - - - {org.name} - + + {({ i18n }) => ( + + + { isModalOpen && ( + {i18n._(t`Delete`)}, + + ]} + > + {warningMsg}
- - ))} -
- - )} - {noInitialResults && ( - - - - <Trans>No Organizations Found</Trans> - - - Please add an organization to populate this list - - - ) || ( - - - - {({ i18n }) => ( + {orgsToDelete.map((org) => ( + + + {org.name} + +
+ + ))} +
+ + )} + {noInitialResults && ( + + + + <Trans>No Organizations Found</Trans> + + + Please add an organization to populate this list + + + ) || ( + +
    { results.map(o => ( ))}
- )} - - - { loading ?
loading...
: '' } - { error ?
error
: '' } - - )} - - + + { loading ?
loading...
: '' } + { error ?
error
: '' } + + )} + + + )} + ); } }