fix: visual issues & user already in role validation

RocketChat · Jul 20, 2021 · 23ea82d · 23ea82d
1 parent 318d7f6
commit 23ea82d
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 32 deletions.
diff --git a/app/api/server/v1/roles.js b/app/api/server/v1/roles.js
@@ -84,9 +84,16 @@ API.v1.addRoute('roles.addUserToRole', { authRequired: true }, {
 		});
 
 		const user = this.getUserFromParams();
+		const { roleName, roomId } = this.bodyParams;
+
+		const users = getUsersInRole(roleName, roomId, { fields: { username: 1 } }).fetch().map(({ username }) => username);
+
+		if (users.indexOf(user.username) > -1) {
+			throw new Meteor.Error('error-user-already-in-role', 'User already in role');
+		}
 
 		Meteor.runAsUser(this.userId, () => {
-			Meteor.call('authorization:addUserToRole', this.bodyParams.roleName, user.username, this.bodyParams.roomId);
+			Meteor.call('authorization:addUserToRole', roleName, user.username, roomId);
 		});
 
 		return API.v1.success({

diff --git a/client/components/UserAutoComplete/UserAutoComplete.js b/client/components/UserAutoComplete/UserAutoComplete.js
@@ -1,4 +1,5 @@
-import { AutoComplete, Option } from '@rocket.chat/fuselage';
+import { AutoComplete, Option, Box, Chip } from '@rocket.chat/fuselage';
+import { useDebouncedValue } from '@rocket.chat/fuselage-hooks';
 import React, { memo, useMemo, useState } from 'react';
 
 import { useEndpointData } from '../../hooks/useEndpointData';
@@ -10,25 +11,37 @@ const query = (term = '', conditions = {}) => ({ selector: JSON.stringify({ term
 const UserAutoComplete = (props) => {
 	const { conditions = {} } = props;
 	const [filter, setFilter] = useState('');
+	const debouncedFilter = useDebouncedValue(filter, 1000);
 	const { value: data } = useEndpointData(
 		'users.autocomplete',
 		// eslint-disable-next-line react-hooks/exhaustive-deps
-		useMemo(() => query(filter, conditions), [filter]),
+		useMemo(() => query(debouncedFilter, conditions), [filter]),
 	);
+
 	const options = useMemo(
 		() => (data && data.items.map((user) => ({ value: user.username, label: user.name }))) || [],
 		[data],
 	);
+
 	return (
 		<AutoComplete
 			{...props}
 			filter={filter}
 			setFilter={setFilter}
-			renderSelected={({ value, label }) => (
-				<>
-					<UserAvatar size='x20' username={value} /> {label}
-				</>
-			)}
+			renderSelected={({ value, label }) => {
+				if (!value) {
+					return '';
+				}
+
+				return (
+					<Chip height='x20' value={value} onClick={() => props.onChange()} mie='x4'>
+						<UserAvatar size='x20' username={value} />
+						<Box verticalAlign='middle' is='span' margin='none' mi='x4'>
+							{label}
+						</Box>
+					</Chip>
+				);
+			}}
 			renderItem={({ value, ...props }) => (
 				<Option key={value} {...props} avatar={<Avatar value={value} />} />
 			)}

diff --git a/client/views/admin/permissions/UsersInRolePage.js b/client/views/admin/permissions/UsersInRolePage.js
@@ -1,4 +1,4 @@
-import { Box, Field, Margins, ButtonGroup, Button, Callout, Flex } from '@rocket.chat/fuselage';
+import { Box, Field, Margins, ButtonGroup, Button, Callout } from '@rocket.chat/fuselage';
 import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
 import React, { useState, useRef } from 'react';
 
@@ -17,8 +17,9 @@ const UsersInRolePage = ({ data }) => {
 
 	const reload = useRef();
 
-	const [user, setUser] = useState();
+	const [user, setUser] = useState('');
 	const [rid, setRid] = useState();
+	const [userError, setUserError] = useState();
 
 	const { _id, name, description } = data;
 
@@ -34,6 +35,10 @@ const UsersInRolePage = ({ data }) => {
 	});
 
 	const handleAdd = useMutableCallback(async () => {
+		if (!user) {
+			return setUserError(t('User_cant_be_empty'));
+		}
+
 		try {
 			await addUser({ roleName: _id, username: user, roomId: rid });
 			dispatchToastMessage({ type: 'success', message: t('User_added') });
@@ -44,6 +49,14 @@ const UsersInRolePage = ({ data }) => {
 		}
 	});
 
+	const handleUserChange = useMutableCallback((user) => {
+		if (user !== '') {
+			setUserError();
+		}
+
+		return setUser(user);
+	});
+
 	return (
 		<Page>
 			<Page.Header title={`${t('Users_in_role')} "${description || name}"`}>
@@ -52,29 +65,33 @@ const UsersInRolePage = ({ data }) => {
 				</ButtonGroup>
 			</Page.Header>
 			<Page.Content>
-				<Box display='flex' flexDirection='row' w='full' mi='neg-x4'>
+				<Box display='flex' flexDirection='column' w='full' mi='neg-x4'>
 					<Margins inline='x4'>
-						<Flex.Item shrink={1}>
-							{data.scope !== 'Users' && (
-								<Field>
-									<Field.Label>{t('Choose_a_room')}</Field.Label>
-									<Field.Row>
-										<RoomAutoComplete value={rid} onChange={setRid} placeholder={t('User')} />
-									</Field.Row>
-								</Field>
-							)}
-							<Field>
-								<Field.Label>{t('Add_user')}</Field.Label>
+						{data.scope !== 'Users' && (
+							<Field mbe='x4'>
+								<Field.Label>{t('Choose_a_room')}</Field.Label>
 								<Field.Row>
-									<UserAutoComplete value={user} onChange={setUser} placeholder={t('User')} />
+									<RoomAutoComplete value={rid} onChange={setRid} placeholder={t('User')} />
 								</Field.Row>
 							</Field>
-							<Box display='flex' flexGrow={1} flexDirection='column' justifyContent='flex-end'>
-								<Button primary onClick={handleAdd}>
-									{t('Add')}
-								</Button>
-							</Box>
-						</Flex.Item>
+						)}
+						<Field>
+							<Field.Label>{t('Add_user')}</Field.Label>
+							<Field.Row>
+								<UserAutoComplete
+									value={user}
+									onChange={handleUserChange}
+									placeholder={t('User')}
+								/>
+
+								<ButtonGroup mis='x8' align='end'>
+									<Button primary onClick={handleAdd}>
+										{t('Add')}
+									</Button>
+								</ButtonGroup>
+							</Field.Row>
+							<Field.Error>{userError}</Field.Error>
+						</Field>
 					</Margins>
 				</Box>
 				<Margins blockStart='x8'>

diff --git a/client/views/admin/permissions/UsersInRoleTable.js b/client/views/admin/permissions/UsersInRoleTable.js
@@ -24,11 +24,10 @@ function UsersInRoleTable({
 	const dispatchToastMessage = useToastMessageDispatch();
 
 	const setModal = useSetModal();
+	const closeModal = () => setModal();
 
 	const removeUser = useEndpoint('POST', 'roles.removeUserFromRole');
 
-	const closeModal = () => setModal();
-
 	const onRemove = useMutableCallback((username) => {
 		const remove = async () => {
 			try {

diff --git a/packages/rocketchat-i18n/i18n/en.i18n.json b/packages/rocketchat-i18n/i18n/en.i18n.json
@@ -4251,6 +4251,7 @@
   "User_added_by": "User <em>__user_added__</em> added by <em>__user_by__</em>.",
   "User_added_successfully": "User added successfully",
   "User_and_group_mentions_only": "User and group mentions only",
+  "User_cant_be_empty": "User cannot be empty",
   "User_created_successfully!": "User create successfully!",
   "User_default": "User default",
   "User_doesnt_exist": "No user exists by the name of `@%s`.",

diff --git a/tests/end-to-end/api/13-roles.js b/tests/end-to-end/api/13-roles.js
@@ -401,7 +401,7 @@ describe('[Roles]', function() {
 			request.post(api('roles.removeUserFromRole'))
 				.set(credentials)
 				.send({
-					roleName: usersScopedRole.name,
+					roleId: usersScopedRole._id,
 					username: login.user,
 				})
 				.expect('Content-Type', 'application/json')