diff --git a/__tests__/api.test.js b/__tests__/api.test.js index a6e12ecdba..c61bd980ea 100644 --- a/__tests__/api.test.js +++ b/__tests__/api.test.js @@ -155,14 +155,14 @@ describe('APIClient (api.js)', () => { done(); }); - test('disassociateInstanceGroup calls expected http method with expected data', async (done) => { + test('disassociate 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); + await api.disassociate(url, id); expect(mockHttp.post).toHaveBeenCalledTimes(1); expect(mockHttp.post.mock.calls[0][0]).toEqual(url); diff --git a/__tests__/components/AccessList.test.jsx b/__tests__/components/AccessList.test.jsx deleted file mode 100644 index d7ab1862fb..0000000000 --- a/__tests__/components/AccessList.test.jsx +++ /dev/null @@ -1,160 +0,0 @@ -import React from 'react'; -import { mount } from 'enzyme'; -import { MemoryRouter } from 'react-router-dom'; -import { I18nProvider } from '@lingui/react'; - -import AccessList from '../../src/components/AccessList'; - -const mockResults = [ - { - id: 1, - username: 'boo', - url: '/foo/bar/', - first_name: 'john', - last_name: 'smith', - summary_fields: { - foo: [ - { - role: { - name: 'foo', - id: 2, - } - } - ], - } - } -]; - -describe('', () => { - test('initially renders succesfully', () => { - mount( - - - {}} - /> - - - ); - }); - - test('api response data passed to component gets set to state properly', (done) => { - const wrapper = mount( - - - ({ data: { count: 1, results: mockResults } })} - /> - - - ).find('AccessList'); - - setImmediate(() => { - expect(wrapper.state().results).toEqual(mockResults); - done(); - }); - }); - - test('onExpand and onCompact methods called when user clicks on Expand and Compact icons respectively', async (done) => { - const onExpand = jest.spyOn(AccessList.prototype, 'onExpand'); - const onCompact = jest.spyOn(AccessList.prototype, 'onCompact'); - const wrapper = mount( - - - ({ data: { count: 1, results: mockResults } })} - /> - - - ).find('AccessList'); - expect(onExpand).not.toHaveBeenCalled(); - expect(onCompact).not.toHaveBeenCalled(); - - setImmediate(() => { - const rendered = wrapper.update(); - rendered.find('button[aria-label="Expand"]').simulate('click'); - rendered.find('button[aria-label="Collapse"]').simulate('click'); - expect(onExpand).toHaveBeenCalled(); - expect(onCompact).toHaveBeenCalled(); - done(); - }); - }); - - test('onSort being passed properly to DataListToolbar component', async (done) => { - const onSort = jest.spyOn(AccessList.prototype, 'onSort'); - const wrapper = mount( - - - ({ data: { count: 1, results: mockResults } })} - /> - - - ).find('AccessList'); - expect(onSort).not.toHaveBeenCalled(); - - setImmediate(() => { - const rendered = wrapper.update(); - rendered.find('button[aria-label="Sort"]').simulate('click'); - expect(onSort).toHaveBeenCalled(); - done(); - }); - }); - - test('getTeamRoles returns empty array if dataset is missing team_id attribute', (done) => { - const mockData = [ - { - id: 1, - username: 'boo', - url: '/foo/bar/', - first_name: 'john', - last_name: 'smith', - summary_fields: { - foo: [ - { - role: { - name: 'foo', - id: 2, - } - } - ], - direct_access: [ - { - role: { - name: 'team user', - id: 3, - } - } - ] - } - } - ]; - const wrapper = mount( - - - ({ data: { count: 1, results: mockData } })} - /> - - - ).find('AccessList'); - - setImmediate(() => { - const { results } = wrapper.state(); - results.forEach(result => { - expect(result.teamRoles).toEqual([]); - }); - done(); - }); - }); -}); diff --git a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx new file mode 100644 index 0000000000..588ff8df7a --- /dev/null +++ b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx @@ -0,0 +1,215 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import { MemoryRouter } from 'react-router-dom'; +import { I18nProvider } from '@lingui/react'; + +import OrganizationAccessList from '../../../../src/pages/Organizations/components/OrganizationAccessList'; + +const mockData = [ + { + id: 1, + username: 'boo', + url: '/foo/bar/', + first_name: 'john', + last_name: 'smith', + summary_fields: { + foo: [ + { + role: { + name: 'foo', + id: 2, + } + } + ], + } + } +]; + +describe('', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('initially renders succesfully', () => { + mount( + + + {}} + removeRole={() => {}} + /> + + + ); + }); + + test('api response data passed to component gets set to state properly', (done) => { + const wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + + setImmediate(() => { + expect(wrapper.state().results).toEqual(mockData); + done(); + }); + }); + + 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 wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + expect(onExpand).not.toHaveBeenCalled(); + expect(onCompact).not.toHaveBeenCalled(); + + setImmediate(() => { + const rendered = wrapper.update(); + rendered.find('button[aria-label="Expand"]').simulate('click'); + rendered.find('button[aria-label="Collapse"]').simulate('click'); + expect(onExpand).toHaveBeenCalled(); + expect(onCompact).toHaveBeenCalled(); + done(); + }); + }); + + test('onSort being passed properly to DataListToolbar component', async (done) => { + const onSort = jest.spyOn(OrganizationAccessList.prototype, 'onSort'); + const wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + expect(onSort).not.toHaveBeenCalled(); + + setImmediate(() => { + const rendered = wrapper.update(); + rendered.find('button[aria-label="Sort"]').simulate('click'); + expect(onSort).toHaveBeenCalled(); + done(); + }); + }); + + test('getTeamRoles returns empty array if dataset is missing team_id attribute', (done) => { + const wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + + setImmediate(() => { + const { results } = wrapper.state(); + results.forEach(result => { + expect(result.teamRoles).toEqual([]); + }); + done(); + }); + }); + + 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 wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + expect(handleWarning).not.toHaveBeenCalled(); + expect(confirmDelete).not.toHaveBeenCalled(); + expect(removeRole).not.toHaveBeenCalled(); + + setImmediate(() => { + 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'); + expect(confirmDelete).toHaveBeenCalled(); + expect(removeRole).toHaveBeenCalled(); + done(); + }); + }); + + test('state is set appropriately when a user tries deleting a role', (done) => { + const wrapper = mount( + + + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + /> + + + ).find('OrganizationAccessList'); + + setImmediate(() => { + const expected = [ + { + deleteType: 'users' + }, + { + deleteRoleId: mockData[0].summary_fields.foo[0].role.id + }, + { + deleteResourceId: mockData[0].id + } + ]; + 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'); + expect(wrapper.state().warningTitle).not.toBe(null); + expect(wrapper.state().warningMsg).not.toBe(null); + expected.forEach(criteria => { + Object.keys(criteria).forEach(key => { + expect(wrapper.state()[key]).toEqual(criteria[key]); + }); + }); + done(); + }); + }); +}); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx index 4a42274379..915c077364 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -8,9 +8,13 @@ const mockAPIAccessList = { foo: 'bar', }; -const mockGetOrganzationAccessList = jest.fn(() => ( - Promise.resolve(mockAPIAccessList) -)); +const mockGetOrganzationAccessList = () => Promise.resolve(mockAPIAccessList); + +const mockResponse = { + status: 'success', +}; + +const mockRemoveRole = () => Promise.resolve(mockResponse); describe('', () => { test('initially renders succesfully', () => { @@ -37,11 +41,14 @@ describe('', () => { params={{}} api={{ getOrganzationAccessList: mockGetOrganzationAccessList, + disassociate: mockRemoveRole }} /> ).find('OrganizationAccess'); const accessList = await wrapper.instance().getOrgAccessList(); expect(accessList).toEqual(mockAPIAccessList); + const resp = await wrapper.instance().removeRole(2, 3, 'users'); + expect(resp).toEqual(mockResponse); }); }); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx index 39c638bba6..1160f6ed4d 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationEdit.test.jsx @@ -183,7 +183,7 @@ describe('', () => { getOrganizationInstanceGroups: getOrganizationInstanceGroupsFn, updateOrganizationDetails: updateOrganizationDetailsFn, associateInstanceGroup: associateInstanceGroupFn, - disassociateInstanceGroup: disassociateInstanceGroupFn + disassociate: disassociateInstanceGroupFn }; const wrapper = mount( diff --git a/src/api.js b/src/api.js index 41cdb7525d..f87c51703e 100644 --- a/src/api.js +++ b/src/api.js @@ -126,7 +126,7 @@ class APIClient { return this.http.post(url, { id }); } - disassociateInstanceGroup (url, id) { + disassociate (url, id) { return this.http.post(url, { id, disassociate: true }); } } diff --git a/src/app.scss b/src/app.scss index 1f0d7ee0c2..7726c4131b 100644 --- a/src/app.scss +++ b/src/app.scss @@ -234,7 +234,6 @@ // note that these should be given a consistent prefix // and bem style, as well as moved into component-based scss files // - .awx-lookup { min-height: 36px; @@ -254,6 +253,52 @@ --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; + --pf-c-alert__title--PaddingBottom: 20px; + --pf-c-alert__title--PaddingLeft: 20px; +} + +.pf-c-alert__description { + --pf-c-alert__description--PaddingRight: 20px; + --pf-c-alert__description--PaddingBottom: 20px; + --pf-c-alert__description--PaddingLeft: 20px; +} + +.pf-c-alert { + position: absolute; + width: 100%; + z-index: 20; +} + +.pf-c-alert__icon { + --pf-c-alert__icon--Color: white; +} + .at-u-textRight { text-align: right; } + +.awx-c-chip { + padding: 3px 0; + height: 24px; + margin-right: 10px; + margin-bottom: 10px; +} + +.awx-c-form-action-group { + float: right; + display: block; + + .pf-m-danger { + margin-top: 20px; + margin-right: 20px; + } +} \ No newline at end of file diff --git a/src/components/AccessList/index.js b/src/components/AccessList/index.js deleted file mode 100644 index f435e8bb1f..0000000000 --- a/src/components/AccessList/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import AccessList from './Access.list'; - -export default AccessList; diff --git a/src/components/BasicChip/basicChip.scss b/src/components/BasicChip/basicChip.scss index 11b6601ec7..8cea76d049 100644 --- a/src/components/BasicChip/basicChip.scss +++ b/src/components/BasicChip/basicChip.scss @@ -7,4 +7,4 @@ .pf-c-button { display: none; } -} \ No newline at end of file +} diff --git a/src/components/AccessList/Access.list.jsx b/src/pages/Organizations/components/OrganizationAccessList.jsx similarity index 66% rename from src/components/AccessList/Access.list.jsx rename to src/pages/Organizations/components/OrganizationAccessList.jsx index c8d3aaec35..60616756c8 100644 --- a/src/components/AccessList/Access.list.jsx +++ b/src/pages/Organizations/components/OrganizationAccessList.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { DataList, DataListItem, DataListCell, Text, - TextContent, TextVariants + TextContent, TextVariants, Chip, Alert, AlertActionCloseButton, Button } from '@patternfly/react-core'; import { I18n, i18nMark } from '@lingui/react'; @@ -13,13 +13,12 @@ import { Link } from 'react-router-dom'; -import BasicChip from '../BasicChip/BasicChip'; -import Pagination from '../Pagination'; -import DataListToolbar from '../DataListToolbar'; +import Pagination from '../../../components/Pagination'; +import DataListToolbar from '../../../components/DataListToolbar'; import { parseQueryString, -} from '../../qs'; +} from '../../../qs'; const userRolesWrapperStyle = { display: 'flex', @@ -79,7 +78,7 @@ const UserName = ({ value, url }) => { return username; }; -class AccessList extends React.Component { +class OrganizationAccessList extends React.Component { columns = [ { name: i18nMark('Name'), key: 'first_name', isSortable: true }, { name: i18nMark('Username'), key: 'username', isSortable: true }, @@ -100,10 +99,17 @@ class AccessList extends React.Component { this.state = { page, page_size, - count: null, + count: 0, sortOrder: 'ascending', sortedColumnKey: 'username', isCompact: false, + showWarning: false, + warningTitle: '', + warningMsg: '', + deleteType: '', + deleteRoleId: null, + deleteResourceId: null, + results: [], }; this.fetchOrgAccessList = this.fetchOrgAccessList.bind(this); @@ -112,7 +118,10 @@ class AccessList extends React.Component { this.onCompact = this.onCompact.bind(this); this.onSort = this.onSort.bind(this); this.getQueryParams = this.getQueryParams.bind(this); - this.getTeamRoles = this.getTeamRoles.bind(this); + this.removeAccessRole = this.removeAccessRole.bind(this); + this.handleWarning = this.handleWarning.bind(this); + this.hideWarning = this.hideWarning.bind(this); + this.confirmDelete = this.confirmDelete.bind(this); } componentDidMount () { @@ -171,23 +180,6 @@ class AccessList extends React.Component { return Object.assign({}, this.defaultParams, searchParams, overrides); } - getRoles = roles => Object.values(roles) - .reduce((val, role) => { - if (role.length > 0) { - val.push(role[0].role); - } - return val; - }, []); - - getTeamRoles = roles => roles - .reduce((val, item) => { - if (item.role.team_id) { - const { role } = item; - val.push(role); - } - return val; - }, []); - async fetchOrgAccessList (queryParams) { const { match, getAccessList } = this.props; @@ -203,7 +195,7 @@ class AccessList extends React.Component { try { const { data: - { count = null, results = null } + { count = 0, results = [] } } = await getAccessList(match.params.id, queryParams); const pageCount = Math.ceil(count / page_size); @@ -216,13 +208,27 @@ class AccessList extends React.Component { sortedColumnKey, results, }; + results.forEach((result) => { - if (result.summary_fields.direct_access) { - result.teamRoles = this.getTeamRoles(result.summary_fields.direct_access); - } else { - result.teamRoles = []; - } - result.userRoles = this.getRoles(result.summary_fields) || []; + // Separate out roles into user roles or team roles + // based on whether or not a team_id attribute is present + const teamRoles = []; + const userRoles = []; + Object.values(result.summary_fields).forEach(field => { + if (field.length > 0) { + field.forEach(item => { + const { role } = item; + if (role.team_id) { + teamRoles.push(role); + } else { + userRoles.push(role); + } + }); + } + }); + + result.teamRoles = teamRoles; + result.userRoles = userRoles; }); this.setState(stateToUpdate); } catch (error) { @@ -230,6 +236,59 @@ class AccessList extends React.Component { } } + async removeAccessRole (roleId, resourceId, type) { + const { removeRole } = this.props; + const url = `/api/v2/${type}/${resourceId}/roles/`; + try { + await removeRole(url, roleId); + } catch (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) { + let warningTitle; + let warningMsg; + + if (type === 'users') { + warningTitle = i18nMark('User Access Removal'); + warningMsg = i18nMark(`Please confirm that you would like 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.`); + } + + this.setState({ + showWarning: true, + warningMsg, + warningTitle, + deleteType: type, + deleteRoleId: roleId, + deleteResourceId: resourceId + }); + } + + hideWarning () { + this.setState({ showWarning: false }); + } + + confirmDelete () { + const { deleteType, deleteResourceId, deleteRoleId } = this.state; + this.removeAccessRole(deleteRoleId, deleteResourceId, deleteType); + } + render () { const { results, @@ -241,13 +300,16 @@ class AccessList extends React.Component { sortedColumnKey, sortOrder, isCompact, + warningMsg, + warningTitle, + showWarning } = this.state; return ( - {!error && !results && ( + {!error && results.length <= 0 && (

Loading...

// TODO: replace with proper loading state )} - {error && !results && ( + {error && results.length <= 0 && (
{error.message}
{error.response && ( @@ -255,7 +317,7 @@ class AccessList extends React.Component { )}
// TODO: replace with proper error handling )} - {results && ( + {results.length > 0 && ( + {showWarning && ( + } + > + {warningMsg} + + + + + + )} + {({ i18n }) => ( @@ -304,10 +380,13 @@ class AccessList extends React.Component { > {i18n._(t`User Roles`)} {result.userRoles.map(role => ( - + className="awx-c-chip" + onClick={() => this.handleWarning(role.name, role.id, result.username, result.id, 'users')} + > + {role.name} + ))} )} @@ -318,10 +397,13 @@ class AccessList extends React.Component { > {i18n._(t`Team Roles`)} {result.teamRoles.map(role => ( - + className="awx-c-chip" + onClick={() => this.handleWarning(role.name, role.id, role.team_name, role.team_id, 'teams')} + > + {role.name} + ))} )} @@ -346,8 +428,9 @@ class AccessList extends React.Component { } } -AccessList.propTypes = { +OrganizationAccessList.propTypes = { getAccessList: PropTypes.func.isRequired, + removeRole: PropTypes.func.isRequired, }; -export default AccessList; +export default OrganizationAccessList; diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index dde9a44621..85b090ee73 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -112,7 +112,7 @@ class Organization extends Component { return ( - + { cardHeader } { - await api.disassociateInstanceGroup(url, id); + await api.disassociate(url, id); })); } catch (err) { this.setState({ error: err });