feat: display leader role group in rooms' members list (#35208)

RocketChat · Feb 20, 2025 · be5031a · be5031a
1 parent 8996414
commit be5031a
Show file tree

Hide file tree

Showing 16 changed files with 160 additions and 210 deletions.
diff --git a/.changeset/rotten-peaches-cry.md b/.changeset/rotten-peaches-cry.md
@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Removes the Leader role from the room header. Leaders are now displayed in their respective group within the room's members list.  
diff --git a/.changeset/weak-kangaroos-admire.md b/.changeset/weak-kangaroos-admire.md
@@ -0,0 +1,9 @@
+---
+'@rocket.chat/model-typings': minor
+'@rocket.chat/core-typings': minor
+'@rocket.chat/models': minor
+'@rocket.chat/i18n': minor
+'@rocket.chat/meteor': minor
+---
+
+Adds the Leader group to rooms' members list for better role visibility and consistency.  
diff --git a/apps/meteor/app/lib/server/functions/createRoom.ts b/apps/meteor/app/lib/server/functions/createRoom.ts
@@ -10,8 +10,9 @@ import { Meteor } from 'meteor/meteor';
 import { createDirectRoom } from './createDirectRoom';
 import { callbacks } from '../../../../lib/callbacks';
 import { beforeCreateRoomCallback } from '../../../../lib/callbacks/beforeCreateRoomCallback';
+import { calculateRoomRolePriorityFromRoles } from '../../../../lib/roles/calculateRoomRolePriorityFromRoles';
 import { getSubscriptionAutotranslateDefaultConfig } from '../../../../server/lib/getSubscriptionAutotranslateDefaultConfig';
-import { calculateRoomRolePriorityFromRoles, syncRoomRolePriorityForUserAndRoom } from '../../../../server/lib/roles/syncRoomRolePriority';
+import { syncRoomRolePriorityForUserAndRoom } from '../../../../server/lib/roles/syncRoomRolePriority';
 import { getDefaultSubscriptionPref } from '../../../utils/lib/getDefaultSubscriptionPref';
 import { getValidRoomName } from '../../../utils/server/lib/getValidRoomName';
 import { notifyOnRoomChanged, notifyOnSubscriptionChangedById } from '../lib/notifyListener';

diff --git a/apps/meteor/app/lib/server/functions/syncRolePrioritiesForRoomIfRequired.ts b/apps/meteor/app/lib/server/functions/syncRolePrioritiesForRoomIfRequired.ts
@@ -1,10 +1,12 @@
 import type { IRoom, IUser } from '@rocket.chat/core-typings';
 import { Subscriptions, Users, Rooms } from '@rocket.chat/models';
 
-import { calculateRoomRolePriorityFromRoles } from '../../../../server/lib/roles/syncRoomRolePriority';
+import { calculateRoomRolePriorityFromRoles } from '../../../../lib/roles/calculateRoomRolePriorityFromRoles';
 
 const READ_BATCH_SIZE = 1000;
 
