From 92c1d3af7bf83dcc9a6df95c8419190789bf0894 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Tue, 23 Jul 2024 11:08:55 +0200 Subject: [PATCH 1/6] wip fix --- .../server/src/modules/group/uc/class-group.uc.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.ts b/apps/server/src/modules/group/uc/class-group.uc.ts index 767591735fe..32712a333cf 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.ts @@ -5,7 +5,7 @@ import { Course } from '@modules/learnroom/domain'; import { CourseDoService } from '@modules/learnroom/service/course-do.service'; import { SchoolYearService } from '@modules/legacy-school'; import { ProvisioningConfig } from '@modules/provisioning'; -import { School, SchoolService } from '@modules/school/domain'; +import { School, SchoolService, SchoolYear } from '@modules/school/domain'; import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { SortHelper } from '@shared/common'; @@ -138,7 +138,8 @@ export class ClassGroupUc { classes: Class[], schoolYearQueryType?: SchoolYearQueryType ): Promise { - const currentYear: SchoolYearEntity = await this.schoolYearService.getCurrentSchoolYear(); + const school: School = await this.schoolService.getSchoolById(classes[0].schoolId); + const currentYear = school.getProps().currentYear; const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await this.addSchoolYearsToClasses( classes @@ -172,7 +173,7 @@ export class ClassGroupUc { } private isClassOfQueryType( - currentYear: SchoolYearEntity, + currentYear: SchoolYear | undefined, schoolYear?: SchoolYearEntity, schoolYearQueryType?: SchoolYearQueryType ): boolean { @@ -180,17 +181,17 @@ export class ClassGroupUc { return true; } - if (schoolYear === undefined) { + if (schoolYear === undefined || currentYear === undefined) { return schoolYearQueryType === SchoolYearQueryType.CURRENT_YEAR; } switch (schoolYearQueryType) { case SchoolYearQueryType.CURRENT_YEAR: - return schoolYear.startDate === currentYear.startDate; + return schoolYear.startDate === currentYear.getProps().startDate; case SchoolYearQueryType.NEXT_YEAR: - return schoolYear.startDate > currentYear.startDate; + return schoolYear.startDate > currentYear.getProps().startDate; case SchoolYearQueryType.PREVIOUS_YEARS: - return schoolYear.startDate < currentYear.startDate; + return schoolYear.startDate < currentYear.getProps().startDate; default: throw new UnknownQueryTypeLoggableException(schoolYearQueryType); } From 0b1b465f83911b6ee4d85d9d276a6745c370f226 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Tue, 23 Jul 2024 13:40:16 +0200 Subject: [PATCH 2/6] move logic to school service --- .../modules/group/uc/class-group.uc.spec.ts | 18 ++++++++++-------- .../src/modules/group/uc/class-group.uc.ts | 3 +-- .../school/domain/service/school.service.ts | 5 +++++ .../modules/school/testing/school.factory.ts | 2 ++ 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.spec.ts b/apps/server/src/modules/group/uc/class-group.uc.spec.ts index 1a9bb2c0abf..1ebfcb362a9 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.spec.ts @@ -12,8 +12,8 @@ import { SchoolYearService } from '@modules/legacy-school'; import { ProvisioningConfig } from '@modules/provisioning'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { School, SchoolService } from '@modules/school/domain'; -import { schoolFactory } from '@modules/school/testing'; +import { School, SchoolProps, SchoolService, SchoolYear, SchoolYearProps } from '@modules/school/domain'; +import { federalStateFactory, schoolFactory, schoolYearFactory } from '@modules/school/testing'; import { System, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; @@ -25,7 +25,7 @@ import { Permission, SortOrder } from '@shared/domain/interface'; import { groupFactory, roleDtoFactory, - schoolYearFactory, + schoolYearFactory as schoolYearEntityFactory, setupEntities, systemFactory, UserAndAccountTestFactory, @@ -155,7 +155,9 @@ describe('ClassGroupUc', () => { describe('when accessing as a normal user', () => { const setup = () => { - const school: School = schoolFactory.build({ permissions: { teacher: { STUDENT_LIST: true } } }); + const school: School = schoolFactory.build({ + permissions: { teacher: { STUDENT_LIST: true } }, + }); const { studentUser } = UserAndAccountTestFactory.buildStudent(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); const teacherRole: RoleDto = roleDtoFactory.buildWithId({ @@ -176,8 +178,8 @@ describe('ClassGroupUc', () => { lastName: studentUser.lastName, roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], }); - const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); - const nextSchoolYear: SchoolYearEntity = schoolYearFactory.buildWithId({ + const schoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId(); + const nextSchoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId({ startDate: schoolYear.endDate, }); const clazz: Class = classFactory.build({ @@ -594,7 +596,7 @@ describe('ClassGroupUc', () => { lastName: studentUser.lastName, roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], }); - const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const schoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId(); let clazzes: Class[] = []; if (generateClasses) { clazzes = classFactory.buildList(11, { @@ -892,7 +894,7 @@ describe('ClassGroupUc', () => { roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }], }); - const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const schoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId(); const clazz: Class = classFactory.build({ name: 'A', teacherIds: [teacherUser.id, notFoundReferenceId], diff --git a/apps/server/src/modules/group/uc/class-group.uc.ts b/apps/server/src/modules/group/uc/class-group.uc.ts index 32712a333cf..020404e9856 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.ts @@ -138,8 +138,7 @@ export class ClassGroupUc { classes: Class[], schoolYearQueryType?: SchoolYearQueryType ): Promise { - const school: School = await this.schoolService.getSchoolById(classes[0].schoolId); - const currentYear = school.getProps().currentYear; + const currentYear = await this.schoolService.getCurrentYear(classes[0].schoolId); const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await this.addSchoolYearsToClasses( classes diff --git a/apps/server/src/modules/school/domain/service/school.service.ts b/apps/server/src/modules/school/domain/service/school.service.ts index b590a6250f5..fe6b9712786 100644 --- a/apps/server/src/modules/school/domain/service/school.service.ts +++ b/apps/server/src/modules/school/domain/service/school.service.ts @@ -53,6 +53,11 @@ export class SchoolService { return schoolsForExternalInvite; } + public async getCurrentYear(schoolId: EntityId) { + const school = await this.getSchoolById(schoolId); + return school.getProps().currentYear; + } + public async doesSchoolExist(schoolId: EntityId): Promise { try { await this.schoolRepo.getSchoolById(schoolId); diff --git a/apps/server/src/modules/school/testing/school.factory.ts b/apps/server/src/modules/school/testing/school.factory.ts index e44195c9cbb..932bf1157a7 100644 --- a/apps/server/src/modules/school/testing/school.factory.ts +++ b/apps/server/src/modules/school/testing/school.factory.ts @@ -1,6 +1,7 @@ import { ObjectId } from '@mikro-orm/mongodb'; import { SchoolFeature } from '@shared/domain/types'; import { BaseFactory } from '@shared/testing'; +import { schoolYearFactory } from '@modules/school/testing/school-year.factory'; import { School, SchoolProps } from '../domain'; import { federalStateFactory } from './federal-state.factory'; @@ -13,5 +14,6 @@ export const schoolFactory = BaseFactory.define(School, ({ federalState: federalStateFactory.build(), features: new Set(), systemIds: [], + currentYear: schoolYearFactory.build(), }; }); From 171d6c652d18a8191d5f829e4c8e7f773ddeb744 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Tue, 23 Jul 2024 15:06:20 +0200 Subject: [PATCH 3/6] fixed tests --- .../src/modules/group/uc/class-group.uc.spec.ts | 13 ++++++++++--- .../src/modules/school/testing/school.factory.ts | 2 -- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.spec.ts b/apps/server/src/modules/group/uc/class-group.uc.spec.ts index 1ebfcb362a9..e3274185de7 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.spec.ts @@ -12,8 +12,8 @@ import { SchoolYearService } from '@modules/legacy-school'; import { ProvisioningConfig } from '@modules/provisioning'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { School, SchoolProps, SchoolService, SchoolYear, SchoolYearProps } from '@modules/school/domain'; -import { federalStateFactory, schoolFactory, schoolYearFactory } from '@modules/school/testing'; +import { School, SchoolService } from '@modules/school/domain'; +import { schoolFactory, schoolYearFactory } from '@modules/school/testing'; import { System, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; @@ -155,9 +155,12 @@ describe('ClassGroupUc', () => { describe('when accessing as a normal user', () => { const setup = () => { + const schoolYearDo = schoolYearFactory.build(); const school: School = schoolFactory.build({ permissions: { teacher: { STUDENT_LIST: true } }, + currentYear: schoolYearDo, }); + const { studentUser } = UserAndAccountTestFactory.buildStudent(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); const teacherRole: RoleDto = roleDtoFactory.buildWithId({ @@ -178,10 +181,13 @@ describe('ClassGroupUc', () => { lastName: studentUser.lastName, roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], }); - const schoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId(); + + const startDate = schoolYearDo.getProps().startDate; + const schoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId({ startDate }); const nextSchoolYear: SchoolYearEntity = schoolYearEntityFactory.buildWithId({ startDate: schoolYear.endDate, }); + const clazz: Class = classFactory.build({ name: 'A', teacherIds: [teacherUser.id], @@ -218,6 +224,7 @@ describe('ClassGroupUc', () => { const synchronizedCourse: Course = courseFactory.build({ syncedWithGroup: group.id }); schoolService.getSchoolById.mockResolvedValueOnce(school); + schoolService.getCurrentYear.mockResolvedValueOnce(schoolYearDo); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); authorizationService.hasAllPermissions.mockReturnValueOnce(false); classService.findAllByUserId.mockResolvedValueOnce([clazz, successorClass, classWithoutSchoolYear]); diff --git a/apps/server/src/modules/school/testing/school.factory.ts b/apps/server/src/modules/school/testing/school.factory.ts index 932bf1157a7..e44195c9cbb 100644 --- a/apps/server/src/modules/school/testing/school.factory.ts +++ b/apps/server/src/modules/school/testing/school.factory.ts @@ -1,7 +1,6 @@ import { ObjectId } from '@mikro-orm/mongodb'; import { SchoolFeature } from '@shared/domain/types'; import { BaseFactory } from '@shared/testing'; -import { schoolYearFactory } from '@modules/school/testing/school-year.factory'; import { School, SchoolProps } from '../domain'; import { federalStateFactory } from './federal-state.factory'; @@ -14,6 +13,5 @@ export const schoolFactory = BaseFactory.define(School, ({ federalState: federalStateFactory.build(), features: new Set(), systemIds: [], - currentYear: schoolYearFactory.build(), }; }); From 3f432982ce06f38858c7d893ee8513c831716b0d Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Tue, 23 Jul 2024 15:17:08 +0200 Subject: [PATCH 4/6] N21-2095 fix current year --- apps/server/src/modules/group/uc/class-group.uc.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.ts b/apps/server/src/modules/group/uc/class-group.uc.ts index 020404e9856..c49671067cf 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.ts @@ -138,7 +138,9 @@ export class ClassGroupUc { classes: Class[], schoolYearQueryType?: SchoolYearQueryType ): Promise { - const currentYear = await this.schoolService.getCurrentYear(classes[0].schoolId); + const currentYear: SchoolYear | undefined = classes[0].schoolId + ? await this.schoolService.getCurrentYear(classes[0].schoolId) + : undefined; const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await this.addSchoolYearsToClasses( classes From 43b6a25cdbd75d9fe09bdfa8e3a0708bb9eea771 Mon Sep 17 00:00:00 2001 From: Gordon Nicholas Date: Tue, 23 Jul 2024 15:21:28 +0200 Subject: [PATCH 5/6] fix conditions --- apps/server/src/modules/group/uc/class-group.uc.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.ts b/apps/server/src/modules/group/uc/class-group.uc.ts index c49671067cf..7368f40fe28 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.ts @@ -138,9 +138,8 @@ export class ClassGroupUc { classes: Class[], schoolYearQueryType?: SchoolYearQueryType ): Promise { - const currentYear: SchoolYear | undefined = classes[0].schoolId - ? await this.schoolService.getCurrentYear(classes[0].schoolId) - : undefined; + const currentYear: SchoolYear | undefined = + classes.length > 0 ? await this.schoolService.getCurrentYear(classes[0].schoolId) : undefined; const classesWithSchoolYear: { clazz: Class; schoolYear?: SchoolYearEntity }[] = await this.addSchoolYearsToClasses( classes From af1cd19590edd1a55d06ec0b4b3293dd4da8c93c Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Tue, 23 Jul 2024 15:30:20 +0200 Subject: [PATCH 6/6] N21-2095 clean code --- apps/server/src/modules/group/uc/class-group.uc.ts | 6 +++--- apps/server/src/modules/school/domain/do/school-year.ts | 6 +++++- apps/server/src/modules/school/domain/do/school.ts | 4 ++++ .../src/modules/school/domain/service/school.service.ts | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/apps/server/src/modules/group/uc/class-group.uc.ts b/apps/server/src/modules/group/uc/class-group.uc.ts index 7368f40fe28..b3085101d1e 100644 --- a/apps/server/src/modules/group/uc/class-group.uc.ts +++ b/apps/server/src/modules/group/uc/class-group.uc.ts @@ -187,11 +187,11 @@ export class ClassGroupUc { switch (schoolYearQueryType) { case SchoolYearQueryType.CURRENT_YEAR: - return schoolYear.startDate === currentYear.getProps().startDate; + return schoolYear.startDate === currentYear.startDate; case SchoolYearQueryType.NEXT_YEAR: - return schoolYear.startDate > currentYear.getProps().startDate; + return schoolYear.startDate > currentYear.startDate; case SchoolYearQueryType.PREVIOUS_YEARS: - return schoolYear.startDate < currentYear.getProps().startDate; + return schoolYear.startDate < currentYear.startDate; default: throw new UnknownQueryTypeLoggableException(schoolYearQueryType); } diff --git a/apps/server/src/modules/school/domain/do/school-year.ts b/apps/server/src/modules/school/domain/do/school-year.ts index eaeeb6384c9..885226ce132 100644 --- a/apps/server/src/modules/school/domain/do/school-year.ts +++ b/apps/server/src/modules/school/domain/do/school-year.ts @@ -1,6 +1,10 @@ import { AuthorizableObject, DomainObject } from '@shared/domain/domain-object'; -export class SchoolYear extends DomainObject {} +export class SchoolYear extends DomainObject { + get startDate() { + return this.props.startDate; + } +} export interface SchoolYearProps extends AuthorizableObject { name: string; diff --git a/apps/server/src/modules/school/domain/do/school.ts b/apps/server/src/modules/school/domain/do/school.ts index 45ea43d255d..f5be38594ba 100644 --- a/apps/server/src/modules/school/domain/do/school.ts +++ b/apps/server/src/modules/school/domain/do/school.ts @@ -20,6 +20,10 @@ interface SchoolInfo { } export class School extends DomainObject { + get currentYear() { + return this.props.currentYear; + } + get systems(): EntityId[] { return this.props.systemIds; } diff --git a/apps/server/src/modules/school/domain/service/school.service.ts b/apps/server/src/modules/school/domain/service/school.service.ts index fe6b9712786..945c85fce98 100644 --- a/apps/server/src/modules/school/domain/service/school.service.ts +++ b/apps/server/src/modules/school/domain/service/school.service.ts @@ -55,7 +55,7 @@ export class SchoolService { public async getCurrentYear(schoolId: EntityId) { const school = await this.getSchoolById(schoolId); - return school.getProps().currentYear; + return school.currentYear; } public async doesSchoolExist(schoolId: EntityId): Promise {