diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6d958fe92..8ec52296db 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -171,6 +171,7 @@ Here are the guidelines for how to name functions. |`replace`| Use for methods that make API `PUT` requests | |`disassociate`| Use for methods that pass `{ disassociate: true }` as a data param to an endpoint | |`associate`| Use for methods that pass a resource id as a data param to an endpoint | +|`can`| Use for props dealing with RBAC to denote whether a user has access to something | ### Default State Initialization When declaring empty initial states, prefer the following instead of leaving them undefined: diff --git a/__tests__/components/NotificationList.test.jsx b/__tests__/components/NotificationList.test.jsx index 47ae953746..2261292ee0 100644 --- a/__tests__/components/NotificationList.test.jsx +++ b/__tests__/components/NotificationList.test.jsx @@ -11,6 +11,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={() => {}} onCreateSuccess={() => {}} + canToggleNotifications /> ); }); @@ -24,6 +25,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={() => {}} onCreateSuccess={() => {}} + canToggleNotifications /> ); expect(spy).toHaveBeenCalled(); @@ -38,6 +40,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={() => {}} onCreateSuccess={() => {}} + canToggleNotifications /> ).find('Notifications'); wrapper.instance().toggleNotification(1, true, 'success'); @@ -56,6 +59,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={() => {}} onCreateSuccess={onCreateSuccess} + canToggleNotifications /> ).find('Notifications'); wrapper.setState({ successTemplateIds: [44] }); @@ -76,6 +80,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={() => {}} onCreateSuccess={() => {}} + canToggleNotifications /> ).find('Notifications'); wrapper.instance().toggleNotification(1, true, 'error'); @@ -94,6 +99,7 @@ describe('', () => { onReadSuccess={() => {}} onCreateError={onCreateError} onCreateSuccess={() => {}} + canToggleNotifications /> ).find('Notifications'); wrapper.setState({ errorTemplateIds: [44] }); @@ -144,6 +150,7 @@ describe('', () => { onReadSuccess={onReadSuccess} onCreateError={() => {}} onCreateSuccess={() => {}} + canToggleNotifications /> ).find('Notifications'); wrapper.instance().updateUrl = jest.fn(); diff --git a/__tests__/components/NotificationListItem.test.jsx b/__tests__/components/NotificationListItem.test.jsx index de5303c318..3ce89b72ba 100644 --- a/__tests__/components/NotificationListItem.test.jsx +++ b/__tests__/components/NotificationListItem.test.jsx @@ -20,6 +20,7 @@ describe('', () => { toggleNotification={toggleNotification} detailUrl="/foo" notificationType="slack" + canToggleNotifications /> ); expect(wrapper.length).toBe(1); @@ -33,6 +34,7 @@ describe('', () => { toggleNotification={toggleNotification} detailUrl="/foo" notificationType="slack" + canToggleNotifications /> ); wrapper.find('Switch').first().find('input').simulate('change'); @@ -47,6 +49,7 @@ describe('', () => { toggleNotification={toggleNotification} detailUrl="/foo" notificationType="slack" + canToggleNotifications /> ); wrapper.find('Switch').first().find('input').simulate('change'); @@ -61,6 +64,7 @@ describe('', () => { toggleNotification={toggleNotification} detailUrl="/foo" notificationType="slack" + canToggleNotifications /> ); wrapper.find('Switch').at(1).find('input').simulate('change'); @@ -75,6 +79,7 @@ describe('', () => { toggleNotification={toggleNotification} detailUrl="/foo" notificationType="slack" + canToggleNotifications /> ); wrapper.find('Switch').at(1).find('input').simulate('change'); diff --git a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx index 42836b436e..1ea9dc9868 100644 --- a/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx +++ b/__tests__/pages/Organizations/components/OrganizationAccessList.test.jsx @@ -16,13 +16,31 @@ const mockData = [ role: { name: 'foo', id: 2, + user_capabilities: { + unattach: true + } } } - ], + ] } } ]; +const organization = { + id: 1, + name: 'Default', + summary_fields: { + object_roles: {}, + user_capabilities: { + edit: true + } + } +}; + +const api = { + foo: () => {} +}; + describe('', () => { afterEach(() => { jest.restoreAllMocks(); @@ -33,7 +51,8 @@ describe('', () => { {}} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ); }); @@ -42,7 +61,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -57,7 +77,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ).find('OrganizationAccessList'); expect(onSort).not.toHaveBeenCalled(); @@ -74,7 +95,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -94,7 +116,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ).find('OrganizationAccessList'); expect(handleWarning).not.toHaveBeenCalled(); expect(confirmDelete).not.toHaveBeenCalled(); @@ -117,7 +140,8 @@ describe('', () => { ({ data: { count: 1, results: mockData } })} removeRole={() => {}} - /> + organization={organization} + />, { context: { network: { api } } } ).find('OrganizationAccessList'); setImmediate(() => { @@ -146,4 +170,36 @@ describe('', () => { done(); }); }); + + test('add role button visible for user that can edit org', () => { + const wrapper = mountWithContexts( + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + organization={organization} + />, { context: { network: { api } } } + ).find('OrganizationAccessList'); + + setImmediate(() => { + const addRole = wrapper.update().find('DataListToolbar').find('PlusIcon'); + expect(addRole.length).toBe(1); + }); + }); + + test('add role button hidden for user that cannot edit org', () => { + const readOnlyOrg = { ...organization }; + readOnlyOrg.summary_fields.user_capabilities.edit = false; + const wrapper = mountWithContexts( + ({ data: { count: 1, results: mockData } })} + removeRole={() => {}} + organization={readOnlyOrg} + />, { context: { network: { api } } } + ).find('OrganizationAccessList'); + + setImmediate(() => { + const addRole = wrapper.update().find('DataListToolbar').find('PlusIcon'); + expect(addRole.length).toBe(0); + }); + }); }); diff --git a/__tests__/pages/Organizations/screens/Organization/Organization.test.jsx b/__tests__/pages/Organizations/screens/Organization/Organization.test.jsx index 25a9da73fd..21e69ac58f 100644 --- a/__tests__/pages/Organizations/screens/Organization/Organization.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/Organization.test.jsx @@ -3,7 +3,21 @@ import { mountWithContexts } from '../../../../enzymeHelpers'; import Organization from '../../../../../src/pages/Organizations/screens/Organization/Organization'; describe('', () => { + const me = { + is_super_user: true, + is_system_auditor: false + }; test('initially renders succesfully', () => { - mountWithContexts(); + mountWithContexts(); + }); + test('notifications tab shown/hidden based on permissions', () => { + const wrapper = mountWithContexts(); + expect(wrapper.find('.pf-c-tabs__item').length).toBe(3); + expect(wrapper.find('.pf-c-tabs__button[children="Notifications"]').length).toBe(0); + wrapper.find('Organization').setState({ + isNotifAdmin: true + }); + expect(wrapper.find('.pf-c-tabs__item').length).toBe(4); + expect(wrapper.find('.pf-c-tabs__button[children="Notifications"]').length).toBe(1); }); }); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx index fd8748d9e8..a998402345 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -3,8 +3,12 @@ import { mountWithContexts } from '../../../../enzymeHelpers'; import OrganizationAccess from '../../../../../src/pages/Organizations/screens/Organization/OrganizationAccess'; describe('', () => { + const organization = { + id: 1, + name: 'Default' + }; test('initially renders succesfully', () => { - mountWithContexts(); + mountWithContexts(); }); test('passed methods as props are called appropriately', async () => { @@ -14,13 +18,14 @@ describe('', () => { const mockResponse = { status: 'success', }; - const wrapper = mountWithContexts(, { context: { network: { - api: { - getOrganizationAccessList: () => Promise.resolve(mockAPIAccessList), - disassociate: () => Promise.resolve(mockResponse) - }, - handleHttpError: () => {} - } } }).find('OrganizationAccess'); + const wrapper = mountWithContexts(, + { context: { network: { + api: { + getOrganizationAccessList: () => Promise.resolve(mockAPIAccessList), + disassociate: () => Promise.resolve(mockResponse) + }, + handleHttpError: () => {} + } } }).find('OrganizationAccess'); const accessList = await wrapper.instance().getOrgAccessList(); expect(accessList).toEqual(mockAPIAccessList); const resp = await wrapper.instance().removeRole(2, 3, 'users'); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx index 177ff714b1..3b7ddda352 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx @@ -8,7 +8,12 @@ describe('', () => { description: 'Bar', custom_virtualenv: 'Fizz', created: 'Bat', - modified: 'Boo' + modified: 'Boo', + summary_fields: { + user_capabilities: { + edit: true + } + } }; test('initially renders succesfully', () => { @@ -83,4 +88,28 @@ describe('', () => { expect(modifiedDetail.find('h6').text()).toBe('Last Modified'); expect(modifiedDetail.find('p').text()).toBe('Boo'); }); + + test('should show edit button for users with edit permission', () => { + const wrapper = mountWithContexts( + + ).find('OrganizationDetail'); + + const editLink = wrapper.findWhere(node => node.props().to === '/organizations/undefined/edit'); + expect(editLink.length).toBe(1); + }); + + test('should hide edit button for users without edit permission', () => { + const readOnlyOrg = { ...mockDetails }; + readOnlyOrg.summary_fields.user_capabilities.edit = false; + const wrapper = mountWithContexts( + + ).find('OrganizationDetail'); + + const editLink = wrapper.findWhere(node => node.props().to === '/organizations/undefined/edit'); + expect(editLink.length).toBe(0); + }); }); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx index 560565c931..5b10cda0c9 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationNotifications.test.jsx @@ -18,7 +18,7 @@ describe('', () => { test('initially renders succesfully', () => { mountWithContexts( - , { context: { network: { + , { context: { network: { api, handleHttpError: () => {} } } } @@ -26,7 +26,7 @@ describe('', () => { }); test('handles api requests', () => { const wrapper = mountWithContexts( - , { context: { network: { + , { context: { network: { api, handleHttpError: () => {} } } } diff --git a/src/api.js b/src/api.js index bda5fd5319..96842e64b6 100644 --- a/src/api.js +++ b/src/api.js @@ -3,6 +3,7 @@ const API_LOGIN = `${API_ROOT}login/`; const API_LOGOUT = `${API_ROOT}logout/`; const API_V2 = `${API_ROOT}v2/`; const API_CONFIG = `${API_V2}config/`; +const API_ME = `${API_V2}me/`; const API_ORGANIZATIONS = `${API_V2}organizations/`; const API_INSTANCE_GROUPS = `${API_V2}instance_groups/`; const API_USERS = `${API_V2}users/`; @@ -58,6 +59,10 @@ class APIClient { return this.http.get(API_CONFIG); } + getMe () { + return this.http.get(API_ME); + } + destroyOrganization (id) { const endpoint = `${API_ORGANIZATIONS}${id}/`; return (this.http.delete(endpoint)); @@ -71,6 +76,10 @@ class APIClient { return this.http.post(API_ORGANIZATIONS, data); } + optionsOrganizations () { + return this.http.options(API_ORGANIZATIONS); + } + getOrganizationAccessList (id, params = {}) { const endpoint = `${API_ORGANIZATIONS}${id}/access_list/`; diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index 738f396b3f..3d8992c69f 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -1,6 +1,6 @@ import React, { Fragment } from 'react'; import PropTypes from 'prop-types'; -import { I18n } from '@lingui/react'; +import { I18n, i18nMark } from '@lingui/react'; import { t } from '@lingui/macro'; import { Button, @@ -28,14 +28,11 @@ import VerticalSeparator from '../VerticalSeparator'; class DataListToolbar extends React.Component { render () { const { + add, addUrl, columns, + deleteTooltip, disableTrashCanIcon, - onSelectAll, - sortedColumnKey, - sortOrder, - showDelete, - showSelectAll, isAllSelected, isCompact, noLeftMargin, @@ -43,8 +40,13 @@ class DataListToolbar extends React.Component { onSearch, onCompact, onExpand, - add, - onOpenDeleteModal + onOpenDeleteModal, + onSelectAll, + showAdd, + showDelete, + showSelectAll, + sortOrder, + sortedColumnKey } = this.props; const showExpandCollapse = (onCompact && onExpand); @@ -112,21 +114,23 @@ class DataListToolbar extends React.Component { { showDelete && ( - + + + )} - {addUrl && ( + {showAdd && addUrl && ( - - + {summary_fields.user_capabilities.edit && ( +
+ + + +
+ )} {error ? 'error!' : ''} )} diff --git a/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx b/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx index 4f6cf13d6d..491071bfee 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationNotifications.jsx @@ -41,13 +41,18 @@ class OrganizationNotifications extends Component { } render () { + const { + canToggleNotifications + } = this.props; + return ( ); } diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index a3746ebaab..035665ed3b 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -62,8 +62,7 @@ class OrganizationsList extends Component { loading: true, results: [], selected: [], - isModalOpen: false, - orgsToDelete: [], + isModalOpen: false }; @@ -74,14 +73,16 @@ class OrganizationsList extends Component { this.onSelectAll = this.onSelectAll.bind(this); this.onSelect = this.onSelect.bind(this); this.updateUrl = this.updateUrl.bind(this); + this.fetchOptionsOrganizations = this.fetchOptionsOrganizations.bind(this); this.fetchOrganizations = this.fetchOrganizations.bind(this); this.handleOrgDelete = this.handleOrgDelete.bind(this); this.handleOpenOrgDeleteModal = this.handleOpenOrgDeleteModal.bind(this); - this.handleClearOrgsToDelete = this.handleClearOrgsToDelete.bind(this); + this.handleCloseOrgDeleteModal = this.handleCloseOrgDeleteModal.bind(this); } componentDidMount () { const queryParams = this.getQueryParams(); + this.fetchOptionsOrganizations(); this.fetchOrganizations(queryParams); } @@ -117,20 +118,20 @@ class OrganizationsList extends Component { onSelectAll (isSelected) { const { results } = this.state; - const selected = isSelected ? results.map(o => o.id) : []; + const selected = isSelected ? results : []; this.setState({ selected }); } - onSelect (id) { + onSelect (row) { const { selected } = this.state; - const isSelected = selected.includes(id); + const isSelected = selected.some(s => s.id === row.id); if (isSelected) { - this.setState({ selected: selected.filter(s => s !== id) }); + this.setState({ selected: selected.filter(s => s.id !== row.id) }); } else { - this.setState({ selected: selected.concat(id) }); + this.setState({ selected: selected.concat(row) }); } } @@ -143,43 +144,35 @@ class OrganizationsList extends Component { return Object.assign({}, this.defaultParams, searchParams, overrides); } - handleClearOrgsToDelete () { + handleCloseOrgDeleteModal () { this.setState({ - isModalOpen: false, - orgsToDelete: [] + isModalOpen: false }); - this.onSelectAll(); } handleOpenOrgDeleteModal () { - const { results, selected } = this.state; + const { selected } = this.state; const warningTitle = selected.length > 1 ? i18nMark('Delete Organization') : i18nMark('Delete Organizations'); const warningMsg = i18nMark('Are you sure you want to delete:'); - - const orgsToDelete = []; - results.forEach((result) => { - selected.forEach((selectedOrg) => { - if (result.id === selectedOrg) { - orgsToDelete.push({ name: result.name, id: selectedOrg }); - } - }); - }); this.setState({ - orgsToDelete, isModalOpen: true, warningTitle, warningMsg, - loading: false }); + loading: false + }); } - async handleOrgDelete (event) { - const { orgsToDelete } = this.state; + async handleOrgDelete () { + const { selected } = this.state; const { api, handleHttpError } = this.props; let errorHandled; try { - await Promise.all(orgsToDelete.map((org) => api.destroyOrganization(org.id))); - this.handleClearOrgsToDelete(); + await Promise.all(selected.map((org) => api.destroyOrganization(org.id))); + this.setState({ + isModalOpen: false, + selected: [] + }); } catch (err) { errorHandled = handleHttpError(err); } finally { @@ -188,7 +181,6 @@ class OrganizationsList extends Component { this.fetchOrganizations(queryParams); } } - event.preventDefault(); } updateUrl (queryParams) { @@ -248,16 +240,35 @@ class OrganizationsList extends Component { } } + async fetchOptionsOrganizations () { + const { api } = this.props; + + try { + const { data } = await api.optionsOrganizations(); + const { actions } = data; + + const stateToUpdate = { + canAdd: Object.prototype.hasOwnProperty.call(actions, 'POST') + }; + + this.setState(stateToUpdate); + } catch (err) { + this.setState({ error: true }); + } finally { + this.setState({ loading: false }); + } + } + render () { const { medium, } = PageSectionVariants; const { + canAdd, count, error, loading, noInitialResults, - orgsToDelete, page, pageCount, page_size, @@ -270,6 +281,12 @@ class OrganizationsList extends Component { warningMsg, } = this.state; const { match } = this.props; + + const disableDelete = ( + selected.length === 0 + || selected.some(row => !row.summary_fields.user_capabilities.delete) + ); + return ( {({ i18n }) => ( @@ -280,15 +297,15 @@ class OrganizationsList extends Component { variant="danger" title={warningTitle} isOpen={isModalOpen} - onClose={this.handleClearOrgsToDelete} + onClose={this.handleCloseOrgDeleteModal} actions={[ , - + ]} > {warningMsg}
- {orgsToDelete.map((org) => ( + {selected.map((org) => ( {org.name} @@ -321,9 +338,27 @@ class OrganizationsList extends Component { onSort={this.onSort} onSelectAll={this.onSelectAll} onOpenDeleteModal={this.handleOpenOrgDeleteModal} - disableTrashCanIcon={selected.length === 0} + disableTrashCanIcon={disableDelete} + deleteTooltip={ + selected.some(row => !row.summary_fields.user_capabilities.delete) ? ( +
+ + You dont have permission to delete the following Organizations: + + {selected + .filter(row => !row.summary_fields.user_capabilities.delete) + .map(row => ( +
+ {row.name} +
+ )) + } +
+ ) : undefined + } showDelete showSelectAll + showAdd={canAdd} />
    { results.map(o => ( @@ -334,8 +369,8 @@ class OrganizationsList extends Component { detailUrl={`${match.url}/${o.id}`} memberCount={o.summary_fields.related_field_counts.users} teamCount={o.summary_fields.related_field_counts.teams} - isSelected={selected.includes(o.id)} - onSelect={() => this.onSelect(o.id, o.name)} + isSelected={selected.some(row => row.id === o.id)} + onSelect={() => this.onSelect(o)} onOpenOrgDeleteModal={this.handleOpenOrgDeleteModal} /> ))}