From f119edf58de96c4e6f76ab8ee25af489293616d0 Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 18 Dec 2023 10:54:51 +0100 Subject: [PATCH 1/4] N21-1602 fixes default limit for classes with introducing unlimited query --- .../src/modules/group/uc/group.uc.spec.ts | 35 +++++++++++++++++-- apps/server/src/modules/group/uc/group.uc.ts | 10 ++++-- .../controller/dto/pagination.params.ts | 6 ++-- 3 files changed, 43 insertions(+), 8 deletions(-) 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 d1cb9748e7d..90b3b71b570 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -522,7 +522,7 @@ describe('GroupUc', () => { }); describe('when accessing as a user with elevated permission', () => { - const setup = () => { + const setup = (generateClasses = false) => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); const { studentUser } = UserAndAccountTestFactory.buildStudent(); const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); @@ -551,6 +551,15 @@ describe('GroupUc', () => { roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], }); const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + let clazzes: Class[] = []; + if (generateClasses) { + clazzes = classFactory.buildList(11, { + name: 'A', + teacherIds: [teacherUser.id], + source: 'LDAP', + year: schoolYear.id, + }); + } const clazz: Class = classFactory.build({ name: 'A', teacherIds: [teacherUser.id], @@ -579,7 +588,7 @@ describe('GroupUc', () => { schoolService.getSchoolById.mockResolvedValueOnce(school); authorizationService.getUserWithPermissions.mockResolvedValueOnce(adminUser); authorizationService.hasAllPermissions.mockReturnValueOnce(true); - classService.findClassesForSchool.mockResolvedValueOnce([clazz]); + classService.findClassesForSchool.mockResolvedValueOnce([...clazzes, clazz]); groupService.findGroupsBySchoolIdAndGroupTypes.mockResolvedValueOnce([group, groupWithSystem]); systemService.findById.mockResolvedValue(system); @@ -788,6 +797,28 @@ describe('GroupUc', () => { total: 3, }); }); + + it('should return classes with expected limit', async () => { + const { adminUser } = setup(true); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + undefined, + 0, + 5 + ); + + expect(result.data.length).toEqual(5); + }); + + it('should return all classes without limit', async () => { + const { adminUser } = setup(true); + + const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(result.data.length).toEqual(14); + }); }); }); diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 13e4741b289..272019c5e77 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -331,8 +331,14 @@ export class GroupUc { return resolvedGroupUsers; } - private applyPagination(combinedClassInfo: ClassInfoDto[], skip: number, limit: number | undefined) { - const page: ClassInfoDto[] = combinedClassInfo.slice(skip, limit ? skip + limit : combinedClassInfo.length); + private applyPagination(combinedClassInfo: ClassInfoDto[], skip: number, limit: number | undefined): ClassInfoDto[] { + let page: ClassInfoDto[]; + + if (limit === -1) { + page = combinedClassInfo.slice(skip); + } else { + page = combinedClassInfo.slice(skip, limit ? skip + limit : combinedClassInfo.length); + } return page; } diff --git a/apps/server/src/shared/controller/dto/pagination.params.ts b/apps/server/src/shared/controller/dto/pagination.params.ts index e44d4a438f8..5cc5434499f 100644 --- a/apps/server/src/shared/controller/dto/pagination.params.ts +++ b/apps/server/src/shared/controller/dto/pagination.params.ts @@ -1,5 +1,5 @@ -import { IsInt, Max, Min } from 'class-validator'; import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsInt, Min } from 'class-validator'; export class PaginationParams { @IsInt() @@ -8,8 +8,6 @@ export class PaginationParams { skip?: number = 0; @IsInt() - @Min(1) - @Max(100) - @ApiPropertyOptional({ description: 'Page limit, defaults to 10.', minimum: 1, maximum: 99 }) + @ApiPropertyOptional({ description: 'Page limit, defaults to 10.' }) limit?: number = 10; } From 31d8e33899962c10d653f9743141213bfeae9894 Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 18 Dec 2023 13:26:27 +0100 Subject: [PATCH 2/4] N21-1602 adds group pagination params --- .../dto/request/group-pagination.params.ts | 14 ++++++++++++++ .../modules/group/controller/dto/request/index.ts | 1 + .../modules/group/controller/group.controller.ts | 12 +++++++++--- .../src/shared/controller/dto/pagination.params.ts | 6 ++++-- 4 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts diff --git a/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts new file mode 100644 index 00000000000..abce61a1117 --- /dev/null +++ b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts @@ -0,0 +1,14 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { PaginationParams } from '@shared/controller'; +import { IsInt, Min } from 'class-validator'; + +export class GroupPaginationParams extends PaginationParams { + @IsInt() + @Min(0) + @ApiPropertyOptional({ description: 'Number of elements (not pages) to be skipped' }) + skip?: number = 0; + + @IsInt() + @ApiPropertyOptional({ description: 'Page limit, defaults to 10.' }) + limit?: number = 10; +} 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 ceef988aa92..cb39cb2f9c5 100644 --- a/apps/server/src/modules/group/controller/dto/request/index.ts +++ b/apps/server/src/modules/group/controller/dto/request/index.ts @@ -1,3 +1,4 @@ export * from './class-sort-params'; export * from './group-id-params'; export * from './class-filter-params'; +export { GroupPaginationParams } from './group-pagination.params'; diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index 2a0e94893a7..7a5117466b7 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -1,12 +1,18 @@ import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; import { Controller, Get, HttpStatus, Param, Query } from '@nestjs/common'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; -import { PaginationParams } from '@shared/controller'; import { Page } from '@shared/domain/domainobject'; import { ErrorResponse } from '@src/core/error/dto'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; -import { ClassFilterParams, ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse } from './dto'; +import { + ClassFilterParams, + ClassInfoSearchListResponse, + ClassSortParams, + GroupIdParams, + GroupResponse, + GroupPaginationParams, +} from './dto'; import { GroupResponseMapper } from './mapper'; @ApiTags('Group') @@ -21,7 +27,7 @@ export class GroupController { @ApiResponse({ status: '5XX', type: ErrorResponse }) @Get('/class') public async findClasses( - @Query() pagination: PaginationParams, + @Query() pagination: GroupPaginationParams, @Query() sortingQuery: ClassSortParams, @Query() filterParams: ClassFilterParams, @CurrentUser() currentUser: ICurrentUser diff --git a/apps/server/src/shared/controller/dto/pagination.params.ts b/apps/server/src/shared/controller/dto/pagination.params.ts index 5cc5434499f..e44d4a438f8 100644 --- a/apps/server/src/shared/controller/dto/pagination.params.ts +++ b/apps/server/src/shared/controller/dto/pagination.params.ts @@ -1,5 +1,5 @@ +import { IsInt, Max, Min } from 'class-validator'; import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsInt, Min } from 'class-validator'; export class PaginationParams { @IsInt() @@ -8,6 +8,8 @@ export class PaginationParams { skip?: number = 0; @IsInt() - @ApiPropertyOptional({ description: 'Page limit, defaults to 10.' }) + @Min(1) + @Max(100) + @ApiPropertyOptional({ description: 'Page limit, defaults to 10.', minimum: 1, maximum: 99 }) limit?: number = 10; } From f22beb5838b2ed6314d7e5f81aa81d51bca1bcea Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 18 Dec 2023 14:16:35 +0100 Subject: [PATCH 3/4] N21-1602 adds test --- .../controller/dto/request/group-pagination.params.ts | 7 +------ apps/server/src/modules/group/uc/group.uc.spec.ts | 8 +++++++- .../modules/legacy-school/controller/school.controller.ts | 3 +++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts index abce61a1117..b75ba96f73e 100644 --- a/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts +++ b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts @@ -1,13 +1,8 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; import { PaginationParams } from '@shared/controller'; -import { IsInt, Min } from 'class-validator'; +import { IsInt } from 'class-validator'; export class GroupPaginationParams extends PaginationParams { - @IsInt() - @Min(0) - @ApiPropertyOptional({ description: 'Number of elements (not pages) to be skipped' }) - skip?: number = 0; - @IsInt() @ApiPropertyOptional({ description: 'Page limit, defaults to 10.' }) limit?: number = 10; 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 90b3b71b570..e49179202c1 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -815,7 +815,13 @@ describe('GroupUc', () => { it('should return all classes without limit', async () => { const { adminUser } = setup(true); - const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + undefined, + 0, + -1 + ); expect(result.data.length).toEqual(14); }); diff --git a/apps/server/src/modules/legacy-school/controller/school.controller.ts b/apps/server/src/modules/legacy-school/controller/school.controller.ts index 1eef6d58229..50c6c0773ef 100644 --- a/apps/server/src/modules/legacy-school/controller/school.controller.ts +++ b/apps/server/src/modules/legacy-school/controller/school.controller.ts @@ -3,6 +3,7 @@ import { Body, Controller, Get, Param, Post } from '@nestjs/common'; import { ApiBody, ApiCreatedResponse, + ApiExtraModels, ApiForbiddenResponse, ApiNotFoundResponse, ApiOkResponse, @@ -45,6 +46,7 @@ export class SchoolController { @ApiForbiddenResponse() @ApiUnprocessableEntityResponse() @ApiNotFoundResponse() + @ApiExtraModels(SchulConneXProvisioningOptionsResponse) public async getProvisioningOptions( @CurrentUser() currentUser: ICurrentUser, @Param() params: SchoolSystemParams @@ -79,6 +81,7 @@ export class SchoolController { @ApiForbiddenResponse() @ApiUnprocessableEntityResponse() @ApiNotFoundResponse() + @ApiExtraModels(SchulConneXProvisioningOptionsResponse) public async setProvisioningOptions( @CurrentUser() currentUser: ICurrentUser, @Param() params: SchoolSystemParams, From 62179be1ffef95aaa3e68191d9738bdc8bf96588 Mon Sep 17 00:00:00 2001 From: Arne Gnisa Date: Mon, 18 Dec 2023 14:25:21 +0100 Subject: [PATCH 4/4] N21-1602 adds override --- .../group/controller/dto/request/group-pagination.params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts index b75ba96f73e..c96860abf04 100644 --- a/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts +++ b/apps/server/src/modules/group/controller/dto/request/group-pagination.params.ts @@ -5,5 +5,5 @@ import { IsInt } from 'class-validator'; export class GroupPaginationParams extends PaginationParams { @IsInt() @ApiPropertyOptional({ description: 'Page limit, defaults to 10.' }) - limit?: number = 10; + override limit?: number = 10; }