From fd513f704bac01617fa07ac1f2ea8f09c05f4d2f Mon Sep 17 00:00:00 2001 From: kialam Date: Tue, 18 Dec 2018 10:30:48 -0500 Subject: [PATCH 01/54] Add a contributing doc to our repo. --- CONTRIBUTING.md | 164 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..c4031e2572 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,164 @@ +# Ansible AWX/Tower V2 + +Hi there! We're excited to have you as a contributor. + +Have questions about this document or anything not covered here? Feel free to reach out to any of the contributors of this repository found here: https://github.com/ansible/awx-pf/graphs/contributors + +## Table of contents + +* [Things to know prior to submitting code](#things-to-know-prior-to-submitting-code) +* [Setting up your development environment](#setting-up-your-development-environment) + * [Prerequisites](#prerequisites) + * [Node and npm](#node-and-npm) +* [Build the user interface](#build-the-user-interface) +* [Accessing the AWX web interface](#accessing-the-awx-web-interface) +* [Working with React](#working-with-react) + * [Class constructors vs Class properties](#class-constructors-vs-class-properties) + * [Binding](#binding) +* [Testing](#testing) + * [Jest](#jest) + * [Enzyme](#enzyme) + +## Things to know prior to submitting code + +- All code submissions are done through pull requests against the `master` branch. +- If collaborating with someone else on the same branch, please use `--force-with-lease` instead of `--force` when pushing up code. This will prevent you from accidentally overwriting commits pushed by someone else. For more information, see https://git-scm.com/docs/git-push#git-push---force-with-leaseltrefnamegt + +## Setting up your development environment + +The UI is built using [ReactJS](https://reactjs.org/docs/getting-started.html) and [Patternfly](https://www.patternfly.org/). + +### Prerequisites + +#### Node and npm + +The AWX UI requires the following: + +- Node 8.x LTS +- NPM 6.x LTS + +Run the following to install all the dependencies: +```bash +(host) $ npm run install +``` + +#### Build the User Interface + +Run the following to build the AWX UI: + +```bash +(host) $ npm run start +``` + +## Accessing the AWX web interface + +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 +### 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. + +BAD: + ```javascript + class MyComponent extends React.Component { + constructor(props) { + super(props); + } + + myEventHandler = () => { + // do a thing + } + } + ``` +GOOD: + ```javascript + class MyComponent extends React.Component { + constructor(props) { + super(props); + this.myEventHandler = this.myEventHandler.bind(this); + } + + myEventHandler() { + // do a thing + } + } + ``` + +### Binding +It is good practice to bind our class methods within our class constructor method for the following reasons: + 1. Avoid defining the method every time `render()` is called. + 2. [Performance advantages](https://stackoverflow.com/a/44844916). + 3. Ease of [testing](https://github.com/airbnb/enzyme/issues/365). + +### Component Lifecycle + +A React Component has various [lifecylce methods](http://projects.wojtekmaj.pl/react-lifecycle-methods-diagram/). Understanding the basic lifecycle of a Component will help you determine which method to utilize for your specific need. Below are some general guidelines: + +BAD: + Use `render` method to make asynchronous calls. + ```javascript + render () { + const { data } = await api.get(API_CONFIG); + + return(
`Hello, ${data.usename}!`
); + } + ``` +GOOD: + Use `componentDidMount()` method to make asynchronous calls to retrieve data a Componenet may need. + ```javascript + async componentDidMount () { + try { + const { data } = await api.get(API_CONFIG); + this.setState({ data }); + } catch (error) { + this.setState({ error }); + } + } + + render() { + return(
`Hello, ${this.state.data.usename}!`
) + } + ``` +## Testing +All code, new or otherwise, should have at least 80% test coverage. +### Jest +We use (Jest)[https://jestjs.io/] for our JS testing framework. +Like many other JS test frameworks (Karma, Mocha, etc), Jest includes their own `spyOn` method as a way for us to test our class methods. +```javascript + const spy = jest.spyOn(MyButton.prototype, 'onSubmit'); +``` + +Jest also allows us to mock the data we expect from an external dependency, such as an API. +```javascript + axios.get.mockImplementation((endpoint) => { + if (endpoint === '/api/v2/config') { + return new Promise((resolve, reject) => { + resolve({ data: { foo: 'bar' }); + }); + } + else { + return 'get results'; + } + }); +``` + +### Enzyme +We use (Enzyme)[https://airbnb.io/enzyme/] to test our React Components. +### Mounting Components wrapped with withRouter +If you are testing a Component wrapped in React Router's `withRouter` class, you can mount the component by wrapping it with the `` component. +```javascript + test('initially renders succesfully', () => { + mount( + + + + ); + }); +``` +You can test the wrapped Component's methods like so: +```javascript + const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onCancel'); +``` From ebd09883fea7900648807c9099a124218a1bebc2 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 11 Dec 2018 12:58:08 -0500 Subject: [PATCH 02/54] update DataListToolbar component and tests --- __tests__/components/DataListToolbar.test.jsx | 126 +++++++++++++++++- .../DataListToolbar/DataListToolbar.jsx | 3 +- 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/__tests__/components/DataListToolbar.test.jsx b/__tests__/components/DataListToolbar.test.jsx index c1c3a4e3af..e4e5591e60 100644 --- a/__tests__/components/DataListToolbar.test.jsx +++ b/__tests__/components/DataListToolbar.test.jsx @@ -4,7 +4,6 @@ import { I18nProvider } from '@lingui/react'; import DataListToolbar from '../../src/components/DataListToolbar'; describe('', () => { - const columns = [{ name: 'Name', key: 'name', isSortable: true }]; let toolbar; afterEach(() => { @@ -15,6 +14,8 @@ describe('', () => { }); test('it triggers the expected callbacks', () => { + const columns = [{ name: 'Name', key: 'name', isSortable: true }]; + const search = 'button[aria-label="Search"]'; const searchTextInput = 'input[aria-label="Search text input"]'; const selectAll = 'input[aria-label="Select all"]'; @@ -55,4 +56,127 @@ describe('', () => { expect(onSearch).toHaveBeenCalledTimes(1); expect(onSearch).toBeCalledWith('test-321'); }); + + test('dropdown items sortable columns work', () => { + const multipleColumns = [ + { name: 'Foo', key: 'foo', isSortable: true }, + { name: 'Bar', key: 'bar', isSortable: true }, + { name: 'Bakery', key: 'bakery', isSortable: true }, + { name: 'Baz', key: 'baz' } + ]; + + const onSearch = jest.fn(); + const onSort = jest.fn(); + const onSelectAll = jest.fn(); + toolbar = mount( + + ); + const sortdropdownToggle = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup .pf-l-toolbar__item button'); + expect(sortdropdownToggle.length).toBe(2); + sortdropdownToggle.at(1).simulate('click'); + sortdropdownToggle.at(0).simulate('click'); + toolbar.update(); + const sortDropdownItems = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup button.pf-c-dropdown__menu-item'); + expect(sortDropdownItems.length).toBe(2); + const mockedSortEvent = { target: { innerText: 'Bar' } }; + sortDropdownItems.at(0).simulate('click', mockedSortEvent); + toolbar = mount( + + ); + toolbar.update(); + const sortdropdownToggleDescending = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup .pf-l-toolbar__item button'); + expect(sortdropdownToggleDescending.length).toBe(2); + sortdropdownToggleDescending.at(1).simulate('click'); + sortdropdownToggleDescending.at(0).simulate('click'); + toolbar.update(); + const sortDropdownItemsDescending = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup button.pf-c-dropdown__menu-item'); + expect(sortDropdownItemsDescending.length).toBe(2); + const mockedSortEventDescending = { target: { innerText: 'Bar' } }; + sortDropdownItems.at(0).simulate('click', mockedSortEventDescending); + toolbar.update(); + const searchDropdownToggle = toolbar.find('.pf-c-dropdown.searchKeyDropdown button.pf-c-dropdown__toggle'); + expect(searchDropdownToggle.length).toBe(1); + searchDropdownToggle.at(0).simulate('click'); + toolbar.update(); + const searchDropdownItems = toolbar.find('.pf-c-dropdown.searchKeyDropdown button.pf-c-dropdown__menu-item'); + expect(searchDropdownItems.length).toBe(3); + const mockedSearchEvent = { target: { innerText: 'Bar' } }; + searchDropdownItems.at(0).simulate('click', mockedSearchEvent); + }); + + test('it displays correct sort icon', () => { + const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; + const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; + const onSearch = jest.fn(); + const onSort = jest.fn(); + const onSelectAll = jest.fn(); + toolbar = mount( + + ); + const downNumericIcon = toolbar.find('SortNumericDownIcon'); + expect(downNumericIcon.length).toBe(1); + toolbar = mount( + + ); + const upNumericIcon = toolbar.find('SortNumericUpIcon'); + expect(upNumericIcon.length).toBe(1); + toolbar = mount( + + ); + const downAlphaIcon = toolbar.find('SortAlphaDownIcon'); + expect(downAlphaIcon.length).toBe(1); + toolbar = mount( + + ); + const upAlphaIcon = toolbar.find('SortAlphaUpIcon'); + expect(upAlphaIcon.length).toBe(1); + }); }); diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index e1d2ea6842..51bf9f782c 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -152,6 +152,7 @@ class DataListToolbar extends React.Component {
- + Date: Tue, 11 Dec 2018 16:53:17 -0500 Subject: [PATCH 03/54] update app and towerlogo tests and remove stale code --- __tests__/App.test.jsx | 29 +++++++------------------ __tests__/components/TowerLogo.test.jsx | 14 ------------ src/App.jsx | 7 +----- src/components/TowerLogo/TowerLogo.jsx | 7 +----- 4 files changed, 10 insertions(+), 47 deletions(-) diff --git a/__tests__/App.test.jsx b/__tests__/App.test.jsx index 7b98adafff..cbffd97f86 100644 --- a/__tests__/App.test.jsx +++ b/__tests__/App.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { HashRouter as Router } from 'react-router-dom'; +import { MemoryRouter } from 'react-router-dom'; import { shallow, mount } from 'enzyme'; import App from '../src/App'; import api from '../src/api'; @@ -7,10 +7,6 @@ import { API_LOGOUT } from '../src/endpoints'; import Dashboard from '../src/pages/Dashboard'; import Login from '../src/pages/Login'; -import { asyncFlush } from '../jest.setup'; - -const DEFAULT_ACTIVE_GROUP = 'views_group'; -const DEFAULT_ACTIVE_ITEM = 'views_group_dashboard'; describe('', () => { test('renders without crashing', () => { @@ -22,7 +18,7 @@ describe('', () => { api.isAuthenticated = jest.fn(); api.isAuthenticated.mockReturnValue(false); - const appWrapper = mount(); + const appWrapper = mount(); const login = appWrapper.find(Login); expect(login.length).toBe(1); @@ -34,7 +30,7 @@ describe('', () => { api.isAuthenticated = jest.fn(); api.isAuthenticated.mockReturnValue(true); - const appWrapper = mount(); + const appWrapper = mount(); const dashboard = appWrapper.find(Dashboard); expect(dashboard.length).toBe(1); @@ -49,24 +45,15 @@ describe('', () => { expect(appWrapper.state().isNavOpen).toBe(false); }); - test('onLogoClick sets selected nav back to defaults', () => { - const appWrapper = shallow(); - appWrapper.setState({ activeGroup: 'foo', activeItem: 'bar' }); - expect(appWrapper.state().activeItem).toBe('bar'); - expect(appWrapper.state().activeGroup).toBe('foo'); - appWrapper.instance().onLogoClick(); - expect(appWrapper.state().activeGroup).toBe(DEFAULT_ACTIVE_GROUP); - }); - test('api.logout called from logout button', async () => { + const logOutButtonSelector = 'button[aria-label="Logout"]'; api.get = jest.fn().mockImplementation(() => Promise.resolve({})); - const appWrapper = shallow(); - appWrapper.instance().onDevLogout(); + const appWrapper = mount(); + const logOutButton = appWrapper.find(logOutButtonSelector); + expect(logOutButton.length).toBe(1); + logOutButton.simulate('click'); appWrapper.setState({ activeGroup: 'foo', activeItem: 'bar' }); expect(api.get).toHaveBeenCalledTimes(1); expect(api.get).toHaveBeenCalledWith(API_LOGOUT); - await asyncFlush(); - expect(appWrapper.state().activeItem).toBe(DEFAULT_ACTIVE_ITEM); - expect(appWrapper.state().activeGroup).toBe(DEFAULT_ACTIVE_GROUP); }); }); diff --git a/__tests__/components/TowerLogo.test.jsx b/__tests__/components/TowerLogo.test.jsx index 3bd40afaee..10e3461443 100644 --- a/__tests__/components/TowerLogo.test.jsx +++ b/__tests__/components/TowerLogo.test.jsx @@ -43,20 +43,6 @@ describe('', () => { expect(towerLogoElem.props().history.length).toBe(2); }); - test('gracefully handles not being passed click handler', () => { - logoWrapper = mount( - - - - - - ); - findChildren(); - expect(towerLogoElem.props().history.length).toBe(1); - logoWrapper.simulate('click'); - expect(towerLogoElem.props().history.length).toBe(1); - }); - test('handles mouse over and out state.hover changes', () => { const onLogoClick = jest.fn(); logoWrapper = mount( diff --git a/src/App.jsx b/src/App.jsx index 38465599d8..3b03de3382 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -81,13 +81,8 @@ class App extends React.Component { this.setState(({ isNavOpen }) => ({ isNavOpen: !isNavOpen })); }; - onLogoClick = () => { - this.setState({ activeGroup: 'views_group' }); - } - onDevLogout = async () => { await api.get(API_LOGOUT); - this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); } render () { @@ -134,7 +129,7 @@ class App extends React.Component { } + logo={} toolbar={PageToolbar} showNavToggle onNavToggle={this.onNavToggle} diff --git a/src/components/TowerLogo/TowerLogo.jsx b/src/components/TowerLogo/TowerLogo.jsx index 082777ba7e..6b9d0e3a30 100644 --- a/src/components/TowerLogo/TowerLogo.jsx +++ b/src/components/TowerLogo/TowerLogo.jsx @@ -15,13 +15,8 @@ class TowerLogo extends Component { } onClick = () => { - const { history, onClick: handleClick } = this.props; - - if (!handleClick) return; - + const { history } = this.props; history.push('/'); - - handleClick(); }; onHover = () => { From e48c734925ed414aa2411d94d18e0544c0412056 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 11 Dec 2018 16:53:33 -0500 Subject: [PATCH 04/54] update datelisttoolbar test --- __tests__/components/DataListToolbar.test.jsx | 179 +++++++++++------- 1 file changed, 109 insertions(+), 70 deletions(-) diff --git a/__tests__/components/DataListToolbar.test.jsx b/__tests__/components/DataListToolbar.test.jsx index e4e5591e60..1c1a3d8b04 100644 --- a/__tests__/components/DataListToolbar.test.jsx +++ b/__tests__/components/DataListToolbar.test.jsx @@ -58,6 +58,11 @@ describe('', () => { }); test('dropdown items sortable columns work', () => { + const sortDropdownToggleSelector = '.pf-l-toolbar__group.sortDropdownGroup .pf-l-toolbar__item button'; + const sortDropdownItemsSelector = '.pf-l-toolbar__group.sortDropdownGroup button.pf-c-dropdown__menu-item'; + const searchDropdownToggleSelector = '.pf-c-dropdown.searchKeyDropdown .pf-c-dropdown__toggle'; + const searchDropdownItemsSelector = '.pf-c-dropdown.searchKeyDropdown button.pf-c-dropdown__menu-item'; + const multipleColumns = [ { name: 'Foo', key: 'foo', isSortable: true }, { name: 'Bar', key: 'bar', isSortable: true }, @@ -68,115 +73,149 @@ describe('', () => { const onSearch = jest.fn(); const onSort = jest.fn(); const onSelectAll = jest.fn(); + toolbar = mount( - + + + ); - const sortdropdownToggle = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup .pf-l-toolbar__item button'); - expect(sortdropdownToggle.length).toBe(2); - sortdropdownToggle.at(1).simulate('click'); - sortdropdownToggle.at(0).simulate('click'); + const sortDropdownToggle = toolbar.find(sortDropdownToggleSelector); + expect(sortDropdownToggle.length).toBe(2); + sortDropdownToggle.at(1).simulate('click'); + sortDropdownToggle.at(0).simulate('click'); toolbar.update(); - const sortDropdownItems = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup button.pf-c-dropdown__menu-item'); + + const sortDropdownItems = toolbar.find(sortDropdownItemsSelector); expect(sortDropdownItems.length).toBe(2); + const mockedSortEvent = { target: { innerText: 'Bar' } }; sortDropdownItems.at(0).simulate('click', mockedSortEvent); toolbar = mount( - + + + ); toolbar.update(); - const sortdropdownToggleDescending = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup .pf-l-toolbar__item button'); - expect(sortdropdownToggleDescending.length).toBe(2); - sortdropdownToggleDescending.at(1).simulate('click'); - sortdropdownToggleDescending.at(0).simulate('click'); + + const sortDropdownToggleDescending = toolbar.find(sortDropdownToggleSelector); + expect(sortDropdownToggleDescending.length).toBe(2); + sortDropdownToggleDescending.at(1).simulate('click'); + sortDropdownToggleDescending.at(0).simulate('click'); toolbar.update(); - const sortDropdownItemsDescending = toolbar.find('.pf-l-toolbar__group.sortDropdownGroup button.pf-c-dropdown__menu-item'); + + const sortDropdownItemsDescending = toolbar.find(sortDropdownItemsSelector); expect(sortDropdownItemsDescending.length).toBe(2); + const mockedSortEventDescending = { target: { innerText: 'Bar' } }; sortDropdownItems.at(0).simulate('click', mockedSortEventDescending); toolbar.update(); - const searchDropdownToggle = toolbar.find('.pf-c-dropdown.searchKeyDropdown button.pf-c-dropdown__toggle'); + + const searchDropdownToggle = toolbar.find(searchDropdownToggleSelector); expect(searchDropdownToggle.length).toBe(1); searchDropdownToggle.at(0).simulate('click'); toolbar.update(); - const searchDropdownItems = toolbar.find('.pf-c-dropdown.searchKeyDropdown button.pf-c-dropdown__menu-item'); + + const searchDropdownItems = toolbar.find(searchDropdownItemsSelector); expect(searchDropdownItems.length).toBe(3); + const mockedSearchEvent = { target: { innerText: 'Bar' } }; searchDropdownItems.at(0).simulate('click', mockedSearchEvent); }); test('it displays correct sort icon', () => { + const downNumericIconSelector = 'SortNumericDownIcon'; + const upNumericIconSelector = 'SortNumericUpIcon'; + const downAlphaIconSelector = 'SortAlphaDownIcon'; + const upAlphaIconSelector = 'SortAlphaUpIcon'; + const numericColumns = [{ name: 'ID', key: 'id', isSortable: true, isNumeric: true }]; const alphaColumns = [{ name: 'Name', key: 'name', isSortable: true, isNumeric: false }]; const onSearch = jest.fn(); const onSort = jest.fn(); const onSelectAll = jest.fn(); + toolbar = mount( - + + + ); - const downNumericIcon = toolbar.find('SortNumericDownIcon'); + + const downNumericIcon = toolbar.find(downNumericIconSelector); expect(downNumericIcon.length).toBe(1); + toolbar = mount( - + + + ); - const upNumericIcon = toolbar.find('SortNumericUpIcon'); + + const upNumericIcon = toolbar.find(upNumericIconSelector); expect(upNumericIcon.length).toBe(1); + toolbar = mount( - + + + ); - const downAlphaIcon = toolbar.find('SortAlphaDownIcon'); + + const downAlphaIcon = toolbar.find(downAlphaIconSelector); expect(downAlphaIcon.length).toBe(1); + toolbar = mount( - + + + ); - const upAlphaIcon = toolbar.find('SortAlphaUpIcon'); + + const upAlphaIcon = toolbar.find(upAlphaIconSelector); expect(upAlphaIcon.length).toBe(1); }); }); From 11583dbff0170c0aacfd78c791920a4f9c34a148 Mon Sep 17 00:00:00 2001 From: John Mitchell Date: Tue, 11 Dec 2018 16:53:44 -0500 Subject: [PATCH 05/54] update pagination tests --- __tests__/components/Pagination.test.jsx | 93 ++++++++++++++++++++++++ src/components/Pagination/Pagination.jsx | 23 +++--- 2 files changed, 102 insertions(+), 14 deletions(-) diff --git a/__tests__/components/Pagination.test.jsx b/__tests__/components/Pagination.test.jsx index 93dbead8ec..6360098c7a 100644 --- a/__tests__/components/Pagination.test.jsx +++ b/__tests__/components/Pagination.test.jsx @@ -72,4 +72,97 @@ describe('', () => { expect(onSetPage).toHaveBeenCalledTimes(2); expect(onSetPage).toBeCalledWith(1, 5); }); + + test('previous button does not work on page 1', () => { + const previous = 'button[aria-label="First"]'; + const onSetPage = jest.fn(); + + pagination = mount( + + + + ); + pagination.find(previous).simulate('click'); + expect(onSetPage).toHaveBeenCalledTimes(0); + }); + + test('changing pageSize works', () => { + const pageSizeDropdownToggleSelector = 'DropdownToggle DropdownToggle[className="togglePageSize"]'; + const pageSizeDropdownItemsSelector = 'DropdownItem'; + const onSetPage = jest.fn(); + + pagination = mount( + + + + ); + const pageSizeDropdownToggle = pagination.find(pageSizeDropdownToggleSelector); + expect(pageSizeDropdownToggle.length).toBe(1); + pageSizeDropdownToggle.at(0).simulate('click'); + + const pageSizeDropdownItems = pagination.find(pageSizeDropdownItemsSelector); + expect(pageSizeDropdownItems.length).toBe(3); + pageSizeDropdownItems.at(1).simulate('click'); + }); + + test('submit a new page by typing in input works', () => { + const textInputSelector = '.pf-l-split__item.pf-m-main .pf-c-form-control'; + const submitFormSelector = '.pf-l-split__item.pf-m-main form'; + + const onSetPage = jest.fn(); + + pagination = mount( + + + + ); + const textInput = pagination.find(textInputSelector); + expect(textInput.length).toBe(1); + textInput.simulate('change'); + pagination.setProps({ page: 2 }); + + const submitForm = pagination.find(submitFormSelector); + expect(submitForm.length).toBe(1); + submitForm.simulate('submit'); + pagination.setState({ value: 'invalid' }); + submitForm.simulate('submit'); + }); + + test('text input page change is disabled when only 1 page', () => { + const onSetPage = jest.fn(); + + pagination = mount( + + + + ); + }); }); diff --git a/src/components/Pagination/Pagination.jsx b/src/components/Pagination/Pagination.jsx index d48f69e72c..001f02f504 100644 --- a/src/components/Pagination/Pagination.jsx +++ b/src/components/Pagination/Pagination.jsx @@ -7,14 +7,9 @@ import { DropdownDirection, DropdownItem, DropdownToggle, - Form, - FormGroup, Level, LevelItem, TextInput, - Toolbar, - ToolbarGroup, - ToolbarItem, Split, SplitItem, } from '@patternfly/react-core'; @@ -32,7 +27,7 @@ class Pagination extends Component { const { page } = this.props; if (prevProps.page !== page) { - this.setState({ value: page }); + this.onPageChange(page); } } @@ -51,13 +46,13 @@ class Pagination extends Component { if (isValid) { onSetPage(value, page_size); - } else{ + } else { this.setState({ value: page }); } }; onFirst = () => { - const { onSetPage, page_size} = this.props; + const { onSetPage, page_size } = this.props; onSetPage(1, page_size); }; @@ -67,7 +62,7 @@ class Pagination extends Component { const previousPage = page - 1; if (previousPage >= 1) { - onSetPage(previousPage, page_size) + onSetPage(previousPage, page_size); } }; @@ -76,7 +71,7 @@ class Pagination extends Component { const nextPage = page + 1; if (nextPage <= pageCount) { - onSetPage(nextPage, page_size) + onSetPage(nextPage, page_size); } }; @@ -139,18 +134,18 @@ class Pagination extends Component { direction={up} isOpen={isOpen} toggle={( - + { page_size } - )}> + )} + > {opts.map(option => ( { option } ))} - Per Page + Per Page From 9bc87b3e806ec8d482a93f6d3f81c1ad176c11d8 Mon Sep 17 00:00:00 2001 From: kialam Date: Mon, 17 Dec 2018 11:44:11 -0500 Subject: [PATCH 06/54] Implement React Context API - Move API GET request to /v2/config out to the top level of our App. - Store /v2/config response data in sessionStorage. - Use Context API to pass down relevant data to Organizations component. - Wrap our AnsibleSelect component as a context consumer and pass in the list of Ansible Environments of the logged in user. - Clear sessionStorage object when user logs out. - Update unit tests. --- __mocks__/axios.js | 21 +- __tests__/components/AnsibleSelect.test.jsx | 25 +- .../views/Organization.add.test.jsx | 23 +- package.json | 1 + src/App.jsx | 228 ++++++++++++------ .../AnsibleSelect/AnsibleSelect.jsx | 15 +- src/context.jsx | 3 + .../Organizations/views/Organization.add.jsx | 41 ++-- 8 files changed, 246 insertions(+), 111 deletions(-) create mode 100644 src/context.jsx diff --git a/__mocks__/axios.js b/__mocks__/axios.js index 23f96b475f..aad497303f 100644 --- a/__mocks__/axios.js +++ b/__mocks__/axios.js @@ -1,5 +1,13 @@ -const axios = require('axios'); +import * as endpoints from '../src/endpoints'; +const axios = require('axios'); +const mockAPIConfigData = { + data: { + custom_virtualenvs: ['foo', 'bar'], + ansible_version: "2.7.2", + version: "2.1.1-40-g2758a3848" + } +}; jest.genMockFromModule('axios'); axios.create = jest.fn(() => axios); @@ -9,7 +17,16 @@ axios.create.mockReturnValue({ get: axios.get, post: axios.post }); -axios.get.mockResolvedValue('get results'); +axios.get.mockImplementation((endpoint) => { + if (endpoint === endpoints.API_CONFIG) { + return new Promise((resolve, reject) => { + resolve(mockAPIConfigData); + }); + } + else { + return 'get results'; + } +}); axios.post.mockResolvedValue('post results'); axios.customClearMocks = () => { diff --git a/__tests__/components/AnsibleSelect.test.jsx b/__tests__/components/AnsibleSelect.test.jsx index 47a068ef4f..d2eacf2129 100644 --- a/__tests__/components/AnsibleSelect.test.jsx +++ b/__tests__/components/AnsibleSelect.test.jsx @@ -2,16 +2,29 @@ import React from 'react'; import { mount } from 'enzyme'; import AnsibleSelect from '../../src/components/AnsibleSelect'; -const mockData = ['foo', 'bar']; +const label = "test select" +const mockData = ["/venv/baz/", "/venv/ansible/"]; describe('', () => { - test('initially renders succesfully', async() => { - const wrapper = mount( {}} />); - wrapper.setState({ isHidden: false }); + test('initially renders succesfully', async () => { + mount( + { }} + labelName={label} + data={mockData} + /> + ); }); test('calls "onSelectChange" on dropdown select change', () => { const spy = jest.spyOn(AnsibleSelect.prototype, 'onSelectChange'); - const wrapper = mount( {}} />); - wrapper.setState({ isHidden: false }); + const wrapper = mount( + { }} + labelName={label} + data={mockData} + /> + ); expect(spy).not.toHaveBeenCalled(); wrapper.find('select').simulate('change'); expect(spy).toHaveBeenCalled(); diff --git a/__tests__/pages/Organizations/views/Organization.add.test.jsx b/__tests__/pages/Organizations/views/Organization.add.test.jsx index 8c25eb1eef..30990942a4 100644 --- a/__tests__/pages/Organizations/views/Organization.add.test.jsx +++ b/__tests__/pages/Organizations/views/Organization.add.test.jsx @@ -1,8 +1,29 @@ import React from 'react'; import { mount } from 'enzyme'; -import OrganizationAdd from '../../../../src/pages/Organizations/views/Organization.add'; import { MemoryRouter } from 'react-router-dom'; +let OrganizationAdd; +const getAppWithConfigContext = (context = { + custom_virtualenvs: ['foo', 'bar'] +}) => { + + // Mock the ConfigContext module being used in our OrganizationAdd component + jest.doMock('../../../../src/context', () => { + return { + ConfigContext: { + Consumer: (props) => props.children(context) + } + } + }); + + // Return the updated OrganizationAdd module with mocked context + return require('../../../../src/pages/Organizations/views/Organization.add').default; +}; + +beforeEach(() => { + OrganizationAdd = getAppWithConfigContext(); +}) + describe('', () => { test('initially renders succesfully', () => { mount( diff --git a/package.json b/package.json index 833f11652c..c2d0e85ac0 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "@patternfly/react-styles": "^2.3.0", "@patternfly/react-tokens": "^1.9.0", "axios": "^0.18.0", + "prop-types": "^15.6.2", "react": "^16.4.1", "react-dom": "^16.4.1", "react-router-dom": "^4.3.1" diff --git a/src/App.jsx b/src/App.jsx index 3b03de3382..7e65262444 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -1,6 +1,11 @@ import React, { Fragment } from 'react'; +<<<<<<< HEAD import { I18nProvider, I18n } from '@lingui/react'; import { t } from '@lingui/macro'; +======= +import { ConfigContext } from './context'; + +>>>>>>> Implement React Context API import { Redirect, Switch, @@ -22,7 +27,7 @@ import { import { global_breakpoint_md as breakpointMd } from '@patternfly/react-tokens'; import api from './api'; -import { API_LOGOUT } from './endpoints'; +import { API_LOGOUT, API_CONFIG } from './endpoints'; import HelpDropdown from './components/HelpDropdown'; import LogoutButton from './components/LogoutButton'; @@ -68,24 +73,39 @@ const languageWithoutRegionCode = language.toLowerCase().split(/[_-]+/)[0]; class App extends React.Component { - constructor (props) { + constructor(props) { super(props); const isNavOpen = typeof window !== 'undefined' && window.innerWidth >= parseInt(breakpointMd.value, 10); this.state = { - isNavOpen + isNavOpen, }; } + getSessionObject(key) { + return JSON.parse(sessionStorage.getItem(key) || '{}'); + } + onNavToggle = () => { this.setState(({ isNavOpen }) => ({ isNavOpen: !isNavOpen })); }; onDevLogout = async () => { await api.get(API_LOGOUT); + this.setState({ activeGroup: 'views_group', activeItem: 'views_group_dashboard' }); + if (sessionStorage.config) { + sessionStorage.clear(); + } } - render () { + async componentDidMount() { + // Grab our config data from the API and store in sessionStorage + if (!sessionStorage.config) { + const { data } = await api.get(API_CONFIG); + sessionStorage.setItem('config', JSON.stringify(data)); + } + } + render() { const { isNavOpen } = this.state; const { logo, loginInfo, history } = this.props; @@ -119,17 +139,12 @@ class App extends React.Component { }} /> - api.isAuthenticated()} - redirectPath="/" - path="/login" - component={() => } - /> + api.isAuthenticated()} redirectPath="/" path="/login" component={() => } /> } + logo={} toolbar={PageToolbar} showNavToggle onNavToggle={this.onNavToggle} @@ -139,66 +154,133 @@ class App extends React.Component { - {({ i18n }) => ( - - )} - + )} /> )} @@ -214,7 +296,9 @@ class App extends React.Component { !api.isAuthenticated()} redirectPath="/login" path="/projects" component={Projects} /> !api.isAuthenticated()} redirectPath="/login" path="/inventories" component={Inventories} /> !api.isAuthenticated()} redirectPath="/login" path="/inventory_scripts" component={InventoryScripts} /> - !api.isAuthenticated()} redirectPath="/login" path="/organizations" component={Organizations} /> + + !api.isAuthenticated()} redirectPath="/login" path="/organizations" component={Organizations} /> + !api.isAuthenticated()} redirectPath="/login" path="/users" component={Users} /> !api.isAuthenticated()} redirectPath="/login" path="/teams" component={Teams} /> !api.isAuthenticated()} redirectPath="/login" path="/credential_types" component={CredentialTypes} /> diff --git a/src/components/AnsibleSelect/AnsibleSelect.jsx b/src/components/AnsibleSelect/AnsibleSelect.jsx index d411c1d17a..001e83853b 100644 --- a/src/components/AnsibleSelect/AnsibleSelect.jsx +++ b/src/components/AnsibleSelect/AnsibleSelect.jsx @@ -1,4 +1,5 @@ import React from 'react'; + import { FormGroup, Select, @@ -16,19 +17,19 @@ class AnsibleSelect extends React.Component { } render() { - const { hidden } = this.props; - if (hidden) { - return null; - } else { + if (this.props.data.length > 1) { return ( - ); + ) + } + else { + return null; } } } diff --git a/src/context.jsx b/src/context.jsx new file mode 100644 index 0000000000..446f82bd63 --- /dev/null +++ b/src/context.jsx @@ -0,0 +1,3 @@ +import React from "react"; + +export const ConfigContext = React.createContext({}); diff --git a/src/pages/Organizations/views/Organization.add.jsx b/src/pages/Organizations/views/Organization.add.jsx index 023817fe1d..75e2147a0c 100644 --- a/src/pages/Organizations/views/Organization.add.jsx +++ b/src/pages/Organizations/views/Organization.add.jsx @@ -1,4 +1,5 @@ import React, { Fragment } from 'react'; +import PropTypes from 'prop-types'; import { withRouter } from 'react-router-dom'; import { Trans } from '@lingui/macro'; import { @@ -17,7 +18,8 @@ import { CardBody, } from '@patternfly/react-core'; -import { API_ORGANIZATIONS, API_CONFIG } from '../../../endpoints'; +import { ConfigContext } from '../../../context'; +import { API_ORGANIZATIONS } from '../../../endpoints'; import api from '../../../api'; import AnsibleSelect from '../../../components/AnsibleSelect' const { light } = PageSectionVariants; @@ -38,8 +40,6 @@ class OrganizationAdd extends React.Component { description: '', instanceGroups: '', custom_virtualenv: '', - custom_virtualenvs: [], - hideAnsibleSelect: true, error:'', }; @@ -69,22 +69,10 @@ class OrganizationAdd extends React.Component { this.props.history.push('/organizations'); } - async componentDidMount() { - try { - const { data } = await api.get(API_CONFIG); - this.setState({ custom_virtualenvs: [...data.custom_virtualenvs] }); - if (this.state.custom_virtualenvs.length > 1) { - // Show dropdown if we have more than one ansible environment - this.setState({ hideAnsibleSelect: !this.state.hideAnsibleSelect }); - } - } catch (error) { - this.setState({ error }) - } - - } render() { const { name } = this.state; const enabled = name.length > 0; // TODO: add better form validation + return ( @@ -128,13 +116,16 @@ class OrganizationAdd extends React.Component { onChange={this.handleChange} /> -