Restrict users shown in Storage edit dialog to those in project

ED-95

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
 }) => (
   <form onSubmit={handleSubmit}>
     <Field
@@ -44,7 +40,7 @@ const EditDataStoreForm = ({
       options={userList}
       getOptionLabel={option => option.label}
       getOptionSelected={(option, value) => option.value === value.value}
-      loading={loadUsersPromise.fetching}
+      loading={usersFetching}
     />
     <UpdateFormControls onClearChanges={reset} onCancel={onCancel} pristine={pristine}/>
   </form>

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;
 

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,
 }) => (
   <Dialog open={true} maxWidth="md" fullWidth>
     <DialogTitle>{title}</DialogTitle>
     <DialogContent>
       <EditDataStoreForm
         userList={sortUsersByLabel(userList.filter(user => 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,

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',

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`] = `
   >
     <div>
       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}]}}
     </div>
   </div>
 </div>

code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.js

@@ -1,68 +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 userActions from '../../actions/userActions';
+import { useProjectUsers } from '../../hooks/projectUsersHooks';
 
-class LoadUserManagementModalWrapper extends Component {
-  componentDidMount() {
-    // Added .catch to prevent unhandled promise error, when lacking permission to view content
-    this.props.actions.listUsers()
-      .catch(() => {});
-  }
+const LoadUserManagementModalWrapper = ({ title, onCancel, Dialog, dataStoreId, projectKey, userKeysMapping, stack, typeName }) => {
+  const dispatch = useDispatch();
 
-  shouldComponentUpdate(nextProps) {
-    const isFetching = nextProps.dataStorage.fetching;
-    return !isFetching;
-  }
+  const dataStorage = useSelector(state => find(state.dataStorage.value, { id: dataStoreId }));
+  const projectUsers = useProjectUsers();
 
-  remapKeys(users) {
-    if (this.props.userKeysMapping) {
-      return users.map(user => mapKeys(user, (value, key) => get(this.props.userKeysMapping, key)));
-    }
+  const remappedProjectUsers = useMemo(() => (userKeysMapping
+    ? projectUsers.value.map(user => mapKeys(user, (_value, key) => get(userKeysMapping, key)))
+    : projectUsers.value),
+  [userKeysMapping, projectUsers]);
 
-    return users;
-  }
+  useEffect(() => dispatch(userActions.listUsers()), [dispatch]);
 
-  getDataStore() {
-    return find(this.props.dataStorage.value, ({ id }) => id === this.props.dataStoreId);
-  }
+  const currentUsers = remappedProjectUsers.filter(user => dataStorage.users.includes(user.value));
 
-  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 (
-      <Dialog
-        onCancel={this.props.onCancel}
-        title={this.props.title}
-        currentUsers={this.getCurrentUsers()}
-        userList={this.remapKeys(this.props.users.value)}
-        loadUsersPromise={this.props.users}
-        stack={this.props.stack}
-        typeName={this.props.typeName}
-        projectKey={this.props.projectKey}
-      />
-    );
-  }
-}
+  return <Dialog
+    onCancel={onCancel}
+    title={title}
+    currentUsers={currentUsers}
+    userList={remappedProjectUsers}
+    usersFetching={projectUsers.fetching.inProgress}
+    stack={stack}
+    typeName={typeName}
+    projectKey={projectKey}
+  />;
+};
 
 LoadUserManagementModalWrapper.propTypes = {
   title: PropTypes.string.isRequired,
@@ -71,19 +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({ dataStorage, users }) {
-  return { dataStorage, users };
-}
-
-function mapDispatchToProps(dispatch) {
-  return {
-    actions: bindActionCreators({
-      ...userActions,
-    }, dispatch),
-  };
-}
-
-export { LoadUserManagementModalWrapper as PureLoadUserManagementModalWrapper }; // export for testing
-export default connect(mapStateToProps, mapDispatchToProps)(LoadUserManagementModalWrapper);
+export default LoadUserManagementModalWrapper;

code/workspaces/web-app/src/containers/modal/LoadUserManagementModalWrapper.spec.js

@@ -1,132 +1,68 @@
 import React from 'react';
 import { shallow } from 'enzyme';
-import createStore from 'redux-mock-store';
-import LoadUserManagementModalWrapper, { PureLoadUserManagementModalWrapper } from './LoadUserManagementModalWrapper';
+import { useDispatch, useSelector } from 'react-redux';
+import LoadUserManagementModalWrapper from './LoadUserManagementModalWrapper';
 import listUsersService from '../../api/listUsersService';
-
+import { useProjectUsers } from '../../hooks/projectUsersHooks';
+
+// https://github.com/enzymejs/enzyme/issues/2086#issuecomment-579510955
+jest.mock('react', () => ({
+  ...jest.requireActual('react'),
+  useEffect: f => f(),
+}));
+jest.mock('react-redux');
 jest.mock('../../api/listUsersService');
+jest.mock('../../hooks/projectUsersHooks');
+
+const projectUsers = {
+  fetching: { inProgress: false },
+  value: [
+    { userId: 'expectedId', name: 'expectedName' },
+    { userId: 'anotherExpectedId', name: 'anotherExpectedName' },
+  ],
+};
+const dataStorage = { users: ['expectedId'] };
 
 let listUsersMock;
 
 beforeEach(() => {
   listUsersMock = jest.fn().mockReturnValue(Promise.resolve('expectedPayload'));
   listUsersService.listUsers = listUsersMock;
-});
-
-describe('LoadUserManagement Modal Wrapper', () => {
-  beforeEach(() => jest.clearAllMocks());
-
-  describe('is a connected component which', () => {
-    function shallowRenderConnected(store) {
-      const props = {
-        store,
-        PrivateComponent: () => {},
-        PublicComponent: () => {},
-        title: 'Title',
-        onCancel: () => {},
-        Dialog: () => {},
-        dataStoreId: 'dataStoreId',
-        projectKey: 'project99',
-        userKeysMapping: {},
-      };
-
-      return shallow(<LoadUserManagementModalWrapper {...props} />).find('LoadUserManagementModalWrapper');
-    }
-
-    const dataStorage = { fetching: false, value: ['expectedArray'] };
-    const users = { fetching: false, value: ['expectedArray'] };
-
-    it('extracts the correct props from redux state', () => {
-      // Arrange
-      const store = createStore()({
-        dataStorage,
-        users,
-      });
-
-      // Act
-      const output = shallowRenderConnected(store);
 
-      // Assert
-      expect(output.prop('dataStorage')).toBe(dataStorage);
-      expect(output.prop('users')).toBe(users);
-    });
-
-    it('binds correct actions', () => {
-      // Arrange
-      const store = createStore()({
-        dataStorage,
-        users,
-      });
+  useDispatch.mockReturnValue(jest.fn().mockName('dispatch'));
 
-      // Act
-      const output = shallowRenderConnected(store).prop('actions');
-
-      // Assert
-      expect(Object.keys(output)).toContain(
-        'listUsers',
-      );
-    });
+  useProjectUsers.mockReturnValue(projectUsers);
 
-    it('listUsers function dispatches correct action', () => {
-      // Arrange
-      const store = createStore()({
-        dataStorage,
-        users,
-      });
-
-      // Act
-      const output = shallowRenderConnected(store);
+  // dataStorage object
+  useSelector.mockReturnValue(dataStorage);
+});
 
-      // Assert
-      expect(store.getActions().length).toBe(0);
-      output.prop('actions').listUsers();
-      const { type, payload } = store.getActions()[0];
-      expect(type).toBe('LIST_USERS');
-      return payload.then(value => expect(value).toBe('expectedPayload'));
-    });
-  });
+describe('LoadUserManagement Modal Wrapper', () => {
+  beforeEach(() => jest.clearAllMocks());
 
   describe('is a container which', () => {
-    function shallowRenderPure(props) {
-      return shallow(<PureLoadUserManagementModalWrapper {...props} />);
-    }
-
     const onCancelMock = jest.fn();
 
     const generateProps = () => ({
       title: 'Title',
       onCancel: onCancelMock,
+      Dialog: () => {},
       dataStoreId: 'expectedDataId',
+      projectKey: 'project99',
       userKeysMapping: {
         userId: 'value',
         name: 'label',
       },
-      projectKey: 'project99',
-      actions: {
-        listUsers: listUsersMock,
-      },
-      dataStorage: {
-        value: [
-          { id: 'expectedDataId', users: [{ userId: 'expectedId', name: 'expectedName' }] },
-        ],
-      },
-      users: {
-        value: [
-          { userId: 'expectedId', name: 'expectedName' },
-          { userId: 'anotherExpectedId', name: 'anotherExpectedName' },
-        ],
-      },
       stack: { displayName: 'displayName', description: 'description' },
       typeName: 'Data Store',
-      Dialog: () => {},
     });
 
-    it('calls loadDataStorage action when mounted', () => {
+    it('calls listUsers action when rendered', () => {
       // Arrange
       const props = generateProps();
 
       // Act
-      shallowRenderPure(props);
+      shallow(<LoadUserManagementModalWrapper {...props} />);
 
       // Assert
       expect(listUsersMock).toHaveBeenCalledTimes(1);
@@ -137,7 +73,7 @@ describe('LoadUserManagement Modal Wrapper', () => {
       const props = generateProps();
 
       // Act
-      const output = shallowRenderPure(props);
+      const output = shallow(<LoadUserManagementModalWrapper {...props} />);
 
       // Assert
       expect(output).toMatchSnapshot();