From 510694ffb05a954d9b20e20264f3265f243b159f Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Thu, 10 Oct 2024 18:25:57 +0200 Subject: [PATCH 1/7] add course-sync service; clean up courseDo and schulconnex sync service; update tests + index --- apps/server/src/modules/learnroom/index.ts | 1 + .../src/modules/learnroom/learnroom.module.ts | 5 + .../service/course-do.service.spec.ts | 96 +---- .../learnroom/service/course-do.service.ts | 29 +- .../service/course-sync.service.spec.ts | 366 ++++++++++++++++++ .../learnroom/service/course-sync.service.ts | 76 ++++ .../src/modules/learnroom/service/index.ts | 5 +- .../learnroom/uc/course-sync.uc.spec.ts | 29 +- .../modules/learnroom/uc/course-sync.uc.ts | 25 +- .../schulconnex-course-sync.service.spec.ts | 188 +-------- .../schulconnex-course-sync.service.ts | 42 +- 11 files changed, 511 insertions(+), 351 deletions(-) create mode 100644 apps/server/src/modules/learnroom/service/course-sync.service.spec.ts create mode 100644 apps/server/src/modules/learnroom/service/course-sync.service.ts diff --git a/apps/server/src/modules/learnroom/index.ts b/apps/server/src/modules/learnroom/index.ts index 20d6d0f7508..4135d6d3677 100644 --- a/apps/server/src/modules/learnroom/index.ts +++ b/apps/server/src/modules/learnroom/index.ts @@ -6,6 +6,7 @@ export { CourseGroupService, CourseService, CourseDoService, + CourseSyncService, DashboardService, CourseRoomsService, } from './service'; diff --git a/apps/server/src/modules/learnroom/learnroom.module.ts b/apps/server/src/modules/learnroom/learnroom.module.ts index 0adf76cacd0..a8bdcbeba4c 100644 --- a/apps/server/src/modules/learnroom/learnroom.module.ts +++ b/apps/server/src/modules/learnroom/learnroom.module.ts @@ -3,6 +3,7 @@ import { ClassModule } from '@modules/class'; import { CopyHelperModule } from '@modules/copy-helper'; import { GroupModule } from '@modules/group'; import { LessonModule } from '@modules/lesson'; +import { RoleModule } from '@modules/role'; import { SchoolModule } from '@modules/school'; import { TaskModule } from '@modules/task'; import { ContextExternalToolModule } from '@modules/tool/context-external-tool'; @@ -34,6 +35,7 @@ import { CourseGroupService, CourseRoomsService, CourseService, + CourseSyncService, DashboardService, GroupDeletedHandlerService, } from './service'; @@ -56,6 +58,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; ClassModule, SchoolModule, GroupModule, + RoleModule, ], providers: [ { @@ -79,6 +82,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; }, CourseService, CourseDoService, + CourseSyncService, DashboardElementRepo, DashboardModelMapper, DashboardService, @@ -92,6 +96,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; CourseCopyService, CourseService, CourseDoService, + CourseSyncService, CourseRoomsService, CommonCartridgeExportService, CommonCartridgeImportService, diff --git a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts index a6fae948299..4710ecc4aaa 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts @@ -7,14 +7,7 @@ import { Page } from '@shared/domain/domainobject'; import { IFindOptions, SortOrder } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { groupFactory } from '@shared/testing'; -import { - Course, - COURSE_REPO, - CourseAlreadySynchronizedLoggableException, - CourseFilter, - CourseNotSynchronizedLoggableException, - CourseRepo, -} from '../domain'; +import { Course, COURSE_REPO, CourseFilter, CourseRepo } from '../domain'; import { courseFactory } from '../testing'; import { CourseDoService } from './course-do.service'; @@ -139,93 +132,6 @@ describe(CourseDoService.name, () => { }); }); - describe('stopSynchronization', () => { - describe('when a course is synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); - - return { - course, - }; - }; - - it('should save a course without a synchronized group', async () => { - const { course } = setup(); - - await service.stopSynchronization(course); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - syncedWithGroup: undefined, - }) - ); - }); - }); - - describe('when a course is not synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build(); - - return { - course, - }; - }; - - it('should throw an unprocessable entity exception', async () => { - const { course } = setup(); - - await expect(service.stopSynchronization(course)).rejects.toThrow(CourseNotSynchronizedLoggableException); - }); - }); - }); - - describe('startSynchronization', () => { - describe('when a course is not synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build(); - const group: Group = groupFactory.build(); - - return { - course, - group, - }; - }; - - it('should save a course with a synchronized group', async () => { - const { course, group } = setup(); - - await service.startSynchronization(course, group); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - syncedWithGroup: group.id, - }) - ); - }); - }); - - describe('when a course is synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); - const group: Group = groupFactory.build(); - - return { - course, - group, - }; - }; - it('should throw an unprocessable entity exception', async () => { - const { course, group } = setup(); - - await expect(service.startSynchronization(course, group)).rejects.toThrow( - CourseAlreadySynchronizedLoggableException - ); - }); - }); - }); - describe('findCourses', () => { describe('when course are found', () => { const setup = () => { diff --git a/apps/server/src/modules/learnroom/service/course-do.service.ts b/apps/server/src/modules/learnroom/service/course-do.service.ts index 951693dee25..76c73fa9ea7 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.ts @@ -4,14 +4,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; -import { - type Course, - COURSE_REPO, - CourseAlreadySynchronizedLoggableException, - CourseFilter, - CourseNotSynchronizedLoggableException, - CourseRepo, -} from '../domain'; +import { COURSE_REPO, CourseFilter, CourseRepo, type Course } from '../domain'; @Injectable() export class CourseDoService implements AuthorizationLoaderServiceGeneric { @@ -35,26 +28,6 @@ export class CourseDoService implements AuthorizationLoaderServiceGeneric { - if (!course.syncedWithGroup) { - throw new CourseNotSynchronizedLoggableException(course.id); - } - - course.syncedWithGroup = undefined; - - await this.courseRepo.save(course); - } - - public async startSynchronization(course: Course, group: Group): Promise { - if (course.syncedWithGroup) { - throw new CourseAlreadySynchronizedLoggableException(course.id); - } - - course.syncedWithGroup = group.id; - - await this.courseRepo.save(course); - } - public async getCourseInfo(filter: CourseFilter, options?: IFindOptions): Promise> { const courses = await this.courseRepo.getCourseInfo(filter, options); 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 new file mode 100644 index 00000000000..07a6140d7ac --- /dev/null +++ b/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts @@ -0,0 +1,366 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; +import { Group, GroupPeriod, GroupUser } from '@modules/group'; +import { RoleDto, RoleService } from '@modules/role'; +import { Test, TestingModule } from '@nestjs/testing'; +import { groupFactory, roleDtoFactory } from '@shared/testing'; +import { + Course, + COURSE_REPO, + CourseAlreadySynchronizedLoggableException, + CourseNotSynchronizedLoggableException, + CourseRepo, +} from '../domain'; +import { courseFactory } from '../testing'; +import { CourseSyncService } from './course-sync.service'; + +describe(CourseSyncService.name, () => { + let module: TestingModule; + let service: CourseSyncService; + let roleService: DeepMocked; + + let courseRepo: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + CourseSyncService, + { + provide: RoleService, + useValue: createMock(), + }, + { + provide: COURSE_REPO, + useValue: createMock(), + }, + ], + }).compile(); + + service = module.get(CourseSyncService); + roleService = module.get(RoleService); + courseRepo = module.get(COURSE_REPO); + }); + + afterAll(async () => { + await module.close(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('stopSynchronization', () => { + describe('when a course is synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); + + return { + course, + }; + }; + + it('should save a course without a synchronized group', async () => { + const { course } = setup(); + + await service.stopSynchronization(course); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: undefined, + }) + ); + }); + }); + + describe('when a course is not synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + + return { + course, + }; + }; + + it('should throw an unprocessable entity exception', async () => { + const { course } = setup(); + + await expect(service.stopSynchronization(course)).rejects.toThrow(CourseNotSynchronizedLoggableException); + }); + }); + }); + + describe('startSynchronization', () => { + describe('when a course is not synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + + const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); + const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; + const group: Group = groupFactory.build({ users: [...students, ...teachers] }); + const groupWithoutTeachers: Group = groupFactory.build({ users: [...students] }); + roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); + + return { + course, + group, + students, + teachers, + groupWithoutTeachers, + }; + }; + + it('should save a course with synchronized group, students, and teachers', async () => { + const { course, group, students, teachers } = setup(); + + await service.startSynchronization(course, group); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: course.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: students.map((student) => student.userId), + teacherIds: teachers.map((teacher) => teacher.userId), + }) + ); + }); + + it('should set an empty list of students if no teachers are present', async () => { + const { course, groupWithoutTeachers } = setup(); + + await service.startSynchronization(course, groupWithoutTeachers); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: groupWithoutTeachers.id, + name: course.name, + startDate: groupWithoutTeachers.validPeriod?.from, + untilDate: groupWithoutTeachers.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when a course is already synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); + const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; + + return { + course, + group, + students, + teachers, + }; + }; + + it('should throw a CourseAlreadySynchronizedLoggableException', async () => { + const { course, group } = setup(); + + await expect(service.startSynchronization(course, group)).rejects.toThrow( + CourseAlreadySynchronizedLoggableException + ); + + expect(courseRepo.save).not.toHaveBeenCalled(); + }); + }); + }); + + describe('synchronizeCourseWithGroup', () => { + describe('when synchronizing with a new group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + const studentId: string = new ObjectId().toHexString(); + const teacherId: string = new ObjectId().toHexString(); + const studentRoleId: string = new ObjectId().toHexString(); + const teacherRoleId: string = new ObjectId().toHexString(); + const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); + const teacherRole: RoleDto = roleDtoFactory.build({ id: teacherRoleId }); + const newGroup: Group = groupFactory.build({ + users: [ + { + userId: studentId, + roleId: studentRoleId, + }, + { + userId: teacherId, + roleId: teacherRoleId, + }, + ], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + studentId, + teacherId, + }; + }; + + it('should synchronize with the group', async () => { + const { course, newGroup, studentId, teacherId } = setup(); + + await service.synchronizeCourseWithGroup(newGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: newGroup.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [studentId], + teacherIds: [teacherId], + }) + ); + }); + }); + + describe('when the course name is the same as the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should synchronize the group name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: newGroup.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when the course name is different from the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Custom Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should keep the old course name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: course.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when the last teacher gets removed from a synced group', () => { + const setup = () => { + const studentUserId = new ObjectId().toHexString(); + const teacherUserId = new ObjectId().toHexString(); + const studentRoleId: string = new ObjectId().toHexString(); + const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); + const teacherRole: RoleDto = roleDtoFactory.build(); + const newGroup: Group = groupFactory.build({ + users: [ + new GroupUser({ + userId: studentUserId, + roleId: studentRoleId, + }), + ], + }); + + const course: Course = courseFactory.build({ + teacherIds: [teacherUserId], + studentIds: [studentUserId], + syncedWithGroup: newGroup.id, + }); + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + teacherUserId, + }; + }; + + it('should keep the last teacher, remove all students', async () => { + const { course, newGroup, teacherUserId } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, newGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: course.name, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [teacherUserId], + syncedWithGroup: course.syncedWithGroup, + }) + ); + }); + }); + }); +}); diff --git a/apps/server/src/modules/learnroom/service/course-sync.service.ts b/apps/server/src/modules/learnroom/service/course-sync.service.ts new file mode 100644 index 00000000000..10be631edc8 --- /dev/null +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -0,0 +1,76 @@ +import { Group, GroupUser } from '@modules/group'; +import { RoleService } from '@modules/role'; +import { Inject, Injectable } from '@nestjs/common'; +import { RoleName } from '@shared/domain/interface'; +import { EntityId } from '@shared/domain/types'; +import { + Course, + COURSE_REPO, + CourseAlreadySynchronizedLoggableException, + CourseNotSynchronizedLoggableException, + CourseRepo, +} from '../domain'; + +@Injectable() +export class CourseSyncService { + constructor( + @Inject(COURSE_REPO) private readonly courseRepo: CourseRepo, + private readonly roleService: RoleService + ) {} + + public async startSynchronization(course: Course, group: Group): Promise { + if (course.syncedWithGroup) { + throw new CourseAlreadySynchronizedLoggableException(course.id); + } + + await this.synchronize(course, group); + } + + public async stopSynchronization(course: Course): Promise { + if (!course.syncedWithGroup) { + throw new CourseNotSynchronizedLoggableException(course.id); + } + + course.syncedWithGroup = undefined; + + await this.courseRepo.save(course); + } + + public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { + const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); + + if (courses.length) { + await Promise.all( + courses.map(async (course) => { + await this.synchronize(course, newGroup, oldGroup); + }) + ); + } + } + + private async synchronize(course: Course, group: Group, oldGroup?: Group): Promise { + course.syncedWithGroup = group.id; + if (!oldGroup || oldGroup.name === course.name) { + course.name = group.name; + } + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; + + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); + + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; + } + + await this.courseRepo.save(course); + } +} diff --git a/apps/server/src/modules/learnroom/service/index.ts b/apps/server/src/modules/learnroom/service/index.ts index 61cc6b2d068..b91fcdc556f 100644 --- a/apps/server/src/modules/learnroom/service/index.ts +++ b/apps/server/src/modules/learnroom/service/index.ts @@ -2,9 +2,10 @@ export * from './board-copy.service'; export * from './common-cartridge-export.service'; export * from './common-cartridge-import.service'; export * from './course-copy.service'; -export * from './course.service'; export { CourseDoService } from './course-do.service'; +export * from './course-rooms.service'; +export { CourseSyncService } from './course-sync.service'; +export * from './course.service'; export * from './coursegroup.service'; export * from './dashboard.service'; -export * from './course-rooms.service'; export { GroupDeletedHandlerService } from './group-deleted-handler.service'; diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts index 60366ab3065..1d366afe06d 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts @@ -5,6 +5,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain/interface'; import { groupFactory, setupEntities, userFactory } from '@shared/testing'; import { CourseDoService } from '../service'; +import { CourseSyncService } from '../service/course-sync.service'; import { courseFactory } from '../testing'; import { CourseSyncUc } from './course-sync.uc'; @@ -15,6 +16,7 @@ describe(CourseSyncUc.name, () => { let authorizationService: DeepMocked; let courseService: DeepMocked; let groupService: DeepMocked; + let courseSyncService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -28,6 +30,10 @@ describe(CourseSyncUc.name, () => { provide: CourseDoService, useValue: createMock(), }, + { + provide: CourseSyncService, + useValue: createMock(), + }, { provide: GroupService, useValue: createMock(), @@ -39,6 +45,8 @@ describe(CourseSyncUc.name, () => { authorizationService = module.get(AuthorizationService); courseService = module.get(CourseDoService); groupService = module.get(GroupService); + courseSyncService = module.get(CourseSyncService); + await setupEntities(); }); @@ -82,7 +90,7 @@ describe(CourseSyncUc.name, () => { await uc.stopSynchronization(user.id, course.id); - expect(courseService.stopSynchronization).toHaveBeenCalledWith(course); + expect(courseSyncService.stopSynchronization).toHaveBeenCalledWith(course); }); }); }); @@ -110,8 +118,6 @@ describe(CourseSyncUc.name, () => { await uc.startSynchronization(user.id, course.id, group.id); - expect(courseService.findById).toHaveBeenCalledWith(course.id); - expect(groupService.findById).toHaveBeenCalledWith(group.id); expect(authorizationService.checkPermission).toHaveBeenCalledWith( user, course, @@ -119,14 +125,27 @@ describe(CourseSyncUc.name, () => { ); }); - it('should start the synchronization', async () => { + it('should call course do service with the correct course id', async () => { const { user, course, group } = setup(); await uc.startSynchronization(user.id, course.id, group.id); expect(courseService.findById).toHaveBeenCalledWith(course.id); + }); + + it('should call group service with the correct group id', async () => { + const { user, course, group } = setup(); + + await uc.startSynchronization(user.id, course.id, group.id); + expect(groupService.findById).toHaveBeenCalledWith(group.id); + }); + + it('should start the synchronization', async () => { + const { user, course, group } = setup(); + + await uc.startSynchronization(user.id, course.id, group.id); - expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group); + expect(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts index 53a4fe2e173..4e018e3bedb 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,37 +1,40 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { Group, GroupService } from '@modules/group'; +import { GroupService } from '@modules/group'; import { Injectable } from '@nestjs/common'; -import { type User as UserEntity } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; -import { Course } from '../domain'; -import { CourseDoService } from '../service'; +import { CourseDoService, CourseSyncService } from '../service'; @Injectable() export class CourseSyncUc { constructor( private readonly authorizationService: AuthorizationService, private readonly courseService: CourseDoService, + private readonly courseSyncService: CourseSyncService, private readonly groupService: GroupService ) {} public async stopSynchronization(userId: EntityId, courseId: EntityId): Promise { - const course: Course = await this.courseService.findById(courseId); + const [course, user] = await Promise.all([ + this.courseService.findById(courseId), + this.authorizationService.getUserWithPermissions(userId), + ]); - const user: UserEntity = await this.authorizationService.getUserWithPermissions(userId); this.authorizationService.checkPermission( user, course, AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.stopSynchronization(course); + await this.courseSyncService.stopSynchronization(course); } public async startSynchronization(userId: string, courseId: string, groupId: string) { - const course: Course = await this.courseService.findById(courseId); - const group: Group = await this.groupService.findById(groupId); - const user: UserEntity = await this.authorizationService.getUserWithPermissions(userId); + const [course, group, user] = await Promise.all([ + this.courseService.findById(courseId), + this.groupService.findById(groupId), + this.authorizationService.getUserWithPermissions(userId), + ]); this.authorizationService.checkPermission( user, @@ -39,6 +42,6 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.startSynchronization(course, group); + await this.courseSyncService.startSynchronization(course, group); } } diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts index b83a48f9d27..9ef64798b77 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts @@ -1,39 +1,28 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ObjectId } from '@mikro-orm/mongodb'; -import { Group, GroupUser } from '@modules/group'; -import { CourseDoService } from '@modules/learnroom'; -import { Course } from '@modules/learnroom/domain'; -import { courseFactory } from '@modules/learnroom/testing'; -import { RoleDto, RoleService } from '@modules/role'; +import { Group } from '@modules/group'; +import { CourseSyncService } from '@modules/learnroom'; import { Test, TestingModule } from '@nestjs/testing'; -import { groupFactory, roleDtoFactory } from '@shared/testing'; +import { groupFactory } from '@shared/testing'; import { SchulconnexCourseSyncService } from './schulconnex-course-sync.service'; describe(SchulconnexCourseSyncService.name, () => { let module: TestingModule; let service: SchulconnexCourseSyncService; - - let courseService: DeepMocked; - let roleService: DeepMocked; + let courseSyncService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ providers: [ SchulconnexCourseSyncService, { - provide: CourseDoService, - useValue: createMock(), - }, - { - provide: RoleService, - useValue: createMock(), + provide: CourseSyncService, + useValue: createMock(), }, ], }).compile(); service = module.get(SchulconnexCourseSyncService); - courseService = module.get(CourseDoService); - roleService = module.get(RoleService); + courseSyncService = module.get(CourseSyncService); }); afterAll(async () => { @@ -47,184 +36,39 @@ describe(SchulconnexCourseSyncService.name, () => { describe('synchronizeCourseWithGroup', () => { describe('when synchronizing with a new group', () => { const setup = () => { - const course: Course = courseFactory.build(); - const studentId: string = new ObjectId().toHexString(); - const teacherId: string = new ObjectId().toHexString(); - const studentRoleId: string = new ObjectId().toHexString(); - const teacherRoleId: string = new ObjectId().toHexString(); - const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); - const teacherRole: RoleDto = roleDtoFactory.build({ id: teacherRoleId }); - const newGroup: Group = groupFactory.build({ - users: [ - { - userId: studentId, - roleId: studentRoleId, - }, - { - userId: teacherId, - roleId: teacherRoleId, - }, - ], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); + const newGroup: Group = groupFactory.build(); return { - course, newGroup, - studentId, - teacherId, }; }; it('should synchronize with the group', async () => { - const { course, newGroup, studentId, teacherId } = setup(); + const { newGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup); - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: newGroup.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [studentId], - teacherIds: [teacherId], - }), - ]); - }); - }); - - describe('when the course name is the same as the old group name', () => { - const setup = () => { - const course: Course = courseFactory.build({ name: 'Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - oldGroup, - }; - }; - - it('should synchronize the group name', async () => { - const { course, newGroup, oldGroup } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: newGroup.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }), - ]); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, undefined); }); }); - describe('when the course name is different from the old group name', () => { + describe('when synchronizing with a new group and an old group', () => { const setup = () => { - const course: Course = courseFactory.build({ name: 'Custom Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); + const newGroup: Group = groupFactory.build(); + const oldGroup: Group = groupFactory.build(); return { - course, newGroup, oldGroup, }; }; - it('should keep the old course name', async () => { - const { course, newGroup, oldGroup } = setup(); + it('should synchronize with the group', async () => { + const { newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: course.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }), - ]); - }); - }); - - describe('when the last teacher gets removed from a synced group', () => { - const setup = () => { - const studentUserId = new ObjectId().toHexString(); - const teacherUserId = new ObjectId().toHexString(); - const course: Course = courseFactory.build({ - teacherIds: [teacherUserId], - studentIds: [studentUserId], - syncedWithGroup: new ObjectId().toHexString(), - }); - const studentRoleId: string = new ObjectId().toHexString(); - const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); - const teacherRole: RoleDto = roleDtoFactory.build(); - const newGroup: Group = groupFactory.build({ - users: [ - new GroupUser({ - userId: studentUserId, - roleId: studentRoleId, - }), - ], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - teacherUserId, - }; - }; - - it('should keep the last teacher, remove all students', async () => { - const { course, newGroup, teacherUserId } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, newGroup); - - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: course.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [teacherUserId], - syncedWithGroup: course.syncedWithGroup, - }), - ]); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, oldGroup); }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts index c62fe52cb81..9f93d76325e 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts @@ -1,46 +1,12 @@ -import { Group, GroupUser } from '@modules/group'; -import { CourseDoService } from '@modules/learnroom'; -import { RoleDto, RoleService } from '@modules/role'; +import { Group } from '@modules/group'; +import { CourseSyncService } from '@modules/learnroom'; import { Injectable } from '@nestjs/common'; -import { RoleName } from '@shared/domain/interface'; -import { EntityId } from '@shared/domain/types'; -import { Course } from '@src/modules/learnroom/domain'; @Injectable() export class SchulconnexCourseSyncService { - constructor(private readonly courseService: CourseDoService, private readonly roleService: RoleService) {} + constructor(private readonly courseSyncService: CourseSyncService) {} async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { - const courses: Course[] = await this.courseService.findBySyncedGroup(newGroup); - - if (courses.length) { - const studentRole: RoleDto = await this.roleService.findByName(RoleName.STUDENT); - const teacherRole: RoleDto = await this.roleService.findByName(RoleName.TEACHER); - - courses.forEach((course: Course): void => { - if (!oldGroup || oldGroup.name === course.name) { - course.name = newGroup.name; - } - - course.startDate = newGroup.validPeriod?.from; - course.untilDate = newGroup.validPeriod?.until; - - const students: GroupUser[] = newGroup.users.filter( - (user: GroupUser): boolean => user.roleId === studentRole.id - ); - const teachers: GroupUser[] = newGroup.users.filter( - (user: GroupUser): boolean => user.roleId === teacherRole.id - ); - - if (teachers.length >= 1) { - course.students = students.map((user: GroupUser): EntityId => user.userId); - course.teachers = teachers.map((user: GroupUser): EntityId => user.userId); - } else { - course.students = []; - } - }); - - await this.courseService.saveAll(courses); - } + await this.courseSyncService.synchronizeCourseWithGroup(newGroup, oldGroup); } } From 749755535988cdfc90d1e60a072e087a5dde079e Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Thu, 10 Oct 2024 18:41:32 +0200 Subject: [PATCH 2/7] fix import --- .../src/modules/learnroom/service/course-sync.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 07a6140d7ac..955d1ffbd6f 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 @@ -1,6 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; -import { Group, GroupPeriod, GroupUser } from '@modules/group'; +import { Group, GroupUser } from '@modules/group'; import { RoleDto, RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; import { groupFactory, roleDtoFactory } from '@shared/testing'; From f18fde8aacbb7e475350678ca36c0c3c9e805ad4 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Fri, 11 Oct 2024 16:07:23 +0200 Subject: [PATCH 3/7] update (schulconnex)-course-sync services; update schulconnex strategy; update tests --- .../service/course-sync.service.spec.ts | 115 +++--------------- .../learnroom/service/course-sync.service.ts | 57 ++++----- .../oidc/schulconnex.strategy.spec.ts | 56 +-------- .../strategy/oidc/schulconnex.strategy.ts | 12 +- .../schulconnex-course-sync.service.spec.ts | 32 +---- .../schulconnex-course-sync.service.ts | 4 +- 6 files changed, 55 insertions(+), 221 deletions(-) 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 955d1ffbd6f..13aa55a506f 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 @@ -116,8 +116,7 @@ describe(CourseSyncService.name, () => { const { course, group, students, teachers } = setup(); await service.startSynchronization(course, group); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), syncedWithGroup: group.id, @@ -126,16 +125,15 @@ describe(CourseSyncService.name, () => { untilDate: group.validPeriod?.until, studentIds: students.map((student) => student.userId), teacherIds: teachers.map((teacher) => teacher.userId), - }) - ); + }), + ]); }); it('should set an empty list of students if no teachers are present', async () => { const { course, groupWithoutTeachers } = setup(); await service.startSynchronization(course, groupWithoutTeachers); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), syncedWithGroup: groupWithoutTeachers.id, @@ -144,8 +142,8 @@ describe(CourseSyncService.name, () => { untilDate: groupWithoutTeachers.validPeriod?.until, studentIds: [], teacherIds: [], - }) - ); + }), + ]); }); }); @@ -171,7 +169,7 @@ describe(CourseSyncService.name, () => { CourseAlreadySynchronizedLoggableException ); - expect(courseRepo.save).not.toHaveBeenCalled(); + expect(courseRepo.saveAll).not.toHaveBeenCalled(); }); }); }); @@ -215,8 +213,7 @@ describe(CourseSyncService.name, () => { const { course, newGroup, studentId, teacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), name: newGroup.name, @@ -225,90 +222,8 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [studentId], teacherIds: [teacherId], - }) - ); - }); - }); - - describe('when the course name is the same as the old group name', () => { - const setup = () => { - const course: Course = courseFactory.build({ name: 'Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - oldGroup, - }; - }; - - it('should synchronize the group name', async () => { - const { course, newGroup, oldGroup } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - name: newGroup.name, - syncedWithGroup: newGroup.id, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }) - ); - }); - }); - - describe('when the course name is different from the old group name', () => { - const setup = () => { - const course: Course = courseFactory.build({ name: 'Custom Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - oldGroup, - }; - }; - - it('should keep the old course name', async () => { - const { course, newGroup, oldGroup } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - name: course.name, - syncedWithGroup: newGroup.id, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }) - ); + }), + ]); }); }); @@ -347,19 +262,19 @@ describe(CourseSyncService.name, () => { it('should keep the last teacher, remove all students', async () => { const { course, newGroup, teacherUserId } = setup(); - await service.synchronizeCourseWithGroup(newGroup, newGroup); + await service.synchronizeCourseWithGroup(newGroup); - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), - name: course.name, + name: newGroup.name, startDate: newGroup.validPeriod?.from, untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [teacherUserId], syncedWithGroup: course.syncedWithGroup, - }) - ); + }), + ]); }); }); }); 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 10be631edc8..568902eb2c4 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -23,7 +23,7 @@ export class CourseSyncService { throw new CourseAlreadySynchronizedLoggableException(course.id); } - await this.synchronize(course, group); + await this.synchronize([course], group); } public async stopSynchronization(course: Course): Promise { @@ -36,41 +36,38 @@ export class CourseSyncService { await this.courseRepo.save(course); } - public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { - const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); + public async synchronizeCourseWithGroup(group: Group): Promise { + const courses: Course[] = await this.courseRepo.findBySyncedGroup(group); + await this.synchronize(courses, group); + } + private async synchronize(courses: Course[], group: Group): Promise { if (courses.length) { - await Promise.all( - courses.map(async (course) => { - await this.synchronize(course, newGroup, oldGroup); - }) - ); - } - } + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); - private async synchronize(course: Course, group: Group, oldGroup?: Group): Promise { - course.syncedWithGroup = group.id; - if (!oldGroup || oldGroup.name === course.name) { - course.name = group.name; - } - course.startDate = group.validPeriod?.from; - course.untilDate = group.validPeriod?.until; + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); - const [studentRole, teacherRole] = await Promise.all([ - this.roleService.findByName(RoleName.STUDENT), - this.roleService.findByName(RoleName.TEACHER), - ]); + const toBeSyncedCourses = courses.map((course) => { + course.syncedWithGroup = group.id; + course.name = group.name; + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; - const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); - const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; + } - if (teachers.length >= 1) { - course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); - course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); - } else { - course.students = []; - } + return course; + }); - await this.courseRepo.save(course); + await this.courseRepo.saveAll(toBeSyncedCourses); + } } } diff --git a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts index 30408619f57..964eaf6b458 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts @@ -385,57 +385,7 @@ describe(SchulconnexProvisioningStrategy.name, () => { }); }); - describe('when an existing group gets provisioned', () => { - const setup = () => { - config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = true; - config.FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED = true; - - const externalUserId = 'externalUserId'; - const externalGroups: ExternalGroupDto[] = externalGroupDtoFactory.buildList(2); - const oauthData: OauthDataDto = new OauthDataDto({ - system: new ProvisioningSystemDto({ - systemId: new ObjectId().toHexString(), - provisioningStrategy: SystemProvisioningStrategy.OIDC, - }), - externalSchool: externalSchoolDtoFactory.build(), - externalUser: new ExternalUserDto({ - externalId: externalUserId, - }), - externalGroups, - }); - - const user: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).build({ - externalId: externalUserId, - }); - const existingGroup: Group = groupFactory.build(); - const updatedGroup: Group = groupFactory.build(); - - schulconnexUserProvisioningService.provisionExternalUser.mockResolvedValueOnce(user); - schulconnexGroupProvisioningService.removeExternalGroupsAndAffiliation.mockResolvedValueOnce([]); - schulconnexGroupProvisioningService.filterExternalGroups.mockResolvedValueOnce(externalGroups); - groupService.findByExternalSource.mockResolvedValueOnce(existingGroup); - schulconnexGroupProvisioningService.provisionExternalGroup.mockResolvedValueOnce(updatedGroup); - - return { - oauthData, - existingGroup, - updatedGroup, - }; - }; - - it('should synchronize all linked courses with the group', async () => { - const { oauthData, updatedGroup, existingGroup } = setup(); - - await strategy.apply(oauthData); - - expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith( - updatedGroup, - existingGroup - ); - }); - }); - - describe('when a new group is provisioned', () => { + describe('when a group is provisioned', () => { const setup = () => { config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = true; config.FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED = true; @@ -476,7 +426,7 @@ describe(SchulconnexProvisioningStrategy.name, () => { await strategy.apply(oauthData); - expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(updatedGroup, undefined); + expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(updatedGroup); }); }); @@ -520,7 +470,7 @@ describe(SchulconnexProvisioningStrategy.name, () => { await strategy.apply(oauthData); - expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group, group); + expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts index 007b70319bc..66f3117fe7c 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts @@ -74,11 +74,6 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate const groupProvisioningPromises: Promise[] = groups.map( async (externalGroup: ExternalGroupDto): Promise => { - const existingGroup: Group | null = await this.groupService.findByExternalSource( - externalGroup.externalId, - data.system.systemId - ); - const provisionedGroup: Group | null = await this.schulconnexGroupProvisioningService.provisionExternalGroup( externalGroup, data.externalSchool, @@ -86,10 +81,7 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate ); if (this.configService.get('FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED') && provisionedGroup) { - await this.schulconnexCourseSyncService.synchronizeCourseWithGroup( - provisionedGroup, - existingGroup ?? undefined - ); + await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(provisionedGroup); } } ); @@ -109,7 +101,7 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate if (this.configService.get('FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED')) { const courseSyncPromises: Promise[] = removedFromGroups.map( async (removedFromGroup: Group): Promise => { - await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(removedFromGroup, removedFromGroup); + await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(removedFromGroup); } ); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts index 9ef64798b77..91e2b4ee6cb 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts @@ -34,41 +34,21 @@ describe(SchulconnexCourseSyncService.name, () => { }); describe('synchronizeCourseWithGroup', () => { - describe('when synchronizing with a new group', () => { + describe('when synchronizing with a group', () => { const setup = () => { - const newGroup: Group = groupFactory.build(); + const group: Group = groupFactory.build(); return { - newGroup, + group, }; }; it('should synchronize with the group', async () => { - const { newGroup } = setup(); + const { group } = setup(); - await service.synchronizeCourseWithGroup(newGroup); + await service.synchronizeCourseWithGroup(group); - expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, undefined); - }); - }); - - describe('when synchronizing with a new group and an old group', () => { - const setup = () => { - const newGroup: Group = groupFactory.build(); - const oldGroup: Group = groupFactory.build(); - - return { - newGroup, - oldGroup, - }; - }; - - it('should synchronize with the group', async () => { - const { newGroup, oldGroup } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, oldGroup); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group); }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts index 9f93d76325e..77128934209 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts @@ -6,7 +6,7 @@ import { Injectable } from '@nestjs/common'; export class SchulconnexCourseSyncService { constructor(private readonly courseSyncService: CourseSyncService) {} - async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { - await this.courseSyncService.synchronizeCourseWithGroup(newGroup, oldGroup); + async synchronizeCourseWithGroup(group: Group): Promise { + await this.courseSyncService.synchronizeCourseWithGroup(group); } } From c800c0b97f4c793daa6be3e09c6f69fc7a9eaab3 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Mon, 14 Oct 2024 12:07:28 +0200 Subject: [PATCH 4/7] Revert "update (schulconnex)-course-sync services; update schulconnex strategy; update tests" This reverts commit f18fde8aacbb7e475350678ca36c0c3c9e805ad4. --- .../service/course-sync.service.spec.ts | 115 +++++++++++++++--- .../learnroom/service/course-sync.service.ts | 57 +++++---- .../oidc/schulconnex.strategy.spec.ts | 56 ++++++++- .../strategy/oidc/schulconnex.strategy.ts | 12 +- .../schulconnex-course-sync.service.spec.ts | 32 ++++- .../schulconnex-course-sync.service.ts | 4 +- 6 files changed, 221 insertions(+), 55 deletions(-) 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 13aa55a506f..955d1ffbd6f 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 @@ -116,7 +116,8 @@ describe(CourseSyncService.name, () => { const { course, group, students, teachers } = setup(); await service.startSynchronization(course, group); - expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + + expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), syncedWithGroup: group.id, @@ -125,15 +126,16 @@ describe(CourseSyncService.name, () => { untilDate: group.validPeriod?.until, studentIds: students.map((student) => student.userId), teacherIds: teachers.map((teacher) => teacher.userId), - }), - ]); + }) + ); }); it('should set an empty list of students if no teachers are present', async () => { const { course, groupWithoutTeachers } = setup(); await service.startSynchronization(course, groupWithoutTeachers); - expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + + expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), syncedWithGroup: groupWithoutTeachers.id, @@ -142,8 +144,8 @@ describe(CourseSyncService.name, () => { untilDate: groupWithoutTeachers.validPeriod?.until, studentIds: [], teacherIds: [], - }), - ]); + }) + ); }); }); @@ -169,7 +171,7 @@ describe(CourseSyncService.name, () => { CourseAlreadySynchronizedLoggableException ); - expect(courseRepo.saveAll).not.toHaveBeenCalled(); + expect(courseRepo.save).not.toHaveBeenCalled(); }); }); }); @@ -213,7 +215,8 @@ describe(CourseSyncService.name, () => { const { course, newGroup, studentId, teacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup); - expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + + expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), name: newGroup.name, @@ -222,8 +225,90 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [studentId], teacherIds: [teacherId], - }), - ]); + }) + ); + }); + }); + + describe('when the course name is the same as the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should synchronize the group name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: newGroup.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when the course name is different from the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Custom Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should keep the old course name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: course.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); }); }); @@ -262,19 +347,19 @@ describe(CourseSyncService.name, () => { it('should keep the last teacher, remove all students', async () => { const { course, newGroup, teacherUserId } = setup(); - await service.synchronizeCourseWithGroup(newGroup); + await service.synchronizeCourseWithGroup(newGroup, newGroup); - expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), - name: newGroup.name, + name: course.name, startDate: newGroup.validPeriod?.from, untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [teacherUserId], syncedWithGroup: course.syncedWithGroup, - }), - ]); + }) + ); }); }); }); 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 568902eb2c4..10be631edc8 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -23,7 +23,7 @@ export class CourseSyncService { throw new CourseAlreadySynchronizedLoggableException(course.id); } - await this.synchronize([course], group); + await this.synchronize(course, group); } public async stopSynchronization(course: Course): Promise { @@ -36,38 +36,41 @@ export class CourseSyncService { await this.courseRepo.save(course); } - public async synchronizeCourseWithGroup(group: Group): Promise { - const courses: Course[] = await this.courseRepo.findBySyncedGroup(group); - await this.synchronize(courses, group); - } + public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { + const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); - private async synchronize(courses: Course[], group: Group): Promise { if (courses.length) { - const [studentRole, teacherRole] = await Promise.all([ - this.roleService.findByName(RoleName.STUDENT), - this.roleService.findByName(RoleName.TEACHER), - ]); - - const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); - const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + await Promise.all( + courses.map(async (course) => { + await this.synchronize(course, newGroup, oldGroup); + }) + ); + } + } - const toBeSyncedCourses = courses.map((course) => { - course.syncedWithGroup = group.id; - course.name = group.name; - course.startDate = group.validPeriod?.from; - course.untilDate = group.validPeriod?.until; + private async synchronize(course: Course, group: Group, oldGroup?: Group): Promise { + course.syncedWithGroup = group.id; + if (!oldGroup || oldGroup.name === course.name) { + course.name = group.name; + } + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; - if (teachers.length >= 1) { - course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); - course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); - } else { - course.students = []; - } + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); - return course; - }); + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); - await this.courseRepo.saveAll(toBeSyncedCourses); + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; } + + await this.courseRepo.save(course); } } diff --git a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts index 964eaf6b458..30408619f57 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.spec.ts @@ -385,7 +385,57 @@ describe(SchulconnexProvisioningStrategy.name, () => { }); }); - describe('when a group is provisioned', () => { + describe('when an existing group gets provisioned', () => { + const setup = () => { + config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = true; + config.FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED = true; + + const externalUserId = 'externalUserId'; + const externalGroups: ExternalGroupDto[] = externalGroupDtoFactory.buildList(2); + const oauthData: OauthDataDto = new OauthDataDto({ + system: new ProvisioningSystemDto({ + systemId: new ObjectId().toHexString(), + provisioningStrategy: SystemProvisioningStrategy.OIDC, + }), + externalSchool: externalSchoolDtoFactory.build(), + externalUser: new ExternalUserDto({ + externalId: externalUserId, + }), + externalGroups, + }); + + const user: UserDO = userDoFactory.withRoles([{ id: 'roleId', name: RoleName.USER }]).build({ + externalId: externalUserId, + }); + const existingGroup: Group = groupFactory.build(); + const updatedGroup: Group = groupFactory.build(); + + schulconnexUserProvisioningService.provisionExternalUser.mockResolvedValueOnce(user); + schulconnexGroupProvisioningService.removeExternalGroupsAndAffiliation.mockResolvedValueOnce([]); + schulconnexGroupProvisioningService.filterExternalGroups.mockResolvedValueOnce(externalGroups); + groupService.findByExternalSource.mockResolvedValueOnce(existingGroup); + schulconnexGroupProvisioningService.provisionExternalGroup.mockResolvedValueOnce(updatedGroup); + + return { + oauthData, + existingGroup, + updatedGroup, + }; + }; + + it('should synchronize all linked courses with the group', async () => { + const { oauthData, updatedGroup, existingGroup } = setup(); + + await strategy.apply(oauthData); + + expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith( + updatedGroup, + existingGroup + ); + }); + }); + + describe('when a new group is provisioned', () => { const setup = () => { config.FEATURE_SANIS_GROUP_PROVISIONING_ENABLED = true; config.FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED = true; @@ -426,7 +476,7 @@ describe(SchulconnexProvisioningStrategy.name, () => { await strategy.apply(oauthData); - expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(updatedGroup); + expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(updatedGroup, undefined); }); }); @@ -470,7 +520,7 @@ describe(SchulconnexProvisioningStrategy.name, () => { await strategy.apply(oauthData); - expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group); + expect(schulconnexCourseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group, group); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts index 66f3117fe7c..007b70319bc 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/schulconnex.strategy.ts @@ -74,6 +74,11 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate const groupProvisioningPromises: Promise[] = groups.map( async (externalGroup: ExternalGroupDto): Promise => { + const existingGroup: Group | null = await this.groupService.findByExternalSource( + externalGroup.externalId, + data.system.systemId + ); + const provisionedGroup: Group | null = await this.schulconnexGroupProvisioningService.provisionExternalGroup( externalGroup, data.externalSchool, @@ -81,7 +86,10 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate ); if (this.configService.get('FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED') && provisionedGroup) { - await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(provisionedGroup); + await this.schulconnexCourseSyncService.synchronizeCourseWithGroup( + provisionedGroup, + existingGroup ?? undefined + ); } } ); @@ -101,7 +109,7 @@ export abstract class SchulconnexProvisioningStrategy extends ProvisioningStrate if (this.configService.get('FEATURE_SCHULCONNEX_COURSE_SYNC_ENABLED')) { const courseSyncPromises: Promise[] = removedFromGroups.map( async (removedFromGroup: Group): Promise => { - await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(removedFromGroup); + await this.schulconnexCourseSyncService.synchronizeCourseWithGroup(removedFromGroup, removedFromGroup); } ); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts index 91e2b4ee6cb..9ef64798b77 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts @@ -34,21 +34,41 @@ describe(SchulconnexCourseSyncService.name, () => { }); describe('synchronizeCourseWithGroup', () => { - describe('when synchronizing with a group', () => { + describe('when synchronizing with a new group', () => { const setup = () => { - const group: Group = groupFactory.build(); + const newGroup: Group = groupFactory.build(); return { - group, + newGroup, }; }; it('should synchronize with the group', async () => { - const { group } = setup(); + const { newGroup } = setup(); - await service.synchronizeCourseWithGroup(group); + await service.synchronizeCourseWithGroup(newGroup); - expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(group); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, undefined); + }); + }); + + describe('when synchronizing with a new group and an old group', () => { + const setup = () => { + const newGroup: Group = groupFactory.build(); + const oldGroup: Group = groupFactory.build(); + + return { + newGroup, + oldGroup, + }; + }; + + it('should synchronize with the group', async () => { + const { newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, oldGroup); }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts index 77128934209..9f93d76325e 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts @@ -6,7 +6,7 @@ import { Injectable } from '@nestjs/common'; export class SchulconnexCourseSyncService { constructor(private readonly courseSyncService: CourseSyncService) {} - async synchronizeCourseWithGroup(group: Group): Promise { - await this.courseSyncService.synchronizeCourseWithGroup(group); + async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { + await this.courseSyncService.synchronizeCourseWithGroup(newGroup, oldGroup); } } From 20b3535f23b38a3d0bd9da35a4d40e46d4e9d1bc Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Mon, 14 Oct 2024 15:06:50 +0200 Subject: [PATCH 5/7] update course sync service --- .../service/course-sync.service.spec.ts | 44 +++++++-------- .../learnroom/service/course-sync.service.ts | 55 +++++++++---------- 2 files changed, 46 insertions(+), 53 deletions(-) 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 955d1ffbd6f..bb7ec125941 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 @@ -117,7 +117,7 @@ describe(CourseSyncService.name, () => { await service.startSynchronization(course, group); - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), syncedWithGroup: group.id, @@ -126,8 +126,8 @@ describe(CourseSyncService.name, () => { untilDate: group.validPeriod?.until, studentIds: students.map((student) => student.userId), teacherIds: teachers.map((teacher) => teacher.userId), - }) - ); + }), + ]); }); it('should set an empty list of students if no teachers are present', async () => { @@ -135,7 +135,7 @@ describe(CourseSyncService.name, () => { await service.startSynchronization(course, groupWithoutTeachers); - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), syncedWithGroup: groupWithoutTeachers.id, @@ -144,8 +144,8 @@ describe(CourseSyncService.name, () => { untilDate: groupWithoutTeachers.validPeriod?.until, studentIds: [], teacherIds: [], - }) - ); + }), + ]); }); }); @@ -171,7 +171,7 @@ describe(CourseSyncService.name, () => { CourseAlreadySynchronizedLoggableException ); - expect(courseRepo.save).not.toHaveBeenCalled(); + expect(courseRepo.saveAll).not.toHaveBeenCalled(); }); }); }); @@ -179,7 +179,6 @@ describe(CourseSyncService.name, () => { describe('synchronizeCourseWithGroup', () => { describe('when synchronizing with a new group', () => { const setup = () => { - const course: Course = courseFactory.build(); const studentId: string = new ObjectId().toHexString(); const teacherId: string = new ObjectId().toHexString(); const studentRoleId: string = new ObjectId().toHexString(); @@ -198,6 +197,7 @@ describe(CourseSyncService.name, () => { }, ], }); + const course: Course = courseFactory.build({ syncedWithGroup: newGroup.id }); courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); roleService.findByName.mockResolvedValueOnce(studentRole); @@ -216,17 +216,16 @@ describe(CourseSyncService.name, () => { await service.synchronizeCourseWithGroup(newGroup); - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), - name: newGroup.name, syncedWithGroup: newGroup.id, startDate: newGroup.validPeriod?.from, untilDate: newGroup.validPeriod?.until, studentIds: [studentId], teacherIds: [teacherId], - }) - ); + }), + ]); }); }); @@ -256,8 +255,7 @@ describe(CourseSyncService.name, () => { const { course, newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), name: newGroup.name, @@ -266,8 +264,8 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [], - }) - ); + }), + ]); }); }); @@ -297,8 +295,7 @@ describe(CourseSyncService.name, () => { const { course, newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), name: course.name, @@ -307,8 +304,8 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [], - }) - ); + }), + ]); }); }); @@ -348,8 +345,7 @@ describe(CourseSyncService.name, () => { const { course, newGroup, teacherUserId } = setup(); await service.synchronizeCourseWithGroup(newGroup, newGroup); - - expect(courseRepo.save).toHaveBeenCalledWith( + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ ...course.getProps(), name: course.name, @@ -358,8 +354,8 @@ describe(CourseSyncService.name, () => { studentIds: [], teacherIds: [teacherUserId], syncedWithGroup: course.syncedWithGroup, - }) - ); + }), + ]); }); }); }); 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 10be631edc8..46d589ff5b9 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -23,7 +23,7 @@ export class CourseSyncService { throw new CourseAlreadySynchronizedLoggableException(course.id); } - await this.synchronize(course, group); + await this.synchronize([course], group); } public async stopSynchronization(course: Course): Promise { @@ -38,39 +38,36 @@ export class CourseSyncService { public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); - - if (courses.length) { - await Promise.all( - courses.map(async (course) => { - await this.synchronize(course, newGroup, oldGroup); - }) - ); - } + await this.synchronize(courses, newGroup, oldGroup); } - private async synchronize(course: Course, group: Group, oldGroup?: Group): Promise { - course.syncedWithGroup = group.id; - if (!oldGroup || oldGroup.name === course.name) { - course.name = group.name; - } - course.startDate = group.validPeriod?.from; - course.untilDate = group.validPeriod?.until; + private async synchronize(courses: Course[], group: Group, oldGroup?: Group): Promise { + if (courses.length) { + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); - const [studentRole, teacherRole] = await Promise.all([ - this.roleService.findByName(RoleName.STUDENT), - this.roleService.findByName(RoleName.TEACHER), - ]); + const coursesToSync = courses.map((course) => { + course.syncedWithGroup = group.id; + if (oldGroup && oldGroup.name === course.name) { + course.name = group.name; + } + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; - const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); - const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + if (teachers.length >= 1) { + course.students = students.map((user: GroupUser): EntityId => user.userId); + course.teachers = teachers.map((user: GroupUser): EntityId => user.userId); + } else { + course.students = []; + } + return course; + }); - if (teachers.length >= 1) { - course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); - course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); - } else { - course.students = []; + await this.courseRepo.saveAll(coursesToSync); } - - await this.courseRepo.save(course); } } From 7b354626dc742363def5b15383b99bba4f7d7631 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Tue, 15 Oct 2024 15:10:53 +0200 Subject: [PATCH 6/7] update course sync service - rm course groups and classes on sync --- .../src/modules/learnroom/domain/do/course.ts | 12 ++++ .../service/course-sync.service.spec.ts | 69 ++++++++++++++++--- .../learnroom/service/course-sync.service.ts | 4 ++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index a6e8eb84bce..64d6fee885d 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -61,14 +61,26 @@ export class Course extends DomainObject { this.props.teacherIds = value; } + set substitutionTeachers(value: EntityId[]) { + this.props.substitutionTeacherIds = value; + } + get substitutionTeachers(): EntityId[] { return this.props.substitutionTeacherIds; } + set classes(value: EntityId[]) { + this.props.classIds = value; + } + get classes(): EntityId[] { return this.props.classIds; } + set groups(value: EntityId[]) { + this.props.groupIds = value; + } + get groups(): EntityId[] { return this.props.groupIds; } 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 bb7ec125941..8afbcd87bb6 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 @@ -93,8 +93,12 @@ describe(CourseSyncService.name, () => { describe('startSynchronization', () => { describe('when a course is not synchronized with a group', () => { const setup = () => { - const course: Course = courseFactory.build(); - + const teacherId = new ObjectId().toHexString(); + const course: Course = courseFactory.build({ + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + substitutionTeacherIds: [teacherId], + }); const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; @@ -109,11 +113,12 @@ describe(CourseSyncService.name, () => { students, teachers, groupWithoutTeachers, + teacherId, }; }; it('should save a course with synchronized group, students, and teachers', async () => { - const { course, group, students, teachers } = setup(); + const { course, group, students, teachers, teacherId } = setup(); await service.startSynchronization(course, group); @@ -126,12 +131,15 @@ describe(CourseSyncService.name, () => { untilDate: group.validPeriod?.until, studentIds: students.map((student) => student.userId), teacherIds: teachers.map((teacher) => teacher.userId), + classIds: [], + groupIds: [], + substitutionTeacherIds: [teacherId], }), ]); }); it('should set an empty list of students if no teachers are present', async () => { - const { course, groupWithoutTeachers } = setup(); + const { course, groupWithoutTeachers, teacherId } = setup(); await service.startSynchronization(course, groupWithoutTeachers); @@ -144,6 +152,9 @@ describe(CourseSyncService.name, () => { untilDate: groupWithoutTeachers.validPeriod?.until, studentIds: [], teacherIds: [], + classIds: [], + groupIds: [], + substitutionTeacherIds: [teacherId], }), ]); }); @@ -197,7 +208,13 @@ describe(CourseSyncService.name, () => { }, ], }); - const course: Course = courseFactory.build({ syncedWithGroup: newGroup.id }); + const substituteTeacherId = new ObjectId().toHexString(); + const course: Course = courseFactory.build({ + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + substitutionTeacherIds: [substituteTeacherId], + syncedWithGroup: newGroup.id, + }); courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); roleService.findByName.mockResolvedValueOnce(studentRole); @@ -208,11 +225,12 @@ describe(CourseSyncService.name, () => { newGroup, studentId, teacherId, + substituteTeacherId, }; }; it('should synchronize with the group', async () => { - const { course, newGroup, studentId, teacherId } = setup(); + const { course, newGroup, studentId, teacherId, substituteTeacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup); @@ -224,6 +242,9 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [studentId], teacherIds: [teacherId], + classIds: [], + groupIds: [], + substitutionTeacherIds: [substituteTeacherId], }), ]); }); @@ -231,7 +252,13 @@ describe(CourseSyncService.name, () => { describe('when the course name is the same as the old group name', () => { const setup = () => { - const course: Course = courseFactory.build({ name: 'Course Name' }); + 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(); const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); @@ -248,11 +275,12 @@ describe(CourseSyncService.name, () => { course, newGroup, oldGroup, + substituteTeacherId, }; }; it('should synchronize the group name', async () => { - const { course, newGroup, oldGroup } = setup(); + const { course, newGroup, oldGroup, substituteTeacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -264,6 +292,9 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [], + classIds: [], + groupIds: [], + substitutionTeacherIds: [substituteTeacherId], }), ]); }); @@ -271,7 +302,13 @@ describe(CourseSyncService.name, () => { describe('when the course name is different from the old group name', () => { const setup = () => { - const course: Course = courseFactory.build({ name: 'Custom Course Name' }); + 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(); const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); @@ -288,11 +325,12 @@ describe(CourseSyncService.name, () => { course, newGroup, oldGroup, + substituteTeacherId, }; }; it('should keep the old course name', async () => { - const { course, newGroup, oldGroup } = setup(); + const { course, newGroup, oldGroup, substituteTeacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -304,6 +342,9 @@ describe(CourseSyncService.name, () => { untilDate: newGroup.validPeriod?.until, studentIds: [], teacherIds: [], + classIds: [], + groupIds: [], + substitutionTeacherIds: [substituteTeacherId], }), ]); }); @@ -311,6 +352,7 @@ describe(CourseSyncService.name, () => { describe('when the last teacher gets removed from a synced group', () => { const setup = () => { + const substituteTeacherId = new ObjectId().toHexString(); const studentUserId = new ObjectId().toHexString(); const teacherUserId = new ObjectId().toHexString(); const studentRoleId: string = new ObjectId().toHexString(); @@ -329,6 +371,7 @@ describe(CourseSyncService.name, () => { teacherIds: [teacherUserId], studentIds: [studentUserId], syncedWithGroup: newGroup.id, + substitutionTeacherIds: [substituteTeacherId], }); courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); roleService.findByName.mockResolvedValueOnce(studentRole); @@ -338,11 +381,12 @@ describe(CourseSyncService.name, () => { course, newGroup, teacherUserId, + substituteTeacherId, }; }; it('should keep the last teacher, remove all students', async () => { - const { course, newGroup, teacherUserId } = setup(); + const { course, newGroup, teacherUserId, substituteTeacherId } = setup(); await service.synchronizeCourseWithGroup(newGroup, newGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -354,6 +398,9 @@ describe(CourseSyncService.name, () => { studentIds: [], teacherIds: [teacherUserId], syncedWithGroup: course.syncedWithGroup, + classIds: [], + groupIds: [], + substitutionTeacherIds: [substituteTeacherId], }), ]); }); 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 46d589ff5b9..92c6c96e1d7 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -64,6 +64,10 @@ export class CourseSyncService { } else { course.students = []; } + + course.classes = []; + course.groups = []; + return course; }); From b9ec274048ab025f46688d8db666c7e669244567 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Tue, 15 Oct 2024 16:30:49 +0200 Subject: [PATCH 7/7] update - rm unused setter from course do --- apps/server/src/modules/learnroom/domain/do/course.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index 64d6fee885d..e95fd85b27e 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -61,10 +61,6 @@ export class Course extends DomainObject { this.props.teacherIds = value; } - set substitutionTeachers(value: EntityId[]) { - this.props.substitutionTeacherIds = value; - } - get substitutionTeachers(): EntityId[] { return this.props.substitutionTeacherIds; }