+const SYNC_VERSION = 2;
+
 async function assignRoomRolePrioritiesFromMap(userIdAndRoomRolePrioritiesMap: Map<IUser['_id'], IUser['roomRolePriorities']>) {
 	const bulk = Users.col.initializeUnorderedBulkOp();
 
@@ -35,7 +37,7 @@ async function assignRoomRolePrioritiesFromMap(userIdAndRoomRolePrioritiesMap: M
 export const syncRolePrioritiesForRoomIfRequired = async (rid: IRoom['_id']) => {
 	const userIdAndRoomRolePrioritiesMap = new Map<IUser['_id'], IUser['roomRolePriorities']>();
 
-	if (await Rooms.hasCreatedRolePrioritiesForRoom(rid)) {
+	if (await Rooms.hasCreatedRolePrioritiesForRoom(rid, SYNC_VERSION)) {
 		return;
 	}
 
@@ -64,5 +66,5 @@ export const syncRolePrioritiesForRoomIfRequired = async (rid: IRoom['_id']) =>
 	// Flush any remaining priorities in the map
 	await assignRoomRolePrioritiesFromMap(userIdAndRoomRolePrioritiesMap);
 
-	await Rooms.markRolePrioritesCreatedForRoom(rid);
+	await Rooms.markRolePrioritesCreatedForRoom(rid, SYNC_VERSION);
 };
diff --git a/apps/meteor/client/views/hooks/useMemberList.spec.tsx b/apps/meteor/client/views/hooks/useMemberList.spec.tsx
@@ -53,21 +53,22 @@ describe('useMembersList', () => {
 
 		fakeMembersPage1 = {
 			offset: 0,
-			count: 2,
-			total: 4,
+			count: 3,
+			total: 5,
 			members: [
 				{ _id: 'user1', username: 'alex', roles: ['owner'], status: UserStatus.ONLINE },
-				{ _id: 'user2', username: 'john', roles: ['moderator'], status: UserStatus.OFFLINE },
+				{ _id: 'user2', username: 'bob', roles: ['leader'], status: UserStatus.OFFLINE },
+				{ _id: 'user3', username: 'john', roles: ['moderator'], status: UserStatus.OFFLINE },
 			],
 		};
 
 		fakeMembersPage2 = {
-			offset: 2,
+			offset: 3,
 			count: 2,
-			total: 4,
+			total: 5,
 			members: [
-				{ _id: 'user3', username: 'chris', roles: [], status: UserStatus.ONLINE },
-				{ _id: 'user4', username: 'zoe', roles: [], status: UserStatus.OFFLINE },
+				{ _id: 'user4', username: 'chris', roles: [], status: UserStatus.ONLINE },
+				{ _id: 'user5', username: 'zoe', roles: [], status: UserStatus.OFFLINE },
 			],
 		};
 
@@ -80,7 +81,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'room123',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'c',
 				}),
@@ -102,7 +103,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'room123',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'p',
 				}),
@@ -125,7 +126,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'directRoomId',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'd',
 				}),
@@ -146,7 +147,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'room123',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'c',
 				}),
@@ -157,22 +158,22 @@ describe('useMembersList', () => {
 						if (offset === 0) {
 							return fakeMembersPage1 as any;
 						}
-						if (offset === 2) {
+						if (offset === 3) {
 							return fakeMembersPage2 as any;
 						}
 						return {
 							members: [],
 							offset,
-							count: 2,
-							total: 4,
+							count: 0,
+							total: 5,
 						};
 					})
 					.build(),
 			},
 		);
 
 		await waitFor(() => expect(result.current.isLoading).toBe(false));
-		expect(result.current.data?.pages[0].members).toHaveLength(2);
+		expect(result.current.data?.pages[0].members).toHaveLength(3);
 
 		await act(async () => {
 			await result.current.fetchNextPage();
@@ -194,7 +195,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'room123',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'c',
 				}),
