diff --git a/apps/server/src/modules/learnroom/controller/api-test/course.api.spec.ts b/apps/server/src/modules/learnroom/controller/api-test/course.api.spec.ts index 6fd315a438b..838abd09cd5 100644 --- a/apps/server/src/modules/learnroom/controller/api-test/course.api.spec.ts +++ b/apps/server/src/modules/learnroom/controller/api-test/course.api.spec.ts @@ -3,7 +3,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; import { ServerTestModule } from '@modules/server/server.module'; import { HttpStatus, INestApplication, StreamableFile } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { Course as CourseEntity } from '@shared/domain/entity'; +import { Course as CourseEntity, SyncAttribute } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { cleanupCollections, @@ -49,6 +49,10 @@ describe('Course Controller (API)', () => { await app.close(); }); + afterEach(() => { + jest.resetAllMocks(); + }); + describe('[GET] /courses/', () => { const setup = () => { const student = createStudent(); @@ -276,6 +280,36 @@ describe('Course Controller (API)', () => { expect(response.statusCode).toEqual(HttpStatus.NO_CONTENT); expect(result.syncedWithGroup?.id).toBe(group.id); }); + + it('should not start the synchronization with validation errord', async () => { + const { loggedInClient, course } = await setup(); + const params = { groupId: 'not-mongo-id' }; + + const response = await loggedInClient.post(`${course.id}/start-sync`).send(params); + + expect(response.statusCode).toEqual(HttpStatus.BAD_REQUEST); + }); + + it('should start partial synchronization', async () => { + const { loggedInClient, course, group } = await setup(); + const params = { groupId: group.id, excludedFields: ['teachers'] }; + + const response = await loggedInClient.post(`${course.id}/start-sync`).send(params); + + const result: CourseEntity = await em.findOneOrFail(CourseEntity, course.id); + expect(response.statusCode).toEqual(HttpStatus.NO_CONTENT); + expect(result.syncedWithGroup?.id).toBe(group.id); + expect(result.syncExcludedFields).toEqual([SyncAttribute.TEACHERS]); + }); + + it('should not start partial synchronization with validation error', async () => { + const { loggedInClient, course, group } = await setup(); + const params = { groupId: group.id, excludedFields: ['not-teachers'] }; + + const response = await loggedInClient.post(`${course.id}/start-sync`).send(params); + + expect(response.statusCode).toEqual(HttpStatus.BAD_REQUEST); + }); }); describe('when a course is already synchronized', () => { diff --git a/apps/server/src/modules/learnroom/controller/course.controller.ts b/apps/server/src/modules/learnroom/controller/course.controller.ts index 7d20c2f5292..c577ee49fb7 100644 --- a/apps/server/src/modules/learnroom/controller/course.controller.ts +++ b/apps/server/src/modules/learnroom/controller/course.controller.ts @@ -122,12 +122,18 @@ export class CourseController { @ApiOperation({ summary: 'Start the synchronization of a course with a group.' }) @ApiNoContentResponse({ description: 'The course was successfully synchronized to a group.' }) @ApiUnprocessableEntityResponse({ description: 'The course is already synchronized with a group.' }) + @ApiBadRequestResponse({ description: 'Request data has invalid format.' }) public async startSynchronization( @CurrentUser() currentUser: ICurrentUser, @Param() params: CourseUrlParams, @Body() bodyParams: CourseSyncBodyParams ): Promise { - await this.courseSyncUc.startSynchronization(currentUser.userId, params.courseId, bodyParams.groupId); + await this.courseSyncUc.startSynchronization( + currentUser.userId, + params.courseId, + bodyParams.groupId, + bodyParams.excludedFields + ); } @Get(':courseId/user-permissions') diff --git a/apps/server/src/modules/learnroom/controller/dto/course-sync.body.params.ts b/apps/server/src/modules/learnroom/controller/dto/course-sync.body.params.ts index 63fdffa5bf3..dd21cb28f55 100644 --- a/apps/server/src/modules/learnroom/controller/dto/course-sync.body.params.ts +++ b/apps/server/src/modules/learnroom/controller/dto/course-sync.body.params.ts @@ -1,5 +1,6 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsMongoId } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { IsArray, IsEnum, IsMongoId, IsOptional } from 'class-validator'; +import { SyncAttribute } from '@shared/domain/entity'; export class CourseSyncBodyParams { @IsMongoId() @@ -9,4 +10,15 @@ export class CourseSyncBodyParams { nullable: false, }) groupId!: string; + + @IsArray() + @IsOptional() + @IsEnum(SyncAttribute, { each: true }) + @ApiPropertyOptional({ + enum: SyncAttribute, + enumName: 'SyncAttribute', + isArray: true, + description: 'Restrict the course synchronization for certain fields', + }) + excludedFields?: SyncAttribute[]; } diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index e95fd85b27e..790435a98d2 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -1,5 +1,5 @@ import { AuthorizableObject, DomainObject } from '@shared/domain/domain-object'; -import { CourseFeatures } from '@shared/domain/entity'; +import { CourseFeatures, SyncAttribute } from '@shared/domain/entity'; import { EntityId } from '@shared/domain/types'; export interface CourseProps extends AuthorizableObject { @@ -34,6 +34,8 @@ export interface CourseProps extends AuthorizableObject { groupIds: EntityId[]; syncedWithGroup?: EntityId; + + syncExcludedFields?: SyncAttribute[]; } export class Course extends DomainObject { @@ -96,4 +98,12 @@ export class Course extends DomainObject { get syncedWithGroup(): EntityId | undefined { return this.props.syncedWithGroup; } + + set syncExcludedFields(values: SyncAttribute[] | undefined) { + this.props.syncExcludedFields = values ? [...new Set(values)] : undefined; + } + + get syncExcludedFields(): SyncAttribute[] | undefined { + return this.props.syncExcludedFields; + } } diff --git a/apps/server/src/modules/learnroom/repo/mikro-orm/course.repo.integration.spec.ts b/apps/server/src/modules/learnroom/repo/mikro-orm/course.repo.integration.spec.ts index 194917950f1..17b134c951c 100644 --- a/apps/server/src/modules/learnroom/repo/mikro-orm/course.repo.integration.spec.ts +++ b/apps/server/src/modules/learnroom/repo/mikro-orm/course.repo.integration.spec.ts @@ -6,7 +6,7 @@ import { classEntityFactory } from '@modules/class/entity/testing'; import { Group } from '@modules/group'; import { GroupEntity } from '@modules/group/entity'; import { Test, TestingModule } from '@nestjs/testing'; -import { Course as CourseEntity, CourseFeatures, CourseGroup, SchoolEntity, User } from '@shared/domain/entity'; +import { Course as CourseEntity, CourseFeatures, CourseGroup, SchoolEntity, SyncAttribute, User } from '@shared/domain/entity'; import { SortOrder } from '@shared/domain/interface'; import { cleanupCollections, @@ -154,6 +154,7 @@ describe(CourseMikroOrmRepo.name, () => { color: '#ACACAC', copyingSince: new Date(), syncedWithGroup: groupEntity.id, + syncExcludedFields: [SyncAttribute.TEACHERS], shareToken: 'shareToken', untilDate: new Date(), startDate: new Date(), diff --git a/apps/server/src/modules/learnroom/repo/mikro-orm/mapper/course.entity.mapper.ts b/apps/server/src/modules/learnroom/repo/mikro-orm/mapper/course.entity.mapper.ts index fdb0925ccc2..29aae56e5be 100644 --- a/apps/server/src/modules/learnroom/repo/mikro-orm/mapper/course.entity.mapper.ts +++ b/apps/server/src/modules/learnroom/repo/mikro-orm/mapper/course.entity.mapper.ts @@ -39,6 +39,7 @@ export class CourseEntityMapper { copyingSince: entity.copyingSince, shareToken: entity.shareToken, syncedWithGroup: entity.syncedWithGroup?.id, + syncExcludedFields: entity.syncExcludedFields, }); return course; @@ -77,6 +78,7 @@ export class CourseEntityMapper { copyingSince: props.copyingSince, shareToken: props.shareToken, syncedWithGroup: props.syncedWithGroup, + syncExcludedFields: props.syncExcludedFields, }; return courseEntityData; 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 8afbcd87bb6..74491a79079 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 @@ -13,6 +13,7 @@ import { } from '../domain'; import { courseFactory } from '../testing'; import { CourseSyncService } from './course-sync.service'; +import { SyncAttribute } from '../../../shared/domain/entity'; describe(CourseSyncService.name, () => { let module: TestingModule; @@ -68,6 +69,7 @@ describe(CourseSyncService.name, () => { new Course({ ...course.getProps(), syncedWithGroup: undefined, + syncExcludedFields: undefined, }) ); }); @@ -94,10 +96,12 @@ describe(CourseSyncService.name, () => { describe('when a course is not synchronized with a group', () => { const setup = () => { const teacherId = new ObjectId().toHexString(); + const courseTeacherId = new ObjectId().toHexString(); const course: Course = courseFactory.build({ classIds: [new ObjectId().toHexString()], groupIds: [new ObjectId().toHexString()], substitutionTeacherIds: [teacherId], + teacherIds: [courseTeacherId], }); const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); @@ -114,6 +118,7 @@ describe(CourseSyncService.name, () => { teachers, groupWithoutTeachers, teacherId, + courseTeacherId, }; }; @@ -160,6 +165,79 @@ describe(CourseSyncService.name, () => { }); }); + describe('when a course is not synchronized with a group (partial sync)', () => { + const setup = () => { + const teacherId = new ObjectId().toHexString(); + const courseTeacherId = new ObjectId().toHexString(); + const course: Course = courseFactory.build({ + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + substitutionTeacherIds: [teacherId], + teacherIds: [courseTeacherId], + }); + 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, + teacherId, + courseTeacherId, + }; + }; + + it('should save a course with synchronized group, students, and teachers ', async () => { + const { course, group, students, teachers, teacherId } = setup(); + + await service.startSynchronization(course, group, []); + + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + 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), + classIds: [], + groupIds: [], + substitutionTeacherIds: [teacherId], + }), + ]); + }); + + it('should save a course with synchronized group, students, and not teachers (partial sync)', async () => { + const { course, group, students, teacherId, courseTeacherId } = setup(); + + await service.startSynchronization(course, group, [SyncAttribute.TEACHERS]); + + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + 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: [courseTeacherId], + classIds: [], + groupIds: [], + substitutionTeacherIds: [teacherId], + syncExcludedFields: [SyncAttribute.TEACHERS], + }), + ]); + }); + }); + describe('when a course is already synchronized with a group', () => { const setup = () => { const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); @@ -350,7 +428,7 @@ describe(CourseSyncService.name, () => { }); }); - describe('when the last teacher gets removed from a synced group', () => { + describe('when the teachers are not synced from group (partial sync)', () => { const setup = () => { const substituteTeacherId = new ObjectId().toHexString(); const studentUserId = new ObjectId().toHexString(); @@ -358,12 +436,17 @@ describe(CourseSyncService.name, () => { const studentRoleId: string = new ObjectId().toHexString(); const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); const teacherRole: RoleDto = roleDtoFactory.build(); + const teacherRoleId: string = new ObjectId().toHexString(); const newGroup: Group = groupFactory.build({ users: [ new GroupUser({ userId: studentUserId, roleId: studentRoleId, }), + new GroupUser({ + userId: substituteTeacherId, + roleId: teacherRoleId, + }), ], }); @@ -372,6 +455,7 @@ describe(CourseSyncService.name, () => { studentIds: [studentUserId], syncedWithGroup: newGroup.id, substitutionTeacherIds: [substituteTeacherId], + syncExcludedFields: [SyncAttribute.TEACHERS], }); courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); roleService.findByName.mockResolvedValueOnce(studentRole); @@ -382,11 +466,12 @@ describe(CourseSyncService.name, () => { newGroup, teacherUserId, substituteTeacherId, + studentUserId, }; }; - it('should keep the last teacher, remove all students', async () => { - const { course, newGroup, teacherUserId, substituteTeacherId } = setup(); + it('should not sync group teachers', async () => { + const { course, newGroup, substituteTeacherId, teacherUserId, studentUserId } = setup(); await service.synchronizeCourseWithGroup(newGroup, newGroup); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ @@ -395,7 +480,7 @@ describe(CourseSyncService.name, () => { name: course.name, startDate: newGroup.validPeriod?.from, untilDate: newGroup.validPeriod?.until, - studentIds: [], + studentIds: [studentUserId], teacherIds: [teacherUserId], syncedWithGroup: course.syncedWithGroup, classIds: [], 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 92c6c96e1d7..98808227d85 100644 --- a/apps/server/src/modules/learnroom/service/course-sync.service.ts +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -1,8 +1,8 @@ import { Group, GroupUser } from '@modules/group'; import { RoleService } from '@modules/role'; import { Inject, Injectable } from '@nestjs/common'; +import { SyncAttribute } from '@shared/domain/entity'; import { RoleName } from '@shared/domain/interface'; -import { EntityId } from '@shared/domain/types'; import { Course, COURSE_REPO, @@ -18,11 +18,15 @@ export class CourseSyncService { private readonly roleService: RoleService ) {} - public async startSynchronization(course: Course, group: Group): Promise { + public async startSynchronization(course: Course, group: Group, excludedFields?: SyncAttribute[]): Promise { if (course.syncedWithGroup) { throw new CourseAlreadySynchronizedLoggableException(course.id); } + if (excludedFields) { + course.syncExcludedFields = excludedFields; + } + await this.synchronize([course], group); } @@ -32,6 +36,7 @@ export class CourseSyncService { } course.syncedWithGroup = undefined; + course.syncExcludedFields = undefined; await this.courseRepo.save(course); } @@ -42,36 +47,41 @@ export class CourseSyncService { } 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 studentIds = group.users + .filter((user: GroupUser) => user.roleId === studentRole.id) + .map((student) => student.userId); + const teacherIds = group.users + .filter((user: GroupUser) => user.roleId === teacherRole.id) + .map((teacher) => teacher.userId); - 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 = []; - } + for (const course of courses) { + course.syncedWithGroup = group.id; + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; + course.classes = []; + course.groups = []; - course.classes = []; - course.groups = []; + if (oldGroup?.name === course.name) { + course.name = group.name; + } - return course; - }); + const excludedAtributes = new Set(course.syncExcludedFields || []); + if (!excludedAtributes.has(SyncAttribute.TEACHERS)) { + course.teachers = teacherIds; + } - await this.courseRepo.saveAll(coursesToSync); + if (!course.teachers.length) { + course.students = []; + } else { + course.students = studentIds; + } } + + await this.courseRepo.saveAll(courses); } } 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 1d366afe06d..20cd635cfd9 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 @@ -8,6 +8,7 @@ import { CourseDoService } from '../service'; import { CourseSyncService } from '../service/course-sync.service'; import { courseFactory } from '../testing'; import { CourseSyncUc } from './course-sync.uc'; +import { SyncAttribute } from '../../../shared/domain/entity'; describe(CourseSyncUc.name, () => { let module: TestingModule; @@ -140,12 +141,27 @@ describe(CourseSyncUc.name, () => { 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(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group, []); + }); + it('should start the synchronization', async () => { const { user, course, group } = setup(); await uc.startSynchronization(user.id, course.id, group.id); - expect(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group); + expect(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group, undefined); + }); + it('should start partial synchronization', async () => { + const { user, course, group } = setup(); + + await uc.startSynchronization(user.id, course.id, group.id, [SyncAttribute.TEACHERS]); + + expect(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group, [SyncAttribute.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 4e018e3bedb..93617837a93 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,6 +1,7 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { GroupService } from '@modules/group'; import { Injectable } from '@nestjs/common'; +import { SyncAttribute } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { CourseDoService, CourseSyncService } from '../service'; @@ -29,7 +30,12 @@ export class CourseSyncUc { await this.courseSyncService.stopSynchronization(course); } - public async startSynchronization(userId: string, courseId: string, groupId: string) { + public async startSynchronization( + userId: string, + courseId: string, + groupId: string, + excludedFields?: SyncAttribute[] + ) { const [course, group, user] = await Promise.all([ this.courseService.findById(courseId), this.groupService.findById(groupId), @@ -42,6 +48,6 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseSyncService.startSynchronization(course, group); + await this.courseSyncService.startSynchronization(course, group, excludedFields); } } diff --git a/apps/server/src/shared/domain/entity/course.entity.ts b/apps/server/src/shared/domain/entity/course.entity.ts index 3bad3a7e62a..c612cb75470 100644 --- a/apps/server/src/shared/domain/entity/course.entity.ts +++ b/apps/server/src/shared/domain/entity/course.entity.ts @@ -1,6 +1,7 @@ import { Collection, Entity, Enum, Index, ManyToMany, ManyToOne, OneToMany, Property, Unique } from '@mikro-orm/core'; import { ClassEntity } from '@modules/class/entity/class.entity'; import { GroupEntity } from '@modules/group/entity/group.entity'; + import { InternalServerErrorException } from '@nestjs/common/exceptions/internal-server-error.exception'; import { EntityWithSchool, Learnroom } from '@shared/domain/interface'; import { EntityId, LearnroomMetadata, LearnroomTypes } from '../types'; @@ -11,6 +12,10 @@ import { SchoolEntity } from './school.entity'; import type { TaskParent } from './task.entity'; import type { User } from './user.entity'; +export enum SyncAttribute { + TEACHERS = 'teachers', +} + export interface CourseProperties { name?: string; description?: string; @@ -27,6 +32,7 @@ export interface CourseProperties { classes?: ClassEntity[]; groups?: GroupEntity[]; syncedWithGroup?: GroupEntity; + syncExcludedFields?: SyncAttribute[]; } // that is really really shit default handling :D constructor, getter, js default, em default...what the hell @@ -105,6 +111,9 @@ export class Course extends BaseEntityWithTimestamps implements Learnroom, Entit @ManyToOne(() => GroupEntity, { nullable: true }) syncedWithGroup?: GroupEntity; + @Enum({ nullable: true, array: true }) + syncExcludedFields?: SyncAttribute[]; + constructor(props: CourseProperties) { super(); this.name = props.name ?? DEFAULT.name; @@ -121,6 +130,7 @@ export class Course extends BaseEntityWithTimestamps implements Learnroom, Entit this.classes.set(props.classes || []); this.groups.set(props.groups || []); if (props.syncedWithGroup) this.syncedWithGroup = props.syncedWithGroup; + if (props.syncExcludedFields) this.syncExcludedFields = props.syncExcludedFields; } public getStudentIds(): EntityId[] { diff --git a/apps/server/src/shared/repo/course/course.repo.integration.spec.ts b/apps/server/src/shared/repo/course/course.repo.integration.spec.ts index 128d742001d..2633d56f8bb 100644 --- a/apps/server/src/shared/repo/course/course.repo.integration.spec.ts +++ b/apps/server/src/shared/repo/course/course.repo.integration.spec.ts @@ -76,6 +76,7 @@ describe('course repo', () => { 'classes', 'groups', 'syncedWithGroup', + 'syncExcludedFields', ].sort(); expect(keysOfFirstElements).toEqual(expectedResult); });