From 36213a0fcc9af7894d48e4914c86cfcdcc838a32 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 09:28:59 +0100 Subject: [PATCH 1/8] add group filter for multiple userIds --- .../group/domain/interface/group-filter.ts | 1 + .../server/src/modules/group/repo/group.repo.ts | 1 + .../src/modules/group/repo/group.scope.ts | 17 ++++++++++++----- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/apps/server/src/modules/group/domain/interface/group-filter.ts b/apps/server/src/modules/group/domain/interface/group-filter.ts index 7baaffbf511..228cf147e28 100644 --- a/apps/server/src/modules/group/domain/interface/group-filter.ts +++ b/apps/server/src/modules/group/domain/interface/group-filter.ts @@ -3,6 +3,7 @@ import { EntityId } from '@shared/domain/types'; export interface GroupFilter { userId?: EntityId; + userIds?: EntityId[]; schoolId?: EntityId; systemId?: EntityId; groupTypes?: GroupTypes[]; diff --git a/apps/server/src/modules/group/repo/group.repo.ts b/apps/server/src/modules/group/repo/group.repo.ts index 180bc4fd6ae..af58c1d4f18 100644 --- a/apps/server/src/modules/group/repo/group.repo.ts +++ b/apps/server/src/modules/group/repo/group.repo.ts @@ -57,6 +57,7 @@ export class GroupRepo extends BaseDomainObjectRepo { public async findGroups(filter: GroupFilter, options?: IFindOptions): Promise> { const scope: GroupScope = new GroupScope(); scope.byUserId(filter.userId); + scope.byUserIds(filter.userIds); scope.byOrganizationId(filter.schoolId); scope.bySystemId(filter.systemId); diff --git a/apps/server/src/modules/group/repo/group.scope.ts b/apps/server/src/modules/group/repo/group.scope.ts index e227f111408..5b879c92609 100644 --- a/apps/server/src/modules/group/repo/group.scope.ts +++ b/apps/server/src/modules/group/repo/group.scope.ts @@ -5,35 +5,42 @@ import { Scope } from '@shared/repo/scope'; import { GroupEntity, GroupEntityTypes } from '../entity'; export class GroupScope extends Scope { - byTypes(types: GroupEntityTypes[] | undefined): this { + public byTypes(types: GroupEntityTypes[] | undefined): this { if (types) { this.addQuery({ type: { $in: types } }); } return this; } - byOrganizationId(id: EntityId | undefined): this { + public byOrganizationId(id: EntityId | undefined): this { if (id) { this.addQuery({ organization: id }); } return this; } - bySystemId(id: EntityId | undefined): this { + public bySystemId(id: EntityId | undefined): this { if (id) { this.addQuery({ externalSource: { system: new ObjectId(id) } }); } return this; } - byUserId(id: EntityId | undefined): this { + public byUserId(id: EntityId | undefined): this { if (id) { this.addQuery({ users: { user: new ObjectId(id) } }); } return this; } - byNameQuery(nameQuery: string | undefined): this { + public byUserIds(ids: EntityId[] | undefined): this { + if (ids) { + this.addQuery({ users: { user: { $in: ids.map((id) => new ObjectId(id)) } } }); + } + return this; + } + + public byNameQuery(nameQuery: string | undefined): this { if (nameQuery) { const escapedName = nameQuery.replace(MongoPatterns.REGEX_MONGO_LANGUAGE_PATTERN_WHITELIST, '').trim(); this.addQuery({ name: new RegExp(escapedName, 'i') }); From cc9da83a9afae5edaaeea958615a61a9f945d0b3 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 09:30:53 +0100 Subject: [PATCH 2/8] adapt accessibility keywords --- .../room-membership/repo/room-membership.repo.ts | 12 ++++++------ apps/server/src/modules/user/service/user.service.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/server/src/modules/room-membership/repo/room-membership.repo.ts b/apps/server/src/modules/room-membership/repo/room-membership.repo.ts index 5db8b3750ce..5477302b252 100644 --- a/apps/server/src/modules/room-membership/repo/room-membership.repo.ts +++ b/apps/server/src/modules/room-membership/repo/room-membership.repo.ts @@ -10,7 +10,7 @@ import { RoomMembershipDomainMapper } from './room-membership-domain.mapper'; export class RoomMembershipRepo { constructor(private readonly em: EntityManager) {} - async findByRoomId(roomId: EntityId): Promise { + public async findByRoomId(roomId: EntityId): Promise { const roomMembershipEntities = await this.em.findOne(RoomMembershipEntity, { roomId }); if (!roomMembershipEntities) return null; @@ -19,28 +19,28 @@ export class RoomMembershipRepo { return roomMemberships; } - async findByRoomIds(roomIds: EntityId[]): Promise { + public async findByRoomIds(roomIds: EntityId[]): Promise { const entities = await this.em.find(RoomMembershipEntity, { roomId: { $in: roomIds } }); const roomMemberships = entities.map((entity) => RoomMembershipDomainMapper.mapEntityToDo(entity)); return roomMemberships; } - async findByGroupId(groupId: EntityId): Promise { + public async findByGroupId(groupId: EntityId): Promise { const entities = await this.em.find(RoomMembershipEntity, { userGroupId: groupId }); const roomMemberships = entities.map((entity) => RoomMembershipDomainMapper.mapEntityToDo(entity)); return roomMemberships; } - async findByGroupIds(groupIds: EntityId[]): Promise { + public async findByGroupIds(groupIds: EntityId[]): Promise { const entities = await this.em.find(RoomMembershipEntity, { userGroupId: { $in: groupIds } }); const roomMemberships = entities.map((entity) => RoomMembershipDomainMapper.mapEntityToDo(entity)); return roomMemberships; } - async save(roomMembership: RoomMembership | RoomMembership[]): Promise { + public async save(roomMembership: RoomMembership | RoomMembership[]): Promise { const roomMemberships = Utils.asArray(roomMembership); roomMemberships.forEach((member) => { @@ -51,7 +51,7 @@ export class RoomMembershipRepo { await this.em.flush(); } - async delete(roomMembership: RoomMembership | RoomMembership[]): Promise { + public async delete(roomMembership: RoomMembership | RoomMembership[]): Promise { const roomMemberships = Utils.asArray(roomMembership); roomMemberships.forEach((member) => { diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index 6f0dc3f798a..ab7c47aec1a 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -174,7 +174,7 @@ export class UserService implements DeletionService, IEventHandler { + public async removeSecondarySchoolFromUsers(userIds: string[], schoolId: EntityId): Promise { const users = await this.userDORepo.findByIds(userIds, true); users.forEach((user) => { From 687ed69c7ba83dd761d7945de64606055d6c7407 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 09:32:30 +0100 Subject: [PATCH 3/8] WIP: implement guest role management on removal --- .../service/room-membership.service.spec.ts | 133 +++++++++++++++++- .../service/room-membership.service.ts | 21 ++- 2 files changed, 146 insertions(+), 8 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 d9134cb2867..d56fc85218e 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 @@ -1,13 +1,15 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { GroupService, GroupTypes } from '@modules/group'; -import { RoleService } from '@modules/role'; +import { MongoMemoryDatabaseModule } from '@infra/database'; +import { Group, GroupService, GroupTypes } from '@modules/group'; +import { RoleDto, RoleService } from '@modules/role'; +import { RoomService } from '@modules/room/domain'; import { roomFactory } from '@modules/room/testing'; +import { schoolFactory } from '@modules/school/testing'; +import { UserService } from '@modules/user'; import { BadRequestException } from '@nestjs/common/exceptions'; import { Test, TestingModule } from '@nestjs/testing'; import { RoleName } from '@shared/domain/interface'; -import { groupFactory, roleDtoFactory, userFactory } from '@shared/testing'; -import { MongoMemoryDatabaseModule } from '@src/infra/database'; -import { RoomService } from '@src/modules/room/domain'; +import { groupFactory, roleDtoFactory, roleFactory, userDoFactory, userFactory } from '@shared/testing'; import { RoomMembershipAuthorizable } from '../do/room-membership-authorizable.do'; import { RoomMembershipRepo } from '../repo/room-membership.repo'; import { roomMembershipFactory } from '../testing'; @@ -20,6 +22,7 @@ describe('RoomMembershipService', () => { let groupService: DeepMocked; let roleService: DeepMocked; let roomService: DeepMocked; + let userService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -42,6 +45,10 @@ describe('RoomMembershipService', () => { provide: RoomService, useValue: createMock(), }, + { + provide: UserService, + useValue: createMock(), + }, ], }).compile(); @@ -50,6 +57,7 @@ describe('RoomMembershipService', () => { groupService = module.get(GroupService); roleService = module.get(RoleService); roomService = module.get(RoomService); + userService = module.get(UserService); }); afterAll(async () => { @@ -176,6 +184,7 @@ describe('RoomMembershipService', () => { roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); groupService.findById.mockResolvedValue(group); + groupService.findGroups.mockResolvedValue({ total: 1, data: [group] }); return { user, @@ -193,6 +202,120 @@ describe('RoomMembershipService', () => { expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [user.id]); }); }); + + const setupUserWithSecondarySchool = () => { + const secondarySchool = schoolFactory.build(); + const otherSchool = schoolFactory.build(); + const role = roleFactory.buildWithId({ name: RoleName.TEACHER }); + const guestTeacher = roleFactory.buildWithId({ name: RoleName.GUESTTEACHER }); + const externalUser = userDoFactory.buildWithId({ + roles: [role], + secondarySchools: [{ schoolId: secondarySchool.id, role: new RoleDto(guestTeacher) }], + }); + return { secondarySchool, externalUser, otherSchool }; + }; + + const setupGroupAndRoom = (schoolId: string) => { + const group = groupFactory.build({ type: GroupTypes.ROOM }); + const room = roomFactory.build({ schoolId }); + const roomMembership = roomMembershipFactory.build({ + roomId: room.id, + userGroupId: group.id, + schoolId, + }); + return { group, room, roomMembership }; + }; + + const mockGroupsAtSchoolAfterRemoval = (groups: Group[]) => { + groupService.findGroups.mockResolvedValue({ total: groups.length, data: groups }); + }; + + describe('when after removal: user is not in any room of that secondary school', () => { + it('should remove user from secondary school', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([]); + + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + + expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith([externalUser.id], secondarySchool.id); + expect(1).toEqual(1); + }); + }); + + describe('when after removal: user is still in a room of that secondary school', () => { + it('should remove user from secondary school', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { group: group2 } = setupGroupAndRoom(secondarySchool.id); + group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([group, group2]); + + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + + expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); + }); + }); + + describe('when after removal: user is still in a room of a different school', () => { + it('should not remove user from secondary school', async () => { + const { secondarySchool, externalUser, otherSchool } = setupUserWithSecondarySchool(); + + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { group: group2 } = setupGroupAndRoom(otherSchool.id); + group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([]); + + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + + expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalled(); + }); + }); + + // describe('when user is removed from last room of primary school', () => { + // it('should remove user from primary school', async () => { + // const primarySchool = schoolEntityFactory.buildWithId(); + // const user = userFactory.buildWithId({ school: primarySchool }); + // const room = roomFactory.build({ schoolId: primarySchool.id }); + // const group = groupFactory.build({ type: GroupTypes.ROOM }); + // const roomMembership = roomMembershipFactory.build({ + // roomId: room.id, + // userGroupId: group.id, + // schoolId: primarySchool.id, + // }); + + // roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + // groupService.findById.mockResolvedValue(group); + // groupService.removeUsersFromGroup.mockResolvedValue(group); + // mockGroupsAtSchoolAfterRemoval([]); + + // await service.removeMembersFromRoom(room.id, [user.id]); + + // expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); + // }); + // }); }); describe('deleteRoomMembership', () => { 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 75e6e6922eb..e6529c1d6df 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 @@ -1,6 +1,7 @@ import { ObjectId } from '@mikro-orm/mongodb'; import { Group, GroupService, GroupTypes } from '@modules/group'; import { RoleDto, RoleService } from '@modules/role'; +import { UserService } from '@modules/user'; import { BadRequestException, Injectable } from '@nestjs/common'; import { RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; @@ -15,14 +16,15 @@ export class RoomMembershipService { private readonly groupService: GroupService, private readonly roomMembershipRepo: RoomMembershipRepo, private readonly roleService: RoleService, - private readonly roomService: RoomService + private readonly roomService: RoomService, + private readonly userService: UserService ) {} private async createNewRoomMembership( roomId: EntityId, userId: EntityId, roleName: RoleName.ROOMEDITOR | RoleName.ROOMVIEWER - ) { + ): Promise { const room = await this.roomService.getSingleRoom(roomId); const group = await this.groupService.createGroup( @@ -65,7 +67,7 @@ export class RoomMembershipService { return roomMembershipAuthorizable; } - public async deleteRoomMembership(roomId: EntityId) { + public async deleteRoomMembership(roomId: EntityId): Promise { const roomMembership = await this.roomMembershipRepo.findByRoomId(roomId); if (roomMembership === null) return; @@ -101,6 +103,8 @@ export class RoomMembershipService { const group = await this.groupService.findById(roomMembership.userGroupId); await this.groupService.removeUsersFromGroup(group.id, userIds); + + await this.handleGuestRoleRemoval(userIds, roomMembership.schoolId); } public async getRoomMembershipAuthorizablesByUserId(userId: EntityId): Promise { @@ -141,4 +145,15 @@ export class RoomMembershipService { return roomMembershipAuthorizable; } + + private async handleGuestRoleRemoval(userIds: EntityId[], schoolId: EntityId): Promise { + const { data: groups } = await this.groupService.findGroups({ userIds, groupTypes: [GroupTypes.ROOM], schoolId }); + + const userIdsInGroups = groups.flatMap((group) => group.users.map((groupUser) => groupUser.userId)); + const removeUserIds = userIds.filter((userId) => !userIdsInGroups.includes(userId)); + + if (removeUserIds.length > 0) { + await this.userService.removeSecondarySchoolFromUsers(removeUserIds, schoolId); + } + } } From e60904cac0c9f436da9ffb010d6763b4434891dc Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 11:49:23 +0100 Subject: [PATCH 4/8] add guestrole when user is added to a room --- .../modules/room-membership/service/room-membership.service.ts | 3 +++ 1 file changed, 3 insertions(+) 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 e6529c1d6df..59fa6167c6d 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 @@ -92,6 +92,9 @@ export class RoomMembershipService { await this.groupService.addUsersToGroup(roomMembership.userGroupId, userIdsAndRoles); + const userIds = userIdsAndRoles.map((user) => user.userId); + await this.userService.addSecondarySchoolToUsers(userIds, roomMembership.schoolId); + return roomMembership.id; } From f6a1c1e068fa8bbdd773d13c44eb0034676221a0 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 11:49:32 +0100 Subject: [PATCH 5/8] chore: fix tests --- .../service/room-membership.service.spec.ts | 87 ++++++++----------- 1 file changed, 36 insertions(+), 51 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 d56fc85218e..76d85959329 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 @@ -121,9 +121,14 @@ describe('RoomMembershipService', () => { describe('when roomMembership exists', () => { const setup = () => { const user = userFactory.buildWithId(); - const group = groupFactory.build({ type: GroupTypes.ROOM }); - const room = roomFactory.build(); - const roomMembership = roomMembershipFactory.build({ roomId: room.id, userGroupId: group.id }); + const school = schoolFactory.build(); + const group = groupFactory.build({ type: GroupTypes.ROOM, organizationId: school.id }); + const room = roomFactory.build({ schoolId: school.id }); + const roomMembership = roomMembershipFactory.build({ + roomId: room.id, + userGroupId: group.id, + schoolId: school.id, + }); roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); @@ -144,6 +149,14 @@ describe('RoomMembershipService', () => { { userId: user.id, roleName: RoleName.ROOMEDITOR }, ]); }); + + it('should add user to school', async () => { + const { user, room } = setup(); + + await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]); + + expect(userService.addSecondarySchoolToUsers).toHaveBeenCalledWith([user.id], room.schoolId); + }); }); }); @@ -230,6 +243,24 @@ describe('RoomMembershipService', () => { groupService.findGroups.mockResolvedValue({ total: groups.length, data: groups }); }; + it('should pass the schoolId of the room', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([]); + + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + + expect(groupService.findGroups).toHaveBeenCalledWith(expect.objectContaining({ schoolId: secondarySchool.id })); + }); + describe('when after removal: user is not in any room of that secondary school', () => { it('should remove user from secondary school', async () => { const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); @@ -246,12 +277,11 @@ describe('RoomMembershipService', () => { await service.removeMembersFromRoom(room.id, [externalUser.id as string]); expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith([externalUser.id], secondarySchool.id); - expect(1).toEqual(1); }); }); describe('when after removal: user is still in a room of that secondary school', () => { - it('should remove user from secondary school', async () => { + it('should not remove user from secondary school', async () => { const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); @@ -264,58 +294,13 @@ describe('RoomMembershipService', () => { roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); groupService.findById.mockResolvedValue(group); groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([group, group2]); + mockGroupsAtSchoolAfterRemoval([group2]); await service.removeMembersFromRoom(room.id, [externalUser.id as string]); expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); }); }); - - describe('when after removal: user is still in a room of a different school', () => { - it('should not remove user from secondary school', async () => { - const { secondarySchool, externalUser, otherSchool } = setupUserWithSecondarySchool(); - - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - const { group: group2 } = setupGroupAndRoom(otherSchool.id); - group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([]); - - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); - - expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalled(); - }); - }); - - // describe('when user is removed from last room of primary school', () => { - // it('should remove user from primary school', async () => { - // const primarySchool = schoolEntityFactory.buildWithId(); - // const user = userFactory.buildWithId({ school: primarySchool }); - // const room = roomFactory.build({ schoolId: primarySchool.id }); - // const group = groupFactory.build({ type: GroupTypes.ROOM }); - // const roomMembership = roomMembershipFactory.build({ - // roomId: room.id, - // userGroupId: group.id, - // schoolId: primarySchool.id, - // }); - - // roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - // groupService.findById.mockResolvedValue(group); - // groupService.removeUsersFromGroup.mockResolvedValue(group); - // mockGroupsAtSchoolAfterRemoval([]); - - // await service.removeMembersFromRoom(room.id, [user.id]); - - // expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); - // }); - // }); }); describe('deleteRoomMembership', () => { From db187e885a652845e92f6844f978e2d916573760 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 12:04:28 +0100 Subject: [PATCH 6/8] chore: clean up following boy scout rule --- .../src/modules/user/service/user.service.ts | 47 ++++++++++--------- apps/server/src/shared/repo/user/user.repo.ts | 6 +-- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index ab7c47aec1a..811cedbfea8 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -56,14 +56,14 @@ export class UserService implements DeletionService, IEventHandler { + public async getUserEntityWithRoles(userId: EntityId): Promise { // only roles required, no need for the other populates const userWithRoles = await this.userRepo.findById(userId, true); return userWithRoles; } - async me(userId: EntityId): Promise<[User, string[]]> { + public async me(userId: EntityId): Promise<[User, string[]]> { const user = await this.userRepo.findById(userId, true); const permissions = user.resolvePermissions(); @@ -73,20 +73,20 @@ export class UserService implements DeletionService, IEventHandler { + public async getUser(id: string): Promise { const userEntity = await this.userRepo.findById(id, true); const userDto = UserMapper.mapFromEntityToDto(userEntity); return userDto; } - async findById(id: string): Promise { + public async findById(id: string): Promise { const userDO = await this.userDORepo.findById(id, true); return userDO; } - async findByIds(ids: string[]): Promise { + public async findByIds(ids: string[]): Promise { const userDOs = await this.userDORepo.findByIds(ids, true); return userDOs; @@ -98,25 +98,25 @@ export class UserService implements DeletionService, IEventHandler { - const savedUser: Promise = this.userDORepo.save(user); + public async save(user: UserDO): Promise { + const savedUser = await this.userDORepo.save(user); return savedUser; } - async saveAll(users: UserDO[]): Promise { - const savedUsers: Promise = this.userDORepo.saveAll(users); + public async saveAll(users: UserDO[]): Promise { + const savedUsers = await this.userDORepo.saveAll(users); return savedUsers; } - async findUsers(query: UserQuery, options?: IFindOptions): Promise> { + public async findUsers(query: UserQuery, options?: IFindOptions): Promise> { const users: Page = await this.userDORepo.find(query, options); return users; } - async findBySchoolRole( + public async findBySchoolRole( schoolId: EntityId, roleName: RoleName, options?: IFindOptions @@ -127,7 +127,7 @@ export class UserService implements DeletionService, IEventHandler): Promise> { + public async findPublicTeachersBySchool(schoolId: EntityId, options?: IFindOptions): Promise> { const discoverabilitySetting = this.configService.get('TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION'); if (discoverabilitySetting === 'disabled') { return new Page([], 0); @@ -147,7 +147,7 @@ export class UserService implements DeletionService, IEventHandler { + public async addSecondarySchoolToUsers(userIds: string[], schoolId: EntityId): Promise { const users = await this.userDORepo.findByIds(userIds, true); const guestStudent = await this.roleService.findByName(RoleName.GUESTSTUDENT); const guestTeacher = await this.roleService.findByName(RoleName.GUESTTEACHER); @@ -185,19 +185,19 @@ export class UserService implements DeletionService, IEventHandler { - const user: Promise = this.userDORepo.findByExternalId(externalId, systemId); + public async findByExternalId(externalId: string, systemId: EntityId): Promise { + const user = await this.userDORepo.findByExternalId(externalId, systemId); return user; } - async findByEmail(email: string): Promise { - const user: Promise = this.userDORepo.findByEmail(email); + public async findByEmail(email: string): Promise { + const user = await this.userDORepo.findByEmail(email); return user; } - async getDisplayName(user: UserDO): Promise { + public async getDisplayName(user: UserDO): Promise { const protectedRoles: RoleDto[] = await this.roleService.getProtectedRoles(); const isProtectedUser: boolean = user.roles.some( (roleRef: RoleReference): boolean => @@ -209,7 +209,7 @@ export class UserService implements DeletionService, IEventHandler { + public async patchLanguage(userId: EntityId, newLanguage: LanguageType): Promise { this.checkAvailableLanguages(newLanguage); const user = await this.userRepo.findById(userId); user.language = newLanguage; @@ -281,7 +281,7 @@ export class UserService implements DeletionService, IEventHandler { - const parentEmails = this.userRepo.getParentEmailsFromUser(userId); + const parentEmails = await this.userRepo.getParentEmailsFromUser(userId); return parentEmails; } @@ -293,7 +293,9 @@ export class UserService implements DeletionService, IEventHandler { - return this.userRepo.findByExternalIds(externalIds); + const userIds = await this.userRepo.findByExternalIds(externalIds); + + return userIds; } public async updateLastSyncedAt(userIds: string[]): Promise { @@ -326,7 +328,8 @@ export class UserService implements DeletionService, IEventHandler { diff --git a/apps/server/src/shared/repo/user/user.repo.ts b/apps/server/src/shared/repo/user/user.repo.ts index 126dbabd99a..7b6fee3bf4e 100644 --- a/apps/server/src/shared/repo/user/user.repo.ts +++ b/apps/server/src/shared/repo/user/user.repo.ts @@ -121,16 +121,16 @@ export class UserRepo extends BaseRepo { return users; } - public async findByExternalIds(externalIds: string[]): Promise { + public async findByExternalIds(externalIds: EntityId[]): Promise { const foundUsers = await this._em.find( User, { externalId: { $in: externalIds } }, { fields: ['id', 'externalId'] } ); - const users = foundUsers.map(({ id }) => id); + const userIds = foundUsers.map(({ id }) => id); - return users; + return userIds; } public async updateAllUserByLastSyncedAt(userIds: string[]): Promise { From ea539f98aec480dc1a5b1f54a94ea2ab4e257549 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 15:26:42 +0100 Subject: [PATCH 7/8] fix module import and tests --- .../modules/room-membership/room-membership.module.ts | 3 ++- .../modules/room/api/test/room-add-members.api.spec.ts | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/room-membership/room-membership.module.ts b/apps/server/src/modules/room-membership/room-membership.module.ts index a261d8ea475..1015fd7b9a8 100644 --- a/apps/server/src/modules/room-membership/room-membership.module.ts +++ b/apps/server/src/modules/room-membership/room-membership.module.ts @@ -4,12 +4,13 @@ import { CqrsModule } from '@nestjs/cqrs'; import { AuthorizationModule } from '../authorization'; import { RoleModule } from '../role'; import { RoomModule } from '../room/room.module'; +import { UserModule } from '../user'; import { RoomMembershipRule } from './authorization/room-membership.rule'; import { RoomMembershipRepo } from './repo/room-membership.repo'; import { RoomMembershipService } from './service/room-membership.service'; @Module({ - imports: [AuthorizationModule, CqrsModule, GroupModule, RoleModule, RoomModule], + imports: [AuthorizationModule, CqrsModule, GroupModule, RoleModule, RoomModule, UserModule], providers: [RoomMembershipService, RoomMembershipRepo, RoomMembershipRule], exports: [RoomMembershipService], }) 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 5fab1cced7e..a1ed8579853 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 @@ -9,6 +9,7 @@ import { cleanupCollections, groupEntityFactory, roleFactory, + schoolEntityFactory, } from '@shared/testing'; import { GroupEntityTypes } from '@modules/group/entity/group.entity'; import { roomMembershipEntityFactory } from '@src/modules/room-membership/testing/room-membership-entity.factory'; @@ -45,10 +46,13 @@ describe('Room Controller (API)', () => { describe('PATCH /rooms/:roomId/members/add', () => { const setupRoomWithMembers = async () => { - const room = roomEntityFactory.buildWithId(); - const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const school = schoolEntityFactory.buildWithId(); + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); const { teacherAccount: otherTeacherAccount, teacherUser: otherTeacherUser } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); + const room = roomEntityFactory.buildWithId({ schoolId: teacherUser.school.id }); + const teacherGuestRole = roleFactory.buildWithId({ name: RoleName.GUESTTEACHER }); + const studentGuestRole = roleFactory.buildWithId({ name: RoleName.GUESTSTUDENT }); const role = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, permissions: [Permission.ROOM_VIEW, Permission.ROOM_EDIT], @@ -67,6 +71,8 @@ describe('Room Controller (API)', () => { roomMemberships, teacherAccount, teacherUser, + teacherGuestRole, + studentGuestRole, otherTeacherUser, otherTeacherAccount, userGroupEntity, From c7364db147ae4f8b79d37b29206bc213bd06f225 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 5 Dec 2024 16:11:45 +0100 Subject: [PATCH 8/8] chore: change spacing --- .../room-membership/service/room-membership.service.spec.ts | 2 ++ 1 file changed, 2 insertions(+) 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 76d85959329..6f6b39139b2 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 @@ -225,6 +225,7 @@ describe('RoomMembershipService', () => { roles: [role], secondarySchools: [{ schoolId: secondarySchool.id, role: new RoleDto(guestTeacher) }], }); + return { secondarySchool, externalUser, otherSchool }; }; @@ -236,6 +237,7 @@ describe('RoomMembershipService', () => { userGroupId: group.id, schoolId, }); + return { group, room, roomMembership }; };