From b821dd16f766358107433b5aa0574233df97ba55 Mon Sep 17 00:00:00 2001 From: Thomas Feldtkeller Date: Fri, 13 Dec 2024 16:54:15 +0100 Subject: [PATCH] room members are always added as admins --- .../service/room-membership.service.spec.ts | 19 ++++++------ .../service/room-membership.service.ts | 12 +++----- .../request/add-room-members.body.params.ts | 30 ++++--------------- .../src/modules/room/api/room.controller.ts | 20 ++++++------- apps/server/src/modules/room/api/room.uc.ts | 13 ++------ .../api/test/room-add-members.api.spec.ts | 14 +++++---- 6 files changed, 40 insertions(+), 68 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts index c6249661dc..7ee7e21aac 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts @@ -87,14 +87,12 @@ describe('RoomMembershipService', () => { }; }; - describe('when no user is provided', () => { - it('should throw an exception', async () => { - const { room } = setup(); + it('should throw an exception', async () => { + const { room, user } = setup(); - roomMembershipRepo.findByRoomId.mockResolvedValue(null); + roomMembershipRepo.findByRoomId.mockResolvedValue(null); - await expect(service.addMembersToRoom(room.id, [])).rejects.toThrow(); - }); + await expect(service.addMembersToRoom(room.id, [user.id])).rejects.toThrow(); }); }); @@ -120,20 +118,21 @@ describe('RoomMembershipService', () => { }; }; - it('should add user to existing roomMembership', async () => { + it('should add user as admin to existing roomMembership', async () => { + // TODO: in the future, once room roles can be changed, this should become ROOMVIEWER const { user, room, group } = setup(); - await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]); + await service.addMembersToRoom(room.id, [user.id]); expect(groupService.addUsersToGroup).toHaveBeenCalledWith(group.id, [ - { userId: user.id, roleName: RoleName.ROOMEDITOR }, + { userId: user.id, roleName: RoleName.ROOMADMIN }, ]); }); it('should add user to school', async () => { const { user, room } = setup(); - await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]); + await service.addMembersToRoom(room.id, [user.id]); expect(userService.addSecondarySchoolToUsers).toHaveBeenCalledWith([user.id], room.schoolId); }); diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.ts b/apps/server/src/modules/room-membership/service/room-membership.service.ts index fd978721d0..ad6af25fa3 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.ts @@ -73,21 +73,17 @@ export class RoomMembershipService { await this.roomMembershipRepo.delete(roomMembership); } - public async addMembersToRoom( - roomId: EntityId, - userIdsAndRoles: Array<{ - userId: EntityId; - roleName: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; - }> - ): Promise { + public async addMembersToRoom(roomId: EntityId, userIds: Array): Promise { const roomMembership = await this.roomMembershipRepo.findByRoomId(roomId); if (roomMembership === null) { throw new Error('Room membership not found'); } + const userIdsAndRoles = userIds.map((userId) => { + return { userId, roleName: RoleName.ROOMADMIN }; + }); await this.groupService.addUsersToGroup(roomMembership.userGroupId, userIdsAndRoles); - const userIds = userIdsAndRoles.map((user) => user.userId); await this.userService.addSecondarySchoolToUsers(userIds, roomMembership.schoolId); return roomMembership.id; diff --git a/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts b/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts index 9980d106fb..f27f142a65 100644 --- a/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts +++ b/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts @@ -1,32 +1,12 @@ import { ApiProperty } from '@nestjs/swagger'; -import { IsMongoId, IsString, ValidateNested } from 'class-validator'; -import { Type } from 'class-transformer'; -import { RoleName, RoomRoleArray } from '@shared/domain/interface'; - -class UserIdAndRole { - @ApiProperty({ - description: 'The ID of the user', - required: true, - }) - @IsMongoId() - userId!: string; - - @ApiProperty({ - description: 'The role of the user', - required: true, - enum: RoomRoleArray, - }) - @IsString() - roleName!: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; -} +import { IsArray, IsMongoId } from 'class-validator'; export class AddRoomMembersBodyParams { @ApiProperty({ - description: 'Array of userIds and their roles inside of the room', + description: 'The IDs of the users', required: true, - type: [UserIdAndRole], }) - @ValidateNested({ each: true }) - @Type(() => UserIdAndRole) - userIdsAndRoles!: UserIdAndRole[]; + @IsArray() + @IsMongoId({ each: true }) + public userIds!: string[]; } diff --git a/apps/server/src/modules/room/api/room.controller.ts b/apps/server/src/modules/room/api/room.controller.ts index b9a125787c..b130516f93 100644 --- a/apps/server/src/modules/room/api/room.controller.ts +++ b/apps/server/src/modules/room/api/room.controller.ts @@ -47,7 +47,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.UNAUTHORIZED, type: UnauthorizedException }) @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async getRooms( + public async getRooms( @CurrentUser() currentUser: ICurrentUser, @Query() pagination: RoomPaginationParams ): Promise { @@ -67,7 +67,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.UNAUTHORIZED, type: UnauthorizedException }) @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async createRoom( + public async createRoom( @CurrentUser() currentUser: ICurrentUser, @Body() createRoomParams: CreateRoomBodyParams ): Promise { @@ -86,7 +86,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: HttpStatus.NOT_FOUND, type: NotFoundException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async getRoomDetails( + public async getRoomDetails( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams ): Promise { @@ -105,7 +105,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: HttpStatus.NOT_FOUND, type: NotFoundException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async getRoomBoards( + public async getRoomBoards( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams ): Promise { @@ -124,7 +124,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: HttpStatus.NOT_FOUND, type: NotFoundException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async updateRoom( + public async updateRoom( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams, @Body() updateRoomParams: UpdateRoomBodyParams @@ -145,7 +145,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.NOT_FOUND, type: NotFoundException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) @HttpCode(204) - async deleteRoom(@CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams): Promise { + public async deleteRoom(@CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams): Promise { await this.roomUc.deleteRoom(currentUser.userId, urlParams.roomId); } @@ -156,12 +156,12 @@ export class RoomController { @ApiResponse({ status: HttpStatus.UNAUTHORIZED, type: UnauthorizedException }) @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async addMembers( + public async addMembers( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams, @Body() bodyParams: AddRoomMembersBodyParams ): Promise { - await this.roomUc.addMembersToRoom(currentUser.userId, urlParams.roomId, bodyParams.userIdsAndRoles); + await this.roomUc.addMembersToRoom(currentUser.userId, urlParams.roomId, bodyParams.userIds); } @Patch(':roomId/members/remove') @@ -171,7 +171,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.UNAUTHORIZED, type: UnauthorizedException }) @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async removeMembers( + public async removeMembers( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams, @Body() bodyParams: RemoveRoomMembersBodyParams @@ -190,7 +190,7 @@ export class RoomController { @ApiResponse({ status: HttpStatus.UNAUTHORIZED, type: UnauthorizedException }) @ApiResponse({ status: HttpStatus.FORBIDDEN, type: ForbiddenException }) @ApiResponse({ status: '5XX', type: ErrorResponse }) - async getMembers( + public async getMembers( @CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams ): Promise { diff --git a/apps/server/src/modules/room/api/room.uc.ts b/apps/server/src/modules/room/api/room.uc.ts index 1de7fd11b5..39ed7379be 100644 --- a/apps/server/src/modules/room/api/room.uc.ts +++ b/apps/server/src/modules/room/api/room.uc.ts @@ -5,7 +5,7 @@ import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { FeatureDisabledLoggableException } from '@shared/common/loggable-exception'; import { Page, UserDO } from '@shared/domain/domainobject'; -import { IFindOptions, Permission, RoleName } from '@shared/domain/interface'; +import { IFindOptions, Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { BoardExternalReferenceType, ColumnBoard, ColumnBoardService } from '@modules/board'; import { Room, RoomService } from '../domain'; @@ -125,17 +125,10 @@ export class RoomUc { return memberResponses; } - public async addMembersToRoom( - currentUserId: EntityId, - roomId: EntityId, - userIdsAndRoles: Array<{ - userId: EntityId; - roleName: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; - }> - ): Promise { + public async addMembersToRoom(currentUserId: EntityId, roomId: EntityId, userIds: Array): Promise { this.checkFeatureEnabled(); await this.checkRoomAuthorization(currentUserId, roomId, Action.write, [Permission.ROOM_MEMBERS_ADD]); - await this.roomMembershipService.addMembersToRoom(roomId, userIdsAndRoles); + await this.roomMembershipService.addMembersToRoom(roomId, userIds); } private mapToMember(member: UserWithRoomRoles, user: UserDO): RoomMemberResponse { diff --git a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts index d4d5761ad5..a4a9a09f6e 100644 --- a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts @@ -119,10 +119,12 @@ describe('Room Controller (API)', () => { }; it('should return forbidden error', async () => { - const { room } = await setupRoomWithMembers(); + const { room, otherTeacherUser } = await setupRoomWithMembers(); const { loggedInClient } = await setupLoggedInUser(); - const response = await loggedInClient.patch(`/${room.id}/members/add`); + const response = await loggedInClient.patch(`/${room.id}/members/add`, { + userIds: [otherTeacherUser.id], + }); expect(response.status).toBe(HttpStatus.FORBIDDEN); }); @@ -130,10 +132,12 @@ describe('Room Controller (API)', () => { describe('when the feature is disabled', () => { it('should return a 403 error', async () => { - const { loggedInClient, room } = await setupRoomWithMembers(); + const { loggedInClient, room, otherTeacherUser } = await setupRoomWithMembers(); config.FEATURE_ROOMS_ENABLED = false; - const response = await loggedInClient.patch(`/${room.id}/members/add`); + const response = await loggedInClient.patch(`/${room.id}/members/add`, { + userIds: [otherTeacherUser.id], + }); expect(response.status).toBe(HttpStatus.FORBIDDEN); }); @@ -144,7 +148,7 @@ describe('Room Controller (API)', () => { const { loggedInClient, room, otherTeacherUser } = await setupRoomWithMembers(); const response = await loggedInClient.patch(`/${room.id}/members/add`, { - userIdsAndRoles: [{ userId: otherTeacherUser.id, roleName: RoleName.ROOMEDITOR }], + userIds: [otherTeacherUser.id], }); expect(response.status).toBe(HttpStatus.OK);