From 3e3f336e37344829cd89b9637d0b4810afca9075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Thu, 14 Nov 2024 12:15:37 +0100 Subject: [PATCH 1/3] fix partial sync start --- .../src/modules/group/domain/group.spec.ts | 118 ++++++++++- apps/server/src/modules/group/domain/group.ts | 14 +- .../learnroom/domain/do/course.spec.ts | 53 +++++ .../src/modules/learnroom/domain/do/course.ts | 6 + .../service/course-sync.service.spec.ts | 199 ++++++++++++++---- .../learnroom/service/course-sync.service.ts | 18 +- .../strategy/iserv/iserv.strategy.spec.ts | 1 + ...ulconnex-user-provisioning.service.spec.ts | 3 +- .../src/modules/role/service/dto/role.dto.ts | 2 +- .../src/modules/role/uc/role.uc.spec.ts | 3 +- 10 files changed, 356 insertions(+), 61 deletions(-) create mode 100644 apps/server/src/modules/learnroom/domain/do/course.spec.ts diff --git a/apps/server/src/modules/group/domain/group.spec.ts b/apps/server/src/modules/group/domain/group.spec.ts index 65018e0eb45..dfb36159c0b 100644 --- a/apps/server/src/modules/group/domain/group.spec.ts +++ b/apps/server/src/modules/group/domain/group.spec.ts @@ -1,11 +1,10 @@ +import { ObjectId } from '@mikro-orm/mongodb'; import { RoleReference, UserDO } from '@shared/domain/domainobject'; import { groupFactory, roleFactory, userDoFactory } from '@shared/testing'; - -import { ObjectId } from '@mikro-orm/mongodb'; import { Group } from './group'; import { GroupUser } from './group-user'; -describe('Group (Domain Object)', () => { +describe(Group.name, () => { describe('removeUser', () => { describe('when the user is in the group', () => { const setup = () => { @@ -193,4 +192,117 @@ describe('Group (Domain Object)', () => { }); }); }); + + describe('removeUser', () => { + describe('when the user is a member of the group', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const roleId = new ObjectId().toHexString(); + const groupUser = new GroupUser({ + userId, + roleId, + }); + const group = groupFactory.build({ + users: [groupUser], + }); + + return { + group, + userId, + }; + }; + + it('should return true', () => { + const { group, userId } = setup(); + + const result = group.isMember(userId); + + expect(result).toEqual(true); + }); + }); + + describe('when the user is a member of the group and has the requested role', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const roleId = new ObjectId().toHexString(); + const groupUser = new GroupUser({ + userId, + roleId, + }); + const group = groupFactory.build({ + users: [groupUser], + }); + + return { + group, + userId, + roleId, + }; + }; + + it('should return true', () => { + const { group, userId, roleId } = setup(); + + const result = group.isMember(userId, roleId); + + expect(result).toEqual(true); + }); + }); + + describe('when the user is a member of the group, but has a different role', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const roleId = new ObjectId().toHexString(); + const groupUser = new GroupUser({ + userId, + roleId: new ObjectId().toHexString(), + }); + const group = groupFactory.build({ + users: [groupUser], + }); + + return { + group, + userId, + roleId, + }; + }; + + it('should return false', () => { + const { group, userId, roleId } = setup(); + + const result = group.isMember(userId, roleId); + + expect(result).toEqual(false); + }); + }); + + describe('when the user is not a member of the group', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const roleId = new ObjectId().toHexString(); + const groupUser = new GroupUser({ + userId: new ObjectId().toHexString(), + roleId: new ObjectId().toHexString(), + }); + const group = groupFactory.build({ + users: [groupUser], + }); + + return { + group, + userId, + roleId, + }; + }; + + it('should return false', () => { + const { group, userId } = setup(); + + const result = group.isMember(userId); + + expect(result).toEqual(false); + }); + }); + }); }); diff --git a/apps/server/src/modules/group/domain/group.ts b/apps/server/src/modules/group/domain/group.ts index 9aa294242da..68e9c44bf84 100644 --- a/apps/server/src/modules/group/domain/group.ts +++ b/apps/server/src/modules/group/domain/group.ts @@ -50,17 +50,25 @@ export class Group extends DomainObject { return this.props.validPeriod; } - removeUser(user: UserDO): void { + public removeUser(user: UserDO): void { this.props.users = this.props.users.filter((groupUser: GroupUser): boolean => groupUser.userId !== user.id); } - isEmpty(): boolean { + public isEmpty(): boolean { return this.props.users.length === 0; } - addUser(user: GroupUser): void { + public addUser(user: GroupUser): void { if (!this.users.find((u: GroupUser): boolean => u.userId === user.userId)) { this.users.push(user); } } + + public isMember(userId: EntityId, roleId?: EntityId): boolean { + const isMember: boolean = this.users.some( + (groupUser: GroupUser) => groupUser.userId === userId && (roleId ? groupUser.roleId === roleId : true) + ); + + return isMember; + } } diff --git a/apps/server/src/modules/learnroom/domain/do/course.spec.ts b/apps/server/src/modules/learnroom/domain/do/course.spec.ts new file mode 100644 index 00000000000..e62a502fac8 --- /dev/null +++ b/apps/server/src/modules/learnroom/domain/do/course.spec.ts @@ -0,0 +1,53 @@ +import { ObjectId } from '@mikro-orm/mongodb'; +import { courseFactory } from '../../testing'; +import { Course } from './course'; + +describe(Course.name, () => { + describe('isTeacher', () => { + describe('when the user is a teacher in the course', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const course = courseFactory.build({ + teacherIds: [userId], + }); + + return { + course, + userId, + }; + }; + + it('should return true', () => { + const { course, userId } = setup(); + + const result = course.isTeacher(userId); + + expect(result).toEqual(true); + }); + }); + + describe('when the user is not a teacher in the course', () => { + const setup = () => { + const userId = new ObjectId().toHexString(); + const course = courseFactory.build({ + teacherIds: [new ObjectId().toHexString()], + studentIds: [userId], + substitutionTeacherIds: [userId], + }); + + return { + course, + userId, + }; + }; + + it('should return false', () => { + const { course, userId } = setup(); + + const result = course.isTeacher(userId); + + expect(result).toEqual(false); + }); + }); + }); +}); diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index a3ae9b7a2ee..734b08f1158 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -106,4 +106,10 @@ export class Course extends DomainObject { get excludeFromSync(): SyncAttribute[] | undefined { return this.props.excludeFromSync; } + + public isTeacher(userId: EntityId): boolean { + const isMember: boolean = this.teachers.some((teacherId: EntityId) => teacherId === userId); + + return isMember; + } } 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 c0a92dccd26..71a0e446572 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 @@ -4,6 +4,7 @@ import { Group, GroupUser } from '@modules/group'; import { RoleDto, RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; import { SyncAttribute } from '@shared/domain/entity'; +import { RoleName } from '@shared/domain/interface'; import { groupFactory, roleDtoFactory, setupEntities, userFactory } from '@shared/testing'; import { Course, @@ -94,43 +95,37 @@ describe(CourseSyncService.name, () => { }); describe('startSynchronization', () => { - describe('when starting partial synchonization with a group', () => { + describe('when the starting user is part of the course and the group', () => { const setup = () => { - const syncingUser = userFactory.asTeacher().buildWithId(); + const syncingUser = userFactory.buildWithId(); - const courseTeacherId = new ObjectId().toHexString(); const course: Course = courseFactory.build({ + teacherIds: [syncingUser.id], classIds: [new ObjectId().toHexString()], groupIds: [new ObjectId().toHexString()], }); - const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); - const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + const studentRole = roleDtoFactory.build({ name: RoleName.STUDENT }); + const teacherRole = roleDtoFactory.build({ name: RoleName.TEACHER }); - const groupStudentId = new ObjectId().toHexString(); - const students: GroupUser[] = [{ roleId: 'student-role-id', userId: groupStudentId }]; + const student = new GroupUser({ roleId: studentRole.id, userId: new ObjectId().toHexString() }); + const teacher = new GroupUser({ roleId: teacherRole.id, userId: syncingUser.id }); - const groupTeacherId = new ObjectId().toHexString(); - const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: groupTeacherId }]; + const group: Group = groupFactory.build({ users: [student, teacher] }); - const group: Group = groupFactory.build({ users: [...students, ...teachers] }); - const groupWithoutTeachers: Group = groupFactory.build({ users: [...students] }); - - roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); return { course, group, - students, - teachers, - groupWithoutTeachers, - groupTeacherId, - courseTeacherId, + student, syncingUser, }; }; - it('should start partial synchronization of a course with a group', async () => { - const { course, group, students, syncingUser } = setup(); + it('should start a full synchronization', async () => { + const { course, group, syncingUser, student } = setup(); await service.startSynchronization(course, group, syncingUser); @@ -141,53 +136,164 @@ describe(CourseSyncService.name, () => { name: course.name, startDate: group.validPeriod?.from, untilDate: group.validPeriod?.until, - studentIds: students.map((student) => student.userId), + studentIds: [student.userId], teacherIds: [syncingUser.id], classIds: [], groupIds: [], - excludeFromSync: [SyncAttribute.TEACHERS], + excludeFromSync: undefined, }), ]); }); }); - describe('when starting full synchonization with a group', () => { + describe('when the starting user is not part of the course, but every teacher of the course is in the group', () => { const setup = () => { + const syncingUser = userFactory.buildWithId(); + const teacher1Id = new ObjectId().toHexString(); + const teacher2Id = new ObjectId().toHexString(); + + const course: Course = courseFactory.build({ + teacherIds: [teacher1Id, teacher2Id], + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + }); + const studentRole = roleDtoFactory.build({ name: RoleName.STUDENT }); + const teacherRole = roleDtoFactory.build({ name: RoleName.TEACHER }); + + const student = new GroupUser({ roleId: studentRole.id, userId: new ObjectId().toHexString() }); + const teacher1 = new GroupUser({ roleId: teacherRole.id, userId: teacher1Id }); + const teacher2 = new GroupUser({ roleId: teacherRole.id, userId: teacher2Id }); + const teacher3 = new GroupUser({ roleId: teacherRole.id, userId: new ObjectId().toHexString() }); + + const group: Group = groupFactory.build({ users: [student, teacher1, teacher2, teacher3] }); + + roleService.findByName.mockResolvedValueOnce(teacherRole); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + group, + student, + teacher1, + teacher2, + teacher3, + syncingUser, + }; + }; + + it('should start a full synchronization', async () => { + const { course, group, syncingUser, student, teacher1, teacher2, teacher3 } = setup(); + + await service.startSynchronization(course, group, syncingUser); + + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: course.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: [student.userId], + teacherIds: [teacher1.userId, teacher2.userId, teacher3.userId], + classIds: [], + groupIds: [], + excludeFromSync: undefined, + }), + ]); + }); + }); + + describe('when the starting user is not part of the course and not every teacher of the course is in the group', () => { + const setup = () => { + const syncingUser = userFactory.buildWithId(); const teacherId = new ObjectId().toHexString(); - const courseTeacher = userFactory.asTeacher().buildWithId(); + const missingTeacherId = new ObjectId().toHexString(); + const course: Course = courseFactory.build({ + teacherIds: [teacherId, missingTeacherId], classIds: [new ObjectId().toHexString()], groupIds: [new ObjectId().toHexString()], - substitutionTeacherIds: [teacherId], }); + const studentRole = roleDtoFactory.build({ name: RoleName.STUDENT }); + const teacherRole = roleDtoFactory.build({ name: RoleName.TEACHER }); - 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' }, - { roleId: 'teacher-role-id', userId: 'teacher-user-id-1' }, - { roleId: 'teacher-role-id', userId: courseTeacher.id }, - ]; - const group: Group = groupFactory.build({ users: [...students, ...teachers] }); - const groupWithoutTeachers: Group = groupFactory.build({ users: [...students] }); - roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); + const student = new GroupUser({ roleId: studentRole.id, userId: new ObjectId().toHexString() }); + const teacher = new GroupUser({ roleId: teacherRole.id, userId: teacherId }); + + const group: Group = groupFactory.build({ users: [student, teacher] }); + + roleService.findByName.mockResolvedValueOnce(teacherRole); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); return { course, group, - students, - teachers, - groupWithoutTeachers, - teacherId, - courseTeacher, + student, + teacher, + missingTeacherId, + syncingUser, + }; + }; + + it('should start a partial synchronization and keep the teachers', async () => { + const { course, group, syncingUser, student, teacher, missingTeacherId } = setup(); + + await service.startSynchronization(course, group, syncingUser); + + expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: course.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: [student.userId], + teacherIds: [teacher.userId, missingTeacherId], + classIds: [], + groupIds: [], + excludeFromSync: [SyncAttribute.TEACHERS], + }), + ]); + }); + }); + + describe('when the starting user is part of the course, but not of the group', () => { + const setup = () => { + const syncingUser = userFactory.buildWithId(); + const teacher2Id = new ObjectId().toHexString(); + + const course: Course = courseFactory.build({ + teacherIds: [syncingUser.id, teacher2Id], + classIds: [new ObjectId().toHexString()], + groupIds: [new ObjectId().toHexString()], + }); + const studentRole = roleDtoFactory.build({ name: RoleName.STUDENT }); + const teacherRole = roleDtoFactory.build({ name: RoleName.TEACHER }); + + const student = new GroupUser({ roleId: studentRole.id, userId: new ObjectId().toHexString() }); + const otherTeacher = new GroupUser({ roleId: teacherRole.id, userId: new ObjectId().toHexString() }); + + const group: Group = groupFactory.build({ users: [student, otherTeacher] }); + + roleService.findByName.mockResolvedValueOnce(teacherRole); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + group, + student, + teacher2Id, + syncingUser, }; }; - it('should start full synchronization of a course with a group', async () => { - const { course, group, students, teachers, courseTeacher } = setup(); + it('should start a partial synchronization and keep the teachers', async () => { + const { course, group, syncingUser, student, teacher2Id } = setup(); - await service.startSynchronization(course, group, courseTeacher); + await service.startSynchronization(course, group, syncingUser); expect(courseRepo.saveAll).toHaveBeenCalledWith<[Course[]]>([ new Course({ @@ -196,10 +302,11 @@ describe(CourseSyncService.name, () => { name: course.name, startDate: group.validPeriod?.from, untilDate: group.validPeriod?.until, - studentIds: students.map((student) => student.userId), - teacherIds: teachers.map((teacher) => teacher.userId), + studentIds: [student.userId], + teacherIds: [syncingUser.id, teacher2Id], classIds: [], groupIds: [], + excludeFromSync: [SyncAttribute.TEACHERS], }), ]); }); 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 d5d0928f4e7..02edd23f703 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,9 @@ import { Group, GroupUser } from '@modules/group'; -import { RoleService } from '@modules/role'; +import { RoleDto, RoleService } from '@modules/role'; import { Inject, Injectable } from '@nestjs/common'; -import { Role, SyncAttribute, User } from '@shared/domain/entity'; +import { SyncAttribute, User } from '@shared/domain/entity'; import { RoleName } from '@shared/domain/interface'; +import { EntityId } from '@shared/domain/types'; import { Course, COURSE_REPO, @@ -23,12 +24,16 @@ export class CourseSyncService { throw new CourseAlreadySynchronizedLoggableException(course.id); } - const isTeacher = user.getRoles().some((role: Role) => role.name === RoleName.TEACHER); - const isInGroup = group.users.some((groupUser) => groupUser.userId === user.id); + const teacherRole: RoleDto = await this.roleService.findByName(RoleName.TEACHER); - if (isTeacher && !isInGroup) { + const isInCourse: boolean = course.isTeacher(user.id); + const isTeacherInBoth: boolean = isInCourse && group.isMember(user.id, teacherRole.id); + const keepsAllTeachers: boolean = + !isInCourse && course.teachers.every((teacherId: EntityId) => group.isMember(teacherId, teacherRole.id)); + const shouldSyncTeachers: boolean = isTeacherInBoth || keepsAllTeachers; + + if (!shouldSyncTeachers) { course.excludeFromSync = [SyncAttribute.TEACHERS]; - course.teachers = [user.id]; } await this.synchronize([course], group); @@ -47,6 +52,7 @@ export class CourseSyncService { public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); + await this.synchronize(courses, newGroup, oldGroup); } diff --git a/apps/server/src/modules/provisioning/strategy/iserv/iserv.strategy.spec.ts b/apps/server/src/modules/provisioning/strategy/iserv/iserv.strategy.spec.ts index 47068ab32e2..3e2d78b75a2 100644 --- a/apps/server/src/modules/provisioning/strategy/iserv/iserv.strategy.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/iserv/iserv.strategy.spec.ts @@ -94,6 +94,7 @@ describe('IservProvisioningStrategy', () => { }); const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId({ externalId: 'schoolExternalId' }); const roleDto: RoleDto = new RoleDto({ + id: new ObjectId().toHexString(), name: RoleName.STUDENT, }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.spec.ts index 05e6fa3d3ad..1f78350f6ec 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-user-provisioning.service.spec.ts @@ -1,4 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; import { AccountSave, AccountService } from '@modules/account'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; @@ -97,7 +98,7 @@ describe(SchulconnexUserProvisioningService.name, () => { }); const minimalViableExternalUser: ExternalUserDto = new ExternalUserDto({ externalId: 'externalUserId' }); const userRole: RoleDto = new RoleDto({ - id: 'roleId', + id: new ObjectId().toHexString(), name: RoleName.USER, }); const hash = 'hash'; diff --git a/apps/server/src/modules/role/service/dto/role.dto.ts b/apps/server/src/modules/role/service/dto/role.dto.ts index a524b5307fc..a09a52fc518 100644 --- a/apps/server/src/modules/role/service/dto/role.dto.ts +++ b/apps/server/src/modules/role/service/dto/role.dto.ts @@ -2,7 +2,7 @@ import { Permission, RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; export class RoleDto { - id?: EntityId; + id: EntityId; name: RoleName; diff --git a/apps/server/src/modules/role/uc/role.uc.spec.ts b/apps/server/src/modules/role/uc/role.uc.spec.ts index 9a9b16e1d9c..62f7e435dd6 100644 --- a/apps/server/src/modules/role/uc/role.uc.spec.ts +++ b/apps/server/src/modules/role/uc/role.uc.spec.ts @@ -1,4 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { RoleService } from '@modules/role/service/role.service'; import { RoleUc } from '@modules/role/uc/role.uc'; @@ -31,7 +32,7 @@ describe('RoleUc', () => { beforeEach(() => { roleDto = new RoleDto({ - id: '', + id: new ObjectId().toHexString(), name: RoleName.STUDENT, permissions: [], }); From a2ca7c90a403b99269ca699b87138655e1d8a0e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Thu, 14 Nov 2024 16:35:40 +0100 Subject: [PATCH 2/3] review changes --- apps/server/src/modules/group/domain/group.spec.ts | 2 +- apps/server/src/modules/learnroom/domain/do/course.ts | 4 ++-- apps/server/src/modules/learnroom/uc/course.uc.spec.ts | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/group/domain/group.spec.ts b/apps/server/src/modules/group/domain/group.spec.ts index dfb36159c0b..7c78a3ab022 100644 --- a/apps/server/src/modules/group/domain/group.spec.ts +++ b/apps/server/src/modules/group/domain/group.spec.ts @@ -193,7 +193,7 @@ describe(Group.name, () => { }); }); - describe('removeUser', () => { + describe('isMember', () => { describe('when the user is a member of the group', () => { const setup = () => { const userId = new ObjectId().toHexString(); diff --git a/apps/server/src/modules/learnroom/domain/do/course.ts b/apps/server/src/modules/learnroom/domain/do/course.ts index 734b08f1158..e5255c3e79a 100644 --- a/apps/server/src/modules/learnroom/domain/do/course.ts +++ b/apps/server/src/modules/learnroom/domain/do/course.ts @@ -108,8 +108,8 @@ export class Course extends DomainObject { } public isTeacher(userId: EntityId): boolean { - const isMember: boolean = this.teachers.some((teacherId: EntityId) => teacherId === userId); + const isTeacher: boolean = this.teachers.some((teacherId: EntityId) => teacherId === userId); - return isMember; + return isTeacher; } } diff --git a/apps/server/src/modules/learnroom/uc/course.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course.uc.spec.ts index 17dbd918dc7..363fffb3c97 100644 --- a/apps/server/src/modules/learnroom/uc/course.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course.uc.spec.ts @@ -1,4 +1,5 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; import { AuthorizationService } from '@modules/authorization'; import { RoleDto, RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; @@ -91,6 +92,7 @@ describe('CourseUc', () => { courseService.findById.mockResolvedValue(course); authorizationService.getUserWithPermissions.mockResolvedValue(teacherUser); const mockRoleDto: RoleDto = { + id: new ObjectId().toHexString(), name: RoleName.TEACHER, permissions: [Permission.COURSE_DELETE], }; From 8b77ceaa3ad2f04d43b11ee0cd3596e021eaa91f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= Date: Thu, 14 Nov 2024 16:49:07 +0100 Subject: [PATCH 3/3] fix test --- .../group/service/group.service.spec.ts | 23 ------------------- .../modules/group/service/group.service.ts | 8 ++----- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/apps/server/src/modules/group/service/group.service.spec.ts b/apps/server/src/modules/group/service/group.service.spec.ts index f558e06f3ba..c5fcc111dc1 100644 --- a/apps/server/src/modules/group/service/group.service.spec.ts +++ b/apps/server/src/modules/group/service/group.service.spec.ts @@ -1,7 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import type { ProvisioningConfig } from '@modules/provisioning'; -import { BadRequestException } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { EventBus } from '@nestjs/cqrs'; import { Test, TestingModule } from '@nestjs/testing'; @@ -561,16 +560,6 @@ describe('GroupService', () => { expect(groupRepo.save).toHaveBeenCalledWith(group); }); - - describe('when the role id is undefined', () => { - it('should throw', async () => { - roleService.findByName.mockResolvedValue(roleDtoFactory.build({ id: undefined })); - - await expect(service.addUserToGroup('groupId', 'userId', RoleName.STUDENT)).rejects.toThrow( - BadRequestException - ); - }); - }); }); }); @@ -618,18 +607,6 @@ describe('GroupService', () => { }); }); - describe('when role has no id', () => { - it('should fail', async () => { - const roleDto = roleDtoFactory.buildWithId({ name: RoleName.STUDENT }); - roleDto.id = undefined; - const { userDo } = setup([roleDto]); - - const method = () => service.addUsersToGroup('groupId', [{ userId: userDo.id!, roleName: RoleName.STUDENT }]); - - await expect(method).rejects.toThrow(); - }); - }); - describe('when user does not exist', () => { it('should fail', async () => { const roleDto = roleDtoFactory.buildWithId({ name: RoleName.STUDENT }); diff --git a/apps/server/src/modules/group/service/group.service.ts b/apps/server/src/modules/group/service/group.service.ts index 76e9f242e1e..ada5e968ba0 100644 --- a/apps/server/src/modules/group/service/group.service.ts +++ b/apps/server/src/modules/group/service/group.service.ts @@ -16,9 +16,9 @@ import { GroupAggregateScope, GroupDeletedEvent, GroupFilter, - GroupVisibilityPermission, - GroupUser, GroupTypes, + GroupUser, + GroupVisibilityPermission, } from '../domain'; import { GroupRepo } from '../repo'; @@ -107,9 +107,6 @@ export class GroupService implements AuthorizationLoaderServiceGeneric { public async addUserToGroup(groupId: EntityId, userId: EntityId, roleName: RoleName): Promise { const role = await this.roleService.findByName(roleName); - if (!role.id) { - throw new BadRequestException('Role has no id.'); - } const group = await this.findById(groupId); const user = await this.userService.findById(userId); // user must have an id, because we are fetching it by id -> fix in service @@ -133,7 +130,6 @@ export class GroupService implements AuthorizationLoaderServiceGeneric { for (const { userId, roleName } of userIdsAndRoles) { const role = roleNamesSet.find((r) => r.name === roleName); if (!role) throw new BadRequestException(`Role ${roleName} not found.`); - if (!role.id) throw new BadRequestException('Role has no id.'); const user = users.find((u) => u.id === userId); // user must have an id, because we are fetching it by id -> fix in service if (!user) throw new BadRequestException('Unknown userId.');