@@ -224,7 +225,7 @@ describe('useMembersList', () => {
 				useMembersList({
 					rid: 'room123',
 					type: 'all',
-					limit: 2,
+					limit: 3,
 					debouncedText: '',
 					roomType: 'c',
 				}),
@@ -233,7 +234,7 @@ describe('useMembersList', () => {
 
 		await waitFor(() => expect(result.current.isLoading).toBe(false));
 		let user2 = result.current.data?.pages[0].members.find((m) => m._id === 'user2') as RoomMember;
-		expect(user2?.roles).toEqual(['moderator']);
+		expect(user2?.roles).toEqual(['leader']);
 
 		// Simulate a roles-change event "added" for user2 -> 'owner'
 		await act(async () => {
@@ -248,20 +249,21 @@ describe('useMembersList', () => {
 		user2 = result.current.data?.pages[0].members.find((m) => m._id === 'user2') as RoomMember;
 
 		await waitFor(() => expect(user2?.roles).toContain('owner'));
-		await waitFor(() => expect(user2?.roles).toContain('moderator'));
+		await waitFor(() => expect(user2?.roles).toContain('leader'));
 	});
 
 	it('sorts members list cache by "roles > status > username/name" logic on roles-change', async () => {
 		const customPage = {
 			offset: 0,
-			count: 5,
-			total: 5,
+			count: 6,
+			total: 6,
 			members: [
-				{ _id: 'u1', username: 'michael', roles: ['owner'], status: UserStatus.OFFLINE },
-				{ _id: 'u2', username: 'karl', roles: ['moderator'], status: UserStatus.ONLINE },
-				{ _id: 'u3', username: 'bob', roles: ['moderator'], status: UserStatus.OFFLINE },
-				{ _id: 'u4', username: 'alex', roles: [], status: UserStatus.OFFLINE },
-				{ _id: 'u5', username: 'john', roles: [], status: UserStatus.ONLINE },
+				{ _id: 'u1', username: 'mark', roles: ['owner'], status: UserStatus.OFFLINE },
+				{ _id: 'u2', username: 'michael', roles: ['leader'], status: UserStatus.OFFLINE },
+				{ _id: 'u3', username: 'karl', roles: ['moderator'], status: UserStatus.ONLINE },
+				{ _id: 'u4', username: 'bob', roles: ['moderator'], status: UserStatus.OFFLINE },
+				{ _id: 'u5', username: 'alex', roles: [], status: UserStatus.OFFLINE },
+				{ _id: 'u6', username: 'john', roles: [], status: UserStatus.ONLINE },
 			],
 		};
 
@@ -291,23 +293,23 @@ describe('useMembersList', () => {
 		await waitFor(() => expect(result.current.isLoading).toBe(false));
 
 		let memberIds = result.current.data?.pages[0].members.map((m) => m._id);
-		expect(memberIds).toEqual(['u1', 'u2', 'u3', 'u4', 'u5']);
+		expect(memberIds).toEqual(['u1', 'u2', 'u3', 'u4', 'u5', 'u6']);
 
-		// Simulate giving user 'alex/u4' the "moderator" role.
-		// That should push 'alex/u4' to the third of the sorted list -
-		// after michael/u1 (since owner), after karl/u2 (since online) and before bob/u3 (since sort by username).
+		// Simulate giving user 'alex/u5' the "moderator" role.
+		// That should push 'alex/u5' to the third of the sorted list -
+		// after mark/u1 and michael/u2 (since owner), after karl/u3 (since online) and before bob/u4 (since sort by username).
 		act(() => {
 			rolesChangeCallback?.({
 				type: 'added',
 				scope: 'room123',
-				u: { _id: 'u4' },
+				u: { _id: 'u5' },
 				_id: 'moderator',
 			});
 		});
 
 		await waitFor(() => {
 			memberIds = result.current.data?.pages[0].members.map((m) => m._id);
-			expect(memberIds).toEqual(['u1', 'u2', 'u4', 'u3', 'u5']);
+			expect(memberIds).toEqual(['u1', 'u2', 'u3', 'u5', 'u4', 'u6']);
 		});
 	});
 

diff --git a/apps/meteor/client/views/hooks/useMembersList.ts b/apps/meteor/client/views/hooks/useMembersList.ts
@@ -4,6 +4,8 @@ import type { InfiniteData, QueryClient } from '@tanstack/react-query';
 import { useInfiniteQuery, useQueryClient } from '@tanstack/react-query';
 import { useEffect } from 'react';
 
+import { calculateRoomRolePriorityFromRoles } from '../../../lib/roles/calculateRoomRolePriorityFromRoles';
+
 type MembersListOptions = {
 	rid: string;
 	type: 'all' | 'online';
@@ -23,20 +25,14 @@ export type RoomMember = Pick<IUser, 'username' | '_id' | 'name' | 'status' | 'f
 type MembersListPage = { members: RoomMember[]; count: number; total: number; offset: number };
 
 const getSortedMembers = (members: RoomMember[], useRealName = false) => {
-	return members.sort((a, b) => {
-		const aRoles = a.roles ?? [];
-		const bRoles = b.roles ?? [];
-		const isOwnerA = aRoles.includes('owner');
-		const isOwnerB = bRoles.includes('owner');
-		const isModeratorA = aRoles.includes('moderator');
-		const isModeratorB = bRoles.includes('moderator');
-
-		if (isOwnerA !== isOwnerB) {
-			return isOwnerA ? -1 : 1;
-		}
-
-		if (isModeratorA !== isModeratorB && !isOwnerA) {
-			return isModeratorA ? -1 : 1;
+	const membersWithRolePriority: (RoomMember & { rolePriority: number })[] = members.map((member) => ({
+		...member,
+		rolePriority: calculateRoomRolePriorityFromRoles(member.roles ?? []),
+	}));
+
+	return membersWithRolePriority.sort((a, b) => {
+		if (a.rolePriority !== b.rolePriority) {
+			return a.rolePriority - b.rolePriority;
 		}
 
 		if (a.status !== b.status) {

diff --git a/apps/meteor/client/views/room/body/LeaderBar.tsx b/apps/meteor/client/views/room/body/LeaderBar.tsx