diff --git a/apps/server/src/modules/group/group-api.module.ts b/apps/server/src/modules/group/group-api.module.ts index 14a564b4741..1f2de66c02a 100644 --- a/apps/server/src/modules/group/group-api.module.ts +++ b/apps/server/src/modules/group/group-api.module.ts @@ -5,12 +5,22 @@ import { RoleModule } from '@modules/role'; import { LegacySchoolModule } from '@modules/legacy-school'; import { SystemModule } from '@modules/system'; import { UserModule } from '@modules/user'; +import { LoggerModule } from '@src/core/logger'; import { GroupController } from './controller'; import { GroupModule } from './group.module'; import { GroupUc } from './uc'; @Module({ - imports: [GroupModule, ClassModule, UserModule, RoleModule, LegacySchoolModule, AuthorizationModule, SystemModule], + imports: [ + GroupModule, + ClassModule, + UserModule, + RoleModule, + LegacySchoolModule, + AuthorizationModule, + SystemModule, + LoggerModule, + ], controllers: [GroupController], providers: [GroupUc], }) diff --git a/apps/server/src/modules/group/uc/group.uc.spec.ts b/apps/server/src/modules/group/uc/group.uc.spec.ts index 0a00d74a15c..03468c092f3 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -25,6 +25,8 @@ import { userDoFactory, userFactory, } from '@shared/testing'; +import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; +import { Logger } from '@src/core/logger'; import { SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; @@ -45,6 +47,7 @@ describe('GroupUc', () => { let schoolService: DeepMocked; let authorizationService: DeepMocked; let schoolYearService: DeepMocked; + let logger: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -82,6 +85,10 @@ describe('GroupUc', () => { provide: SchoolYearService, useValue: createMock(), }, + { + provide: Logger, + useValue: createMock(), + }, ], }).compile(); @@ -94,6 +101,7 @@ describe('GroupUc', () => { schoolService = module.get(LegacySchoolService); authorizationService = module.get(AuthorizationService); schoolYearService = module.get(SchoolYearService); + logger = module.get(Logger); await setupEntities(); }); @@ -217,6 +225,17 @@ describe('GroupUc', () => { throw new Error(); }); + userService.findByIdOrNull.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === studentUser.id) { + return Promise.resolve(studentUserDo); + } + + throw new Error(); + }); roleService.findById.mockImplementation((roleId: string): Promise => { if (roleId === teacherUser.roles[0].id) { return Promise.resolve(teacherRole); @@ -567,6 +586,21 @@ describe('GroupUc', () => { throw new Error(); }); + userService.findByIdOrNull.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === studentUser.id) { + return Promise.resolve(studentUserDo); + } + + if (userId === adminUser.id) { + return Promise.resolve(adminUserDo); + } + + throw new Error(); + }); roleService.findById.mockImplementation((roleId: string): Promise => { if (roleId === teacherUser.roles[0].id) { return Promise.resolve(teacherRole); @@ -733,6 +767,130 @@ describe('GroupUc', () => { }); }); }); + + describe('when class has a user referenced which is not existing', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const notFoundReferenceId = new ObjectId().toHexString(); + const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); + + const teacherRole: RoleDto = roleDtoFactory.buildWithId({ + id: teacherUser.roles[0].id, + name: teacherUser.roles[0].name, + }); + + const teacherUserDo: UserDO = userDoFactory.buildWithId({ + id: teacherUser.id, + lastName: teacherUser.lastName, + roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }], + }); + + const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const clazz: Class = classFactory.build({ + name: 'A', + teacherIds: [teacherUser.id, notFoundReferenceId], + source: 'LDAP', + year: schoolYear.id, + }); + const system: SystemDto = new SystemDto({ + id: new ObjectId().toHexString(), + displayName: 'External System', + type: 'oauth2', + }); + const group: Group = groupFactory.build({ + name: 'B', + users: [ + { userId: teacherUser.id, roleId: teacherUser.roles[0].id }, + { userId: notFoundReferenceId, roleId: teacherUser.roles[0].id }, + ], + externalSource: undefined, + }); + + schoolService.getSchoolById.mockResolvedValueOnce(school); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); + classService.findAllByUserId.mockResolvedValueOnce([clazz]); + groupService.findByUser.mockResolvedValueOnce([group]); + systemService.findById.mockResolvedValue(system); + + userService.findById.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + throw new Error(); + }); + userService.findByIdOrNull.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === notFoundReferenceId) { + return Promise.resolve(null); + } + + throw new Error(); + }); + roleService.findById.mockImplementation((roleId: string): Promise => { + if (roleId === teacherUser.roles[0].id) { + return Promise.resolve(teacherRole); + } + + throw new Error(); + }); + schoolYearService.findById.mockResolvedValue(schoolYear); + + return { + teacherUser, + clazz, + group, + notFoundReferenceId, + schoolYear, + }; + }; + + it('should return class without missing user', async () => { + const { teacherUser, clazz, group, schoolYear } = setup(); + + const result = await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + studentCount: 2, + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + studentCount: 0, + }, + ], + total: 2, + }); + }); + + it('should log the missing user', async () => { + const { teacherUser, clazz, group, notFoundReferenceId } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + + expect(logger.warning).toHaveBeenCalledWith( + new ReferencedEntityNotFoundLoggable(Class.name, clazz.id, UserDO.name, notFoundReferenceId) + ); + expect(logger.warning).toHaveBeenCalledWith( + new ReferencedEntityNotFoundLoggable(Group.name, group.id, UserDO.name, notFoundReferenceId) + ); + }); + }); }); describe('getGroup', () => { @@ -817,8 +975,10 @@ describe('GroupUc', () => { groupService.findById.mockResolvedValueOnce(group); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); userService.findById.mockResolvedValueOnce(teacherUserDo); + userService.findByIdOrNull.mockResolvedValueOnce(teacherUserDo); roleService.findById.mockResolvedValueOnce(teacherRole); userService.findById.mockResolvedValueOnce(studentUserDo); + userService.findByIdOrNull.mockResolvedValueOnce(studentUserDo); roleService.findById.mockResolvedValueOnce(studentRole); return { diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 150e951b129..637a9b97f7b 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -4,13 +4,15 @@ import { Class } from '@modules/class/domain'; import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { LegacySystemService, SystemDto } from '@modules/system'; import { UserService } from '@modules/user'; import { Injectable } from '@nestjs/common'; +import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; import { LegacySchoolDo, Page, UserDO } from '@shared/domain/domainobject'; import { SchoolYearEntity, User } from '@shared/domain/entity'; import { Permission, SortOrder } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; +import { Logger } from '@src/core/logger'; +import { LegacySystemService, SystemDto } from '@src/modules/system'; import { SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupUser } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; @@ -29,7 +31,8 @@ export class GroupUc { private readonly roleService: RoleService, private readonly schoolService: LegacySchoolService, private readonly authorizationService: AuthorizationService, - private readonly schoolYearService: SchoolYearService + private readonly schoolYearService: SchoolYearService, + private readonly logger: Logger ) {} public async findAllClasses( @@ -143,14 +146,14 @@ export class GroupUc { this.isClassOfQueryType(currentYear, classWithSchoolYear.schoolYear, schoolYearQueryType) ); - const classInfosFromClasses = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear); + const classInfosFromClasses: ClassInfoDto[] = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear); return classInfosFromClasses; } private async addSchoolYearsToClasses(classes: Class[]): Promise<{ clazz: Class; schoolYear?: SchoolYearEntity }[]> { const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await Promise.all( - classes.map(async (clazz) => { + classes.map(async (clazz: Class) => { let schoolYear: SchoolYearEntity | undefined; if (clazz.year) { schoolYear = await this.schoolYearService.findById(clazz.year); @@ -162,6 +165,7 @@ export class GroupUc { }; }) ); + return classesWithSchoolYear; } @@ -193,11 +197,10 @@ export class GroupUc { private async mapClassInfosFromClasses( filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] ): Promise { - const classInfosFromClasses = await Promise.all( + const classInfosFromClasses: ClassInfoDto[] = await Promise.all( filteredClassesForSchoolYear.map(async (classWithSchoolYear): Promise => { - const teachers: UserDO[] = await Promise.all( - classWithSchoolYear.clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) - ); + const { teacherIds } = classWithSchoolYear.clazz; + const teachers: UserDO[] = await this.getTeachersByIds(teacherIds, classWithSchoolYear.clazz.id); const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto( classWithSchoolYear.clazz, @@ -208,9 +211,28 @@ export class GroupUc { return mapped; }) ); + return classInfosFromClasses; } + private async getTeachersByIds(teacherIds: EntityId[], classId: EntityId): Promise { + const teacherPromises: Promise[] = teacherIds.map( + async (teacherId: EntityId): Promise => { + const teacher: UserDO | null = await this.userService.findByIdOrNull(teacherId); + if (!teacher) { + this.logger.warning(new ReferencedEntityNotFoundLoggable(Class.name, classId, UserDO.name, teacherId)); + } + return teacher; + } + ); + + const teachers: UserDO[] = (await Promise.all(teacherPromises)).filter( + (teacher: UserDO | null): teacher is UserDO => teacher !== null + ); + + return teachers; + } + private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise { const groupsOfTypeClass: Group[] = await this.groupService.findClassesForSchool(schoolId); @@ -271,20 +293,33 @@ export class GroupUc { } private async findUsersForGroup(group: Group): Promise { - const resolvedGroupUsers: ResolvedGroupUser[] = await Promise.all( - group.users.map(async (groupUser: GroupUser): Promise => { - const user: UserDO = await this.userService.findById(groupUser.userId); - const role: RoleDto = await this.roleService.findById(groupUser.roleId); - - const resolvedGroups = new ResolvedGroupUser({ - user, - role, - }); + const resolvedGroupUsersOrNull: (ResolvedGroupUser | null)[] = await Promise.all( + group.users.map(async (groupUser: GroupUser): Promise => { + const user: UserDO | null = await this.userService.findByIdOrNull(groupUser.userId); + let resolvedGroup: ResolvedGroupUser | null = null; + + if (!user) { + this.logger.warning( + new ReferencedEntityNotFoundLoggable(Group.name, group.id, UserDO.name, groupUser.userId) + ); + } else { + const role: RoleDto = await this.roleService.findById(groupUser.roleId); + + resolvedGroup = new ResolvedGroupUser({ + user, + role, + }); + } - return resolvedGroups; + return resolvedGroup; }) ); + const resolvedGroupUsers: ResolvedGroupUser[] = resolvedGroupUsersOrNull.filter( + (resolvedGroupUser: ResolvedGroupUser | null): resolvedGroupUser is ResolvedGroupUser => + resolvedGroupUser !== null + ); + return resolvedGroupUsers; } diff --git a/apps/server/src/modules/user/service/user.service.spec.ts b/apps/server/src/modules/user/service/user.service.spec.ts index ecee10025a7..c7223c45322 100644 --- a/apps/server/src/modules/user/service/user.service.spec.ts +++ b/apps/server/src/modules/user/service/user.service.spec.ts @@ -12,6 +12,7 @@ import { EntityId } from '@shared/domain/types'; import { UserRepo } from '@shared/repo'; import { UserDORepo } from '@shared/repo/user/user-do.repo'; import { roleFactory, setupEntities, userDoFactory, userFactory } from '@shared/testing'; +import { ObjectId } from '@mikro-orm/mongodb'; import { UserDto } from '../uc/dto/user.dto'; import { UserQuery } from './user-query.type'; import { UserService } from './user.service'; @@ -135,6 +136,48 @@ describe('UserService', () => { }); }); + describe('findByIdOrNull', () => { + describe('when a user with this id exists', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const user: UserDO = userDoFactory.buildWithId({ id: userId }); + + userDORepo.findByIdOrNull.mockResolvedValue(user); + + return { + user, + userId, + }; + }; + + it('should return the user', async () => { + const { user, userId } = setup(); + + const result: UserDO | null = await service.findByIdOrNull(userId); + + expect(result).toEqual(user); + }); + }); + + describe('when a user with this id does not exist', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + + userDORepo.findByIdOrNull.mockResolvedValue(null); + + return { userId }; + }; + + it('should return null', async () => { + const { userId } = setup(); + + const result: UserDO | null = await service.findByIdOrNull(userId); + + expect(result).toBeNull(); + }); + }); + }); + describe('getResolvedUser is called', () => { describe('when a resolved user is requested', () => { const setup = () => { diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index a35de214447..4e8f2fe2394 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -60,6 +60,12 @@ export class UserService { return userDO; } + public async findByIdOrNull(id: string): Promise { + const userDO: UserDO | null = await this.userDORepo.findByIdOrNull(id, true); + + return userDO; + } + async save(user: UserDO): Promise { const savedUser: Promise = this.userDORepo.save(user); diff --git a/apps/server/src/shared/common/loggable/index.ts b/apps/server/src/shared/common/loggable/index.ts new file mode 100644 index 00000000000..5f21a462625 --- /dev/null +++ b/apps/server/src/shared/common/loggable/index.ts @@ -0,0 +1 @@ +export { ReferencedEntityNotFoundLoggable } from './referenced-entity-not-found-loggable'; diff --git a/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.spec.ts b/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.spec.ts new file mode 100644 index 00000000000..f58e29f0db8 --- /dev/null +++ b/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.spec.ts @@ -0,0 +1,43 @@ +import { ObjectId } from 'bson'; +import { ReferencedEntityNotFoundLoggable } from './referenced-entity-not-found-loggable'; +import { EntityId } from '../../domain/types'; + +describe(ReferencedEntityNotFoundLoggable.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const sourceEntityName = 'sourceEntityName'; + const sourceEntityId: EntityId = new ObjectId().toHexString(); + const referencedEntityName = 'referencedEntityName'; + const referencedEntityId: EntityId = new ObjectId().toHexString(); + + const loggable: ReferencedEntityNotFoundLoggable = new ReferencedEntityNotFoundLoggable( + sourceEntityName, + sourceEntityId, + referencedEntityName, + referencedEntityId + ); + + return { + loggable, + referencedEntityName, + referencedEntityId, + sourceEntityName, + sourceEntityId, + }; + }; + + it('should return the correct log message', () => { + const { loggable, referencedEntityName, referencedEntityId, sourceEntityName, sourceEntityId } = setup(); + + expect(loggable.getLogMessage()).toEqual({ + message: 'The requested entity could not been found, but it is still referenced.', + data: { + referencedEntityName, + referencedEntityId, + sourceEntityName, + sourceEntityId, + }, + }); + }); + }); +}); diff --git a/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.ts b/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.ts new file mode 100644 index 00000000000..10031eeb67e --- /dev/null +++ b/apps/server/src/shared/common/loggable/referenced-entity-not-found-loggable.ts @@ -0,0 +1,23 @@ +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { EntityId } from '../../domain/types'; + +export class ReferencedEntityNotFoundLoggable implements Loggable { + constructor( + private readonly sourceEntityName: string, + private readonly sourceEntityId: EntityId, + private readonly referencedEntityName: string, + private readonly referencedEntityId: EntityId + ) {} + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + message: 'The requested entity could not been found, but it is still referenced.', + data: { + referencedEntityName: this.referencedEntityName, + referencedEntityId: this.referencedEntityId, + sourceEntityName: this.sourceEntityName, + sourceEntityId: this.sourceEntityId, + }, + }; + } +} diff --git a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts index 96e0ed8bafa..291e687f00c 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.integration.spec.ts @@ -522,4 +522,49 @@ describe('UserRepo', () => { }); }); }); + + describe('findByIdOrNull', () => { + describe('when user not found', () => { + const setup = () => { + const id = new ObjectId().toHexString(); + + return { id }; + }; + + it('should return null', async () => { + const { id } = setup(); + + const result: UserDO | null = await repo.findByIdOrNull(id); + + expect(result).toBeNull(); + }); + }); + + describe('when user was found', () => { + const setup = async () => { + const role: Role = roleFactory.build(); + + const user: User = userFactory.buildWithId({ roles: [role] }); + + await em.persistAndFlush([user, role]); + em.clear(); + + return { user, role }; + }; + + it('should return user with role', async () => { + const { user, role } = await setup(); + + const result: UserDO | null = await repo.findByIdOrNull(user.id, true); + + expect(result?.id).toEqual(user.id); + expect(result?.roles).toEqual([ + { + id: role.id, + name: role.name, + }, + ]); + }); + }); + }); }); diff --git a/apps/server/src/shared/repo/user/user-do.repo.ts b/apps/server/src/shared/repo/user/user-do.repo.ts index 1603563ec69..c2185ffbd5e 100644 --- a/apps/server/src/shared/repo/user/user-do.repo.ts +++ b/apps/server/src/shared/repo/user/user-do.repo.ts @@ -58,6 +58,23 @@ export class UserDORepo extends BaseDORepo { return this.mapEntityToDO(userEntity); } + async findByIdOrNull(id: EntityId, populate = false): Promise { + const user: User | null = await this._em.findOne(this.entityName, id as FilterQuery); + + if (!user) { + return null; + } + + if (populate) { + await this._em.populate(user, ['roles', 'school.systems', 'school.schoolYear']); + await this.populateRoles(user.roles.getItems()); + } + + const domainObject: UserDO = this.mapEntityToDO(user); + + return domainObject; + } + async findByExternalIdOrFail(externalId: string, systemId: string): Promise { const userDo: UserDO | null = await this.findByExternalId(externalId, systemId); if (userDo) {