diff --git a/.changeset/nice-bottles-breathe.md b/.changeset/nice-bottles-breathe.md new file mode 100644 index 0000000000000..89a85ce3afba8 --- /dev/null +++ b/.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. diff --git a/apps/meteor/server/services/team/service.ts b/apps/meteor/server/services/team/service.ts index 60fb07226d624..cde46a95ce8f0 100644 --- a/apps/meteor/server/services/team/service.ts +++ b/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('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): Promise { + 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): Promise { const isMember = await TeamMember.findOneByUserIdAndTeamId(userId, teamId, { projection: { _id: 1 }, diff --git a/apps/meteor/tests/end-to-end/api/teams.ts b/apps/meteor/tests/end-to-end/api/teams.ts index 9e1816f353151..8623c97c7f8df 100644 --- a/apps/meteor/tests/end-to-end/api/teams.ts +++ b/apps/meteor/tests/end-to-end/api/teams.ts @@ -430,11 +430,6 @@ describe('/teams.addMembers', () => { let testUser: TestUser; let testUser2: TestUser; - 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', () => { diff --git a/packages/core-services/src/types/ITeamService.ts b/packages/core-services/src/types/ITeamService.ts index bd103eaed7332..e3f77b3c7d274 100644 --- a/packages/core-services/src/types/ITeamService.ts +++ b/packages/core-services/src/types/ITeamService.ts @@ -125,6 +125,7 @@ export interface ITeamService { getStatistics(): Promise; findBySubscribedUserIds(userId: string, callerId?: string): Promise; addRolesToMember(teamId: string, userId: string, roles: Array): Promise; + addRolesToSubscription(roomId: string, userId: string, roles: Array): Promise; getRoomInfo( room: AtLeast, ): Promise<{ team?: Pick; parentRoom?: Pick }>;