Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[N21-2242] Server: course sync add handling for substitute teachers #5352

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
be9e569
init add substitute teachers to provisioning and sync
sdinkov Nov 21, 2024
2aaf772
migration: Migration20241121180221
sdinkov Nov 22, 2024
bb20d44
update course sync - sync subst teachers
sdinkov Nov 22, 2024
4c71903
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
sdinkov Nov 22, 2024
26b830b
update tests
sdinkov Nov 25, 2024
d78100b
cleanup test schulconnex response mapper
sdinkov Nov 25, 2024
15f73ab
update group seed data
sdinkov Nov 25, 2024
ba4e6a7
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
sdinkov Nov 26, 2024
843fb8d
update seed data - groups
sdinkov Nov 26, 2024
6df2c0e
filter substitution teachers by teachers
MarvinOehlerkingCap Dec 9, 2024
2f27125
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MarvinOehlerkingCap Dec 10, 2024
31467a7
VLehr
MarvinOehlerkingCap Dec 10, 2024
3429b90
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MarvinOehlerkingCap Dec 10, 2024
8627d14
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MBergCap Dec 12, 2024
994ec66
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MarvinOehlerkingCap Jan 20, 2025
53099da
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
mrikallab Jan 20, 2025
b449204
Merge remote-tracking branch 'origin/N21-2242-course-sync-add-handlin…
mrikallab Jan 20, 2025
9159dab
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MarvinOehlerkingCap Jan 20, 2025
79af8d2
Merge branch 'main' into N21-2242-course-sync-add-handling-for-substi…
MarvinOehlerkingCap Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export enum SchulconnexGroupRole {
TEACHER = 'Lehr',
SUBSTITUTE_TEACHER = 'VLehr',
STUDENT = 'Lern',
CLASS_LEADER = 'KlLeit',
SUPPORT_TEACHER = 'Foerd',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export const schulconnexResponseFactory = Factory.define<SchulconnexResponse>(()
rollen: [SchulconnexGroupRole.STUDENT],
ktid: 'ktid',
},
{
rollen: [SchulconnexGroupRole.SUBSTITUTE_TEACHER],
ktid: 'ktid1',
},
],
},
],
Expand Down
21 changes: 21 additions & 0 deletions apps/server/src/migrations/mikro-orm/Migration20241121180221.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Migration } from '@mikro-orm/migrations-mongodb';

export class Migration20241121180221 extends Migration {
async up(): Promise<void> {
await this.driver.nativeInsert('roles', {
name: 'groupSubstitutionTeacher',
roles: [],
permissions: [],
});

// eslint-disable-next-line no-console
console.info('Added groupSubstitutionTeacher role');
}

async down(): Promise<void> {
await this.driver.nativeDelete('roles', { name: 'groupSubstitutionTeacher' });

// eslint-disable-next-line no-console
console.info('Rollback: Removed groupSubstitutionTeacher role');
}
}
8 changes: 8 additions & 0 deletions apps/server/src/modules/group/repo/group.repo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions apps/server/src/modules/learnroom/domain/do/course.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ export class Course extends DomainObject<CourseProps> {
return this.props.substitutionTeacherIds;
}

set substitutionTeachers(value: EntityId[]) {
this.props.substitutionTeacherIds = value;
}

set classes(value: EntityId[]) {
this.props.classIds = value;
}
Expand Down
100 changes: 82 additions & 18 deletions apps/server/src/modules/learnroom/service/course-sync.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -398,22 +398,93 @@ 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: [],
}),
]);
});
});

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();
Expand All @@ -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[]]>([
Expand All @@ -450,20 +520,18 @@ describe(CourseSyncService.name, () => {
teacherIds: [],
classIds: [],
groupIds: [],
substitutionTeacherIds: [substituteTeacherId],
substitutionTeacherIds: [],
}),
]);
});
});

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();
Expand All @@ -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[]]>([
Expand All @@ -500,15 +567,14 @@ describe(CourseSyncService.name, () => {
teacherIds: [],
classIds: [],
groupIds: [],
substitutionTeacherIds: [substituteTeacherId],
substitutionTeacherIds: [],
}),
]);
});
});

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();
Expand All @@ -526,7 +592,6 @@ describe(CourseSyncService.name, () => {

const course: Course = courseFactory.build({
syncedWithGroup: newGroup.id,
substitutionTeacherIds: [substituteTeacherId],
teacherIds: [teacherUserId],
excludeFromSync: [],
});
Expand All @@ -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[]]>([
Expand All @@ -559,7 +623,7 @@ describe(CourseSyncService.name, () => {
classIds: [],
groupIds: [],
excludeFromSync: [],
substitutionTeacherIds: [substituteTeacherId],
substitutionTeacherIds: [],
}),
]);
});
Expand Down Expand Up @@ -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[]]>([
Expand All @@ -622,7 +686,7 @@ describe(CourseSyncService.name, () => {
classIds: [],
groupIds: [],
excludeFromSync: [SyncAttribute.TEACHERS],
substitutionTeacherIds: [substituteTeacherId],
substitutionTeacherIds: [],
}),
]);
});
Expand Down
24 changes: 18 additions & 6 deletions apps/server/src/modules/learnroom/service/course-sync.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,21 @@ export class CourseSyncService {
}

private async synchronize(courses: Course[], group: Group, oldGroup?: Group): Promise<void> {
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;
Expand All @@ -80,14 +84,22 @@ export class CourseSyncService {
course.name = group.name;
}

const excludedFromSync = new Set(course.excludeFromSync || []);
const excludedFromSync: Set<SyncAttribute> = new Set(course.excludeFromSync || []);

if (excludedFromSync.has(SyncAttribute.TEACHERS)) {
course.students = studentIds;
} else {
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<EntityId> = new Set(course.teachers);
const filteredSubstituteTeacherIds: string[] = substituteTeacherIds.filter(
(substituteTeacherId: EntityId) => !teacherSet.has(substituteTeacherId)
);

course.substitutionTeachers = filteredSubstituteTeacherIds;
}

await this.courseRepo.saveAll(courses);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const RoleMapping: Partial<Record<SchulconnexRole | string, RoleName>> = {

const GroupRoleMapping: Partial<Record<SchulconnexGroupRole | string, RoleName>> = {
[SchulconnexGroupRole.TEACHER]: RoleName.TEACHER,
[SchulconnexGroupRole.SUBSTITUTE_TEACHER]: RoleName.GROUPSUBSTITUTIONTEACHER,
[SchulconnexGroupRole.STUDENT]: RoleName.STUDENT,
};

Expand Down
1 change: 1 addition & 0 deletions apps/server/src/shared/domain/interface/rolename.enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export enum RoleName {
COURSEADMINISTRATOR = 'courseAdministrator',
COURSESTUDENT = 'courseStudent',
COURSESUBSTITUTIONTEACHER = 'courseSubstitutionTeacher',
GROUPSUBSTITUTIONTEACHER = 'groupSubstitutionTeacher',
COURSETEACHER = 'courseTeacher',
DEMO = 'demo',
DEMOSTUDENT = 'demoStudent',
Expand Down
Loading
Loading