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 index d7ab1862fb..1cb93ac9da 100644 --- a/__tests__/components/AccessList.test.jsx +++ b/__tests__/components/AccessList.test.jsx @@ -34,6 +34,7 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '1' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => {}} + removeRole={() => {}} /> @@ -48,6 +49,7 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} + removeRole={() => {}} /> @@ -69,6 +71,7 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} + removeRole={() => {}} /> @@ -95,6 +98,7 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} + removeRole={() => {}} /> @@ -144,6 +148,7 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} /> @@ -157,4 +162,36 @@ describe('', () => { done(); }); }); + + test('test handleWarning, confirmDelete, and removeRole methods for Alert component', async (done) => { + const handleWarning = jest.spyOn(AccessList.prototype, 'handleWarning'); + const confirmDelete = jest.spyOn(AccessList.prototype, 'confirmDelete'); + const removeRole = jest.spyOn(AccessList.prototype, 'removeRole'); + const wrapper = mount( + + + ({ data: { count: 1, results: mockResults } })} + removeRole={() => {}} + /> + + + ).find('AccessList'); + 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(); + }); + }); }); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx index 4a42274379..3ba80d596f 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -12,6 +12,14 @@ const mockGetOrganzationAccessList = jest.fn(() => ( Promise.resolve(mockAPIAccessList) )); +const mockResponse = { + status: 'success', +}; + +const mockRemoveRole = jest.fn(() => ( + Promise.resolve(mockResponse) +)); + describe('', () => { test('initially renders succesfully', () => { mount( @@ -37,11 +45,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 934240376c..b768b036e4 100644 --- a/src/app.scss +++ b/src/app.scss @@ -220,14 +220,6 @@ // and bem style, as well as moved into component-based scss files // -.awx-lookup { - min-height: 36px; - - .pf-c-form-control { - --pf-c-form-control--Height: auto; - } -} - .awx-c-list { border-bottom: 1px solid #d7d7d7; } @@ -238,3 +230,28 @@ --pf-c-card__body--PaddingX: 0; --pf-c-card__body--PaddingY: 0; } + + +.awx-c-card { + position: relative; +} + +.awx-c-alert { + position: absolute; + top: 0; + left: 0; + width: 100%; +} + +.pf-c-alert { + position: absolute; + width: 100%; + + & button { + margin-right: 20px; + } +} + +.pf-c-alert__icon > svg { + fill: white; +} \ No newline at end of file diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx index c8d3aaec35..cb0e9a09f9 100644 --- a/src/components/AccessList/Access.list.jsx +++ b/src/components/AccessList/Access.list.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,7 +13,6 @@ import { Link } from 'react-router-dom'; -import BasicChip from '../BasicChip/BasicChip'; import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; @@ -46,6 +45,10 @@ const hiddenStyle = { display: 'none', }; +const buttonGroupStyle = { + float: 'right', +}; + const Detail = ({ label, value, url, customStyles }) => { let detail = null; if (value) { @@ -104,6 +107,7 @@ class AccessList extends React.Component { sortOrder: 'ascending', sortedColumnKey: 'username', isCompact: false, + showWarning: false, }; this.fetchOrgAccessList = this.fetchOrgAccessList.bind(this); @@ -112,7 +116,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.removeRole = this.removeRole.bind(this); + this.handleWarning = this.handleWarning.bind(this); + this.hideWarning = this.hideWarning.bind(this); + this.confirmDelete = this.confirmDelete.bind(this); } componentDidMount () { @@ -171,23 +178,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; @@ -216,13 +206,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 +234,55 @@ class AccessList extends React.Component { } } + async removeRole (roleId, resourceId, type) { + const { removeRole } = this.props; + const url = `/api/v2/${type}/${resourceId}/roles/`; + await removeRole(url, roleId); + 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.removeRole(deleteRoleId, deleteResourceId, deleteType); + } + render () { const { results, @@ -241,6 +294,9 @@ class AccessList extends React.Component { sortedColumnKey, sortOrder, isCompact, + warningMsg, + warningTitle, + showWarning } = this.state; return ( @@ -268,6 +324,20 @@ class AccessList extends React.Component { isCompact={isCompact} showExpandCollapse /> + {showWarning && ( + } + > + {warningMsg} + + + + + + )} + {({ i18n }) => ( @@ -304,10 +374,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 +391,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} + ))} )} @@ -348,6 +424,7 @@ class AccessList extends React.Component { AccessList.propTypes = { getAccessList: PropTypes.func.isRequired, + removeRole: PropTypes.func.isRequired, }; export default AccessList; diff --git a/src/components/BasicChip/basicChip.scss b/src/components/BasicChip/basicChip.scss index 11b6601ec7..41e7e8f5df 100644 --- a/src/components/BasicChip/basicChip.scss +++ b/src/components/BasicChip/basicChip.scss @@ -7,4 +7,11 @@ .pf-c-button { display: none; } +} + +.awx-c-chip { + padding: 3px 0; + height: 24px; + margin-right: 10px; + margin-bottom: 10px; } \ No newline at end of file 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 });