From 36f1066126eb929be2366377c160f5de8d2e2e79 Mon Sep 17 00:00:00 2001 From: Josh Holland Date: Mon, 17 Jun 2024 11:53:10 +0100 Subject: [PATCH] Restrict users shown in Storage edit dialog to those in project ED-95 --- .../dataStorage/editDataStoreForm.js | 10 +- .../dataStorage/editDataStoreForm.spec.js | 2 +- .../components/modal/EditDataStoreDialog.js | 10 +- .../modal/EditDataStoreDialog.spec.js | 6 +- .../EditDataStoreDialog.spec.js.snap | 2 +- .../modal/LoadUserManagementModalWrapper.js | 129 ++++-------------- .../LoadUserManagementModalWrapper.spec.js | 10 +- 7 files changed, 42 insertions(+), 127 deletions(-) diff --git a/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.js b/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.js index efe3b540e..44f1833aa 100644 --- a/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.js +++ b/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.js @@ -13,16 +13,12 @@ const formPropTypes = { value: PropTypes.string, }), ).isRequired, - loadUsersPromise: PropTypes.shape({ - error: PropTypes.any, - fetching: PropTypes.bool.isRequired, - value: PropTypes.array.isRequired, - }).isRequired, + usersFetching: PropTypes.bool.isRequired, }; const EditDataStoreForm = ({ handleSubmit, reset, pristine, // from redux form - userList, loadUsersPromise, onCancel, // user provided + userList, usersFetching, onCancel, // user provided }) => (
option.label} getOptionSelected={(option, value) => option.value === value.value} - loading={loadUsersPromise.fetching} + loading={usersFetching} /> diff --git a/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.spec.js b/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.spec.js index 06533d7f3..a159bcef4 100644 --- a/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.spec.js +++ b/code/workspaces/web-app/src/components/dataStorage/editDataStoreForm.spec.js @@ -15,7 +15,7 @@ describe('EditDataStoreForm', () => { pristine={true} onCancel={jest.fn().mockName('onCancel')} userList={[{ label: 'User 1', value: 'user1' }, { label: 'User 2', value: 'user2' }]} - loadUsersPromise={{ error: null, fetching: false, value: [] }} + usersFetching={false} />, ).container; diff --git a/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.js b/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.js index 9d15a97d6..b938b2220 100644 --- a/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.js +++ b/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.js @@ -38,14 +38,14 @@ export const getOnDetailsEditSubmit = (projectKey, stackName, typeName) => async }; const EditDataStoreDialog = ({ - onCancel, title, currentUsers, userList, loadUsersPromise, stack, projectKey, typeName, + onCancel, title, currentUsers, userList, usersFetching, stack, projectKey, typeName, }) => ( {title} user.verified))} - loadUsersPromise={loadUsersPromise} + usersFetching={usersFetching} onSubmit={getOnDetailsEditSubmit(projectKey, stack.name, typeName)} onCancel={onCancel} initialValues={{ @@ -73,11 +73,7 @@ EditDataStoreDialog.propTypes = { value: PropTypes.string, }), ).isRequired, - loadUsersPromise: PropTypes.shape({ - error: PropTypes.any, - fetching: PropTypes.bool.isRequired, - value: PropTypes.array.isRequired, - }).isRequired, + usersFetching: PropTypes.bool.isRequired, stack: PropTypes.shape({ name: PropTypes.string.isRequired, displayName: PropTypes.string.isRequired, diff --git a/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.spec.js b/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.spec.js index e44c315e0..c18d4fd62 100644 --- a/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.spec.js +++ b/code/workspaces/web-app/src/components/modal/EditDataStoreDialog.spec.js @@ -23,11 +23,7 @@ describe('Edit data store dialog', () => { title: 'expectedTitle', currentUsers: [{ label: 'expectedLabelOne', value: 'expectedValueOne', verified: true }], userList: [{ label: 'expectedLabelTwo', value: 'expectedValueTwo', verified: true }, { label: 'expectedLabelThree', value: 'expectedValueThree', verified: false }], - loadUsersPromise: { - error: null, - fetching: false, - value: [], - }, + usersFetching: false, stack: { name: 'stackName', displayName: 'Stack Display Name', description: 'Stack description' }, projectKey: 'project99', typeName: 'Data Store', diff --git a/code/workspaces/web-app/src/components/modal/__snapshots__/EditDataStoreDialog.spec.js.snap b/code/workspaces/web-app/src/components/modal/__snapshots__/EditDataStoreDialog.spec.js.snap index b123f98e0..c65eed63e 100644 --- a/code/workspaces/web-app/src/components/modal/__snapshots__/EditDataStoreDialog.spec.js.snap +++ b/code/workspaces/web-app/src/components/modal/__snapshots__/EditDataStoreDialog.spec.js.snap @@ -17,7 +17,7 @@ exports[`Edit data store dialog creates correct snapshot 1`] = ` >
EditDataStoreForm mock - {"userList":[{"label":"expectedLabelTwo","value":"expectedValueTwo","verified":true}],"loadUsersPromise":{"error":null,"fetching":false,"value":[]},"initialValues":{"displayName":"Stack Display Name","description":"Stack description","users":[{"label":"expectedLabelOne","value":"expectedValueOne","verified":true}]}} + {"userList":[{"label":"expectedLabelTwo","value":"expectedValueTwo","verified":true}],"usersFetching":false,"initialValues":{"displayName":"Stack Display Name","description":"Stack description","users":[{"label":"expectedLabelOne","value":"expectedValueOne","verified":true}]}}
diff --git a/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.js b/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.js index 49a84fce9..650751538 100644 --- a/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.js +++ b/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.js @@ -1,96 +1,36 @@ -import React, { Component } from 'react'; +import React, { useEffect, useMemo } from 'react'; import PropTypes from 'prop-types'; -import { connect } from 'react-redux'; -import { bindActionCreators } from 'redux'; +import { useDispatch, useSelector } from 'react-redux'; import { mapKeys, get, find } from 'lodash'; -import dataStorageActions from '../../actions/dataStorageActions'; import userActions from '../../actions/userActions'; -import notify from '../../components/common/notify'; +import { useProjectUsers } from '../../hooks/projectUsersHooks'; -class LoadUserManagementModalWrapper extends Component { - constructor(props, context) { - super(props, context); - this.addUser = this.addUser.bind(this); - this.removeUser = this.removeUser.bind(this); - } +const LoadUserManagementModalWrapper = ({ title, onCancel, Dialog, dataStoreId, projectKey, userKeysMapping, stack, typeName }) => { + const dispatch = useDispatch(); - componentDidMount() { - // Added .catch to prevent unhandled promise error, when lacking permission to view content - this.props.actions.listUsers() - .catch(() => {}); - } + const dataStorage = useSelector(state => find(state.dataStorage.value, ({ id }) => id === dataStoreId)); + const projectUsers = useProjectUsers(); - shouldComponentUpdate(nextProps) { - const isFetching = nextProps.dataStorage.fetching; - return !isFetching; - } + const remappedProjectUsers = useMemo(() => (userKeysMapping + ? projectUsers.value.map(user => mapKeys(user, (_value, key) => get(userKeysMapping, key))) + : projectUsers.value), + [userKeysMapping, projectUsers]); - addUser({ value }) { - const { name } = this.getDataStore(); - this.props.actions.addUserToDataStore(this.props.projectKey, { name, users: [value] }) - .then(() => notify.success('User added to data store')) - .then(() => this.props.actions.loadDataStorage(this.props.projectKey)); - } + useEffect(() => dispatch(userActions.listUsers()), [dispatch]); - removeUser({ value }) { - if (this.props.loginUserId !== value) { - const { name } = this.getDataStore(); - this.props.actions.removeUserFromDataStore(this.props.projectKey, { name, users: [value] }) - .then(() => notify.success('User removed from data store')) - .then(() => this.props.actions.loadDataStorage(this.props.projectKey)); - } else { - notify.error('Unable to remove self'); - } - } + const currentUsers = remappedProjectUsers.filter(user => dataStorage.users.includes(user.value)); - remapKeys(users) { - if (this.props.userKeysMapping) { - return users.map(user => mapKeys(user, (value, key) => get(this.props.userKeysMapping, key))); - } - - return users; - } - - getDataStore() { - return find(this.props.dataStorage.value, ({ id }) => id === this.props.dataStoreId); - } - - getCurrentUsers() { - const { users } = this.getDataStore(); - const invertedMapping = Object.entries(this.props.userKeysMapping) - .map(([key, value]) => ({ [value]: key })) - .reduce((previous, current) => ({ ...previous, ...current }), {}); - - let currentUsers; - - if (!this.props.users.fetching && this.props.users.value.length > 0) { - currentUsers = users.map(user => find(this.props.users.value, { [invertedMapping.value]: user })); - } else { - currentUsers = []; - } - - return this.remapKeys(currentUsers); - } - - render() { - const { Dialog } = this.props; - - return ( - - ); - } -} + return ; +}; LoadUserManagementModalWrapper.propTypes = { title: PropTypes.string.isRequired, @@ -99,20 +39,11 @@ LoadUserManagementModalWrapper.propTypes = { dataStoreId: PropTypes.string.isRequired, projectKey: PropTypes.string.isRequired, userKeysMapping: PropTypes.object.isRequired, + stack: PropTypes.shape({ + displayName: PropTypes.string.isRequired, + description: PropTypes.string.isRequired, + }).isRequired, + typeName: PropTypes.string.isRequired, }; -function mapStateToProps({ authentication: { identity: { sub } }, dataStorage, users }) { - return { loginUserId: sub, dataStorage, users }; -} - -function mapDispatchToProps(dispatch) { - return { - actions: bindActionCreators({ - ...dataStorageActions, - ...userActions, - }, dispatch), - }; -} - -export { LoadUserManagementModalWrapper as PureLoadUserManagementModalWrapper }; // export for testing -export default connect(mapStateToProps, mapDispatchToProps)(LoadUserManagementModalWrapper); +export default LoadUserManagementModalWrapper; diff --git a/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.spec.js b/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.spec.js index 208fff6e3..24b1b01d3 100644 --- a/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.spec.js +++ b/code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.spec.js @@ -1,7 +1,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import createStore from 'redux-mock-store'; -import LoadUserManagementModalWrapper, { PureLoadUserManagementModalWrapper } from './LoadUserManagementModalWrapper'; +import LoadUserManagementModalWrapper from './LoadUserManagementModalWrapper'; import dataStorageService from '../../api/dataStorageService'; import listUsersService from '../../api/listUsersService'; @@ -166,10 +166,6 @@ describe('LoadUserManagement Modal Wrapper', () => { }); describe('is a container which', () => { - function shallowRenderPure(props) { - return shallow(); - } - const onCancelMock = jest.fn(); const generateProps = () => ({ @@ -213,7 +209,7 @@ describe('LoadUserManagement Modal Wrapper', () => { const props = generateProps(); // Act - shallowRenderPure(props); + shallow(); // Assert expect(listUsersMock).toHaveBeenCalledTimes(1); @@ -224,7 +220,7 @@ describe('LoadUserManagement Modal Wrapper', () => { const props = generateProps(); // Act - const output = shallowRenderPure(props); + const output = shallow(); // Assert expect(output).toMatchSnapshot();