From 612b02914a723a34d94c7db19547a8f857e431a1 Mon Sep 17 00:00:00 2001 From: Igor Richter <93926487+IgorCapCoder@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:36:48 +0100 Subject: [PATCH] N21-1623 fix class visibility for course (#4659) * class visibility fix * use new schoolService --- .../interface/class-request-context.enum.ts | 4 ++ .../group/controller/dto/interface/index.ts | 1 + .../dto/request/class-caller-params.ts | 10 ++++ .../group/controller/dto/request/index.ts | 1 + .../group/controller/group.controller.ts | 3 + .../src/modules/group/group-api.module.ts | 2 + .../src/modules/group/uc/group.uc.spec.ts | 55 ++++++++++++++----- apps/server/src/modules/group/uc/group.uc.ts | 17 ++++-- .../modules/school/domain/do/school.spec.ts | 39 +++++++++++++ .../src/modules/school/domain/do/school.ts | 6 ++ 10 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 apps/server/src/modules/group/controller/dto/interface/class-request-context.enum.ts create mode 100644 apps/server/src/modules/group/controller/dto/request/class-caller-params.ts diff --git a/apps/server/src/modules/group/controller/dto/interface/class-request-context.enum.ts b/apps/server/src/modules/group/controller/dto/interface/class-request-context.enum.ts new file mode 100644 index 00000000000..5f24479c2e8 --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/interface/class-request-context.enum.ts @@ -0,0 +1,4 @@ +export enum ClassRequestContext { + COURSE = 'course', + CLASS_OVERVIEW = 'class-overview', +} diff --git a/apps/server/src/modules/group/controller/dto/interface/index.ts b/apps/server/src/modules/group/controller/dto/interface/index.ts index fa69bb70b30..a94213271b7 100644 --- a/apps/server/src/modules/group/controller/dto/interface/index.ts +++ b/apps/server/src/modules/group/controller/dto/interface/index.ts @@ -1,2 +1,3 @@ export * from './class-sort-by.enum'; export * from './school-year-query-type.enum'; +export { ClassRequestContext } from './class-request-context.enum'; diff --git a/apps/server/src/modules/group/controller/dto/request/class-caller-params.ts b/apps/server/src/modules/group/controller/dto/request/class-caller-params.ts new file mode 100644 index 00000000000..60765802ffb --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/request/class-caller-params.ts @@ -0,0 +1,10 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsEnum, IsOptional } from 'class-validator'; +import { ClassRequestContext } from '../interface'; + +export class ClassCallerParams { + @IsOptional() + @IsEnum(ClassRequestContext) + @ApiPropertyOptional({ enum: ClassRequestContext, enumName: 'ClassRequestContext' }) + calledFrom?: ClassRequestContext; +} diff --git a/apps/server/src/modules/group/controller/dto/request/index.ts b/apps/server/src/modules/group/controller/dto/request/index.ts index cb39cb2f9c5..e7da3889f23 100644 --- a/apps/server/src/modules/group/controller/dto/request/index.ts +++ b/apps/server/src/modules/group/controller/dto/request/index.ts @@ -2,3 +2,4 @@ export * from './class-sort-params'; export * from './group-id-params'; export * from './class-filter-params'; export { GroupPaginationParams } from './group-pagination.params'; +export { ClassCallerParams } from './class-caller-params'; diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index 7a5117466b7..e5df1dec514 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -12,6 +12,7 @@ import { GroupIdParams, GroupResponse, GroupPaginationParams, + ClassCallerParams, } from './dto'; import { GroupResponseMapper } from './mapper'; @@ -30,12 +31,14 @@ export class GroupController { @Query() pagination: GroupPaginationParams, @Query() sortingQuery: ClassSortParams, @Query() filterParams: ClassFilterParams, + @Query() callerParams: ClassCallerParams, @CurrentUser() currentUser: ICurrentUser ): Promise { const board: Page = await this.groupUc.findAllClasses( currentUser.userId, currentUser.schoolId, filterParams.type, + callerParams.calledFrom, pagination.skip, pagination.limit, sortingQuery.sortBy, diff --git a/apps/server/src/modules/group/group-api.module.ts b/apps/server/src/modules/group/group-api.module.ts index 1f2de66c02a..1b3d14645c6 100644 --- a/apps/server/src/modules/group/group-api.module.ts +++ b/apps/server/src/modules/group/group-api.module.ts @@ -6,6 +6,7 @@ import { LegacySchoolModule } from '@modules/legacy-school'; import { SystemModule } from '@modules/system'; import { UserModule } from '@modules/user'; import { LoggerModule } from '@src/core/logger'; +import { SchoolModule } from '@modules/school'; import { GroupController } from './controller'; import { GroupModule } from './group.module'; import { GroupUc } from './uc'; @@ -17,6 +18,7 @@ import { GroupUc } from './uc'; UserModule, RoleModule, LegacySchoolModule, + SchoolModule, AuthorizationModule, SystemModule, LoggerModule, diff --git a/apps/server/src/modules/group/uc/group.uc.spec.ts b/apps/server/src/modules/group/uc/group.uc.spec.ts index e49179202c1..3a53d6d2f80 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -4,7 +4,7 @@ import { Action, AuthorizationContext, AuthorizationService } from '@modules/aut import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; -import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; +import { SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { LegacySystemService, SystemDto } from '@modules/system'; @@ -13,13 +13,12 @@ import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; -import { LegacySchoolDo, Page, UserDO } from '@shared/domain/domainobject'; +import { Page, UserDO } from '@shared/domain/domainobject'; import { SchoolYearEntity, User } from '@shared/domain/entity'; import { Permission, SortOrder } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { groupFactory, - legacySchoolDoFactory, roleDtoFactory, schoolYearFactory, setupEntities, @@ -28,7 +27,9 @@ import { userFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; -import { SchoolYearQueryType } from '../controller/dto/interface'; +import { School, SchoolService } from '@modules/school/domain'; +import { schoolFactory } from '@modules/school/testing'; +import { ClassRequestContext, SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; import { GroupService } from '../service'; @@ -45,7 +46,7 @@ describe('GroupUc', () => { let systemService: DeepMocked; let userService: DeepMocked; let roleService: DeepMocked; - let schoolService: DeepMocked; + let schoolService: DeepMocked; let authorizationService: DeepMocked; let schoolYearService: DeepMocked; let logger: DeepMocked; @@ -75,8 +76,8 @@ describe('GroupUc', () => { useValue: createMock(), }, { - provide: LegacySchoolService, - useValue: createMock(), + provide: SchoolService, + useValue: createMock(), }, { provide: AuthorizationService, @@ -99,7 +100,7 @@ describe('GroupUc', () => { systemService = module.get(LegacySystemService); userService = module.get(UserService); roleService = module.get(RoleService); - schoolService = module.get(LegacySchoolService); + schoolService = module.get(SchoolService); authorizationService = module.get(AuthorizationService); schoolYearService = module.get(SchoolYearService); logger = module.get(Logger); @@ -118,7 +119,7 @@ describe('GroupUc', () => { describe('findAllClasses', () => { describe('when the user has no permission', () => { const setup = () => { - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const school: School = schoolFactory.build(); const user: User = userFactory.buildWithId(); const error = new ForbiddenException(); @@ -146,7 +147,7 @@ describe('GroupUc', () => { describe('when accessing as a normal user', () => { const setup = () => { - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const school: School = schoolFactory.build({ permissions: { teacher: { STUDENT_LIST: true } } }); const { studentUser } = UserAndAccountTestFactory.buildStudent(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); const teacherRole: RoleDto = roleDtoFactory.buildWithId({ @@ -271,7 +272,7 @@ describe('GroupUc', () => { await uc.findAllClasses(teacherUser.id, teacherUser.school.id, SchoolYearQueryType.CURRENT_YEAR); - expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( + expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, School, AuthorizationContext]>( teacherUser, school, { @@ -292,6 +293,26 @@ describe('GroupUc', () => { ]); }); + describe('when accessing form course as a teacher', () => { + it('should call findClassesForSchool method from classService', async () => { + const { teacherUser } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id, undefined, ClassRequestContext.COURSE); + + expect(classService.findClassesForSchool).toHaveBeenCalled(); + }); + }); + + describe('when accessing form class overview as a teacher', () => { + it('should call findAllByUserId method from classService', async () => { + const { teacherUser } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id, undefined, ClassRequestContext.CLASS_OVERVIEW); + + expect(classService.findAllByUserId).toHaveBeenCalled(); + }); + }); + describe('when no pagination is given', () => { it('should return all classes sorted by name', async () => { const { @@ -385,6 +406,7 @@ describe('GroupUc', () => { SchoolYearQueryType.CURRENT_YEAR, undefined, undefined, + undefined, 'externalSourceName', SortOrder.desc ); @@ -441,6 +463,7 @@ describe('GroupUc', () => { teacherUser.id, teacherUser.school.id, SchoolYearQueryType.CURRENT_YEAR, + undefined, 2, 1, 'name', @@ -523,7 +546,7 @@ describe('GroupUc', () => { describe('when accessing as a user with elevated permission', () => { const setup = (generateClasses = false) => { - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const school: School = schoolFactory.build(); const { studentUser } = UserAndAccountTestFactory.buildStudent(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); const { adminUser } = UserAndAccountTestFactory.buildAdmin(); @@ -652,7 +675,7 @@ describe('GroupUc', () => { await uc.findAllClasses(adminUser.id, adminUser.school.id); - expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( + expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, School, AuthorizationContext]>( adminUser, school, { @@ -733,6 +756,7 @@ describe('GroupUc', () => { undefined, undefined, undefined, + undefined, 'externalSourceName', SortOrder.desc ); @@ -778,6 +802,7 @@ describe('GroupUc', () => { adminUser.id, adminUser.school.id, undefined, + undefined, 1, 1, 'name', @@ -805,6 +830,7 @@ describe('GroupUc', () => { adminUser.id, adminUser.school.id, undefined, + undefined, 0, 5 ); @@ -819,6 +845,7 @@ describe('GroupUc', () => { adminUser.id, adminUser.school.id, undefined, + undefined, 0, -1 ); @@ -830,7 +857,7 @@ describe('GroupUc', () => { describe('when class has a user referenced which is not existing', () => { const setup = () => { - const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const school: School = schoolFactory.build(); const notFoundReferenceId = new ObjectId().toHexString(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 272019c5e77..18bb1c5e466 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -1,20 +1,21 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; -import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; +import { SchoolYearService } from '@modules/legacy-school'; import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { UserService } from '@modules/user'; import { Injectable } from '@nestjs/common'; import { SortHelper } from '@shared/common'; import { ReferencedEntityNotFoundLoggable } from '@shared/common/loggable'; -import { LegacySchoolDo, Page, UserDO } from '@shared/domain/domainobject'; +import { Page, UserDO } from '@shared/domain/domainobject'; import { SchoolYearEntity, User } from '@shared/domain/entity'; import { Permission, SortOrder } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { Logger } from '@src/core/logger'; import { LegacySystemService, SystemDto } from '@src/modules/system'; -import { SchoolYearQueryType } from '../controller/dto/interface'; +import { School, SchoolService } from '@modules/school/domain'; +import { ClassRequestContext, SchoolYearQueryType } from '../controller/dto/interface'; import { Group, GroupTypes, GroupUser } from '../domain'; import { UnknownQueryTypeLoggableException } from '../loggable'; import { GroupService } from '../service'; @@ -29,7 +30,7 @@ export class GroupUc { private readonly systemService: LegacySystemService, private readonly userService: UserService, private readonly roleService: RoleService, - private readonly schoolService: LegacySchoolService, + private readonly schoolService: SchoolService, private readonly authorizationService: AuthorizationService, private readonly schoolYearService: SchoolYearService, private readonly logger: Logger @@ -41,12 +42,13 @@ export class GroupUc { userId: EntityId, schoolId: EntityId, schoolYearQueryType?: SchoolYearQueryType, + calledFrom?: ClassRequestContext, skip = 0, limit?: number, sortBy: keyof ClassInfoDto = 'name', sortOrder: SortOrder = SortOrder.asc ): Promise> { - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolId); + const school: School = await this.schoolService.getSchoolById(schoolId); const user: User = await this.authorizationService.getUserWithPermissions(userId); this.authorizationService.checkPermission( @@ -60,8 +62,11 @@ export class GroupUc { Permission.GROUP_FULL_ADMIN, ]); + const calledFromCourse: boolean = + calledFrom === ClassRequestContext.COURSE && school.getPermissions()?.teacher?.STUDENT_LIST === true; + let combinedClassInfo: ClassInfoDto[]; - if (canSeeFullList) { + if (canSeeFullList || calledFromCourse) { combinedClassInfo = await this.findCombinedClassListForSchool(schoolId, schoolYearQueryType); } else { combinedClassInfo = await this.findCombinedClassListForUser(userId, schoolYearQueryType); diff --git a/apps/server/src/modules/school/domain/do/school.spec.ts b/apps/server/src/modules/school/domain/do/school.spec.ts index faa9214eaba..479f48f28f6 100644 --- a/apps/server/src/modules/school/domain/do/school.spec.ts +++ b/apps/server/src/modules/school/domain/do/school.spec.ts @@ -42,6 +42,45 @@ describe('School', () => { expect(school.getProps().features).not.toContain(feature); }); }); + // TODO N21-1623 add test for getPermissions + describe('getPermissions', () => { + describe('when permissions exist', () => { + const setup = () => { + const permissions = { teacher: { STUDENT_LIST: true } }; + const school = schoolFactory.build({ + permissions, + }); + + return { school, permissions }; + }; + + it('should return permissions', () => { + const { school, permissions } = setup(); + + const result = school.getPermissions(); + + expect(result).toEqual(permissions); + }); + }); + + describe('when permissions are undefined', () => { + const setup = () => { + const school = schoolFactory.build({ + permissions: undefined, + }); + + return { school }; + }; + + it('should return undefined', () => { + const { school } = setup(); + + const result = school.getPermissions(); + + expect(result).toBeUndefined(); + }); + }); + }); describe('isInMaintenance', () => { describe('when inMaintenanceSince is in the past', () => { diff --git a/apps/server/src/modules/school/domain/do/school.ts b/apps/server/src/modules/school/domain/do/school.ts index cb1ecca400f..0cfb3594c7e 100644 --- a/apps/server/src/modules/school/domain/do/school.ts +++ b/apps/server/src/modules/school/domain/do/school.ts @@ -14,6 +14,12 @@ export class School extends DomainObject { this.props.features.delete(feature); } + public getPermissions(): SchoolPermissions | undefined { + const { permissions } = this.props; + + return permissions; + } + public isInMaintenance(): boolean { const result = this.props.inMaintenanceSince ? this.props.inMaintenanceSince <= new Date() : false;