room members are always added as admins

hpi-schul-cloud · Dec 13, 2024 · b821dd1 · b821dd1
1 parent 0df7b67
commit b821dd1
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 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
@@ -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
@@ -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<EntityId> {
+	public async addMembersToRoom(roomId: EntityId, userIds: Array<EntityId>): Promise<EntityId> {
 		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
@@ -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
@@ -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<RoomListResponse> {
@@ -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<RoomItemResponse> {
@@ -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<RoomDetailsResponse> {
@@ -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<RoomBoardListResponse> {
@@ -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<void> {
+	public async deleteRoom(@CurrentUser() currentUser: ICurrentUser, @Param() urlParams: RoomUrlParams): Promise<void> {
 		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<void> {
-		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<RoomMemberListResponse> {

diff --git 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<void> {
+	public async addMembersToRoom(currentUserId: EntityId, roomId: EntityId, userIds: Array<EntityId>): Promise<void> {
 		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
@@ -119,21 +119,25 @@ 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);
 			});
 		});
 
 		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);