Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add partial sync attributes #5316

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -49,6 +49,10 @@ describe('Course Controller (API)', () => {
await app.close();
});

afterEach(() => {
jest.resetAllMocks();
});

describe('[GET] /courses/', () => {
const setup = () => {
const student = createStudent();
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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[];
}
12 changes: 11 additions & 1 deletion apps/server/src/modules/learnroom/domain/do/course.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -34,6 +34,8 @@ export interface CourseProps extends AuthorizableObject {
groupIds: EntityId[];

syncedWithGroup?: EntityId;

syncExcludedFields?: SyncAttribute[];
}

export class Course extends DomainObject<CourseProps> {
Expand Down Expand Up @@ -96,4 +98,12 @@ export class Course extends DomainObject<CourseProps> {
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ 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,
Expand Down Expand Up @@ -154,6 +161,7 @@ describe(CourseMikroOrmRepo.name, () => {
color: '#ACACAC',
copyingSince: new Date(),
syncedWithGroup: groupEntity.id,
syncExcludedFields: [SyncAttribute.TEACHERS],
shareToken: 'shareToken',
untilDate: new Date(),
startDate: new Date(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class CourseEntityMapper {
copyingSince: entity.copyingSince,
shareToken: entity.shareToken,
syncedWithGroup: entity.syncedWithGroup?.id,
syncExcludedFields: entity.syncExcludedFields,
});

return course;
Expand Down Expand Up @@ -77,6 +78,7 @@ export class CourseEntityMapper {
copyingSince: props.copyingSince,
shareToken: props.shareToken,
syncedWithGroup: props.syncedWithGroup,
syncExcludedFields: props.syncExcludedFields,
};

return courseEntityData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ObjectId } from '@mikro-orm/mongodb';
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 { groupFactory, roleDtoFactory } from '@shared/testing';
import {
Course,
Expand Down Expand Up @@ -68,6 +69,7 @@ describe(CourseSyncService.name, () => {
new Course({
...course.getProps(),
syncedWithGroup: undefined,
syncExcludedFields: undefined,
})
);
});
Expand All @@ -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' });
Expand All @@ -114,6 +118,7 @@ describe(CourseSyncService.name, () => {
teachers,
groupWithoutTeachers,
teacherId,
courseTeacherId,
};
};

Expand Down Expand Up @@ -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() });
Expand Down Expand Up @@ -350,20 +428,25 @@ 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();
const teacherUserId = new ObjectId().toHexString();
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,
}),
],
});

Expand All @@ -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);
Expand All @@ -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[]]>([
Expand All @@ -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: [],
Expand Down
Loading
Loading