From 5a6fabf28bba59dd62127b229b581b4077658ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Tue, 16 Jan 2024 10:15:36 +0100 Subject: [PATCH 1/3] add query param to turn of loading of users --- .../dto/request/class-filter-params.ts | 7 +- .../group/controller/group.controller.ts | 7 +- apps/server/src/modules/group/uc/group.uc.ts | 79 +++++++++++++------ 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts index d6a7e3ba62f..5a3aca6e218 100644 --- a/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts +++ b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts @@ -1,5 +1,5 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsEnum, IsOptional } from 'class-validator'; +import { IsBoolean, IsEnum, IsOptional } from 'class-validator'; import { SchoolYearQueryType } from '../interface'; export class ClassFilterParams { @@ -7,4 +7,9 @@ export class ClassFilterParams { @IsEnum(SchoolYearQueryType) @ApiPropertyOptional({ enum: SchoolYearQueryType, enumName: 'SchoolYearQueryType' }) type?: SchoolYearQueryType; + + @IsOptional() + @IsBoolean() + @ApiPropertyOptional({ default: false }) + loadUsers?: boolean; } diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index e5df1dec514..d4116a9bc4e 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -6,13 +6,13 @@ import { ErrorResponse } from '@src/core/error/dto'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; import { + ClassCallerParams, ClassFilterParams, ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, - GroupResponse, GroupPaginationParams, - ClassCallerParams, + GroupResponse, } from './dto'; import { GroupResponseMapper } from './mapper'; @@ -42,7 +42,8 @@ export class GroupController { pagination.skip, pagination.limit, sortingQuery.sortBy, - sortingQuery.sortOrder + sortingQuery.sortOrder, + filterParams.loadUsers ); const response: ClassInfoSearchListResponse = GroupResponseMapper.mapToClassInfosToListResponse( diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 18bb1c5e466..404cb0daa73 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -4,6 +4,7 @@ import { Class } from '@modules/class/domain'; import { SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; +import { School, SchoolService } from '@modules/school/domain'; import { UserService } from '@modules/user'; import { Injectable } from '@nestjs/common'; import { SortHelper } from '@shared/common'; @@ -14,7 +15,6 @@ 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 { School, SchoolService } from '@modules/school/domain'; import { ClassRequestContext, SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes, GroupUser } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; @@ -46,7 +46,8 @@ export class GroupUc { skip = 0, limit?: number, sortBy: keyof ClassInfoDto = 'name', - sortOrder: SortOrder = SortOrder.asc + sortOrder: SortOrder = SortOrder.asc, + loadUsers?: boolean ): Promise> { const school: School = await this.schoolService.getSchoolById(schoolId); @@ -67,9 +68,9 @@ export class GroupUc { let combinedClassInfo: ClassInfoDto[]; if (canSeeFullList || calledFromCourse) { - combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType); + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType, loadUsers); } else { - combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType); + combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType, loadUsers); } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => @@ -85,14 +86,15 @@ export class GroupUc { private async findCombinedClassListForSchool( schoolId: EntityId, - schoolYearQueryType?: SchoolYearQueryType + schoolYearQueryType?: SchoolYearQueryType, + loadUsers?: boolean ): Promise { let classInfosFromGroups: ClassInfoDto[] = []; - const classInfosFromClasses = await this.findClassesForSchool(schoolId, schoolYearQueryType); + const classInfosFromClasses = await this.findClassesForSchool(schoolId, schoolYearQueryType, loadUsers); if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { - classInfosFromGroups = await this.findGroupsForSchool(schoolId); + classInfosFromGroups = await this.findGroupsForSchool(schoolId, loadUsers); } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; @@ -102,14 +104,15 @@ export class GroupUc { private async findCombinedClassListForUser( userId: EntityId, - schoolYearQueryType?: SchoolYearQueryType + schoolYearQueryType?: SchoolYearQueryType, + loadUsers?: boolean ): Promise { let classInfosFromGroups: ClassInfoDto[] = []; - const classInfosFromClasses = await this.findClassesForUser(userId, schoolYearQueryType); + const classInfosFromClasses = await this.findClassesForUser(userId, schoolYearQueryType, loadUsers); if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { - classInfosFromGroups = await this.findGroupsForUser(userId); + classInfosFromGroups = await this.findGroupsForUser(userId, loadUsers); } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; @@ -119,29 +122,40 @@ export class GroupUc { private async findClassesForSchool( schoolId: EntityId, - schoolYearQueryType?: SchoolYearQueryType + schoolYearQueryType?: SchoolYearQueryType, + loadUsers?: boolean ): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); - const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses( + classes, + schoolYearQueryType, + loadUsers + ); return classInfosFromClasses; } private async findClassesForUser( userId: EntityId, - schoolYearQueryType?: SchoolYearQueryType + schoolYearQueryType?: SchoolYearQueryType, + loadUsers?: boolean ): Promise { const classes: Class[] = await this.classService.findAllByUserId(userId); - const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses( + classes, + schoolYearQueryType, + loadUsers + ); return classInfosFromClasses; } private async getClassInfosFromClasses( classes: Class[], - schoolYearQueryType?: SchoolYearQueryType + schoolYearQueryType?: SchoolYearQueryType, + loadUsers?: boolean ): Promise { const currentYear: SchoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); @@ -153,7 +167,10 @@ export class GroupUc { this.isClassOfQueryType(currentYear, classWithSchoolYear.schoolYear, schoolYearQueryType) ); - const classInfosFromClasses: ClassInfoDto[] = await this.mapClassInfosFromClasses(filteredClassesForSchoolYear); + const classInfosFromClasses: ClassInfoDto[] = await this.mapClassInfosFromClasses( + filteredClassesForSchoolYear, + loadUsers + ); return classInfosFromClasses; } @@ -202,12 +219,15 @@ export class GroupUc { } private async mapClassInfosFromClasses( - filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] + filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[], + loadUsers?: boolean ): Promise { const classInfosFromClasses: ClassInfoDto[] = await Promise.all( filteredClassesForSchoolYear.map(async (classWithSchoolYear): Promise => { - const { teacherIds } = classWithSchoolYear.clazz; - const teachers: UserDO[] = await this.getTeachersByIds(teacherIds, classWithSchoolYear.clazz.id); + let teachers: UserDO[] = []; + if (loadUsers) { + teachers = await this.getTeachersByIds(classWithSchoolYear.clazz.teacherIds, classWithSchoolYear.clazz.id); + } const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto( classWithSchoolYear.clazz, @@ -240,7 +260,7 @@ export class GroupUc { return teachers; } - private async findGroupsForSchool(schoolId: EntityId): Promise { + private async findGroupsForSchool(schoolId: EntityId, loadUsers?: boolean): Promise { const groups: Group[] = await this.groupService.findGroupsBySchoolIdAndGroupTypes( schoolId, this.ALLOWED_GROUP_TYPES @@ -249,13 +269,13 @@ export class GroupUc { const systemMap: Map = await this.findSystemNamesForGroups(groups); const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groups.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) + groups.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap, loadUsers)) ); return classInfosFromGroups; } - private async findGroupsForUser(userId: EntityId): Promise { + private async findGroupsForUser(userId: EntityId, loadUsers?: boolean): Promise { const user: UserDO = await this.userService.findById(userId); const groupsOfTypeClass: Group[] = await this.groupService.findGroupsByUserAndGroupTypes( @@ -266,19 +286,28 @@ export class GroupUc { const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) + groupsOfTypeClass.map( + async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap, loadUsers) + ) ); return classInfosFromGroups; } - private async getClassInfoFromGroup(group: Group, systemMap: Map): Promise { + private async getClassInfoFromGroup( + group: Group, + systemMap: Map, + loadUsers?: boolean + ): Promise { let system: SystemDto | undefined; if (group.externalSource) { system = systemMap.get(group.externalSource.systemId); } - const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + let resolvedUsers: ResolvedGroupUser[] = []; + if (loadUsers) { + resolvedUsers = await this.findUsersForGroup(group); + } const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); From 261b60acc2c3ef141693eae0424b41a415f39b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Tue, 16 Jan 2024 16:58:03 +0100 Subject: [PATCH 2/3] don't return users in groups --- .../controller/api-test/group.api.spec.ts | 8 +- .../dto/request/class-filter-params.ts | 7 +- .../group/controller/group.controller.ts | 3 +- .../src/modules/group/uc/group.uc.spec.ts | 72 +++++------ apps/server/src/modules/group/uc/group.uc.ts | 121 +++++------------- 5 files changed, 70 insertions(+), 141 deletions(-) diff --git a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts index 32a9740cd7d..ff60d7e625c 100644 --- a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts +++ b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts @@ -7,13 +7,13 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Role, SchoolEntity, SchoolYearEntity, SystemEntity, User } from '@shared/domain/entity'; import { RoleName, SortOrder } from '@shared/domain/interface'; import { - TestApiClient, - UserAndAccountTestFactory, groupEntityFactory, roleFactory, schoolFactory, schoolYearFactory, systemEntityFactory, + TestApiClient, + UserAndAccountTestFactory, userFactory, } from '@shared/testing'; import { ObjectId } from 'bson'; @@ -121,14 +121,14 @@ describe('Group (API)', () => { type: ClassRootType.GROUP, name: group.name, externalSourceName: system.displayName, - teachers: [adminUser.lastName], + teachers: [], studentCount: 0, }, { id: clazz.id, type: ClassRootType.CLASS, name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, - teachers: [teacherUser.lastName], + teachers: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 0, diff --git a/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts index 5a3aca6e218..d6a7e3ba62f 100644 --- a/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts +++ b/apps/server/src/modules/group/controller/dto/request/class-filter-params.ts @@ -1,5 +1,5 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsBoolean, IsEnum, IsOptional } from 'class-validator'; +import { IsEnum, IsOptional } from 'class-validator'; import { SchoolYearQueryType } from '../interface'; export class ClassFilterParams { @@ -7,9 +7,4 @@ export class ClassFilterParams { @IsEnum(SchoolYearQueryType) @ApiPropertyOptional({ enum: SchoolYearQueryType, enumName: 'SchoolYearQueryType' }) type?: SchoolYearQueryType; - - @IsOptional() - @IsBoolean() - @ApiPropertyOptional({ default: false }) - loadUsers?: boolean; } diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index d4116a9bc4e..4ed85bd2755 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -42,8 +42,7 @@ export class GroupController { pagination.skip, pagination.limit, sortingQuery.sortBy, - sortingQuery.sortOrder, - filterParams.loadUsers + sortingQuery.sortOrder ); const response: ClassInfoSearchListResponse = GroupResponseMapper.mapToClassInfosToListResponse( 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 3a53d6d2f80..37101b069bb 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -7,11 +7,12 @@ import { classFactory } from '@modules/class/domain/testing/factory/class.factor import { SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; +import { School, SchoolService } from '@modules/school/domain'; +import { schoolFactory } from '@modules/school/testing'; import { LegacySystemService, SystemDto } from '@modules/system'; import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { Page, UserDO } from '@shared/domain/domainobject'; import { SchoolYearEntity, User } from '@shared/domain/entity'; @@ -27,8 +28,6 @@ import { userFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; -import { School, SchoolService } from '@modules/school/domain'; -import { schoolFactory } from '@modules/school/testing'; import { ClassRequestContext, SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; @@ -336,7 +335,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 2, @@ -348,7 +347,7 @@ describe('GroupUc', () => { : successorClass.name, type: ClassRootType.CLASS, externalSourceName: successorClass.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: nextSchoolYear.name, isUpgradable: false, studentCount: 2, @@ -360,7 +359,7 @@ describe('GroupUc', () => { : classWithoutSchoolYear.name, type: ClassRootType.CLASS, externalSourceName: classWithoutSchoolYear.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], isUpgradable: false, studentCount: 2, }, @@ -368,7 +367,7 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, { @@ -376,8 +375,8 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teacherNames: [teacherUser.lastName], - studentCount: 1, + teacherNames: [], + studentCount: 0, }, ], total: 5, @@ -420,7 +419,7 @@ describe('GroupUc', () => { : classWithoutSchoolYear.name, type: ClassRootType.CLASS, externalSourceName: classWithoutSchoolYear.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], isUpgradable: false, studentCount: 2, }, @@ -429,7 +428,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 2, @@ -439,14 +438,14 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teacherNames: [teacherUser.lastName], - studentCount: 1, + teacherNames: [], + studentCount: 0, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, ], @@ -476,7 +475,7 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, ], @@ -504,7 +503,7 @@ describe('GroupUc', () => { : successorClass.name, externalSourceName: successorClass.source, type: ClassRootType.CLASS, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: nextSchoolYear.name, isUpgradable: false, studentCount: 2, @@ -698,7 +697,7 @@ describe('GroupUc', () => { describe('when no pagination is given', () => { it('should return all classes sorted by name', async () => { - const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + const { adminUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); @@ -709,7 +708,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 2, @@ -718,7 +717,7 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, { @@ -726,8 +725,8 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teacherNames: [teacherUser.lastName], - studentCount: 1, + teacherNames: [], + studentCount: 0, }, ], total: 3, @@ -748,7 +747,7 @@ describe('GroupUc', () => { describe('when sorting by external source name in descending order', () => { it('should return all classes sorted by external source name in descending order', async () => { - const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + const { adminUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); const result: Page = await uc.findAllClasses( adminUser.id, @@ -768,7 +767,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 2, @@ -778,14 +777,14 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teacherNames: [teacherUser.lastName], - studentCount: 1, + teacherNames: [], + studentCount: 0, }, { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, ], @@ -796,7 +795,7 @@ describe('GroupUc', () => { describe('when using pagination', () => { it('should return the selected page', async () => { - const { adminUser, teacherUser, group } = setup(); + const { adminUser, group } = setup(); const result: Page = await uc.findAllClasses( adminUser.id, @@ -815,7 +814,7 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], studentCount: 0, }, ], @@ -948,7 +947,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teacherNames: [teacherUser.lastName], + teacherNames: [], schoolYear: schoolYear.name, isUpgradable: false, studentCount: 2, @@ -957,26 +956,13 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teacherNames: [teacherUser.lastName], + teacherNames: [], 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) - ); - }); }); }); diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 404cb0daa73..babed7387d2 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -3,18 +3,17 @@ import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; import { SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; -import { RoleDto } from '@modules/role/service/dto/role.dto'; import { School, SchoolService } from '@modules/school/domain'; import { UserService } from '@modules/user'; import { Injectable } from '@nestjs/common'; import { SortHelper } from '@shared/common'; -import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; import { 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 { RoleDto } from '../../role/service/dto/role.dto'; import { ClassRequestContext, SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes, GroupUser } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; @@ -46,8 +45,7 @@ export class GroupUc { skip = 0, limit?: number, sortBy: keyof ClassInfoDto = 'name', - sortOrder: SortOrder = SortOrder.asc, - loadUsers?: boolean + sortOrder: SortOrder = SortOrder.asc ): Promise> { const school: School = await this.schoolService.getSchoolById(schoolId); @@ -68,9 +66,9 @@ export class GroupUc { let combinedClassInfo: ClassInfoDto[]; if (canSeeFullList || calledFromCourse) { - combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType, loadUsers); + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType); } else { - combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType, loadUsers); + combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType); } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => @@ -86,15 +84,14 @@ export class GroupUc { private async findCombinedClassListForSchool( schoolId: EntityId, - schoolYearQueryType?: SchoolYearQueryType, - loadUsers?: boolean + schoolYearQueryType?: SchoolYearQueryType ): Promise { let classInfosFromGroups: ClassInfoDto[] = []; - const classInfosFromClasses = await this.findClassesForSchool(schoolId, schoolYearQueryType, loadUsers); + const classInfosFromClasses = await this.findClassesForSchool(schoolId, schoolYearQueryType); if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { - classInfosFromGroups = await this.findGroupsForSchool(schoolId, loadUsers); + classInfosFromGroups = await this.findGroupsForSchool(schoolId); } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; @@ -104,15 +101,14 @@ export class GroupUc { private async findCombinedClassListForUser( userId: EntityId, - schoolYearQueryType?: SchoolYearQueryType, - loadUsers?: boolean + schoolYearQueryType?: SchoolYearQueryType ): Promise { let classInfosFromGroups: ClassInfoDto[] = []; - const classInfosFromClasses = await this.findClassesForUser(userId, schoolYearQueryType, loadUsers); + const classInfosFromClasses = await this.findClassesForUser(userId, schoolYearQueryType); if (!schoolYearQueryType || schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR) { - classInfosFromGroups = await this.findGroupsForUser(userId, loadUsers); + classInfosFromGroups = await this.findGroupsForUser(userId); } const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; @@ -122,40 +118,29 @@ export class GroupUc { private async findClassesForSchool( schoolId: EntityId, - schoolYearQueryType?: SchoolYearQueryType, - loadUsers?: boolean + schoolYearQueryType?: SchoolYearQueryType ): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); - const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses( - classes, - schoolYearQueryType, - loadUsers - ); + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); return classInfosFromClasses; } private async findClassesForUser( userId: EntityId, - schoolYearQueryType?: SchoolYearQueryType, - loadUsers?: boolean + schoolYearQueryType?: SchoolYearQueryType ): Promise { const classes: Class[] = await this.classService.findAllByUserId(userId); - const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses( - classes, - schoolYearQueryType, - loadUsers - ); + const classInfosFromClasses: ClassInfoDto[] = await this.getClassInfosFromClasses(classes, schoolYearQueryType); return classInfosFromClasses; } private async getClassInfosFromClasses( classes: Class[], - schoolYearQueryType?: SchoolYearQueryType, - loadUsers?: boolean + schoolYearQueryType?: SchoolYearQueryType ): Promise { const currentYear: SchoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); @@ -167,10 +152,7 @@ export class GroupUc { this.isClassOfQueryType(currentYear, classWithSchoolYear.schoolYear, schoolYearQueryType) ); - const classInfosFromClasses: ClassInfoDto[] = await this.mapClassInfosFromClasses( - filteredClassesForSchoolYear, - loadUsers - ); + const classInfosFromClasses: ClassInfoDto[] = this.mapClassInfosFromClasses(filteredClassesForSchoolYear); return classInfosFromClasses; } @@ -218,16 +200,12 @@ export class GroupUc { } } - private async mapClassInfosFromClasses( - filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[], - loadUsers?: boolean - ): Promise { - const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - filteredClassesForSchoolYear.map(async (classWithSchoolYear): Promise => { - let teachers: UserDO[] = []; - if (loadUsers) { - teachers = await this.getTeachersByIds(classWithSchoolYear.clazz.teacherIds, classWithSchoolYear.clazz.id); - } + private mapClassInfosFromClasses( + filteredClassesForSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] + ): ClassInfoDto[] { + const classInfosFromClasses: ClassInfoDto[] = filteredClassesForSchoolYear.map( + (classWithSchoolYear): ClassInfoDto => { + const teachers: UserDO[] = []; const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto( classWithSchoolYear.clazz, @@ -236,31 +214,12 @@ 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; + return classInfosFromClasses; } - private async findGroupsForSchool(schoolId: EntityId, loadUsers?: boolean): Promise { + private async findGroupsForSchool(schoolId: EntityId): Promise { const groups: Group[] = await this.groupService.findGroupsBySchoolIdAndGroupTypes( schoolId, this.ALLOWED_GROUP_TYPES @@ -268,14 +227,13 @@ export class GroupUc { const systemMap: Map = await this.findSystemNamesForGroups(groups); - const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groups.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap, loadUsers)) + const classInfosFromGroups: ClassInfoDto[] = groups.map( + (group: Group): ClassInfoDto => this.getClassInfoFromGroup(group, systemMap) ); - return classInfosFromGroups; } - private async findGroupsForUser(userId: EntityId, loadUsers?: boolean): Promise { + private async findGroupsForUser(userId: EntityId): Promise { const user: UserDO = await this.userService.findById(userId); const groupsOfTypeClass: Group[] = await this.groupService.findGroupsByUserAndGroupTypes( @@ -285,29 +243,20 @@ export class GroupUc { const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); - const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groupsOfTypeClass.map( - async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap, loadUsers) - ) + const classInfosFromGroups: ClassInfoDto[] = groupsOfTypeClass.map( + (group: Group): ClassInfoDto => this.getClassInfoFromGroup(group, systemMap) ); return classInfosFromGroups; } - private async getClassInfoFromGroup( - group: Group, - systemMap: Map, - loadUsers?: boolean - ): Promise { + private getClassInfoFromGroup(group: Group, systemMap: Map): ClassInfoDto { let system: SystemDto | undefined; if (group.externalSource) { system = systemMap.get(group.externalSource.systemId); } - let resolvedUsers: ResolvedGroupUser[] = []; - if (loadUsers) { - resolvedUsers = await this.findUsersForGroup(group); - } + const resolvedUsers: ResolvedGroupUser[] = []; const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); @@ -340,11 +289,11 @@ export class GroupUc { const user: UserDO | null = await this.userService.findByIdOrNull(groupUser.userId); let resolvedGroup: ResolvedGroupUser | null = null; - if (!user) { - this.logger.warning( + /* TODO add this log back later + this.logger.warning( new ReferencedEntityNotFoundLoggable(Group.name, group.id, UserDO.name, groupUser.userId) - ); - } else { + ); */ + if (user) { const role: RoleDto = await this.roleService.findById(groupUser.roleId); resolvedGroup = new ResolvedGroupUser({ From f399985e20e6501725df95e662bd34045085d569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Tue, 16 Jan 2024 17:23:28 +0100 Subject: [PATCH 3/3] fix lint --- .../src/modules/group/controller/api-test/group.api.spec.ts | 2 +- apps/server/src/modules/group/uc/group.uc.spec.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts index ff60d7e625c..592580b47c6 100644 --- a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts +++ b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts @@ -104,7 +104,7 @@ describe('Group (API)', () => { }; it('should return the classes of his school', async () => { - const { adminClient, group, clazz, system, adminUser, teacherUser, schoolYear } = await setup(); + const { adminClient, group, clazz, system, schoolYear } = await setup(); const response = await adminClient.get(`/class`).query({ skip: 0, 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 37101b069bb..96bbd9d15dc 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -48,6 +48,7 @@ describe('GroupUc', () => { let schoolService: DeepMocked; let authorizationService: DeepMocked; let schoolYearService: DeepMocked; + // eslint-disable-next-line @typescript-eslint/no-unused-vars let logger: DeepMocked; beforeAll(async () => {