From 5f0ebe08df3ee21617beb0f65c5a7ac234a61f4d Mon Sep 17 00:00:00 2001 From: dougfabris Date: Fri, 10 May 2024 16:03:33 -0300 Subject: [PATCH 1/5] fix: Users in role list not working properly --- .../UsersInRole/UsersInRolePage.tsx | 88 ++++++++------ .../UsersInRoleTable/UsersInRoleTable.tsx | 107 +++++++++++------- .../UsersInRoleTableWithData.tsx | 87 -------------- .../UsersInRole/UsersInRoleTable/index.ts | 2 +- 4 files changed, 124 insertions(+), 160 deletions(-) delete mode 100644 apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableWithData.tsx diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx index effb72691f93..b696728caad5 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx @@ -1,9 +1,10 @@ import type { IRole, IRoom } from '@rocket.chat/core-typings'; -import { Box, Field, FieldLabel, FieldRow, Margins, ButtonGroup, Button, Callout } from '@rocket.chat/fuselage'; -import { useMutableCallback } from '@rocket.chat/fuselage-hooks'; -import { useToastMessageDispatch, useRoute, useEndpoint, useTranslation } from '@rocket.chat/ui-contexts'; +import { Box, Field, FieldLabel, FieldRow, Margins, ButtonGroup, Button, Callout, FieldError } from '@rocket.chat/fuselage'; +import { useEffectEvent, useUniqueId } from '@rocket.chat/fuselage-hooks'; +import { useToastMessageDispatch, useEndpoint, useTranslation, useRouter } from '@rocket.chat/ui-contexts'; +import { useQueryClient } from '@tanstack/react-query'; import type { ReactElement } from 'react'; -import React, { useRef } from 'react'; +import React from 'react'; import { useForm, Controller } from 'react-hook-form'; import { Page, PageHeader, PageContent } from '../../../../components/Page'; @@ -18,42 +19,35 @@ type UsersInRolePayload = { const UsersInRolePage = ({ role }: { role: IRole }): ReactElement => { const t = useTranslation(); - const reload = useRef<() => void>(() => undefined); const dispatchToastMessage = useToastMessageDispatch(); + const queryClient = useQueryClient(); const { control, handleSubmit, - formState: { isDirty }, - reset, - getValues, + formState: { errors, isDirty }, + watch, } = useForm({ defaultValues: { users: [] } }); const { _id, name, description } = role; - const router = useRoute('admin-permissions'); - const addUser = useEndpoint('POST', '/v1/roles.addUserToRole'); + const router = useRouter(); + const addUserToRoleEndpoint = useEndpoint('POST', '/v1/roles.addUserToRole'); - const rid = getValues('rid'); + const { rid } = watch(); + const roomFieldId = useUniqueId(); + const usersFieldId = useUniqueId(); - const handleReturn = useMutableCallback(() => { - router.push({ - context: 'edit', - _id, - }); - }); - - const handleAdd = useMutableCallback(async ({ users, rid }: UsersInRolePayload) => { + const handleAdd = useEffectEvent(async ({ users, rid }: UsersInRolePayload) => { try { await Promise.all( users.map(async (user) => { if (user) { - await addUser({ roleName: _id, username: user, roomId: rid }); + await addUserToRoleEndpoint({ roleName: _id, username: user, roomId: rid }); } }), ); dispatchToastMessage({ type: 'success', message: t('Users_added') }); - reload.current(); - reset(); + queryClient.invalidateQueries(['getUsersInRole']); } catch (error) { dispatchToastMessage({ type: 'error', message: error }); } @@ -63,7 +57,7 @@ const UsersInRolePage = ({ role }: { role: IRole }): ReactElement => { - + @@ -71,40 +65,68 @@ const UsersInRolePage = ({ role }: { role: IRole }): ReactElement => { {role.scope !== 'Users' && ( - {t('Choose_a_room')} + {t('Choose_a_room')} ( - + rules={{ required: t('error-the-field-is-required', { field: t('Room') }) }} + render={({ field: { onChange, value } }) => ( + )} /> + {errors.rid && ( + + {errors.rid.message} + + )} )} - {t('Add_users')} + {t('Add_users')} ( - + rules={{ required: t('error-the-field-is-required', { field: t('Users') }) }} + render={({ field: { onChange, value } }) => ( + )} /> - + {errors.users && ( + + + {errors.users.message} + + + )} - {(role.scope === 'Users' || rid) && ( - - )} + {(role.scope === 'Users' || rid) && } {role.scope !== 'Users' && !rid && {t('Select_a_room')}} diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx index ab8c5926f97f..b78f950c0102 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx @@ -1,79 +1,106 @@ -import type { IRole, IRoom, IUserInRole } from '@rocket.chat/core-typings'; +import type { IRole, IRoom } from '@rocket.chat/core-typings'; import { Pagination } from '@rocket.chat/fuselage'; -import { useMutableCallback } from '@rocket.chat/fuselage-hooks'; +import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { useSetModal, useToastMessageDispatch, useEndpoint, useTranslation } from '@rocket.chat/ui-contexts'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; import type { ReactElement } from 'react'; -import React from 'react'; +import React, { useMemo } from 'react'; +import GenericError from '../../../../../components/GenericError'; import GenericModal from '../../../../../components/GenericModal'; import GenericNoResults from '../../../../../components/GenericNoResults'; -import { GenericTable, GenericTableHeader, GenericTableHeaderCell, GenericTableBody } from '../../../../../components/GenericTable'; -import type { usePagination } from '../../../../../components/GenericTable/hooks/usePagination'; +import { + GenericTable, + GenericTableHeader, + GenericTableHeaderCell, + GenericTableBody, + GenericTableLoadingTable, +} from '../../../../../components/GenericTable'; +import { usePagination } from '../../../../../components/GenericTable/hooks/usePagination'; import UsersInRoleTableRow from './UsersInRoleTableRow'; type UsersInRoleTableProps = { - users: IUserInRole[]; - reload: () => void; roleName: IRole['name']; roleId: IRole['_id']; description: IRole['description']; - total: number; rid?: IRoom['_id']; - paginationData: ReturnType; }; -// TODO: Missing error state -const UsersInRoleTable = ({ - users, - reload, - roleName, - roleId, - description, - total, - rid, - paginationData, -}: UsersInRoleTableProps): ReactElement => { +const UsersInRoleTable = ({ rid, roleId, roleName, description }: UsersInRoleTableProps): ReactElement => { const t = useTranslation(); const setModal = useSetModal(); const dispatchToastMessage = useToastMessageDispatch(); - const removeUser = useEndpoint('POST', '/v1/roles.removeUserFromRole'); - const { current, itemsPerPage, setItemsPerPage: onSetItemsPerPage, setCurrent: onSetCurrent, ...paginationProps } = paginationData; + const queryClient = useQueryClient(); - const closeModal = (): void => setModal(); + const getUsersInRoleEndpoint = useEndpoint('GET', '/v1/roles.getUsersInRole'); + const removeUserFromRoleEndpoint = useEndpoint('POST', '/v1/roles.removeUserFromRole'); - const handleRemove = useMutableCallback((username) => { - const remove = async (): Promise => { + const { current, itemsPerPage, setItemsPerPage: onSetItemsPerPage, setCurrent: onSetCurrent, ...paginationProps } = usePagination(); + + const query = useMemo( + () => ({ + role: roleId, + ...(rid && { roomId: rid }), + ...(itemsPerPage && { count: itemsPerPage }), + ...(current && { offset: current }), + }), + [itemsPerPage, current, rid, roleId], + ); + + const { data, isLoading, isSuccess, refetch, isError } = useQuery(['getUsersInRole', roleId, query], async () => + getUsersInRoleEndpoint(query), + ); + + const users = data?.users?.map((user) => ({ + ...user, + createdAt: new Date(user.createdAt), + _updatedAt: new Date(user._updatedAt), + })); + + const handleRemove = useEffectEvent((username) => { + const remove = async () => { try { - await removeUser({ roleId, username, scope: rid }); + await removeUserFromRoleEndpoint({ roleId, username, scope: rid }); dispatchToastMessage({ type: 'success', message: t('User_removed') }); - } catch (error: unknown) { + queryClient.invalidateQueries(['getUsersInRole']); + } catch (error) { dispatchToastMessage({ type: 'error', message: error }); } finally { - closeModal(); - reload(); + setModal(null); } }; setModal( - + setModal(null)} confirmText={t('Delete')}> {t('The_user_s_will_be_removed_from_role_s', username, description || roleName)} , ); }); + const headers = ( + <> + {t('Name')} + {t('Email')} + + + ); + return ( <> - {users.length === 0 && } - {users.length > 0 && ( + {isLoading && ( + + {headers} + + + + + )} + {isSuccess && users && users.length > 0 && ( <> - - {t('Name')} - {t('Email')} - - + {headers} - {users.map((user) => ( + {users?.map((user) => ( ))} @@ -82,13 +109,15 @@ const UsersInRoleTable = ({ divider current={current} itemsPerPage={itemsPerPage} - count={total} + count={users.length || 0} onSetItemsPerPage={onSetItemsPerPage} onSetCurrent={onSetCurrent} {...paginationProps} /> )} + {users && users.length === 0 && } + {isError && } ); }; diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableWithData.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableWithData.tsx deleted file mode 100644 index a7fe837b6e72..000000000000 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableWithData.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import type { IRole, IRoom, IUserInRole } from '@rocket.chat/core-typings'; -import { useEndpoint, useToastMessageDispatch, useTranslation } from '@rocket.chat/ui-contexts'; -import { useQuery } from '@tanstack/react-query'; -import type { ReactElement, MutableRefObject } from 'react'; -import React, { useEffect, useMemo } from 'react'; - -import { usePagination } from '../../../../../components/GenericTable/hooks/usePagination'; -import UsersInRoleTable from './UsersInRoleTable'; - -type UsersInRoleTableWithDataProps = { - rid?: IRoom['_id']; - roleId: IRole['_id']; - roleName: IRole['name']; - description: IRole['description']; - reloadRef: MutableRefObject<() => void>; -}; - -const UsersInRoleTableWithData = ({ - rid, - roleId, - roleName, - description, - reloadRef, -}: UsersInRoleTableWithDataProps): ReactElement | null => { - const { itemsPerPage, current, ...paginationData } = usePagination(); - const t = useTranslation(); - - const query = useMemo( - () => ({ - role: roleId, - ...(rid && { roomId: rid }), - ...(itemsPerPage && { count: itemsPerPage }), - ...(current && { offset: current }), - }), - [itemsPerPage, current, rid, roleId], - ); - - const getUsersInRole = useEndpoint('GET', '/v1/roles.getUsersInRole'); - - const dispatchToastMessage = useToastMessageDispatch(); - - const { refetch, ...result } = useQuery( - ['roles', roleId, 'users', query], - async () => { - const { users } = await getUsersInRole(query); - - if (users.length === 0) { - throw new Error(t('No_results_found')); - } - return users; - }, - { - onError: (error) => { - dispatchToastMessage({ type: 'error', message: error }); - }, - }, - ); - - useEffect(() => { - reloadRef.current = refetch; - }, [refetch, reloadRef]); - - if (result.isLoading || result.error || !result.data) { - return null; - } - - const users: IUserInRole[] = result.data.map((user) => ({ - ...user, - createdAt: new Date(user.createdAt), - _updatedAt: new Date(user._updatedAt), - })); - - return ( - - ); -}; - -export default UsersInRoleTableWithData; diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/index.ts b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/index.ts index e7195afe4fd1..1557d53dff12 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/index.ts +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/index.ts @@ -1 +1 @@ -export { default } from './UsersInRoleTableWithData'; +export { default } from './UsersInRoleTable'; From 8ebf3cc633d30f813ff005c63271add1edacce9b Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 16 May 2024 19:05:34 -0300 Subject: [PATCH 2/5] fix: ui tweaks --- .../UsersInRole/UsersInRolePage.tsx | 2 +- .../UsersInRoleTable/UsersInRoleTableRow.tsx | 32 ++++++++----------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx index b696728caad5..4bfb2afc81d3 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx @@ -57,7 +57,7 @@ const UsersInRolePage = ({ role }: { role: IRole }): ReactElement => { - + diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableRow.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableRow.tsx index ea4b999d694e..4e860ca8f759 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableRow.tsx +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTableRow.tsx @@ -1,7 +1,8 @@ import type { IUserInRole } from '@rocket.chat/core-typings'; -import { Box, Button, Icon } from '@rocket.chat/fuselage'; -import { useMutableCallback } from '@rocket.chat/fuselage-hooks'; +import { Box, IconButton } from '@rocket.chat/fuselage'; +import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { UserAvatar } from '@rocket.chat/ui-avatar'; +import { useTranslation } from '@rocket.chat/ui-contexts'; import type { ReactElement } from 'react'; import React, { memo } from 'react'; @@ -14,10 +15,11 @@ type UsersInRoleTableRowProps = { }; const UsersInRoleTableRow = ({ user, onRemove }: UsersInRoleTableRowProps): ReactElement => { + const t = useTranslation(); const { _id, name, username, avatarETag } = user; const email = getUserEmailAddress(user); - const handleRemove = useMutableCallback(() => { + const handleRemove = useEffectEvent(() => { onRemove(username); }); @@ -27,26 +29,20 @@ const UsersInRoleTableRow = ({ user, onRemove }: UsersInRoleTableRowProps): Reac - - - {name || username} - - {name && ( - - {' '} - {`@${username}`}{' '} - - )} + + {name || username} + {name && ( + + {`@${username}`} + + )} {email} - - {/* FIXME: Replace to IconButton */} - + + ); From e9a1de4a3fe1f234aad301afdcdba18e20a172e0 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 16 May 2024 19:05:41 -0300 Subject: [PATCH 3/5] test: add e2e tests --- apps/meteor/tests/e2e/administration.spec.ts | 45 ++++++++++++++++++-- apps/meteor/tests/e2e/page-objects/admin.ts | 14 +++++- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/apps/meteor/tests/e2e/administration.spec.ts b/apps/meteor/tests/e2e/administration.spec.ts index 48657efea73a..f97da0fcdf0b 100644 --- a/apps/meteor/tests/e2e/administration.spec.ts +++ b/apps/meteor/tests/e2e/administration.spec.ts @@ -2,7 +2,7 @@ import { faker } from '@faker-js/faker'; import { IS_EE } from './config/constants'; import { Users } from './fixtures/userStates'; -import { Admin } from './page-objects'; +import { Admin, Utils } from './page-objects'; import { createTargetChannel } from './utils'; import { setSettingValueById } from './utils/setSettingValueById'; import { test, expect } from './utils/test'; @@ -11,10 +11,12 @@ test.use({ storageState: Users.admin.state }); test.describe.parallel('administration', () => { let poAdmin: Admin; + let poUtils: Utils; let targetChannel: string; test.beforeEach(async ({ page }) => { poAdmin = new Admin(page); + poUtils = new Utils(page); }); test.describe('Workspace', () => { @@ -182,7 +184,7 @@ test.describe.parallel('administration', () => { test('expect open upsell modal if not enterprise', async ({ page }) => { test.skip(IS_EE); await poAdmin.btnCreateRole.click(); - await page.waitForSelector('role=dialog[name="Custom roles"]'); + await expect(page.getByRole('dialog', { name: 'Custom roles' })).toBeVisible(); }); test.describe('Users in role', () => { @@ -194,7 +196,7 @@ test.describe.parallel('administration', () => { await api.post('/channels.addOwner', { roomId: channel._id, userId: Users.user1.data._id }); await api.post('/channels.removeOwner', { roomId: channel._id, userId: Users.admin.data._id }); - }) + }); test('admin should be able to get the owners of a room that wasnt created by him', async ({ page }) => { await poAdmin.openRoleByName('Owner').click(); @@ -203,7 +205,42 @@ test.describe.parallel('administration', () => { await page.getByRole('option', { name: channelName }).click(); await expect(poAdmin.getUserRowByUsername('user1')).toBeVisible(); - }) + }); + + test('should add user1 as moderator of target channel', async ({ page }) => { + await poAdmin.openRoleByName('Moderator').click(); + await poAdmin.btnUsersInRole.click(); + + await poAdmin.inputRoom.fill(channelName); + await page.getByRole('option', { name: channelName }).click(); + + await poAdmin.inputUsers.fill('user1'); + await page.getByRole('option', { name: 'user1' }).click(); + await poAdmin.btnAdd.click(); + + await expect(poAdmin.getUserRowByUsername('user1')).toBeVisible(); + }); + + test('should remove user1 as moderator of target channel', async ({ page }) => { + await poAdmin.openRoleByName('Moderator').click(); + await poAdmin.btnUsersInRole.click(); + + await poAdmin.inputRoom.fill(channelName); + await page.getByRole('option', { name: channelName }).click(); + + await poAdmin.getUserRowByUsername('user1').getByRole('button', { name: 'Remove' }).click(); + await poUtils.btnModalConfirmDelete.click(); + + await expect(page.locator('h3 >> text="No results found"')).toBeVisible(); + }); + + test('should back to the permissions page', async ({ page }) => { + await poAdmin.openRoleByName('Moderator').click(); + await poAdmin.btnUsersInRole.click(); + await poAdmin.btnBack.click(); + + await expect(page.locator('h1 >> text="Permissions"')).toBeVisible(); + }); }) }); diff --git a/apps/meteor/tests/e2e/page-objects/admin.ts b/apps/meteor/tests/e2e/page-objects/admin.ts index 5f61d2ef43e1..8582d7be5911 100644 --- a/apps/meteor/tests/e2e/page-objects/admin.ts +++ b/apps/meteor/tests/e2e/page-objects/admin.ts @@ -197,7 +197,19 @@ export class Admin { return this.page.locator('input[placeholder="Room"]'); } + get inputUsers(): Locator { + return this.page.locator('input[placeholder="Users"]'); + } + + get btnAdd(): Locator { + return this.page.getByRole('button', { name: 'Add' }); + } + + get btnBack(): Locator { + return this.page.getByRole('button', { name: 'Back' }); + } + getUserRowByUsername(username: string): Locator { - return this.page.locator('tr', { hasText: username }) + return this.page.locator('tr', { hasText: username }); } } From 68984cd0dbaf7d7d9f7daf3317c501748a749cc3 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 23 May 2024 11:47:28 -0300 Subject: [PATCH 4/5] chore: changeset --- .changeset/rare-colts-repair.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rare-colts-repair.md diff --git a/.changeset/rare-colts-repair.md b/.changeset/rare-colts-repair.md new file mode 100644 index 000000000000..9011de6ff483 --- /dev/null +++ b/.changeset/rare-colts-repair.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes an issue not rendering the proper error and empty state on users in role table From 3b06b2bddc890827e3f117ae8fabd2d51c8c499c Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 23 May 2024 14:40:50 -0300 Subject: [PATCH 5/5] fix: review --- .../UsersInRoleTable/UsersInRoleTable.tsx | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx index b78f950c0102..ed7fdd93d79c 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx +++ b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/UsersInRoleTable.tsx @@ -51,11 +51,12 @@ const UsersInRoleTable = ({ rid, roleId, roleName, description }: UsersInRoleTab getUsersInRoleEndpoint(query), ); - const users = data?.users?.map((user) => ({ - ...user, - createdAt: new Date(user.createdAt), - _updatedAt: new Date(user._updatedAt), - })); + const users = + data?.users?.map((user) => ({ + ...user, + createdAt: new Date(user.createdAt), + _updatedAt: new Date(user._updatedAt), + })) || []; const handleRemove = useEffectEvent((username) => { const remove = async () => { @@ -95,7 +96,7 @@ const UsersInRoleTable = ({ rid, roleId, roleName, description }: UsersInRoleTab )} - {isSuccess && users && users.length > 0 && ( + {isSuccess && users?.length > 0 && ( <> {headers} @@ -116,7 +117,7 @@ const UsersInRoleTable = ({ rid, roleId, roleName, description }: UsersInRoleTab /> )} - {users && users.length === 0 && } + {users?.length === 0 && } {isError && } );