From db57aaa1fc7fe3657d5684cec35ff87141d1dbb2 Mon Sep 17 00:00:00 2001 From: Mike Stone Date: Fri, 7 Oct 2016 13:07:02 -0400 Subject: [PATCH] Filter unchanged attributes when updating user (#293) * Only send changed user attributes to the server * Improve flash message styles * Do not allow admins to demote or disable their own account * Disable admin actions against self --- .../components/FlashMessage/FlashMessage.jsx | 16 ++++--- frontend/components/FlashMessage/styles.js | 43 ++++++++++++++----- .../forms/Admin/EditUserForm/EditUserForm.jsx | 6 +-- .../Admin/EditUserForm/EditUserForm.tests.jsx | 29 +++++++++++++ .../forms/fields/Dropdown/Dropdown.jsx | 4 +- .../UserBlock/UserBlock.jsx | 12 +++--- .../UserManagementPage/UserManagementPage.jsx | 20 +++++++-- 7 files changed, 98 insertions(+), 32 deletions(-) create mode 100644 frontend/components/forms/Admin/EditUserForm/EditUserForm.tests.jsx diff --git a/frontend/components/FlashMessage/FlashMessage.jsx b/frontend/components/FlashMessage/FlashMessage.jsx index 642770d1e9..604e2acb62 100644 --- a/frontend/components/FlashMessage/FlashMessage.jsx +++ b/frontend/components/FlashMessage/FlashMessage.jsx @@ -5,7 +5,13 @@ import { hideFlash } from '../../redux/nodes/notifications/actions'; const FlashMessage = ({ notification, dispatch }) => { const { alertType, isVisible, message, undoAction } = notification; - const { containerStyles, contentStyles, undoStyles } = componentStyles; + const { + containerStyles, + contentStyles, + flashActionStyles, + removeFlashMessageStyles, + undoStyles, + } = componentStyles; const submitUndoAction = () => { dispatch(undoAction); @@ -25,11 +31,9 @@ const FlashMessage = ({ notification, dispatch }) => {
{message}
-
- Undo -
-
- X +
+
{undoAction && 'undo'}
+
x
); diff --git a/frontend/components/FlashMessage/styles.js b/frontend/components/FlashMessage/styles.js index b0c74303f1..29c976c665 100644 --- a/frontend/components/FlashMessage/styles.js +++ b/frontend/components/FlashMessage/styles.js @@ -1,26 +1,49 @@ import Style from '../../styles'; -const { color } = Style; +const { color, padding } = Style; export default { containerStyles: (alertType) => { - const successAlert = { - backgroundColor: color.success, - }; - + const successAlert = { backgroundColor: color.success }; + const errorAlert = { backgroundColor: color.alert }; const baseStyles = { + alignItems: 'center', color: color.white, + display: 'flex', + height: '50px', + justifyContent: 'space-between', + paddingLeft: padding.half, + paddingRight: padding.half, }; if (alertType === 'success') { - return { - ...baseStyles, - ...successAlert, - }; + return { ...baseStyles, ...successAlert }; + } + + if (alertType === 'error') { + return { ...baseStyles, ...errorAlert }; } return {}; }, contentStyles: {}, - undoStyles: {}, + flashActionStyles: { + display: 'flex', + justifyContent: 'space-between', + width: '96px', + }, + removeFlashMessageStyles: (alertType) => { + const backgroundColor = alertType === 'success' ? color.successLight : color.alertLight; + return { + backgroundColor, + borderRadius: '50%', + cursor: 'pointer', + height: '30px', + textAlign: 'center', + width: '30px', + }; + }, + undoStyles: { + cursor: 'pointer', + }, }; diff --git a/frontend/components/forms/Admin/EditUserForm/EditUserForm.jsx b/frontend/components/forms/Admin/EditUserForm/EditUserForm.jsx index 7dd31609b0..ce1f96aa44 100644 --- a/frontend/components/forms/Admin/EditUserForm/EditUserForm.jsx +++ b/frontend/components/forms/Admin/EditUserForm/EditUserForm.jsx @@ -36,12 +36,8 @@ class EditUserForm extends Component { constructor (props) { super(props); - const { user } = props; - this.state = { - formData: { - ...user, - }, + formData: {}, }; } diff --git a/frontend/components/forms/Admin/EditUserForm/EditUserForm.tests.jsx b/frontend/components/forms/Admin/EditUserForm/EditUserForm.tests.jsx new file mode 100644 index 0000000000..608cfa9973 --- /dev/null +++ b/frontend/components/forms/Admin/EditUserForm/EditUserForm.tests.jsx @@ -0,0 +1,29 @@ +import React from 'react'; +import expect, { createSpy, restoreSpies } from 'expect'; +import { mount } from 'enzyme'; + +import EditUserForm from './EditUserForm'; +import { fillInFormInput } from '../../../../test/helpers'; + +describe('EditUserForm - form', () => { + afterEach(restoreSpies); + + const user = { + email: 'hi@gnar.dog', + name: 'Gnar Dog', + position: 'Head of Everything', + username: 'gnardog', + }; + + it('sends the users changed attributes when the form is submitted', () => { + const email = 'newEmail@gnar.dog'; + const onSubmit = createSpy(); + const form = mount(); + const emailInput = form.find({ name: 'email' }); + + fillInFormInput(emailInput, email); + form.simulate('submit'); + + expect(onSubmit).toHaveBeenCalledWith({ email }); + }); +}); diff --git a/frontend/components/forms/fields/Dropdown/Dropdown.jsx b/frontend/components/forms/fields/Dropdown/Dropdown.jsx index 485c74ca5c..0cc094f4c6 100644 --- a/frontend/components/forms/fields/Dropdown/Dropdown.jsx +++ b/frontend/components/forms/fields/Dropdown/Dropdown.jsx @@ -28,11 +28,11 @@ class Dropdown extends Component { } renderOption = (option) => { - const { value, text } = option; + const { disabled = false, value, text } = option; const { optionWrapperStyles } = componentStyles; return ( - ); diff --git a/frontend/pages/Admin/UserManagementPage/UserBlock/UserBlock.jsx b/frontend/pages/Admin/UserManagementPage/UserBlock/UserBlock.jsx index a67ba9fd22..1adeafaa2e 100644 --- a/frontend/pages/Admin/UserManagementPage/UserBlock/UserBlock.jsx +++ b/frontend/pages/Admin/UserManagementPage/UserBlock/UserBlock.jsx @@ -7,17 +7,19 @@ import EditUserForm from '../../../../components/forms/Admin/EditUserForm'; class UserBlock extends Component { static propTypes = { + currentUser: PropTypes.object, onEditUser: PropTypes.func, onSelect: PropTypes.func, user: PropTypes.object, }; - static userActionOptions = (user) => { + static userActionOptions = (currentUser, user) => { + const disableActions = currentUser.id === user.id; const userEnableAction = user.enabled - ? { text: 'Disable Account', value: 'disable_account' } + ? { disabled: disableActions, text: 'Disable Account', value: 'disable_account' } : { text: 'Enable Account', value: 'enable_account' }; const userPromotionAction = user.admin - ? { text: 'Demote User', value: 'demote_user' } + ? { disabled: disableActions, text: 'Demote User', value: 'demote_user' } : { text: 'Promote User', value: 'promote_user' }; return [ @@ -88,7 +90,7 @@ class UserBlock extends Component { userStatusWrapperStyles, userWrapperStyles, } = componentStyles; - const { user } = this.props; + const { currentUser, user } = this.props; const { admin, email, @@ -99,7 +101,7 @@ class UserBlock extends Component { } = user; const userLabel = admin ? 'Admin' : 'User'; const activeLabel = enabled ? 'Active' : 'Disabled'; - const userActionOptions = UserBlock.userActionOptions(user); + const userActionOptions = UserBlock.userActionOptions(currentUser, user); const { isEdit } = this.state; const { onEditUserFormSubmit, onToggleEditing } = this; diff --git a/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx b/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx index dac9cd252d..e98075d90e 100644 --- a/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx +++ b/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx @@ -11,6 +11,7 @@ import { renderFlash } from '../../../redux/nodes/notifications/actions'; class UserManagementPage extends Component { static propTypes = { + currentUser: PropTypes.object, dispatch: PropTypes.func, users: PropTypes.arrayOf(PropTypes.object), }; @@ -33,21 +34,29 @@ class UserManagementPage extends Component { } onUserActionSelect = (user, action) => { - const { dispatch } = this.props; + const { currentUser, dispatch } = this.props; const { update } = userActions; if (action) { switch (action) { - case 'demote_user': + case 'demote_user': { + if (currentUser.id === user.id) { + return dispatch(renderFlash('error', 'You cannot demote yourself')); + } return dispatch(update(user, { admin: false })) .then(() => { return dispatch(renderFlash('success', 'User demoted', update(user, { admin: true }))); }); - case 'disable_account': + } + case 'disable_account': { + if (currentUser.id === user.id) { + return dispatch(renderFlash('error', 'You cannot disable your own account')); + } return dispatch(userActions.update(user, { enabled: false })) .then(() => { return dispatch(renderFlash('success', 'User account disabled', update(user, { enabled: true }))); }); + } case 'enable_account': return dispatch(update(user, { enabled: true })) .then(() => { @@ -105,10 +114,12 @@ class UserManagementPage extends Component { } renderUserBlock = (user) => { + const { currentUser } = this.props; const { onEditUser, onUserActionSelect } = this; return ( { + const { user: currentUser } = state.auth; const { entities: users } = entityGetter(state).get('users'); - return { users }; + return { currentUser, users }; }; export default connect(mapStateToProps)(UserManagementPage);