From 54336ad9f0c5b8a9837a1d3dc52e57ba0a242b34 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 14:03:21 +0200 Subject: [PATCH 1/5] update course sync --- .../learnroom/service/course-do.service.ts | 19 ++++++++-- .../learnroom/uc/course-sync.uc.spec.ts | 36 ++++++++++++++++--- .../modules/learnroom/uc/course-sync.uc.ts | 27 ++++++++++---- 3 files changed, 68 insertions(+), 14 deletions(-) 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..c5590db6bee 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.ts @@ -1,5 +1,5 @@ import { AuthorizationLoaderServiceGeneric } from '@modules/authorization'; -import { type Group } from '@modules/group'; +import { GroupUser, type Group } from '@modules/group'; import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; @@ -45,12 +45,27 @@ export class CourseDoService implements AuthorizationLoaderServiceGeneric { + public async startSynchronization( + course: Course, + group: Group, + students: GroupUser[], + teachers: GroupUser[] + ): Promise { if (course.syncedWithGroup) { throw new CourseAlreadySynchronizedLoggableException(course.id); } course.syncedWithGroup = group.id; + 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 = []; + } await this.courseRepo.save(course); } 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..ad386434cd5 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 @@ -1,9 +1,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { GroupService } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; -import { Permission } from '@shared/domain/interface'; -import { groupFactory, setupEntities, userFactory } from '@shared/testing'; +import { Permission, RoleName } from '@shared/domain/interface'; +import { groupFactory, roleDtoFactory, setupEntities, userFactory } from '@shared/testing'; import { CourseDoService } from '../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 roleService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -32,6 +34,10 @@ describe(CourseSyncUc.name, () => { provide: GroupService, useValue: createMock(), }, + { + provide: RoleService, + useValue: createMock(), + }, ], }).compile(); @@ -39,6 +45,7 @@ describe(CourseSyncUc.name, () => { authorizationService = module.get(AuthorizationService); courseService = module.get(CourseDoService); groupService = module.get(GroupService); + roleService = module.get(RoleService); await setupEntities(); }); @@ -93,15 +100,30 @@ describe(CourseSyncUc.name, () => { const user = userFactory.buildWithId(); const course = courseFactory.build(); const group = groupFactory.build(); + const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); + const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + + group.users = [ + { roleId: 'student-role-id', userId: 'student-user-id' }, + { roleId: 'teacher-role-id', userId: 'teacher-user-id' }, + ]; + + const students = group.users.filter((groupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser) => groupUser.roleId === teacherRole.id); courseService.findById.mockResolvedValueOnce(course); groupService.findById.mockResolvedValueOnce(group); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); return { user, course, group, + studentRole, + teacherRole, + students, + teachers, }; }; @@ -119,14 +141,18 @@ describe(CourseSyncUc.name, () => { ); }); - it('should start the synchronization', async () => { - const { user, course, group } = setup(); + it('should start the synchronization with correct roles', async () => { + const { user, course, group, students, teachers } = setup(); await uc.startSynchronization(user.id, course.id, group.id); + expect(courseService.findById).toHaveBeenCalledWith(course.id); expect(groupService.findById).toHaveBeenCalledWith(group.id); - expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.STUDENT); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.TEACHER); + + expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group, students, teachers); }); }); }); 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..0dd5675da87 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,8 +1,10 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { Group, GroupService } from '@modules/group'; +import { Group, GroupService, GroupUser } from '@modules/group'; import { Injectable } from '@nestjs/common'; +import { RoleDto, RoleService } from '@modules/role'; + import { type User as UserEntity } from '@shared/domain/entity'; -import { Permission } from '@shared/domain/interface'; +import { Permission, RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { Course } from '../domain'; import { CourseDoService } from '../service'; @@ -12,7 +14,8 @@ export class CourseSyncUc { constructor( private readonly authorizationService: AuthorizationService, private readonly courseService: CourseDoService, - private readonly groupService: GroupService + private readonly groupService: GroupService, + private readonly roleService: RoleService ) {} public async stopSynchronization(userId: EntityId, courseId: EntityId): Promise { @@ -29,9 +32,11 @@ export class CourseSyncUc { } 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 +44,14 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.startSynchronization(course, group); + 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 this.courseService.startSynchronization(course, group, students, teachers); } } From 9a077b2980f7c8e3822f769bc83ded497ae9520f Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 14:14:22 +0200 Subject: [PATCH 2/5] clean up imports --- apps/server/src/modules/learnroom/uc/course-sync.uc.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 0dd5675da87..770268fe882 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,7 +1,7 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { Group, GroupService, GroupUser } from '@modules/group'; +import { GroupService, GroupUser } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Injectable } from '@nestjs/common'; -import { RoleDto, RoleService } from '@modules/role'; import { type User as UserEntity } from '@shared/domain/entity'; import { Permission, RoleName } from '@shared/domain/interface'; From 8872973a669c119da207d808e4472707cd3737eb Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 14:48:20 +0200 Subject: [PATCH 3/5] update course do service tests --- .../service/course-do.service.spec.ts | 59 ++++++++++++++++--- 1 file changed, 51 insertions(+), 8 deletions(-) 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..c81191242fa 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 @@ -1,6 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; -import { Group } from '@modules/group'; +import { Group, GroupUser } from '@modules/group'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { Page } from '@shared/domain/domainobject'; @@ -185,44 +185,87 @@ describe(CourseDoService.name, () => { const setup = () => { const course: Course = courseFactory.build(); 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 save a course with a synchronized group', async () => { - const { course, group } = setup(); + it('should save a course with synchronized group, students, and teachers', async () => { + const { course, group, students, teachers } = setup(); - await service.startSynchronization(course, group); + await service.startSynchronization(course, group, students, teachers); expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), syncedWithGroup: group.id, + name: group.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, group, students } = setup(); + const teachers: GroupUser[] = []; // No teachers + + await service.startSynchronization(course, group, students, teachers); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: group.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: [], + teacherIds: [], }) ); }); }); - describe('when a course is synchronized with a group', () => { + 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 an unprocessable entity exception', async () => { - const { course, group } = setup(); - await expect(service.startSynchronization(course, group)).rejects.toThrow( + it('should throw a CourseAlreadySynchronizedLoggableException', async () => { + const { course, group, students, teachers } = setup(); + + await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( CourseAlreadySynchronizedLoggableException ); }); + + it('should not attempt to save the course if already synchronized', async () => { + const { course, group, students, teachers } = setup(); + + try { + await service.startSynchronization(course, group, students, teachers); + } catch (error) { + expect(courseRepo.save).not.toHaveBeenCalled(); + } + }); }); }); From 6e004a32602d039095dc0b3789b53e18000cc28a Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 15:09:23 +0200 Subject: [PATCH 4/5] update course do service tests --- .../learnroom/service/course-do.service.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 c81191242fa..b4728e61f29 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 @@ -260,11 +260,11 @@ describe(CourseDoService.name, () => { it('should not attempt to save the course if already synchronized', async () => { const { course, group, students, teachers } = setup(); - try { - await service.startSynchronization(course, group, students, teachers); - } catch (error) { - expect(courseRepo.save).not.toHaveBeenCalled(); - } + await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( + CourseAlreadySynchronizedLoggableException + ); + + expect(courseRepo.save).not.toHaveBeenCalled(); }); }); }); From 3e4a154c84d93182928092ef84b75fb19efc9598 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 16:16:48 +0200 Subject: [PATCH 5/5] update course do service tests --- .../modules/learnroom/service/course-do.service.spec.ts | 8 -------- 1 file changed, 8 deletions(-) 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 b4728e61f29..9f9591c5c19 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 @@ -255,14 +255,6 @@ describe(CourseDoService.name, () => { await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( CourseAlreadySynchronizedLoggableException ); - }); - - it('should not attempt to save the course if already synchronized', async () => { - const { course, group, students, teachers } = setup(); - - await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( - CourseAlreadySynchronizedLoggableException - ); expect(courseRepo.save).not.toHaveBeenCalled(); });