From 3cd54c45ebebe17e340ce5f3f3e5142d9a4ea1c4 Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Tue, 26 Feb 2019 08:39:13 -0500 Subject: [PATCH 1/7] Add Access List to Orgs. --- src/api.js | 26 +++++++++++++++++++ .../screens/Organization/Organization.jsx | 10 ++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/api.js b/src/api.js index f0d3c866b8..1c9c03ff31 100644 --- a/src/api.js +++ b/src/api.js @@ -5,6 +5,8 @@ const API_V2 = `${API_ROOT}v2/`; const API_CONFIG = `${API_V2}config/`; const API_ORGANIZATIONS = `${API_V2}organizations/`; const API_INSTANCE_GROUPS = `${API_V2}instance_groups/`; +const API_USERS = `${API_V2}users/`; +const API_TEAMS = `${API_V2}teams/`; const LOGIN_CONTENT_TYPE = 'application/x-www-form-urlencoded'; @@ -64,6 +66,12 @@ class APIClient { return this.http.post(API_ORGANIZATIONS, data); } + getOrganzationAccessList (id) { + const endpoint = `${API_ORGANIZATIONS}${id}/access_list/`; + + return this.http.get(endpoint); + } + getOrganizationDetails (id) { const endpoint = `${API_ORGANIZATIONS}${id}/`; @@ -76,6 +84,24 @@ class APIClient { return this.http.get(endpoint, { params }); } + getOrganizationUserRoles (id) { + const endpoint = `${API_USERS}${id}/roles/`; + + return this.http.get(endpoint); + } + + getUserTeams (id) { + const endpoint = `${API_USERS}${id}/teams/`; + + return this.http.get(endpoint); + } + + getTeamRoles (id) { + const endpoint = `${API_TEAMS}${id}/roles/`; + + return this.http.get(endpoint); + } + getOrganizationNotifications (id, params = {}) { const endpoint = `${API_ORGANIZATIONS}${id}/notification_templates/`; diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index 2b63bf0877..e611fea024 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -14,6 +14,7 @@ import { PageSection } from '@patternfly/react-core'; +import OrganizationAccess from './OrganizationAccess'; import OrganizationDetail from './OrganizationDetail'; import OrganizationEdit from './OrganizationEdit'; import OrganizationNotifications from './OrganizationNotifications'; @@ -141,7 +142,14 @@ class Organization extends Component { )}

Access

} + render={() => ( + + )} /> Date: Tue, 26 Feb 2019 13:23:34 -0500 Subject: [PATCH 2/7] Abstract out Access List as shared component. --- src/api.js | 40 +- src/components/AccessList/Access.list.jsx | 343 ++++++++++++++++++ src/components/AccessList/index.js | 3 + .../DataListToolbar/DataListToolbar.jsx | 33 +- .../NotificationsList/Notifications.list.jsx | 2 +- .../Organization/OrganizationAccess.jsx | 55 +++ 6 files changed, 451 insertions(+), 25 deletions(-) create mode 100644 src/components/AccessList/Access.list.jsx create mode 100644 src/components/AccessList/index.js create mode 100644 src/pages/Organizations/screens/Organization/OrganizationAccess.jsx diff --git a/src/api.js b/src/api.js index 1c9c03ff31..95e4520541 100644 --- a/src/api.js +++ b/src/api.js @@ -66,10 +66,10 @@ class APIClient { return this.http.post(API_ORGANIZATIONS, data); } - getOrganzationAccessList (id) { + getOrganzationAccessList (id, params = {}) { const endpoint = `${API_ORGANIZATIONS}${id}/access_list/`; - return this.http.get(endpoint); + return this.http.get(endpoint, { params }); } getOrganizationDetails (id) { @@ -84,24 +84,6 @@ class APIClient { return this.http.get(endpoint, { params }); } - getOrganizationUserRoles (id) { - const endpoint = `${API_USERS}${id}/roles/`; - - return this.http.get(endpoint); - } - - getUserTeams (id) { - const endpoint = `${API_USERS}${id}/teams/`; - - return this.http.get(endpoint); - } - - getTeamRoles (id) { - const endpoint = `${API_TEAMS}${id}/roles/`; - - return this.http.get(endpoint); - } - getOrganizationNotifications (id, params = {}) { const endpoint = `${API_ORGANIZATIONS}${id}/notification_templates/`; @@ -139,6 +121,24 @@ class APIClient { createInstanceGroups (url, id) { return this.http.post(url, { id }); } + + getUserRoles (id) { + const endpoint = `${API_USERS}${id}/roles/`; + + return this.http.get(endpoint); + } + + getUserTeams (id) { + const endpoint = `${API_USERS}${id}/teams/`; + + return this.http.get(endpoint); + } + + getTeamRoles (id) { + const endpoint = `${API_TEAMS}${id}/roles/`; + + return this.http.get(endpoint); + } } export default APIClient; diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx new file mode 100644 index 0000000000..dc9a53a592 --- /dev/null +++ b/src/components/AccessList/Access.list.jsx @@ -0,0 +1,343 @@ +import React, { Fragment } from 'react'; +import PropTypes from 'prop-types'; + +import { + DataList, DataListItem, DataListCell, Text, + TextContent, TextVariants, Badge +} from '@patternfly/react-core'; + +import { I18n, i18nMark } from '@lingui/react'; +import { t } from '@lingui/macro'; + +import { + Link +} from 'react-router-dom'; + +import BasicChip from '../BasicChip/BasicChip'; +import Pagination from '../Pagination/Pagination'; +import DataListToolbar from '../DataListToolbar/DataListToolbar'; + +import { + parseQueryString, +} from '../../qs'; + +const userRolesWrapperStyle = { + display: 'flex', + flexWrap: 'wrap', +}; + +const detailWrapperStyle = { + display: 'grid', + gridTemplateColumns: 'minmax(70px, max-content) minmax(60px, max-content)', +}; + +const detailLabelStyle = { + fontWeight: '700', + lineHeight: '24px', + marginRight: '20px', +}; + +const detailValueStyle = { + lineHeight: '28px', + overflow: 'visible', +}; + +const hiddenStyle = { + display: 'none', +}; + +const Detail = ({ label, value, url, isBadge, customStyles }) => { + let detail = null; + if (value) { + detail = ( + + {url ? ( + + {label} + ) : ({label} + )} + {isBadge ? ( + + {value} + + ) : ( + {value} + )} + + ); + } + return detail; +}; + +class AccessList extends React.Component { + columns = [ + { name: i18nMark('Username'), key: 'username', isSortable: true }, + { name: i18nMark('First Name'), key: 'first_name', isSortable: true, isNumeric: true }, + { name: i18nMark('Last Name'), key: 'last_name', isSortable: true, isNumeric: true }, + ]; + + defaultParams = { + page: 1, + page_size: 5, + order_by: 'username', + }; + + constructor (props) { + super(props); + + const { page, page_size } = this.getQueryParams(); + + this.state = { + page, + page_size, + count: null, + results: [], + sortOrder: 'ascending', + sortedColumnKey: 'username', + isCompact: true, + }; + + this.fetchOrgAccessList = this.fetchOrgAccessList.bind(this); + this.onSetPage = this.onSetPage.bind(this); + this.onExpand = this.onExpand.bind(this); + this.onCompact = this.onCompact.bind(this); + this.onSort = this.onSort.bind(this); + this.getQueryParams = this.getQueryParams.bind(this); + } + + componentDidMount () { + const queryParams = this.getQueryParams(); + + this.fetchOrgAccessList(queryParams); + } + + onExpand = () => { + this.setState({ isCompact: false }); + } + + onCompact = () => { + this.setState({ isCompact: true }); + } + + onSetPage = (pageNumber, pageSize) => { + const { sortOrder, sortedColumnKey } = this.state; + const page = parseInt(pageNumber, 10); + const page_size = parseInt(pageSize, 10); + let order_by = sortedColumnKey; + + if (sortOrder === 'descending') { + order_by = `-${order_by}`; + } + + const queryParams = this.getQueryParams({ page, page_size, order_by }); + + this.fetchOrgAccessList(queryParams); + }; + + getQueryParams (overrides = {}) { + const { location } = this.props; + const { search } = location; + + const searchParams = parseQueryString(search.substring(1)); + + return Object.assign({}, this.defaultParams, searchParams, overrides); + } + + onSort = (sortedColumnKey, sortOrder) => { + const { page_size } = this.state; + + let order_by = sortedColumnKey; + + if (sortOrder === 'descending') { + order_by = `-${order_by}`; + } + + const queryParams = this.getQueryParams({ order_by, page_size }); + + this.fetchOrgAccessList(queryParams); + }; + + async fetchOrgAccessList (queryParams) { + const { match, getAccessList, getUserRoles, getTeamRoles, getUserTeams } = this.props; + + const { page, page_size, order_by } = queryParams; + + let sortOrder = 'ascending'; + let sortedColumnKey = order_by; + + if (order_by.startsWith('-')) { + sortOrder = 'descending'; + sortedColumnKey = order_by.substring(1); + } + + try { + const { data: + { count = null, results = null } + } = await getAccessList(match.params.id, queryParams); + const pageCount = Math.ceil(count / page_size); + + const stateToUpdate = { + count, + page, + pageCount, + page_size, + sortOrder, + sortedColumnKey, + results, + }; + + results.map(async result => { + result.user_roles = []; + result.team_roles = []; + // Grab each Role Type and set as a top-level value + Object.values(result.summary_fields).filter(value => value.length > 0).forEach(roleType => { + result.roleType = roleType[0].role.name; + }); + + // Grab User Roles and set as a top-level value + try { + const resp = await getUserRoles(result.id); + const roles = resp.data.results; + roles.forEach(role => { + result.user_roles.push(role); + }); + this.setState(stateToUpdate); + } catch (error) { + this.setState({ error }); + } + + // Grab Team Roles and set as a top-level value + try { + const team_data = await getUserTeams(result.id); + const teams = team_data.data.results; + teams.forEach(async team => { + let team_roles = await getTeamRoles(team.id); + team_roles = team_roles.data.results; + team_roles.forEach(role => { + result.team_roles.push(role); + }); + this.setState(stateToUpdate); + }); + } catch (error) { + this.setState({ error }); + } + }); + } catch (error) { + this.setState({ error }); + } + } + + render () { + const { + results, + error, + count, + page_size, + pageCount, + page, + sortedColumnKey, + sortOrder, + isCompact, + } = this.state; + + if (!results) { + return null; + } + return ( + + {({ i18n }) => ( + + {}} + onSort={this.onSort} + onCompact={this.onCompact} + onExpand={this.onExpand} + isCompact={isCompact} + showExpandCollapse + /> + + {results.map(result => ( + + + + {result.first_name || result.last_name ? ( + + ) : ( + null + )} + + + + {result.user_roles.length > 0 && ( +
    + {i18n._(t`User Roles`)} + {result.user_roles.map(role => ( + + ))} +
+ )} + {result.team_roles.length > 0 && ( +
    + {i18n._(t`Team Roles`)} + {result.team_roles.map(role => ( + + ))} +
+ )} + + + ))} + + + { error ?
{error}
: '' } + + )} + + ); + } +} + +AccessList.propTypes = { + getAccessList: PropTypes.func.isRequired, + getUserRoles: PropTypes.func.isRequired, + getUserTeams: PropTypes.func.isRequired, + getTeamRoles: PropTypes.func.isRequired, +}; + +export default AccessList; diff --git a/src/components/AccessList/index.js b/src/components/AccessList/index.js new file mode 100644 index 0000000000..f435e8bb1f --- /dev/null +++ b/src/components/AccessList/index.js @@ -0,0 +1,3 @@ +import AccessList from './Access.list'; + +export default AccessList; diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index a84f086a00..074b804e58 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -24,7 +24,7 @@ import { SortNumericDownIcon, SortNumericUpIcon, TrashAltIcon, - PlusIcon + PlusIcon, } from '@patternfly/react-icons'; import { Link @@ -37,6 +37,12 @@ const flexGrowStyling = { flexGrow: '1' }; +const ToolbarActiveStyle = { + backgroundColor: 'rgb(0, 123, 186)', + color: 'white', + padding: '0 5px', +}; + class DataListToolbar extends React.Component { constructor (props) { super(props); @@ -56,6 +62,18 @@ class DataListToolbar extends React.Component { this.onSearchDropdownSelect = this.onSearchDropdownSelect.bind(this); this.onSearch = this.onSearch.bind(this); this.onSort = this.onSort.bind(this); + this.onExpand = this.onExpand.bind(this); + this.onCompact = this.onCompact.bind(this); + } + + onExpand () { + const { onExpand } = this.props; + onExpand(); + } + + onCompact () { + const { onCompact } = this.props; + onCompact(); } onSortDropdownToggle (isSortDropdownOpen) { @@ -114,7 +132,8 @@ class DataListToolbar extends React.Component { showExpandCollapse, showDelete, showSelectAll, - isLookup + isLookup, + isCompact, } = this.props; const { isSearchDropdownOpen, @@ -246,16 +265,20 @@ class DataListToolbar extends React.Component { { (showDelete || addUrl) && ( @@ -306,6 +329,7 @@ DataListToolbar.propTypes = { onSelectAll: PropTypes.func, onSort: PropTypes.func, showDelete: PropTypes.bool, + showExpandCollapse: PropTypes.bool, showSelectAll: PropTypes.bool, sortOrder: PropTypes.string, sortedColumnKey: PropTypes.string, @@ -317,6 +341,7 @@ DataListToolbar.defaultProps = { onSelectAll: null, onSort: null, showDelete: false, + showExpandCollapse: false, showSelectAll: false, sortOrder: 'ascending', sortedColumnKey: 'name', diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index 38f37044ce..1fe385823d 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -335,7 +335,7 @@ class Notifications extends Component { } } -Notifications.propType = { +Notifications.propTypes = { getError: PropTypes.func.isRequired, getNotifications: PropTypes.func.isRequired, getSuccess: PropTypes.func.isRequired, diff --git a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx new file mode 100644 index 0000000000..969bb4a370 --- /dev/null +++ b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx @@ -0,0 +1,55 @@ +import React from 'react'; +import AccessList from '../../../../components/AccessList/Access.list'; + +class OrganizationAccess extends React.Component { + constructor (props) { + super(props); + + this.getOrgAccessList = this.getOrgAccessList.bind(this); + this.getUserRoles = this.getUserRoles.bind(this); + this.getUserTeams = this.getUserTeams.bind(this); + this.getTeamRoles = this.getTeamRoles.bind(this); + } + + getOrgAccessList (id, params) { + const { api } = this.props; + return api.getOrganzationAccessList(id, params); + } + + getUserRoles (id) { + const { api } = this.props; + return api.getUserRoles(id); + } + + getUserTeams (id) { + const { api } = this.props; + return api.getUserTeams(id); + } + + getTeamRoles (id) { + const { api } = this.props; + return api.getTeamRoles(id); + } + + render () { + const { + location, + match, + history, + } = this.props; + + return ( + + ); + } +} + +export default OrganizationAccess; From 5659270d3e04413f971ef1aa0c24e3a47991cb76 Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Wed, 27 Feb 2019 17:30:02 -0500 Subject: [PATCH 3/7] Add unit test for Access List component. --- __tests__/components/AccessList.test.jsx | 142 ++++++++++++ .../Organization/OrganizationAccess.test.jsx | 80 +++++++ src/components/AccessList/Access.list.jsx | 202 ++++++++++-------- .../Organization/OrganizationAccess.jsx | 2 +- 4 files changed, 330 insertions(+), 96 deletions(-) create mode 100644 __tests__/components/AccessList.test.jsx create mode 100644 __tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx diff --git a/__tests__/components/AccessList.test.jsx b/__tests__/components/AccessList.test.jsx new file mode 100644 index 0000000000..1afb88b127 --- /dev/null +++ b/__tests__/components/AccessList.test.jsx @@ -0,0 +1,142 @@ +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', + } + } + ] + } + } +]; + +const mockUserRoles = [ + { + id: 2, + name: 'bar', + } +]; + +const mockUserTeams = [ + { + id: 3, + } +]; + +const mockTeamRoles = [ + { + id: 4, + name: 'baz', + } +]; + +describe('', () => { + test('initially renders succesfully', () => { + mount( + + + {}} + getUserRoles={() => {}} + getUserTeams={() => {}} + getTeamRoles={() => {}} + /> + + + ); + }); + + test('api response data passed to component gets set to state properly', (done) => { + const wrapper = mount( + + + ({ data: { count: 1, results: mockResults } })} + getUserRoles={() => ({ data: { results: mockUserRoles } })} + getUserTeams={() => ({ data: { results: mockUserTeams } })} + getTeamRoles={() => ({ data: { results: mockTeamRoles } })} + /> + + + ).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 } })} + getUserRoles={() => ({ data: { results: mockUserRoles } })} + getUserTeams={() => ({ data: { results: mockUserTeams } })} + getTeamRoles={() => ({ data: { results: mockTeamRoles } })} + /> + + + ).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 } })} + getUserRoles={() => ({ data: { results: mockUserRoles } })} + getUserTeams={() => ({ data: { results: mockUserTeams } })} + getTeamRoles={() => ({ data: { results: mockTeamRoles } })} + /> + + + ).find('AccessList'); + expect(onSort).not.toHaveBeenCalled(); + + setImmediate(() => { + const rendered = wrapper.update(); + rendered.find('button[aria-label="Sort"]').simulate('click'); + expect(onSort).toHaveBeenCalled(); + done(); + }); + }); +}); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx new file mode 100644 index 0000000000..4771f7a4e8 --- /dev/null +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -0,0 +1,80 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import { MemoryRouter } from 'react-router-dom'; + +import OrganizationAccess from '../../../../../src/pages/Organizations/screens/Organization/OrganizationAccess'; + +const mockAPIAccessList = { + foo: 'bar', +}; +const mockAPIRoles = { + bar: 'baz', +}; +const mockAPITeams = { + qux: 'quux', +}; +const mockAPITeamRoles = { + quuz: 'quuz', +}; + +const mockGetOrganzationAccessList = jest.fn(() => ( + Promise.resolve(mockAPIAccessList) +)); + +const mockGetUserRoles = jest.fn(() => ( + Promise.resolve(mockAPIRoles) +)); + +const mockGetUserTeams = jest.fn(() => ( + Promise.resolve(mockAPITeams) +)); + +const mockGetTeamRoles = jest.fn(() => ( + Promise.resolve(mockAPITeamRoles) +)); + +describe('', () => { + test('initially renders succesfully', () => { + mount( + + + + ); + }); + + test('passed methods as props are called appropriately', async () => { + const wrapper = mount( + + + + ).find('OrganizationAccess'); + const accessList = await wrapper.instance().getOrgAccessList(); + expect(accessList).toEqual(mockAPIAccessList); + const userRoles = await wrapper.instance().getUserRoles(); + expect(userRoles).toEqual(mockAPIRoles); + const userTeams = await wrapper.instance().getUserTeams(); + expect(userTeams).toEqual(mockAPITeams); + const teamRoles = await wrapper.instance().getTeamRoles(); + expect(teamRoles).toEqual(mockAPITeamRoles); + }); +}); diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx index dc9a53a592..8f096622be 100644 --- a/src/components/AccessList/Access.list.jsx +++ b/src/components/AccessList/Access.list.jsx @@ -14,8 +14,8 @@ import { } from 'react-router-dom'; import BasicChip from '../BasicChip/BasicChip'; -import Pagination from '../Pagination/Pagination'; -import DataListToolbar from '../DataListToolbar/DataListToolbar'; +import Pagination from '../Pagination'; +import DataListToolbar from '../DataListToolbar'; import { parseQueryString, @@ -91,7 +91,6 @@ class AccessList extends React.Component { page, page_size, count: null, - results: [], sortOrder: 'ascending', sortedColumnKey: 'username', isCompact: true, @@ -107,19 +106,22 @@ class AccessList extends React.Component { componentDidMount () { const queryParams = this.getQueryParams(); - - this.fetchOrgAccessList(queryParams); + try { + this.fetchOrgAccessList(queryParams); + } catch (error) { + this.setState({ error }); + } } - onExpand = () => { + onExpand () { this.setState({ isCompact: false }); } - onCompact = () => { + onCompact () { this.setState({ isCompact: true }); } - onSetPage = (pageNumber, pageSize) => { + onSetPage (pageNumber, pageSize) { const { sortOrder, sortedColumnKey } = this.state; const page = parseInt(pageNumber, 10); const page_size = parseInt(pageSize, 10); @@ -132,18 +134,9 @@ class AccessList extends React.Component { const queryParams = this.getQueryParams({ page, page_size, order_by }); this.fetchOrgAccessList(queryParams); - }; - - getQueryParams (overrides = {}) { - const { location } = this.props; - const { search } = location; - - const searchParams = parseQueryString(search.substring(1)); - - return Object.assign({}, this.defaultParams, searchParams, overrides); } - onSort = (sortedColumnKey, sortOrder) => { + onSort (sortedColumnKey, sortOrder) { const { page_size } = this.state; let order_by = sortedColumnKey; @@ -155,7 +148,16 @@ class AccessList extends React.Component { const queryParams = this.getQueryParams({ order_by, page_size }); this.fetchOrgAccessList(queryParams); - }; + } + + getQueryParams (overrides = {}) { + const { location } = this.props; + const { search } = location; + + const searchParams = parseQueryString(search.substring(1)); + + return Object.assign({}, this.defaultParams, searchParams, overrides); + } async fetchOrgAccessList (queryParams) { const { match, getAccessList, getUserRoles, getTeamRoles, getUserTeams } = this.props; @@ -186,7 +188,7 @@ class AccessList extends React.Component { results, }; - results.map(async result => { + results.forEach(async result => { result.user_roles = []; result.team_roles = []; // Grab each Role Type and set as a top-level value @@ -197,7 +199,7 @@ class AccessList extends React.Component { // Grab User Roles and set as a top-level value try { const resp = await getUserRoles(result.id); - const roles = resp.data.results; + const roles = resp.data.results || []; roles.forEach(role => { result.user_roles.push(role); }); @@ -209,14 +211,18 @@ class AccessList extends React.Component { // Grab Team Roles and set as a top-level value try { const team_data = await getUserTeams(result.id); - const teams = team_data.data.results; + const teams = team_data.data.results || []; teams.forEach(async team => { - let team_roles = await getTeamRoles(team.id); - team_roles = team_roles.data.results; - team_roles.forEach(role => { - result.team_roles.push(role); - }); - this.setState(stateToUpdate); + try { + let team_roles = await getTeamRoles(team.id); + team_roles = team_roles.data.results || []; + team_roles.forEach(role => { + result.team_roles.push(role); + }); + this.setState(stateToUpdate); + } catch (error) { + this.setState({ error }); + } }); } catch (error) { this.setState({ error }); @@ -239,85 +245,90 @@ class AccessList extends React.Component { sortOrder, isCompact, } = this.state; - - if (!results) { - return null; - } return ( - - {({ i18n }) => ( + + {!results && ( +

Loading...

// TODO: replace with something nicer + )} + {results && ( {}} + onSearch={() => { }} onSort={this.onSort} onCompact={this.onCompact} onExpand={this.onExpand} isCompact={isCompact} showExpandCollapse /> - - {results.map(result => ( - - - - {result.first_name || result.last_name ? ( - - ) : ( - null - )} - - - - {result.user_roles.length > 0 && ( -
    - {i18n._(t`User Roles`)} - {result.user_roles.map(role => ( - + + {({ i18n }) => ( + + {results.map(result => ( + + + - ))} -
- )} - {result.team_roles.length > 0 && ( -
    - {i18n._(t`Team Roles`)} - {result.team_roles.map(role => ( - + ) : ( + null + )} + + + - ))} -
- )} - - - ))} - + {result.user_roles.length > 0 && ( +
    + {i18n._(t`User Roles`)} + {result.user_roles.map(role => ( + + ))} +
+ )} + {result.team_roles.length > 0 && ( +
    + {i18n._(t`Team Roles`)} + {result.team_roles.map(role => ( + + ))} +
+ )} + + + ))} + + )} + + - { error ?
{error}
: '' } + {error ?
{error}
: ''} )} - + + ); } } diff --git a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx index 969bb4a370..108d409885 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import AccessList from '../../../../components/AccessList/Access.list'; +import AccessList from '../../../../components/AccessList'; class OrganizationAccess extends React.Component { constructor (props) { From a3a80bc23eaab7ec07f2d45ae0656d1cbb902b24 Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Wed, 27 Feb 2019 18:03:15 -0500 Subject: [PATCH 4/7] Fix prop errors in unit tests. --- __tests__/components/Lookup.test.jsx | 6 ++-- .../components/NotificationList.test.jsx | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/__tests__/components/Lookup.test.jsx b/__tests__/components/Lookup.test.jsx index 9402dcbbcc..7ac2c9575b 100644 --- a/__tests__/components/Lookup.test.jsx +++ b/__tests__/components/Lookup.test.jsx @@ -198,10 +198,11 @@ describe('', () => { { }} - data={mockData} + value={mockData} selected={[]} columns={mockColumns} sortedColumnKey="name" + getItems={() => { }} /> ).find('Lookup'); @@ -217,10 +218,11 @@ describe('', () => { { }} - data={mockData} + value={mockData} selected={[]} columns={mockColumns} sortedColumnKey="name" + getItems={() => { }} /> ).find('Lookup'); diff --git a/__tests__/components/NotificationList.test.jsx b/__tests__/components/NotificationList.test.jsx index 0b53f3c7e9..c08de77350 100644 --- a/__tests__/components/NotificationList.test.jsx +++ b/__tests__/components/NotificationList.test.jsx @@ -12,6 +12,11 @@ describe('', () => { @@ -25,6 +30,11 @@ describe('', () => { @@ -39,6 +49,11 @@ describe('', () => { @@ -54,6 +69,10 @@ describe('', () => { @@ -75,6 +94,11 @@ describe('', () => { @@ -90,7 +114,11 @@ describe('', () => { @@ -141,6 +169,8 @@ describe('', () => { getNotifications={getNotificationsFn} getSuccess={getSuccessFn} getError={getErrorFn} + postError={jest.fn()} + postSuccess={jest.fn()} /> From bf7e6201a2dd4b95bfbc42a8b46c892645e7a9bc Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Fri, 1 Mar 2019 10:12:36 -0500 Subject: [PATCH 5/7] Refactor Access List. - Derive Team Roles without making extra API call. - Consistent variable naming convention use camelCase. - More informative error display. --- __tests__/components/AccessList.test.jsx | 21 ---- .../Organization/OrganizationAccess.test.jsx | 22 ---- src/api.js | 13 --- src/components/AccessList/Access.list.jsx | 106 ++++++++++-------- .../Organization/OrganizationAccess.jsx | 14 --- 5 files changed, 62 insertions(+), 114 deletions(-) diff --git a/__tests__/components/AccessList.test.jsx b/__tests__/components/AccessList.test.jsx index 1afb88b127..624ae78fa2 100644 --- a/__tests__/components/AccessList.test.jsx +++ b/__tests__/components/AccessList.test.jsx @@ -31,19 +31,6 @@ const mockUserRoles = [ } ]; -const mockUserTeams = [ - { - id: 3, - } -]; - -const mockTeamRoles = [ - { - id: 4, - name: 'baz', - } -]; - describe('', () => { test('initially renders succesfully', () => { mount( @@ -54,8 +41,6 @@ describe('', () => { location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => {}} getUserRoles={() => {}} - getUserTeams={() => {}} - getTeamRoles={() => {}} /> @@ -71,8 +56,6 @@ describe('', () => { location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} getUserRoles={() => ({ data: { results: mockUserRoles } })} - getUserTeams={() => ({ data: { results: mockUserTeams } })} - getTeamRoles={() => ({ data: { results: mockTeamRoles } })} /> @@ -95,8 +78,6 @@ describe('', () => { location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} getUserRoles={() => ({ data: { results: mockUserRoles } })} - getUserTeams={() => ({ data: { results: mockUserTeams } })} - getTeamRoles={() => ({ data: { results: mockTeamRoles } })} /> @@ -124,8 +105,6 @@ describe('', () => { location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} getUserRoles={() => ({ data: { results: mockUserRoles } })} - getUserTeams={() => ({ data: { results: mockUserTeams } })} - getTeamRoles={() => ({ data: { results: mockTeamRoles } })} /> diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx index 4771f7a4e8..9f6c63accd 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -10,12 +10,6 @@ const mockAPIAccessList = { const mockAPIRoles = { bar: 'baz', }; -const mockAPITeams = { - qux: 'quux', -}; -const mockAPITeamRoles = { - quuz: 'quuz', -}; const mockGetOrganzationAccessList = jest.fn(() => ( Promise.resolve(mockAPIAccessList) @@ -25,14 +19,6 @@ const mockGetUserRoles = jest.fn(() => ( Promise.resolve(mockAPIRoles) )); -const mockGetUserTeams = jest.fn(() => ( - Promise.resolve(mockAPITeams) -)); - -const mockGetTeamRoles = jest.fn(() => ( - Promise.resolve(mockAPITeamRoles) -)); - describe('', () => { test('initially renders succesfully', () => { mount( @@ -44,8 +30,6 @@ describe('', () => { api={{ getOrganzationAccessList: jest.fn(), getUserRoles: jest.fn(), - getUserTeams: jest.fn(), - getTeamRoles: jest.fn(), }} /> @@ -62,8 +46,6 @@ describe('', () => { api={{ getOrganzationAccessList: mockGetOrganzationAccessList, getUserRoles: mockGetUserRoles, - getUserTeams: mockGetUserTeams, - getTeamRoles: mockGetTeamRoles, }} /> @@ -72,9 +54,5 @@ describe('', () => { expect(accessList).toEqual(mockAPIAccessList); const userRoles = await wrapper.instance().getUserRoles(); expect(userRoles).toEqual(mockAPIRoles); - const userTeams = await wrapper.instance().getUserTeams(); - expect(userTeams).toEqual(mockAPITeams); - const teamRoles = await wrapper.instance().getTeamRoles(); - expect(teamRoles).toEqual(mockAPITeamRoles); }); }); diff --git a/src/api.js b/src/api.js index 95e4520541..60106abe6b 100644 --- a/src/api.js +++ b/src/api.js @@ -6,7 +6,6 @@ const API_CONFIG = `${API_V2}config/`; const API_ORGANIZATIONS = `${API_V2}organizations/`; const API_INSTANCE_GROUPS = `${API_V2}instance_groups/`; const API_USERS = `${API_V2}users/`; -const API_TEAMS = `${API_V2}teams/`; const LOGIN_CONTENT_TYPE = 'application/x-www-form-urlencoded'; @@ -127,18 +126,6 @@ class APIClient { return this.http.get(endpoint); } - - getUserTeams (id) { - const endpoint = `${API_USERS}${id}/teams/`; - - return this.http.get(endpoint); - } - - getTeamRoles (id) { - const endpoint = `${API_TEAMS}${id}/roles/`; - - return this.http.get(endpoint); - } } export default APIClient; diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx index 8f096622be..23e02cf497 100644 --- a/src/components/AccessList/Access.list.jsx +++ b/src/components/AccessList/Access.list.jsx @@ -71,15 +71,15 @@ const Detail = ({ label, value, url, isBadge, customStyles }) => { class AccessList extends React.Component { columns = [ + { name: i18nMark('Name'), key: 'first_name', isSortable: true }, { name: i18nMark('Username'), key: 'username', isSortable: true }, - { name: i18nMark('First Name'), key: 'first_name', isSortable: true, isNumeric: true }, - { name: i18nMark('Last Name'), key: 'last_name', isSortable: true, isNumeric: true }, + { name: i18nMark('Last Name'), key: 'last_name', isSortable: true }, ]; defaultParams = { page: 1, page_size: 5, - order_by: 'username', + order_by: 'first_name', }; constructor (props) { @@ -102,6 +102,9 @@ class AccessList extends React.Component { this.onCompact = this.onCompact.bind(this); this.onSort = this.onSort.bind(this); this.getQueryParams = this.getQueryParams.bind(this); + this.getRoleType = this.getRoleType.bind(this); + this.fetchUserRoles = this.fetchUserRoles.bind(this); + this.getTeamRoles = this.getTeamRoles.bind(this); } componentDidMount () { @@ -127,6 +130,7 @@ class AccessList extends React.Component { const page_size = parseInt(pageSize, 10); let order_by = sortedColumnKey; + // Preserve sort order when paginating if (sortOrder === 'descending') { order_by = `-${order_by}`; } @@ -159,8 +163,39 @@ class AccessList extends React.Component { return Object.assign({}, this.defaultParams, searchParams, overrides); } + getTeamRoles (arr) { + this.arr = arr; + const filtered = this.arr.filter(entry => entry.role.team_id); + return filtered.reduce((val, item) => { + if (item.role.team_id) { + const { role } = item; + val = role; + } + return val; + }, {}); + } + + getRoleType (arr, index, type) { + return Object.values(arr).filter(value => value.length > 0).map(roleType => { + if (type === 'user') { + return roleType[index].role.name; + } + if (type === 'team') { + return this.getTeamRoles(roleType); + } + return null; + }); + } + + async fetchUserRoles (id) { + const { getUserRoles } = this.props; + const { data: { results: userRoles = [] } } = await getUserRoles(id); + + return userRoles; + } + async fetchOrgAccessList (queryParams) { - const { match, getAccessList, getUserRoles, getTeamRoles, getUserTeams } = this.props; + const { match, getAccessList } = this.props; const { page, page_size, order_by } = queryParams; @@ -189,44 +224,22 @@ class AccessList extends React.Component { }; results.forEach(async result => { - result.user_roles = []; - result.team_roles = []; + result.userRoles = []; + result.teamRoles = []; + result.directRole = null; + // Grab each Role Type and set as a top-level value - Object.values(result.summary_fields).filter(value => value.length > 0).forEach(roleType => { - result.roleType = roleType[0].role.name; - }); + result.directRole = this.getRoleType(result.summary_fields, 0, 'user') || null; + result.teamRoles = this.getRoleType(result.summary_fields, 1, 'team').filter(teamRole => teamRole.id); // Grab User Roles and set as a top-level value try { - const resp = await getUserRoles(result.id); - const roles = resp.data.results || []; - roles.forEach(role => { - result.user_roles.push(role); - }); + const roles = await this.fetchUserRoles(result.id); + roles.map(role => result.userRoles.push(role)); this.setState(stateToUpdate); } catch (error) { this.setState({ error }); } - - // Grab Team Roles and set as a top-level value - try { - const team_data = await getUserTeams(result.id); - const teams = team_data.data.results || []; - teams.forEach(async team => { - try { - let team_roles = await getTeamRoles(team.id); - team_roles = team_roles.data.results || []; - team_roles.forEach(role => { - result.team_roles.push(role); - }); - this.setState(stateToUpdate); - } catch (error) { - this.setState({ error }); - } - }); - } catch (error) { - this.setState({ error }); - } }); } catch (error) { this.setState({ error }); @@ -247,8 +260,16 @@ class AccessList extends React.Component { } = this.state; return ( - {!results && ( -

Loading...

// TODO: replace with something nicer + {!error && !results && ( +

Loading...

// TODO: replace with proper loading state + )} + {error && !results && ( + +
{error.message}
+ {error.response && ( +
{error.response.data.detail}
+ )} + // TODO: replace with proper error handling )} {results && ( @@ -272,7 +293,7 @@ class AccessList extends React.Component { @@ -294,13 +315,13 @@ class AccessList extends React.Component { url={null} customStyles={isCompact ? hiddenStyle : null} /> - {result.user_roles.length > 0 && ( + {result.userRoles.length > 0 && (
    {i18n._(t`User Roles`)} - {result.user_roles.map(role => ( + {result.userRoles.map(role => ( )} - {result.team_roles.length > 0 && ( + {result.teamRoles.length > 0 && (
      {i18n._(t`Team Roles`)} - {result.team_roles.map(role => ( + {result.teamRoles.map(role => ( - {error ?
      {error}
      : ''} )} @@ -348,8 +368,6 @@ class AccessList extends React.Component { AccessList.propTypes = { getAccessList: PropTypes.func.isRequired, getUserRoles: PropTypes.func.isRequired, - getUserTeams: PropTypes.func.isRequired, - getTeamRoles: PropTypes.func.isRequired, }; export default AccessList; diff --git a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx index 108d409885..5301fcb05b 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx @@ -7,8 +7,6 @@ class OrganizationAccess extends React.Component { this.getOrgAccessList = this.getOrgAccessList.bind(this); this.getUserRoles = this.getUserRoles.bind(this); - this.getUserTeams = this.getUserTeams.bind(this); - this.getTeamRoles = this.getTeamRoles.bind(this); } getOrgAccessList (id, params) { @@ -21,16 +19,6 @@ class OrganizationAccess extends React.Component { return api.getUserRoles(id); } - getUserTeams (id) { - const { api } = this.props; - return api.getUserTeams(id); - } - - getTeamRoles (id) { - const { api } = this.props; - return api.getTeamRoles(id); - } - render () { const { location, @@ -42,8 +30,6 @@ class OrganizationAccess extends React.Component { Date: Mon, 4 Mar 2019 14:38:58 -0500 Subject: [PATCH 6/7] Some cosmetic changes. - Reverse order of Expand/Compact icons in DataListToolbar to Compact/Expand. - Make Expanded the default view for Access Lists. - Make role badge styling closer to Chip component styling. --- src/components/AccessList/Access.list.jsx | 46 +++++++++++++++---- .../DataListToolbar/DataListToolbar.jsx | 20 ++++---- src/components/Tooltip/Tooltip.jsx | 14 ++++-- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx index 23e02cf497..1df8377ddf 100644 --- a/src/components/AccessList/Access.list.jsx +++ b/src/components/AccessList/Access.list.jsx @@ -16,6 +16,7 @@ import { import BasicChip from '../BasicChip/BasicChip'; import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; +import Tooltip from '../Tooltip'; import { parseQueryString, @@ -46,6 +47,15 @@ const hiddenStyle = { display: 'none', }; +const badgeStyle = { + borderRadius: 'var(--pf-global--BorderRadius--sm)', + height: '24px', +} + +const badgeTextStyle = { + lineHeight: '24px', +} + const Detail = ({ label, value, url, isBadge, customStyles }) => { let detail = null; if (value) { @@ -57,8 +67,8 @@ const Detail = ({ label, value, url, isBadge, customStyles }) => { ) : ({label} )} {isBadge ? ( - - {value} + + {value} ) : ( {value} @@ -93,7 +103,7 @@ class AccessList extends React.Component { count: null, sortOrder: 'ascending', sortedColumnKey: 'username', - isCompact: true, + isCompact: false, }; this.fetchOrgAccessList = this.fetchOrgAccessList.bind(this); @@ -321,12 +331,30 @@ class AccessList extends React.Component { : userRolesWrapperStyle} > {i18n._(t`User Roles`)} - {result.userRoles.map(role => ( - - ))} + {result.userRoles.map(role => { + // Show tooltips for associated Org/Team of role name displayed + if (role.summary_fields.resource_name) { + return ( + + + + ) + } else { + return ( + + ) + } + })}
    )} {result.teamRoles.length > 0 && ( diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index 074b804e58..3968d4742d 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -261,16 +261,6 @@ class DataListToolbar extends React.Component { {showExpandCollapse && ( - - - + + + { (showDelete || addUrl) && ( )} diff --git a/src/components/Tooltip/Tooltip.jsx b/src/components/Tooltip/Tooltip.jsx index 2ecb1ce832..c3d8a2ac4f 100644 --- a/src/components/Tooltip/Tooltip.jsx +++ b/src/components/Tooltip/Tooltip.jsx @@ -1,6 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; +const toolTipContent = { + padding: '10px', + minWidth: '100px', +} + class Tooltip extends React.Component { transforms = { top: { @@ -42,7 +47,10 @@ class Tooltip extends React.Component { const { isDisplayed } = this.state; - + + if (message === '') { + return null; + } return ( -
    -
    +
    +
    { message }
    From 9f86fc2def5d2cef346de0373949b42eb3926fb6 Mon Sep 17 00:00:00 2001 From: Kia Lam Date: Tue, 5 Mar 2019 12:01:06 -0500 Subject: [PATCH 7/7] Respond to PR feedback. --- __tests__/components/AccessList.test.jsx | 63 ++++++-- .../Organization/OrganizationAccess.test.jsx | 11 -- src/api.js | 7 - src/components/AccessList/Access.list.jsx | 140 ++++++------------ .../DataListToolbar/DataListToolbar.jsx | 2 +- src/components/Tooltip/Tooltip.jsx | 6 +- .../Organization/OrganizationAccess.jsx | 7 - 7 files changed, 101 insertions(+), 135 deletions(-) diff --git a/__tests__/components/AccessList.test.jsx b/__tests__/components/AccessList.test.jsx index 624ae78fa2..d7ab1862fb 100644 --- a/__tests__/components/AccessList.test.jsx +++ b/__tests__/components/AccessList.test.jsx @@ -17,20 +17,14 @@ const mockResults = [ { role: { name: 'foo', + id: 2, } } - ] + ], } } ]; -const mockUserRoles = [ - { - id: 2, - name: 'bar', - } -]; - describe('', () => { test('initially renders succesfully', () => { mount( @@ -40,7 +34,6 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '1' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => {}} - getUserRoles={() => {}} /> @@ -55,7 +48,6 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} - getUserRoles={() => ({ data: { results: mockUserRoles } })} /> @@ -77,7 +69,6 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} - getUserRoles={() => ({ data: { results: mockUserRoles } })} /> @@ -104,7 +95,6 @@ describe('', () => { match={{ path: '/organizations/:id', url: '/organizations/1', params: { id: '0' } }} location={{ search: '', pathname: '/organizations/1/access' }} getAccessList={() => ({ data: { count: 1, results: mockResults } })} - getUserRoles={() => ({ data: { results: mockUserRoles } })} /> @@ -118,4 +108,53 @@ describe('', () => { 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/screens/Organization/OrganizationAccess.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx index 9f6c63accd..4a42274379 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationAccess.test.jsx @@ -7,18 +7,11 @@ import OrganizationAccess from '../../../../../src/pages/Organizations/screens/O const mockAPIAccessList = { foo: 'bar', }; -const mockAPIRoles = { - bar: 'baz', -}; const mockGetOrganzationAccessList = jest.fn(() => ( Promise.resolve(mockAPIAccessList) )); -const mockGetUserRoles = jest.fn(() => ( - Promise.resolve(mockAPIRoles) -)); - describe('', () => { test('initially renders succesfully', () => { mount( @@ -29,7 +22,6 @@ describe('', () => { params={{}} api={{ getOrganzationAccessList: jest.fn(), - getUserRoles: jest.fn(), }} /> @@ -45,14 +37,11 @@ describe('', () => { params={{}} api={{ getOrganzationAccessList: mockGetOrganzationAccessList, - getUserRoles: mockGetUserRoles, }} /> ).find('OrganizationAccess'); const accessList = await wrapper.instance().getOrgAccessList(); expect(accessList).toEqual(mockAPIAccessList); - const userRoles = await wrapper.instance().getUserRoles(); - expect(userRoles).toEqual(mockAPIRoles); }); }); diff --git a/src/api.js b/src/api.js index 54fe242f5e..41cdb7525d 100644 --- a/src/api.js +++ b/src/api.js @@ -5,7 +5,6 @@ const API_V2 = `${API_ROOT}v2/`; const API_CONFIG = `${API_V2}config/`; const API_ORGANIZATIONS = `${API_V2}organizations/`; const API_INSTANCE_GROUPS = `${API_V2}instance_groups/`; -const API_USERS = `${API_V2}users/`; const LOGIN_CONTENT_TYPE = 'application/x-www-form-urlencoded'; @@ -130,12 +129,6 @@ class APIClient { disassociateInstanceGroup (url, id) { return this.http.post(url, { id, disassociate: true }); } - - getUserRoles (id) { - const endpoint = `${API_USERS}${id}/roles/`; - - return this.http.get(endpoint); - } } export default APIClient; diff --git a/src/components/AccessList/Access.list.jsx b/src/components/AccessList/Access.list.jsx index 1df8377ddf..c8d3aaec35 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, Badge + TextContent, TextVariants } from '@patternfly/react-core'; import { I18n, i18nMark } from '@lingui/react'; @@ -16,7 +16,6 @@ import { import BasicChip from '../BasicChip/BasicChip'; import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; -import Tooltip from '../Tooltip'; import { parseQueryString, @@ -47,16 +46,7 @@ const hiddenStyle = { display: 'none', }; -const badgeStyle = { - borderRadius: 'var(--pf-global--BorderRadius--sm)', - height: '24px', -} - -const badgeTextStyle = { - lineHeight: '24px', -} - -const Detail = ({ label, value, url, isBadge, customStyles }) => { +const Detail = ({ label, value, url, customStyles }) => { let detail = null; if (value) { detail = ( @@ -66,19 +56,29 @@ const Detail = ({ label, value, url, isBadge, customStyles }) => { {label} ) : ({label} )} - {isBadge ? ( - - {value} - - ) : ( - {value} - )} + {value} ); } return detail; }; +const UserName = ({ value, url }) => { + let username = null; + if (value) { + username = ( + + {url ? ( + + {value} + ) : ({value} + )} + + ); + } + return username; +}; + class AccessList extends React.Component { columns = [ { name: i18nMark('Name'), key: 'first_name', isSortable: true }, @@ -112,8 +112,6 @@ class AccessList extends React.Component { this.onCompact = this.onCompact.bind(this); this.onSort = this.onSort.bind(this); this.getQueryParams = this.getQueryParams.bind(this); - this.getRoleType = this.getRoleType.bind(this); - this.fetchUserRoles = this.fetchUserRoles.bind(this); this.getTeamRoles = this.getTeamRoles.bind(this); } @@ -173,36 +171,22 @@ class AccessList extends React.Component { return Object.assign({}, this.defaultParams, searchParams, overrides); } - getTeamRoles (arr) { - this.arr = arr; - const filtered = this.arr.filter(entry => entry.role.team_id); - return filtered.reduce((val, item) => { - if (item.role.team_id) { - const { role } = item; - val = role; + getRoles = roles => Object.values(roles) + .reduce((val, role) => { + if (role.length > 0) { + val.push(role[0].role); } return val; - }, {}); - } + }, []); - getRoleType (arr, index, type) { - return Object.values(arr).filter(value => value.length > 0).map(roleType => { - if (type === 'user') { - return roleType[index].role.name; + getTeamRoles = roles => roles + .reduce((val, item) => { + if (item.role.team_id) { + const { role } = item; + val.push(role); } - if (type === 'team') { - return this.getTeamRoles(roleType); - } - return null; - }); - } - - async fetchUserRoles (id) { - const { getUserRoles } = this.props; - const { data: { results: userRoles = [] } } = await getUserRoles(id); - - return userRoles; - } + return val; + }, []); async fetchOrgAccessList (queryParams) { const { match, getAccessList } = this.props; @@ -232,25 +216,15 @@ class AccessList extends React.Component { sortedColumnKey, results, }; - - results.forEach(async result => { - result.userRoles = []; - result.teamRoles = []; - result.directRole = null; - - // Grab each Role Type and set as a top-level value - result.directRole = this.getRoleType(result.summary_fields, 0, 'user') || null; - result.teamRoles = this.getRoleType(result.summary_fields, 1, 'team').filter(teamRole => teamRole.id); - - // Grab User Roles and set as a top-level value - try { - const roles = await this.fetchUserRoles(result.id); - roles.map(role => result.userRoles.push(role)); - this.setState(stateToUpdate); - } catch (error) { - this.setState({ error }); + 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) || []; }); + this.setState(stateToUpdate); } catch (error) { this.setState({ error }); } @@ -301,11 +275,9 @@ class AccessList extends React.Component { {results.map(result => ( - {result.first_name || result.last_name ? ( {i18n._(t`User Roles`)} - {result.userRoles.map(role => { - // Show tooltips for associated Org/Team of role name displayed - if (role.summary_fields.resource_name) { - return ( - - - - ) - } else { - return ( - - ) - } - })} + {result.userRoles.map(role => ( + + ))}
)} {result.teamRoles.length > 0 && ( @@ -388,14 +342,12 @@ class AccessList extends React.Component { )} - ); } } AccessList.propTypes = { getAccessList: PropTypes.func.isRequired, - getUserRoles: PropTypes.func.isRequired, }; export default AccessList; diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index 3968d4742d..2810a52435 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -38,7 +38,7 @@ const flexGrowStyling = { }; const ToolbarActiveStyle = { - backgroundColor: 'rgb(0, 123, 186)', + backgroundColor: '#007bba', color: 'white', padding: '0 5px', }; diff --git a/src/components/Tooltip/Tooltip.jsx b/src/components/Tooltip/Tooltip.jsx index c3d8a2ac4f..c2309b443d 100644 --- a/src/components/Tooltip/Tooltip.jsx +++ b/src/components/Tooltip/Tooltip.jsx @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'; const toolTipContent = { padding: '10px', minWidth: '100px', -} +}; class Tooltip extends React.Component { transforms = { @@ -47,7 +47,7 @@ class Tooltip extends React.Component { const { isDisplayed } = this.state; - + if (message === '') { return null; } @@ -64,7 +64,7 @@ class Tooltip extends React.Component { style={{ position: 'absolute', zIndex: '10', ...this.transforms[position] }} className={`pf-c-tooltip pf-m-${position}`} > -
+
{ message }
diff --git a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx index 5301fcb05b..f1b8cb98b2 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationAccess.jsx @@ -6,7 +6,6 @@ class OrganizationAccess extends React.Component { super(props); this.getOrgAccessList = this.getOrgAccessList.bind(this); - this.getUserRoles = this.getUserRoles.bind(this); } getOrgAccessList (id, params) { @@ -14,11 +13,6 @@ class OrganizationAccess extends React.Component { return api.getOrganzationAccessList(id, params); } - getUserRoles (id) { - const { api } = this.props; - return api.getUserRoles(id); - } - render () { const { location, @@ -29,7 +23,6 @@ class OrganizationAccess extends React.Component { return (