Skip to content

Commit

Permalink
N21-1602 fixes default limit for classes with introducing unlimited q…
Browse files Browse the repository at this point in the history
…uery
  • Loading branch information
arnegns committed Dec 18, 2023
1 parent e72dd50 commit f119edf
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 8 deletions.
35 changes: 33 additions & 2 deletions apps/server/src/modules/group/uc/group.uc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -788,6 +797,28 @@ describe('GroupUc', () => {
total: 3,
});
});

it('should return classes with expected limit', async () => {
const { adminUser } = setup(true);

const result: Page<ClassInfoDto> = 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<ClassInfoDto> = await uc.findAllClasses(adminUser.id, adminUser.school.id);

expect(result.data.length).toEqual(14);
});
});
});

Expand Down
10 changes: 8 additions & 2 deletions apps/server/src/modules/group/uc/group.uc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 2 additions & 4 deletions apps/server/src/shared/controller/dto/pagination.params.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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;
}

0 comments on commit f119edf

Please sign in to comment.