From 79deeab0b4f09fd389d60c92fb2174e6c3d63480 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov <133751031+sdinkov@users.noreply.github.com> Date: Tue, 21 Jan 2025 09:27:21 +0100 Subject: [PATCH] N21-2242 Course sync add handling for substitute teachers (#5352) --- .../response/schulconnex-group-role.ts | 1 + .../testing/schulconnex-response-factory.ts | 4 + .../mikro-orm/Migration20241121180221.ts | 21 ++++ .../src/modules/group/repo/group.repo.spec.ts | 8 ++ .../src/modules/learnroom/domain/do/course.ts | 4 + .../service/course-sync.service.spec.ts | 100 ++++++++++++++--- .../learnroom/service/course-sync.service.ts | 24 +++- .../schulconnex-response-mapper.spec.ts | 8 +- .../schulconnex-response-mapper.ts | 1 + .../shared/domain/interface/rolename.enum.ts | 1 + .../testing/factory/group-entity.factory.ts | 4 + backup/setup/groups.json | 104 ++++++++++-------- backup/setup/migrations.json | 9 ++ backup/setup/roles.json | 16 ++- 14 files changed, 235 insertions(+), 70 deletions(-) create mode 100644 apps/server/src/migrations/mikro-orm/Migration20241121180221.ts diff --git a/apps/server/src/infra/schulconnex-client/response/schulconnex-group-role.ts b/apps/server/src/infra/schulconnex-client/response/schulconnex-group-role.ts index 1ce90fbc9e..faa2e302a0 100644 --- a/apps/server/src/infra/schulconnex-client/response/schulconnex-group-role.ts +++ b/apps/server/src/infra/schulconnex-client/response/schulconnex-group-role.ts @@ -1,5 +1,6 @@ export enum SchulconnexGroupRole { TEACHER = 'Lehr', + SUBSTITUTE_TEACHER = 'VLehr', STUDENT = 'Lern', CLASS_LEADER = 'KlLeit', SUPPORT_TEACHER = 'Foerd', diff --git a/apps/server/src/infra/schulconnex-client/testing/schulconnex-response-factory.ts b/apps/server/src/infra/schulconnex-client/testing/schulconnex-response-factory.ts index 78edb0e1a7..691c97a6bb 100644 --- a/apps/server/src/infra/schulconnex-client/testing/schulconnex-response-factory.ts +++ b/apps/server/src/infra/schulconnex-client/testing/schulconnex-response-factory.ts @@ -52,6 +52,10 @@ export const schulconnexResponseFactory = Factory.define(() rollen: [SchulconnexGroupRole.STUDENT], ktid: 'ktid', }, + { + rollen: [SchulconnexGroupRole.SUBSTITUTE_TEACHER], + ktid: 'ktid1', + }, ], }, ], diff --git a/apps/server/src/migrations/mikro-orm/Migration20241121180221.ts b/apps/server/src/migrations/mikro-orm/Migration20241121180221.ts new file mode 100644 index 0000000000..02571e78aa --- /dev/null +++ b/apps/server/src/migrations/mikro-orm/Migration20241121180221.ts @@ -0,0 +1,21 @@ +import { Migration } from '@mikro-orm/migrations-mongodb'; + +export class Migration20241121180221 extends Migration { + async up(): Promise { + await this.driver.nativeInsert('roles', { + name: 'groupSubstitutionTeacher', + roles: [], + permissions: [], + }); + + // eslint-disable-next-line no-console + console.info('Added groupSubstitutionTeacher role'); + } + + async down(): Promise { + await this.driver.nativeDelete('roles', { name: 'groupSubstitutionTeacher' }); + + // eslint-disable-next-line no-console + console.info('Rollback: Removed groupSubstitutionTeacher role'); + } +} diff --git a/apps/server/src/modules/group/repo/group.repo.spec.ts b/apps/server/src/modules/group/repo/group.repo.spec.ts index c405b2538e..5c95c8bcda 100644 --- a/apps/server/src/modules/group/repo/group.repo.spec.ts +++ b/apps/server/src/modules/group/repo/group.repo.spec.ts @@ -77,6 +77,10 @@ describe(GroupRepo.name, () => { userId: group.users[1].user.id, roleId: group.users[1].role.id, }), + new GroupUser({ + userId: group.users[2].user.id, + roleId: group.users[2].role.id, + }), ], organizationId: group.organization?.id, validPeriod: group.validPeriod, @@ -706,6 +710,10 @@ describe(GroupRepo.name, () => { userId: groupEntity.users[1].user.id, roleId: groupEntity.users[1].role.id, }), + new GroupUser({ + userId: groupEntity.users[2].user.id, + roleId: groupEntity.users[2].role.id, + }), ], organizationId: groupEntity.organization?.id, validPeriod: groupEntity.validPeriod, diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index e5255c3e79..9627ed1dec 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -67,6 +67,10 @@ export class Course extends DomainObject { return this.props.substitutionTeacherIds; } + set substitutionTeachers(value: EntityId[]) { + this.props.substitutionTeacherIds = value; + } + set classes(value: EntityId[]) { this.props.classIds = value; } diff --git a/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts b/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts index b75369d505..ded10fcd11 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts @@ -386,7 +386,7 @@ describe(CourseSyncService.name, () => { }; it('should synchronize with the new group', async () => { - const { course, newGroup, studentId, teacherId, substituteTeacherId } = setup(); + const { course, newGroup, studentId, teacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup); @@ -398,9 +398,82 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [studentId], teacherIds: [teacherId], + substitutionTeacherIds: [], classIds: [], groupIds: [], + }), + ]); + }); + }); + + describe('when synchronizing with a new group with substitute teacher', () => { + const setup = () => { + const studentId: string = new ObjectId().toHexString(); + const teacherId: string = new ObjectId().toHexString(); + const substituteTeacherId: string = new ObjectId().toHexString(); + const studentRoleId: string = new ObjectId().toHexString(); + const teacherRoleId: string = new ObjectId().toHexString(); + const substituteTeacherRoleId: string = new ObjectId().toHexString(); + const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); + const teacherRole: RoleDto = roleDtoFactory.build({ id: teacherRoleId }); + const substituteTeacherRole: RoleDto = roleDtoFactory.build({ id: substituteTeacherRoleId }); + const newGroup: Group = groupFactory.build({ + users: [ + { + userId: studentId, + roleId: studentRoleId, + }, + { + userId: teacherId, + roleId: teacherRoleId, + }, + { + userId: substituteTeacherId, + roleId: substituteTeacherRoleId, + }, + { + userId: teacherId, + roleId: substituteTeacherRoleId, + }, + ], + }); + const course: Course = courseFactory.build({ + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + substitutionTeacherIds: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName + .mockResolvedValueOnce(studentRole) + .mockResolvedValueOnce(teacherRole) + .mockResolvedValueOnce(substituteTeacherRole); + + return { + course, + newGroup, + studentId, + teacherId, + substituteTeacherId, + }; + }; + + it('should synchronize the substitution teachers, without creating duplicates in teacherIds', async () => { + const { course, newGroup, studentId, teacherId, substituteTeacherId } = setup(); + + await service.synchronizeCourseWithGroup(newGroup); + + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + new Course({ + ...course.getProps(), + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [studentId], + teacherIds: [teacherId], substitutionTeacherIds: [substituteTeacherId], + classIds: [], + groupIds: [], }), ]); }); @@ -408,12 +481,10 @@ describe(CourseSyncService.name, () => { describe('when the course name is the same as the old group name', () => { const setup = () => { - const substituteTeacherId = new ObjectId().toHexString(); const course: Course = courseFactory.build({ name: 'Course Name', classIds: [new ObjectId().toHexString()], groupIds: [new ObjectId().toHexString()], - substitutionTeacherIds: [substituteTeacherId], }); const studentRole: RoleDto = roleDtoFactory.build(); const teacherRole: RoleDto = roleDtoFactory.build(); @@ -431,12 +502,11 @@ describe(CourseSyncService.name, () => { course, newGroup, oldGroup, - substituteTeacherId, }; }; it('should synchronize the group name', async () => { - const { course, newGroup, oldGroup, substituteTeacherId } = setup(); + const { course, newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -450,7 +520,7 @@ describe(CourseSyncService.name, () => { teacherIds: [], classIds: [], groupIds: [], - substitutionTeacherIds: [substituteTeacherId], + substitutionTeacherIds: [], }), ]); }); @@ -458,12 +528,10 @@ describe(CourseSyncService.name, () => { describe('when the course name is different from the old group name', () => { const setup = () => { - const substituteTeacherId = new ObjectId().toHexString(); const course: Course = courseFactory.build({ name: 'Custom Course Name', classIds: [new ObjectId().toHexString()], groupIds: [new ObjectId().toHexString()], - substitutionTeacherIds: [substituteTeacherId], }); const studentRole: RoleDto = roleDtoFactory.build(); const teacherRole: RoleDto = roleDtoFactory.build(); @@ -481,12 +549,11 @@ describe(CourseSyncService.name, () => { course, newGroup, oldGroup, - substituteTeacherId, }; }; it('should keep the old course name', async () => { - const { course, newGroup, oldGroup, substituteTeacherId } = setup(); + const { course, newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -500,7 +567,7 @@ describe(CourseSyncService.name, () => { teacherIds: [], classIds: [], groupIds: [], - substitutionTeacherIds: [substituteTeacherId], + substitutionTeacherIds: [], }), ]); }); @@ -508,7 +575,6 @@ describe(CourseSyncService.name, () => { describe('when the teachers are not synced from group', () => { const setup = () => { - const substituteTeacherId = new ObjectId().toHexString(); const studentUserId = new ObjectId().toHexString(); const teacherUserId = new ObjectId().toHexString(); const studentRoleId: string = new ObjectId().toHexString(); @@ -526,7 +592,6 @@ describe(CourseSyncService.name, () => { const course: Course = courseFactory.build({ syncedWithGroup: newGroup.id, - substitutionTeacherIds: [substituteTeacherId], teacherIds: [teacherUserId], excludeFromSync: [], }); @@ -538,13 +603,12 @@ describe(CourseSyncService.name, () => { course, newGroup, teacherUserId, - substituteTeacherId, studentUserId, }; }; it('should not sync group students', async () => { - const { course, newGroup, teacherUserId, substituteTeacherId } = setup(); + const { course, newGroup, teacherUserId } = setup(); await service.synchronizeCourseWithGroup(newGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -559,7 +623,7 @@ describe(CourseSyncService.name, () => { classIds: [], groupIds: [], excludeFromSync: [], - substitutionTeacherIds: [substituteTeacherId], + substitutionTeacherIds: [], }), ]); }); @@ -607,7 +671,7 @@ describe(CourseSyncService.name, () => { }; it('should not sync group teachers', async () => { - const { course, newGroup, substituteTeacherId, teacherUserId, studentUserId } = setup(); + const { course, newGroup, teacherUserId, studentUserId } = setup(); await service.synchronizeCourseWithGroup(newGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -622,7 +686,7 @@ describe(CourseSyncService.name, () => { classIds: [], groupIds: [], excludeFromSync: [SyncAttribute.TEACHERS], - substitutionTeacherIds: [substituteTeacherId], + substitutionTeacherIds: [], }), ]); }); diff --git a/apps/server/src/modules/learnroom/service/course-sync.service.ts b/apps/server/src/modules/learnroom/service/course-sync.service.ts index 02edd23f70..f285edc7f9 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -57,17 +57,21 @@ export class CourseSyncService { } private async synchronize(courses: Course[], group: Group, oldGroup?: Group): Promise { - const [studentRole, teacherRole] = await Promise.all([ + const [studentRole, teacherRole, substituteTeacherRole] = await Promise.all([ this.roleService.findByName(RoleName.STUDENT), this.roleService.findByName(RoleName.TEACHER), + this.roleService.findByName(RoleName.GROUPSUBSTITUTIONTEACHER), ]); - const studentIds = group.users + const studentIds: EntityId[] = group.users .filter((user: GroupUser) => user.roleId === studentRole.id) - .map((student) => student.userId); - const teacherIds = group.users + .map((student: GroupUser) => student.userId); + const teacherIds: EntityId[] = group.users .filter((user: GroupUser) => user.roleId === teacherRole.id) - .map((teacher) => teacher.userId); + .map((teacher: GroupUser) => teacher.userId); + const substituteTeacherIds: EntityId[] = group.users + .filter((user: GroupUser) => user.roleId === substituteTeacherRole.id) + .map((substituteTeacher: GroupUser) => substituteTeacher.userId); for (const course of courses) { course.syncedWithGroup = group.id; @@ -80,7 +84,7 @@ export class CourseSyncService { course.name = group.name; } - const excludedFromSync = new Set(course.excludeFromSync || []); + const excludedFromSync: Set = new Set(course.excludeFromSync || []); if (excludedFromSync.has(SyncAttribute.TEACHERS)) { course.students = studentIds; @@ -88,6 +92,14 @@ export class CourseSyncService { course.teachers = teacherIds.length > 0 ? teacherIds : course.teachers; course.students = teacherIds.length > 0 ? studentIds : []; } + + // To ensure unique teachers per course, filter out already assigned teachers from the substitution teacher list + const teacherSet: Set = new Set(course.teachers); + const filteredSubstituteTeacherIds: string[] = substituteTeacherIds.filter( + (substituteTeacherId: EntityId) => !teacherSet.has(substituteTeacherId) + ); + + course.substitutionTeachers = filteredSubstituteTeacherIds; } await this.courseRepo.saveAll(courses); diff --git a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts index 7af9e42aec..f5b9de1698 100644 --- a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.spec.ts @@ -159,17 +159,19 @@ describe(SchulconnexResponseMapper.name, () => { const personenkontext: SchulconnexPersonenkontextResponse = schulconnexResponse.personenkontexte[0]; const group: SchulconnexGruppenResponse = personenkontext.gruppen![0]; const otherParticipant: SchulconnexSonstigeGruppenzugehoerigeResponse = group.sonstige_gruppenzugehoerige![0]; + const otherParticipant1: SchulconnexSonstigeGruppenzugehoerigeResponse = group.sonstige_gruppenzugehoerige![1]; return { schulconnexResponse, group, personenkontext, otherParticipant, + otherParticipant1, }; }; it('should map the schulconnex response to external group dtos', () => { - const { schulconnexResponse, group, personenkontext, otherParticipant } = setup(); + const { schulconnexResponse, group, personenkontext, otherParticipant, otherParticipant1 } = setup(); const result: ExternalGroupDto[] | undefined = mapper.mapToExternalGroupDtos(schulconnexResponse); @@ -186,6 +188,10 @@ describe(SchulconnexResponseMapper.name, () => { externalUserId: otherParticipant.ktid, roleName: RoleName.STUDENT, }, + { + externalUserId: otherParticipant1.ktid, + roleName: RoleName.GROUPSUBSTITUTIONTEACHER, + }, ], from: new Date('2024-08-01'), until: new Date('2025-07-31'), diff --git a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts index 196814b685..76687f282c 100644 --- a/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts +++ b/apps/server/src/modules/provisioning/strategy/schulconnex/schulconnex-response-mapper.ts @@ -36,6 +36,7 @@ const RoleMapping: Partial> = { const GroupRoleMapping: Partial> = { [SchulconnexGroupRole.TEACHER]: RoleName.TEACHER, + [SchulconnexGroupRole.SUBSTITUTE_TEACHER]: RoleName.GROUPSUBSTITUTIONTEACHER, [SchulconnexGroupRole.STUDENT]: RoleName.STUDENT, }; diff --git a/apps/server/src/shared/domain/interface/rolename.enum.ts b/apps/server/src/shared/domain/interface/rolename.enum.ts index 310f80cf84..c097137f7f 100644 --- a/apps/server/src/shared/domain/interface/rolename.enum.ts +++ b/apps/server/src/shared/domain/interface/rolename.enum.ts @@ -3,6 +3,7 @@ export enum RoleName { COURSEADMINISTRATOR = 'courseAdministrator', COURSESTUDENT = 'courseStudent', COURSESUBSTITUTIONTEACHER = 'courseSubstitutionTeacher', + GROUPSUBSTITUTIONTEACHER = 'groupSubstitutionTeacher', COURSETEACHER = 'courseTeacher', DEMO = 'demo', DEMOSTUDENT = 'demoStudent', diff --git a/apps/server/src/testing/factory/group-entity.factory.ts b/apps/server/src/testing/factory/group-entity.factory.ts index 4bf8b925a8..0792d44122 100644 --- a/apps/server/src/testing/factory/group-entity.factory.ts +++ b/apps/server/src/testing/factory/group-entity.factory.ts @@ -20,6 +20,10 @@ export const groupEntityFactory = BaseFactory.define