fix: teams.addMembers API to assign roles properly (RocketChat#36852)

Author: yash-rajpal

.changeset/nice-bottles-breathe.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/core-services': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes `teams.addMembers` API to assign team member roles properly.

apps/meteor/server/services/team/service.ts

@@ -1,4 +1,4 @@
-import { Room, Authorization, Message, ServiceClassInternal } from '@rocket.chat/core-services';
+import { Room, Authorization, Message, ServiceClassInternal, api } from '@rocket.chat/core-services';
 import type {
 	IListRoomsFilter,
 	ITeamAutocompleteResult,
@@ -723,7 +723,21 @@ export class TeamService extends ServiceClassInternal implements ITeamService {
 			await addUserToRoom(team.roomId, user, createdBy, { skipSystemMessage: false });
 
 			if (member.roles) {
-				await this.addRolesToMember(teamId, member.userId, member.roles);
+				const isRoleAddedToTeam = await this.addRolesToMember(teamId, member.userId, member.roles);
+				const isRoleAddedToSubscription = await this.addRolesToSubscription(team.roomId, member.userId, member.roles);
+				if (settings.get<boolean>('UI_DisplayRoles') && isRoleAddedToTeam && isRoleAddedToSubscription) {
+					member.roles.forEach((role) => {
+						void api.broadcast('user.roleUpdate', {
+							type: 'added',
+							_id: role,
+							u: {
+								_id: user._id,
+								username: user.username,
+							},
+							scope: team.roomId,
+						});
+					});
+				}
 			}
 		}
 	}
@@ -939,6 +953,17 @@ export class TeamService extends ServiceClassInternal implements ITeamService {
 		return !!(await TeamMember.updateRolesByTeamIdAndUserId(teamId, userId, roles));
 	}
 
+	async addRolesToSubscription(roomId: string, userId: string, roles: Array<string>): Promise<boolean> {
+		const subscription = await Subscriptions.findOneByRoomIdAndUserId(roomId, userId);
+
+		if (!subscription) {
+			// TODO should this throw an error instead?
+			return false;
+		}
+
+		return !!(await Subscriptions.addRolesByUserId(userId, roles, roomId));
+	}
+
 	async removeRolesFromMember(teamId: string, userId: string, roles: Array<string>): Promise<boolean> {
 		const isMember = await TeamMember.findOneByUserIdAndTeamId(userId, teamId, {
 			projection: { _id: 1 },

apps/meteor/tests/end-to-end/api/teams.ts

@@ -430,11 +430,6 @@ describe('/teams.addMembers', () => {
 	let testUser: TestUser<IUser>;
 	let testUser2: TestUser<IUser>;
 
-	before(async () => {
-		testUser = await createUser();
-		testUser2 = await createUser();
-	});
-
 	before('Create test team', (done) => {
 		void request
 			.post(api('teams.create'))
@@ -449,7 +444,14 @@ describe('/teams.addMembers', () => {
 			});
 	});
 
-	after(() => Promise.all([deleteUser(testUser), deleteUser(testUser2), deleteTeam(credentials, teamName)]));
+	after(() => deleteTeam(credentials, teamName));
+
+	beforeEach(async () => {
+		testUser = await createUser();
+		testUser2 = await createUser();
+	});
+
+	afterEach(() => Promise.all([deleteUser(testUser), deleteUser(testUser2)]));
 
 	it('should add members to a public team', (done) => {
 		void request
@@ -516,6 +518,72 @@ describe('/teams.addMembers', () => {
 			.then(() => done())
 			.catch(done);
 	});
+
+	it('should add members and assign roles to them properly', (done) => {
+		void request
+			.post(api('teams.addMembers'))
+			.set(credentials)
+			.send({
+				teamName: testTeam.name,
+				members: [
+					{
+						userId: testUser._id,
+						roles: ['owner', 'leader'],
+					},
+					{
+						userId: testUser2._id,
+						roles: ['moderator'],
+					},
+				],
+			})
+			.expect('Content-Type', 'application/json')
+			.expect(200)
+			.expect((res) => {
+				expect(res.body).to.have.property('success', true);
+			})
+			.then(() =>
+				request
+					.get(api('rooms.membersOrderedByRole'))
+					.set(credentials)
+					.query({
+						roomId: testTeam.roomId,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((response) => {
+						expect(response.body).to.have.property('success', true);
+						expect(response.body).to.have.property('members');
+						expect(response.body.members).to.have.length(3);
+						expect(response.body.members[1]).to.have.property('_id');
+						expect(response.body.members[1]).to.have.property('roles');
+						expect(response.body.members[1]).to.have.property('username');
+						expect(response.body.members[1]).to.have.property('name');
+
+						const members = (response.body.members as IUser[]).map(({ _id, username, name, roles }) => ({
+							_id,
+							username,
+							name,
+							roles,
+						}));
+
+						expect(members).to.deep.own.include({
+							_id: testUser._id,
+							username: testUser.username,
+							name: testUser.name,
+							roles: ['owner', 'leader'],
+						});
+
+						expect(members).to.deep.own.include({
+							_id: testUser2._id,
+							username: testUser2.username,
+							name: testUser2.name,
+							roles: ['moderator'],
+						});
+					}),
+			)
+			.then(() => done())
+			.catch(done);
+	});
 });
 
 describe('/teams.members', () => {

packages/core-services/src/types/ITeamService.ts

@@ -125,6 +125,7 @@ export interface ITeamService {
 	getStatistics(): Promise<ITeamStats>;
 	findBySubscribedUserIds(userId: string, callerId?: string): Promise<ITeam[]>;
 	addRolesToMember(teamId: string, userId: string, roles: Array<string>): Promise<boolean>;
+	addRolesToSubscription(roomId: string, userId: string, roles: Array<string>): Promise<boolean>;
 	getRoomInfo(
 		room: AtLeast<IRoom, 'teamId' | 'teamMain' | '_id'>,
 	): Promise<{ team?: Pick<ITeam, 'name' | 'roomId' | 'type'>; parentRoom?: Pick<IRoom, 'name' | 'fname' | 't' | '_id'> }>;