From 92d3cb6dc4c7fefbc2695210ae1dfccf497e4836 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Fri, 22 Feb 2019 14:35:58 -0500 Subject: [PATCH 1/9] Replace Lookup tags with pf-react Chip component --- __tests__/components/Lookup.test.jsx | 10 +++++----- src/app.scss | 16 ---------------- src/components/Lookup/Lookup.jsx | 28 ++++++++++++++++------------ 3 files changed, 21 insertions(+), 33 deletions(-) diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index 9402dcbbcc..c505fe1bc1 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -114,8 +114,7 @@ describe('', () => { removeIcon.simulate('click'); expect(spy).toHaveBeenCalled(); }); - test('"wrapTags" method properly handles data', () => { - const spy = jest.spyOn(Lookup.prototype, 'wrapTags'); + test('renders chips from prop value', () => { mockData = [{ name: 'foo', id: 0 }, { name: 'bar', id: 1 }]; const wrapper = mount( @@ -130,9 +129,10 @@ describe('', () => { /> ); - expect(spy).toHaveBeenCalled(); - const pill = wrapper.find('span.awx-c-tag--pill'); - expect(pill).toHaveLength(2); + const chip = wrapper.find('li.pf-c-chip'); + const overflowChip = wrapper.find('.pf-c-chip.pf-m-overflow'); + expect(chip).toHaveLength(1); + expect(overflowChip).toHaveLength(1); }); test('toggleSelected successfully adds/removes row from lookupSelectedItems state', () => { mockData = [{ name: 'foo', id: 1 }]; diff --git a/src/app.scss b/src/app.scss index 0a1e430ded..934240376c 100644 --- a/src/app.scss +++ b/src/app.scss @@ -228,26 +228,10 @@ } } -.awx-c-icon--remove { - padding-left: 10px; - &:hover { - cursor: pointer; - } -} - .awx-c-list { border-bottom: 1px solid #d7d7d7; } -.awx-c-tag--pill { - color: var(--pf-global--BackgroundColor--light-100); - background-color: rgb(0, 123, 186); - border-radius: 3px; - margin: 1px 2px; - padding: 0 10px; - display: inline-block; -} - .at-c-listCardBody { --pf-c-card__footer--PaddingX: 0; --pf-c-card__footer--PaddingY: 0; diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index 54fb3dbebf..8568498f8e 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -2,6 +2,8 @@ import React, { Fragment } from 'react'; import PropTypes from 'prop-types'; import { SearchIcon, CubesIcon } from '@patternfly/react-icons'; import { + Chip, + ChipGroup, Modal, Button, EmptyState, @@ -134,17 +136,6 @@ class Lookup extends React.Component { this.handleModalToggle(); } - wrapTags (tags = []) { - return tags.map(tag => ( - - {tag.name} - - - )); - } - render () { const { isModalOpen, @@ -159,6 +150,19 @@ class Lookup extends React.Component { } = this.state; const { lookupHeader = 'items', value, columns } = this.props; + let chips = null; + if (value) { + chips = ( + + {value.map(chip => ( + this.toggleSelected(chip)}> + {chip.name} + + ))} + + ); + } + return ( {({ i18n }) => ( @@ -166,7 +170,7 @@ class Lookup extends React.Component { -
{this.wrapTags(value)}
+
{chips}
Date: Fri, 22 Feb 2019 14:47:25 -0500 Subject: [PATCH 2/9] Rename createInstanceGroups api method to associateInstanceGroup --- .../pages/Organizations/screens/OrganizationAdd.test.jsx | 8 ++++---- src/api.js | 2 +- src/pages/Organizations/screens/OrganizationAdd.jsx | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx index 1ce8ae0a6e..9d5079ce70 100644 --- a/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx +++ b/__tests__/pages/Organizations/screens/OrganizationAdd.test.jsx @@ -71,7 +71,7 @@ describe('', () => { test('Successful form submission triggers redirect', (done) => { const onSuccess = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onSuccess'); const mockedResp = { data: { id: 1, related: { instance_groups: '/bar' } } }; - const api = { createOrganization: jest.fn().mockResolvedValue(mockedResp), createInstanceGroups: jest.fn().mockResolvedValue('done') }; + const api = { createOrganization: jest.fn().mockResolvedValue(mockedResp), associateInstanceGroup: jest.fn().mockResolvedValue('done') }; const wrapper = mount( @@ -131,10 +131,10 @@ describe('', () => { } } }); - const createInstanceGroupsFn = jest.fn().mockResolvedValue('done'); + const associateInstanceGroupFn = jest.fn().mockResolvedValue('done'); const api = { createOrganization: createOrganizationFn, - createInstanceGroups: createInstanceGroupsFn + associateInstanceGroup: associateInstanceGroupFn }; const wrapper = mount( @@ -156,7 +156,7 @@ describe('', () => { description: '', name: 'mock org' }); - expect(createInstanceGroupsFn).toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 1); + expect(associateInstanceGroupFn).toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 1); }); test('AnsibleSelect component renders if there are virtual environments', () => { diff --git a/src/api.js b/src/api.js index f0d3c866b8..1bbb68ce2f 100644 --- a/src/api.js +++ b/src/api.js @@ -110,7 +110,7 @@ class APIClient { return this.http.get(API_INSTANCE_GROUPS, { params }); } - createInstanceGroups (url, id) { + associateInstanceGroup (url, id) { return this.http.post(url, { id }); } } diff --git a/src/pages/Organizations/screens/OrganizationAdd.jsx b/src/pages/Organizations/screens/OrganizationAdd.jsx index 6dbd224781..5f38a237f8 100644 --- a/src/pages/Organizations/screens/OrganizationAdd.jsx +++ b/src/pages/Organizations/screens/OrganizationAdd.jsx @@ -61,7 +61,7 @@ class OrganizationAdd extends React.Component { try { if (instanceGroups.length > 0) { instanceGroups.forEach(async (select) => { - await api.createInstanceGroups(instanceGroupsUrl, select.id); + await api.associateInstanceGroup(instanceGroupsUrl, select.id); }); } } catch (err) { From f1fefbf5f08cbe816cf0de2e2373e7219ecff889 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Fri, 22 Feb 2019 14:51:38 -0500 Subject: [PATCH 3/9] Add organization details patch and instance groups disassociate methods to api --- __tests__/api.test.js | 32 ++++++++++++++++++++++++++++++++ src/api.js | 10 ++++++++++ 2 files changed, 42 insertions(+) diff --git a/__tests__/api.test.js b/__tests__/api.test.js index 31b69913b5..a6e12ecdba 100644 --- a/__tests__/api.test.js +++ b/__tests__/api.test.js @@ -138,4 +138,36 @@ describe('APIClient (api.js)', () => { done(); }); + + test('associateInstanceGroup calls expected http method with expected data', async (done) => { + const createPromise = () => Promise.resolve(); + const mockHttp = ({ post: jest.fn(createPromise) }); + + const api = new APIClient(mockHttp); + const url = 'foo/bar/'; + const id = 1; + await api.associateInstanceGroup(url, id); + + expect(mockHttp.post).toHaveBeenCalledTimes(1); + expect(mockHttp.post.mock.calls[0][0]).toEqual(url); + expect(mockHttp.post.mock.calls[0][1]).toEqual({ id }); + + done(); + }); + + test('disassociateInstanceGroup calls expected http method with expected data', async (done) => { + const createPromise = () => Promise.resolve(); + const mockHttp = ({ post: jest.fn(createPromise) }); + + const api = new APIClient(mockHttp); + const url = 'foo/bar/'; + const id = 1; + await api.disassociateInstanceGroup(url, id); + + expect(mockHttp.post).toHaveBeenCalledTimes(1); + expect(mockHttp.post.mock.calls[0][0]).toEqual(url); + expect(mockHttp.post.mock.calls[0][1]).toEqual({ id, disassociate: true }); + + done(); + }); }); diff --git a/src/api.js b/src/api.js index 1bbb68ce2f..1dc6d5a74b 100644 --- a/src/api.js +++ b/src/api.js @@ -70,6 +70,12 @@ class APIClient { return this.http.get(endpoint); } + updateOrganizationDetails (id, data) { + const endpoint = `${API_ORGANIZATIONS}${id}/`; + + return this.http.patch(endpoint, data); + } + getOrganizationInstanceGroups (id, params = {}) { const endpoint = `${API_ORGANIZATIONS}${id}/instance_groups/`; @@ -113,6 +119,10 @@ class APIClient { associateInstanceGroup (url, id) { return this.http.post(url, { id }); } + + disassociateInstanceGroup (url, id) { + return this.http.post(url, { id, disassociate: true }); + } } export default APIClient; From d2cf2c275b5a44f3ae02f3d6f7b4326b519def8d Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Fri, 22 Feb 2019 14:53:17 -0500 Subject: [PATCH 4/9] Add Organization Edit view --- src/components/Lookup/Lookup.jsx | 4 +- .../screens/Organization/Organization.jsx | 20 +- .../screens/Organization/OrganizationEdit.jsx | 304 +++++++++++++++++- 3 files changed, 304 insertions(+), 24 deletions(-) diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index 8568498f8e..bc57dbb1c4 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -30,9 +30,10 @@ const paginationStyling = { class Lookup extends React.Component { constructor (props) { super(props); + this.state = { isModalOpen: false, - lookupSelectedItems: [], + lookupSelectedItems: props.value || [], results: [], count: 0, page: 1, @@ -43,7 +44,6 @@ class Lookup extends React.Component { }; this.onSetPage = this.onSetPage.bind(this); this.handleModalToggle = this.handleModalToggle.bind(this); - this.wrapTags = this.wrapTags.bind(this); this.toggleSelected = this.toggleSelected.bind(this); this.saveModal = this.saveModal.bind(this); this.getData = this.getData.bind(this); diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index 2b63bf0877..248b93c6d3 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -119,14 +119,18 @@ class Organization extends Component { to="/organizations/:id/details" exact /> - ( - - )} - /> + {organization && ( + ( + + )} + /> + )} {organization && ( ( - -

edit view

- - save/cancel and go back to view - - -); +import { ConfigContext } from '../../../../context'; +import Lookup from '../../../../components/Lookup'; +import FormActionGroup from '../../../../components/FormActionGroup'; +import AnsibleSelect from '../../../../components/AnsibleSelect'; -export default OrganizationEdit; +class OrganizationEdit extends Component { + constructor (props) { + super(props); + + this.getInstanceGroups = this.getInstanceGroups.bind(this); + this.getRelatedInstanceGroups = this.getRelatedInstanceGroups.bind(this); + this.checkValidity = this.checkValidity.bind(this); + this.onFieldChange = this.onFieldChange.bind(this); + this.onLookupSave = this.onLookupSave.bind(this); + this.onSubmit = this.onSubmit.bind(this); + this.postInstanceGroups = this.postInstanceGroups.bind(this); + this.onCancel = this.onCancel.bind(this); + this.onSuccess = this.onSuccess.bind(this); + + this.state = { + form: { + name: { + value: '', + isValid: true, + validation: { + required: true + }, + helperTextInvalid: i18nMark('This field must not be blank') + }, + description: { + value: '' + }, + instanceGroups: { + value: null, + initialValue: null + }, + custom_virtualenv: { + value: '', + defaultValue: '/venv/ansible/' + } + }, + error: '', + formIsValid: true + }; + } + + async componentDidMount () { + const { organization } = this.props; + const { form: formData } = this.state; + + formData.name.value = organization.name; + formData.description.value = organization.description; + formData.custom_virtualenv.value = organization.custom_virtualenv; + + try { + formData.instanceGroups.value = await this.getRelatedInstanceGroups(); + formData.instanceGroups.initialValue = [...formData.instanceGroups.value]; + } catch (err) { + this.setState({ error: err }); + } + + this.setState({ form: formData }); + } + + onFieldChange (val, evt) { + const targetName = evt.target.name; + const value = val; + + const { form: updatedForm } = this.state; + const updatedFormEl = { ...updatedForm[targetName] }; + + updatedFormEl.value = value; + updatedForm[targetName] = updatedFormEl; + + let formIsValid = true; + if (updatedFormEl.validation) { + updatedFormEl.isValid = this.checkValidity(updatedFormEl.value, updatedFormEl.validation); + formIsValid = updatedFormEl.isValid && formIsValid; + } + + this.setState({ form: updatedForm, formIsValid }); + } + + onLookupSave (val, targetName) { + const { form: updatedForm } = this.state; + updatedForm[targetName].value = val; + + this.setState({ form: updatedForm }); + } + + async onSubmit () { + const { api, organization } = this.props; + const { form: { name, description, custom_virtualenv } } = this.state; + const formData = { name, description, custom_virtualenv }; + + const updatedData = {}; + Object.keys(formData) + .forEach(formId => { + updatedData[formId] = formData[formId].value; + }); + + try { + await api.updateOrganizationDetails(organization.id, updatedData); + await this.postInstanceGroups(); + } catch (err) { + this.setState({ error: err }); + } finally { + this.onSuccess(); + } + } + + onCancel () { + const { organization: { id }, history } = this.props; + history.push(`/organizations/${id}`); + } + + onSuccess () { + const { organization: { id }, history } = this.props; + history.push(`/organizations/${id}`); + } + + async getInstanceGroups (params) { + const { api } = this.props; + const data = await api.getInstanceGroups(params); + return data; + } + + async getRelatedInstanceGroups () { + const { + api, + organization: { id } + } = this.props; + const { data } = await api.getOrganizationInstanceGroups(id); + const { results } = data; + return results; + } + + checkValidity = (value, validation) => { + let isValid = true; + if (validation.required) { + isValid = value.trim() !== '' && isValid; + } + return isValid; + } + + async postInstanceGroups () { + const { api, organization } = this.props; + const { form: { instanceGroups } } = this.state; + const url = organization.related.instance_groups; + + const initialInstanceGroups = instanceGroups.initialValue.map(ig => ig.id); + const updatedInstanceGroups = instanceGroups.value.map(ig => ig.id); + + const groupsToAssociate = [...updatedInstanceGroups] + .filter(x => !initialInstanceGroups.includes(x)); + const groupsToDisassociate = [...initialInstanceGroups] + .filter(x => !updatedInstanceGroups.includes(x)); + + try { + await Promise.all(groupsToAssociate.map(async id => { + await api.associateInstanceGroup(url, id); + })); + await Promise.all(groupsToDisassociate.map(async id => { + await api.disassociateInstanceGroup(url, id); + })); + } catch (err) { + this.setState({ error: err }); + } + } + + render () { + const { + form: { + name, + description, + instanceGroups, + custom_virtualenv + }, + formIsValid, + error + } = this.state; + + const instanceGroupsLookupColumns = [ + { name: i18nMark('Name'), key: 'name', isSortable: true }, + { name: i18nMark('Modified'), key: 'modified', isSortable: false, isNumeric: true }, + { name: i18nMark('Created'), key: 'created', isSortable: false, isNumeric: true } + ]; + + return ( + +
+ + {({ i18n }) => ( + + + + + + + + + { instanceGroups.value + && ( + + ) + } + + + {({ custom_virtualenvs }) => ( + custom_virtualenvs && custom_virtualenvs.length > 1 && ( + + + + ) + )} + + + )} + + + { error ?
error
: '' } + +
+ ); + } +} + +OrganizationEdit.contextTypes = { + custom_virtualenvs: PropTypes.arrayOf(PropTypes.string) +}; + +export default withRouter(OrganizationEdit); From c3fc00c45a7dc904f93ffab8f0f001706abc5c9d Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Fri, 22 Feb 2019 14:53:47 -0500 Subject: [PATCH 5/9] Add Organization Edit tests and fix Lookup unit tests --- __tests__/components/Lookup.test.jsx | 17 +- .../Organization/OrganizationEdit.test.jsx | 238 +++++++++++++++++- 2 files changed, 241 insertions(+), 14 deletions(-) diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index c505fe1bc1..95763d4912 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -60,7 +60,7 @@ describe('', () => { ).find('Lookup'); expect(spy).not.toHaveBeenCalled(); - expect(wrapper.state('lookupSelectedItems')).toEqual([]); + expect(wrapper.state('lookupSelectedItems')).toEqual(mockSelected); const searchItem = wrapper.find('.pf-c-input-group__text#search'); searchItem.first().simulate('click'); expect(spy).toHaveBeenCalled(); @@ -110,7 +110,7 @@ describe('', () => { /> ); - const removeIcon = wrapper.find('.awx-c-icon--remove').first(); + const removeIcon = wrapper.find('button[aria-label="close"]').first(); removeIcon.simulate('click'); expect(spy).toHaveBeenCalled(); }); @@ -135,14 +135,13 @@ describe('', () => { expect(overflowChip).toHaveLength(1); }); test('toggleSelected successfully adds/removes row from lookupSelectedItems state', () => { - mockData = [{ name: 'foo', id: 1 }]; + mockData = []; const wrapper = mount( { }} value={mockData} - selected={[]} getItems={() => { }} columns={mockColumns} sortedColumnKey="name" @@ -164,7 +163,7 @@ describe('', () => { expect(wrapper.state('lookupSelectedItems')).toEqual([]); }); test('saveModal calls callback with selected items', () => { - mockData = [{ name: 'foo', id: 1 }]; + mockData = []; const onLookupSaveFn = jest.fn(); const wrapper = mount( @@ -198,10 +197,10 @@ describe('', () => { { }} - data={mockData} - selected={[]} + value={mockData} columns={mockColumns} sortedColumnKey="name" + getItems={() => { }} /> ).find('Lookup'); @@ -217,10 +216,10 @@ describe('', () => { { }} - data={mockData} - selected={[]} + value={mockData} columns={mockColumns} sortedColumnKey="name" + getItems={() => { }} /> ).find('Lookup'); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx index 6bcb5c6b37..39c638bba6 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx @@ -1,16 +1,244 @@ 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 APIClient from '../../../../../src/api'; import OrganizationEdit from '../../../../../src/pages/Organizations/screens/Organization/OrganizationEdit'; describe('', () => { - test('initially renders succesfully', () => { + const mockData = { + name: 'Foo', + description: 'Bar', + custom_virtualenv: 'Fizz', + id: 1, + related: { + instance_groups: '/api/v2/organizations/1/instance_groups' + } + }; + + test('should request related instance groups from api', () => { + const mockInstanceGroups = [ + { name: 'One', id: 1 }, + { name: 'Two', id: 2 } + ]; + const getOrganizationInstanceGroups = jest.fn(() => ( + Promise.resolve({ data: { results: mockInstanceGroups } }) + )); mount( - - + + + + + + ).find('OrganizationEdit'); + + expect(getOrganizationInstanceGroups).toHaveBeenCalledTimes(1); + }); + + test('componentDidMount should set instanceGroups to state', async () => { + const mockInstanceGroups = [ + { name: 'One', id: 1 }, + { name: 'Two', id: 2 } + ]; + const getOrganizationInstanceGroups = jest.fn(() => ( + Promise.resolve({ data: { results: mockInstanceGroups } }) + )); + const wrapper = mount( + + + + + + ).find('OrganizationEdit'); + + await wrapper.instance().componentDidMount(); + expect(wrapper.state().form.instanceGroups.value).toEqual(mockInstanceGroups); + }); + test('onLookupSave successfully sets instanceGroups state', () => { + const api = jest.fn(); + const wrapper = mount( + + + + + + ).find('OrganizationEdit'); + + wrapper.instance().onLookupSave([ + { + id: 1, + name: 'foo' + } + ], 'instanceGroups'); + expect(wrapper.state().form.instanceGroups.value).toEqual([ + { + id: 1, + name: 'foo' + } + ]); + }); + test('calls "onFieldChange" when input values change', () => { + const api = new APIClient(); + const spy = jest.spyOn(OrganizationEdit.WrappedComponent.prototype, 'onFieldChange'); + const wrapper = mount( + + + + + + ).find('OrganizationEdit'); + + expect(spy).not.toHaveBeenCalled(); + wrapper.instance().onFieldChange('foo', { target: { name: 'name' } }); + wrapper.instance().onFieldChange('bar', { target: { name: 'description' } }); + expect(spy).toHaveBeenCalledTimes(2); + }); + test('AnsibleSelect component renders if there are virtual environments', () => { + const api = jest.fn(); + const config = { + custom_virtualenvs: ['foo', 'bar'], + }; + const wrapper = mount( + + + + + + ); + expect(wrapper.find('FormSelect')).toHaveLength(1); + expect(wrapper.find('FormSelectOption')).toHaveLength(2); + }); + test('calls onSubmit when Save button is clicked', () => { + const api = jest.fn(); + const spy = jest.spyOn(OrganizationEdit.WrappedComponent.prototype, 'onSubmit'); + const wrapper = mount( + + + + + + ); + expect(spy).not.toHaveBeenCalled(); + wrapper.find('button[aria-label="Save"]').prop('onClick')(); + expect(spy).toBeCalled(); + }); + test('onSubmit associates and disassociates instance groups', async () => { + const mockInstanceGroups = [ + { name: 'One', id: 1 }, + { name: 'Two', id: 2 } + ]; + const getOrganizationInstanceGroupsFn = jest.fn(() => ( + Promise.resolve({ data: { results: mockInstanceGroups } }) + )); + const mockDataForm = { + name: 'Foo', + description: 'Bar', + custom_virtualenv: 'Fizz', + }; + const updateOrganizationDetailsFn = jest.fn().mockResolvedValue(1, mockDataForm); + const associateInstanceGroupFn = jest.fn().mockResolvedValue('done'); + const disassociateInstanceGroupFn = jest.fn().mockResolvedValue('done'); + const api = { + getOrganizationInstanceGroups: getOrganizationInstanceGroupsFn, + updateOrganizationDetails: updateOrganizationDetailsFn, + associateInstanceGroup: associateInstanceGroupFn, + disassociateInstanceGroup: disassociateInstanceGroupFn + }; + const wrapper = mount( + + + + + + ).find('OrganizationEdit'); + + await wrapper.instance().componentDidMount(); + + wrapper.instance().onLookupSave([ + { name: 'One', id: 1 }, + { name: 'Three', id: 3 } + ], 'instanceGroups'); + + await wrapper.instance().onSubmit(); + expect(updateOrganizationDetailsFn).toHaveBeenCalledWith(1, mockDataForm); + expect(associateInstanceGroupFn).toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 3); + expect(associateInstanceGroupFn).not.toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 1); + expect(associateInstanceGroupFn).not.toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 2); + + expect(disassociateInstanceGroupFn).toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 2); + expect(disassociateInstanceGroupFn).not.toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 1); + expect(disassociateInstanceGroupFn).not.toHaveBeenCalledWith('/api/v2/organizations/1/instance_groups', 3); + }); + + test('calls "onCancel" when Cancel button is clicked', () => { + const api = jest.fn(); + const spy = jest.spyOn(OrganizationEdit.WrappedComponent.prototype, 'onCancel'); + const wrapper = mount( + + + + + + ); + expect(spy).not.toHaveBeenCalled(); + wrapper.find('button[aria-label="Cancel"]').prop('onClick')(); + expect(spy).toBeCalled(); }); }); From ff339e0eba6c8ce44be40c3c303aaf98805a353b Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Mon, 25 Feb 2019 09:15:21 -0500 Subject: [PATCH 6/9] Update org edit form layout and remove chip group component from lookup; --- src/components/Lookup/Lookup.jsx | 22 +++--- .../screens/Organization/OrganizationEdit.jsx | 67 +++++++++---------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index bc57dbb1c4..454b937edb 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import { SearchIcon, CubesIcon } from '@patternfly/react-icons'; import { Chip, - ChipGroup, Modal, Button, EmptyState, @@ -150,18 +149,15 @@ class Lookup extends React.Component { } = this.state; const { lookupHeader = 'items', value, columns } = this.props; - let chips = null; - if (value) { - chips = ( - - {value.map(chip => ( - this.toggleSelected(chip)}> - {chip.name} - - ))} - - ); - } + const chips = value ? ( +
+ {value.map(chip => ( + this.toggleSelected(chip)}> + {chip.name} + + ))} +
+ ) : null; return ( diff --git a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx index 0ed3e8887d..2fd9f280ba 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx @@ -7,7 +7,6 @@ import { CardBody, Form, FormGroup, - Gallery, TextInput, } from '@patternfly/react-core'; @@ -201,10 +200,10 @@ class OrganizationEdit extends Component { return ( -
- - {({ i18n }) => ( - + + {({ i18n }) => ( + +
@@ -230,28 +228,9 @@ class OrganizationEdit extends Component { id="edit-org-form-description" name="description" onChange={this.onFieldChange} - type="text" value={description.value || ''} /> - - { instanceGroups.value - && ( - - ) - } - {({ custom_virtualenvs }) => ( custom_virtualenvs && custom_virtualenvs.length > 1 && ( @@ -271,16 +250,34 @@ class OrganizationEdit extends Component { ) )} - - )} - - - { error ?
error
: '' } - +
+ + { instanceGroups.value + && ( + + ) + } + + + { error ?
error
: '' } + + )} +
); } From ffefba9bf9aeab6e4e7aa89fbe80a16de0862cd1 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Mon, 25 Feb 2019 11:05:42 -0500 Subject: [PATCH 7/9] Fix bug where lookup parent state is not updated on toggleSelected --- src/components/Lookup/Lookup.jsx | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index 454b937edb..54be10b2d9 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -32,7 +32,7 @@ class Lookup extends React.Component { this.state = { isModalOpen: false, - lookupSelectedItems: props.value || [], + lookupSelectedItems: [...props.value] || [], results: [], count: 0, page: 1, @@ -101,17 +101,27 @@ class Lookup extends React.Component { }; toggleSelected (row) { - const { lookupSelectedItems } = this.state; - const selectedIndex = lookupSelectedItems + const { name, onLookupSave } = this.props; + const { lookupSelectedItems: updatedSelectedItems, isModalOpen } = this.state; + + const selectedIndex = updatedSelectedItems .findIndex(selectedRow => selectedRow.id === row.id); + if (selectedIndex > -1) { - lookupSelectedItems.splice(selectedIndex, 1); - this.setState({ lookupSelectedItems }); + updatedSelectedItems.splice(selectedIndex, 1); + this.setState({ lookupSelectedItems: updatedSelectedItems }); } else { this.setState(prevState => ({ lookupSelectedItems: [...prevState.lookupSelectedItems, row] })); } + + // Updates the selected items from parent state + // This handles the case where the user removes chips from the lookup input + // while the modal is closed + if (!isModalOpen) { + onLookupSave(updatedSelectedItems, name); + } } handleModalToggle () { From a7b51c526a0913a0fb89a4142f45f2f90944269d Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Mon, 25 Feb 2019 11:42:18 -0500 Subject: [PATCH 8/9] Update Lookup chip test --- __tests__/components/Lookup.test.jsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index 95763d4912..84d454fb3a 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -128,11 +128,9 @@ describe('', () => { sortedColumnKey="name" /> - ); + ).find('Lookup'); const chip = wrapper.find('li.pf-c-chip'); - const overflowChip = wrapper.find('.pf-c-chip.pf-m-overflow'); - expect(chip).toHaveLength(1); - expect(overflowChip).toHaveLength(1); + expect(chip).toHaveLength(2); }); test('toggleSelected successfully adds/removes row from lookupSelectedItems state', () => { mockData = []; From 053b21e832195358fa5fa0937d8beae789c0eb09 Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Tue, 26 Feb 2019 14:21:27 -0500 Subject: [PATCH 9/9] Use ternary operator in Org Edit form --- .../screens/Organization/OrganizationEdit.jsx | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx index 2fd9f280ba..fe1d99fa5c 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx @@ -43,8 +43,8 @@ class OrganizationEdit extends Component { value: '' }, instanceGroups: { - value: null, - initialValue: null + value: [], + initialValue: [] }, custom_virtualenv: { value: '', @@ -84,11 +84,10 @@ class OrganizationEdit extends Component { updatedFormEl.value = value; updatedForm[targetName] = updatedFormEl; - let formIsValid = true; - if (updatedFormEl.validation) { - updatedFormEl.isValid = this.checkValidity(updatedFormEl.value, updatedFormEl.validation); - formIsValid = updatedFormEl.isValid && formIsValid; - } + updatedFormEl.isValid = (updatedFormEl.validation) + ? this.checkValidity(updatedFormEl.value, updatedFormEl.validation) : true; + + const formIsValid = (updatedFormEl.validation) ? updatedFormEl.isValid : true; this.setState({ form: updatedForm, formIsValid }); } @@ -148,10 +147,9 @@ class OrganizationEdit extends Component { } checkValidity = (value, validation) => { - let isValid = true; - if (validation.required) { - isValid = value.trim() !== '' && isValid; - } + const isValid = (validation.required) + ? (value.trim() !== '') : true; + return isValid; } @@ -255,19 +253,15 @@ class OrganizationEdit extends Component { fieldId="edit-org-form-instance-groups" label={i18n._(t`Instance Groups`)} > - { instanceGroups.value - && ( - - ) - } +