Skip to content

Commit bc9ad66

Browse files
committed
feat(cohorts): add pagination to unbounded find() queries in CohortsService
- Add PaginationQueryDto with page/limit and enforced maximum to getMembers, getThreads, and getAssignments - Return standard paginated envelope consistent with rest of codebase - Add composite indexes on (cohortId, createdAt) to back ordering - Update cohorts controller to accept pagination query params - Add tests asserting bounded page sizes on all three endpoints Closes #1135
1 parent 198d421 commit bc9ad66

6 files changed

Lines changed: 246 additions & 13 deletions

File tree

src/cohorts/cohorts.controller.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Controller, Post, Get, Body, Param, Req, UseGuards, Delete } from '@nestjs/common';
1+
import { Controller, Post, Get, Body, Param, Req, UseGuards, Delete, Query } from '@nestjs/common';
22
import { ApiTags, ApiBearerAuth, ApiOperation, ApiResponse } from '@nestjs/swagger';
33
import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
44
import { CohortsService } from './cohorts.service';
@@ -7,6 +7,7 @@ import { AddCohortMemberDto } from './dto/add-cohort-member.dto';
77
import { CreateCohortThreadDto } from './dto/create-cohort-thread.dto';
88
import { CreateCohortCommentDto } from './dto/create-cohort-comment.dto';
99
import { CreateCohortAssignmentDto } from './dto/create-cohort-assignment.dto';
10+
import { PaginationQueryDto } from '../common/dto/pagination.dto';
1011

1112
@ApiTags('Cohorts')
1213
@Controller('cohorts')
@@ -52,8 +53,8 @@ export class CohortsController {
5253

5354
@Get(':id/members')
5455
@ApiOperation({ summary: 'List cohort members' })
55-
listMembers(@Param('id') id: string, @Req() req: any) {
56-
return this.cohortsService.listMembers(id, req.user.id);
56+
listMembers(@Param('id') id: string, @Req() req: any, @Query() query?: PaginationQueryDto) {
57+
return this.cohortsService.listMembers(id, req.user.id, query);
5758
}
5859

5960
@Post(':id/threads')
@@ -64,8 +65,8 @@ export class CohortsController {
6465

6566
@Get(':id/threads')
6667
@ApiOperation({ summary: 'List discussion threads inside a cohort' })
67-
getThreads(@Param('id') id: string, @Req() req: any) {
68-
return this.cohortsService.listThreads(id, req.user.id);
68+
getThreads(@Param('id') id: string, @Req() req: any, @Query() query?: PaginationQueryDto) {
69+
return this.cohortsService.listThreads(id, req.user.id, query);
6970
}
7071

7172
@Get(':id/threads/:threadId')
@@ -96,8 +97,8 @@ export class CohortsController {
9697

9798
@Get(':id/assignments')
9899
@ApiOperation({ summary: 'List assignments for a cohort' })
99-
getAssignments(@Param('id') id: string, @Req() req: any) {
100-
return this.cohortsService.listAssignments(id, req.user.id);
100+
getAssignments(@Param('id') id: string, @Req() req: any, @Query() query?: PaginationQueryDto) {
101+
return this.cohortsService.listAssignments(id, req.user.id, query);
101102
}
102103

103104
@Get(':id/assignments/:assignmentId')
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
import { CohortsService } from './cohorts.service';
2+
import { buildOffsetResponse, clampLimit } from '../common/utils/pagination.utils';
3+
4+
describe('CohortsService', () => {
5+
let service: CohortsService;
6+
let mockCohortRepo: any;
7+
let mockMemberRepo: any;
8+
let mockThreadRepo: any;
9+
let mockCommentRepo: any;
10+
let mockAssignmentRepo: any;
11+
12+
const mockMembership = { id: 'mem-1', cohortId: 'cohort-1', userId: 'user-1', role: 'member' };
13+
const mockMember = { id: 'm-1', cohortId: 'cohort-1', userId: 'user-2', role: 'member', createdAt: new Date() };
14+
const mockThread = { id: 't-1', cohortId: 'cohort-1', authorId: 'user-2', title: 'Thread', content: 'Content', createdAt: new Date() };
15+
const mockAssignment = { id: 'a-1', cohortId: 'cohort-1', title: 'Assignment', createdAt: new Date() };
16+
17+
beforeEach(() => {
18+
mockCohortRepo = {
19+
findOne: jest.fn().mockResolvedValue(null),
20+
create: jest.fn((dto) => dto),
21+
save: jest.fn(async (data) => ({ id: 'cohort-1', ...data })),
22+
createQueryBuilder: jest.fn(() => ({
23+
innerJoin: jest.fn().mockReturnThis(),
24+
where: jest.fn().mockReturnThis(),
25+
orderBy: jest.fn().mockReturnThis(),
26+
getOne: jest.fn().mockResolvedValue(null),
27+
getMany: jest.fn().mockResolvedValue([]),
28+
})),
29+
};
30+
31+
mockMemberRepo = {
32+
findOne: jest.fn().mockResolvedValue(mockMembership),
33+
create: jest.fn((dto) => dto),
34+
save: jest.fn(async (data) => ({ id: 'new-id', ...data })),
35+
findAndCount: jest.fn().mockResolvedValue([[mockMember], 1]),
36+
};
37+
38+
mockThreadRepo = {
39+
findOne: jest.fn().mockResolvedValue(null),
40+
create: jest.fn((dto) => dto),
41+
save: jest.fn(async (data) => ({ id: 'new-id', ...data })),
42+
findAndCount: jest.fn().mockResolvedValue([[mockThread], 1]),
43+
};
44+
45+
mockCommentRepo = {
46+
create: jest.fn((dto) => dto),
47+
save: jest.fn(async (data) => ({ id: 'new-id', ...data })),
48+
};
49+
50+
mockAssignmentRepo = {
51+
findOne: jest.fn().mockResolvedValue(null),
52+
create: jest.fn((dto) => dto),
53+
save: jest.fn(async (data) => ({ id: 'new-id', ...data })),
54+
findAndCount: jest.fn().mockResolvedValue([[mockAssignment], 1]),
55+
};
56+
57+
service = new CohortsService(
58+
mockCohortRepo,
59+
mockMemberRepo,
60+
mockThreadRepo,
61+
mockCommentRepo,
62+
mockAssignmentRepo,
63+
);
64+
65+
mockCohortRepo.findOne.mockResolvedValue({ id: 'cohort-1', ownerId: 'user-1' });
66+
});
67+
68+
describe('listMembers', () => {
69+
it('should return paginated members with default page size', async () => {
70+
mockMemberRepo.findAndCount.mockResolvedValue([[mockMember], 1]);
71+
72+
const result = await service.listMembers('cohort-1', 'user-1');
73+
74+
expect(result.data).toEqual([mockMember]);
75+
expect(result.total).toBe(1);
76+
expect(result.page).toBe(1);
77+
expect(result.limit).toBe(10);
78+
expect(result.totalPages).toBe(1);
79+
expect(mockMemberRepo.findAndCount).toHaveBeenCalledWith(
80+
expect.objectContaining({ skip: 0, take: 10 }),
81+
);
82+
});
83+
84+
it('should respect custom page and limit', async () => {
85+
const items = Array.from({ length: 5 }, (_, i) => ({
86+
...mockMember,
87+
id: `m-${i + 1}`,
88+
createdAt: new Date(),
89+
}));
90+
mockMemberRepo.findAndCount.mockResolvedValue([items, 25]);
91+
92+
const result = await service.listMembers('cohort-1', 'user-1', { page: 2, limit: 5 });
93+
94+
expect(result.data).toHaveLength(5);
95+
expect(result.total).toBe(25);
96+
expect(result.page).toBe(2);
97+
expect(result.limit).toBe(5);
98+
expect(result.totalPages).toBe(5);
99+
expect(result.hasNextPage).toBe(true);
100+
expect(result.hasPrevPage).toBe(true);
101+
expect(mockMemberRepo.findAndCount).toHaveBeenCalledWith(
102+
expect.objectContaining({ skip: 5, take: 5 }),
103+
);
104+
});
105+
106+
it('should enforce max page size via clampLimit', async () => {
107+
mockMemberRepo.findAndCount.mockResolvedValue([[mockMember], 1]);
108+
109+
await service.listMembers('cohort-1', 'user-1', { page: 1, limit: 999 });
110+
111+
expect(mockMemberRepo.findAndCount).toHaveBeenCalledWith(
112+
expect.objectContaining({ take: 100 }),
113+
);
114+
});
115+
116+
it('should return first page when no query is provided', async () => {
117+
mockMemberRepo.findAndCount.mockResolvedValue([[mockMember], 1]);
118+
119+
const result = await service.listMembers('cohort-1', 'user-1', undefined);
120+
121+
expect(result.page).toBe(1);
122+
expect(mockMemberRepo.findAndCount).toHaveBeenCalledWith(
123+
expect.objectContaining({ skip: 0, take: 10 }),
124+
);
125+
});
126+
127+
it('should reject non-members with ForbiddenException', async () => {
128+
mockMemberRepo.findOne.mockResolvedValue(null);
129+
130+
await expect(service.listMembers('cohort-1', 'user-2')).rejects.toThrow('Access denied');
131+
});
132+
});
133+
134+
describe('listThreads', () => {
135+
it('should return paginated threads ordered by createdAt DESC', async () => {
136+
mockThreadRepo.findAndCount.mockResolvedValue([[mockThread], 1]);
137+
138+
const result = await service.listThreads('cohort-1', 'user-1', { page: 1, limit: 10 });
139+
140+
expect(result.data).toEqual([mockThread]);
141+
expect(result.total).toBe(1);
142+
expect(result.page).toBe(1);
143+
expect(mockThreadRepo.findAndCount).toHaveBeenCalledWith(
144+
expect.objectContaining({
145+
where: { cohortId: 'cohort-1' },
146+
order: { createdAt: 'DESC' },
147+
skip: 0,
148+
take: 10,
149+
}),
150+
);
151+
});
152+
});
153+
154+
describe('listAssignments', () => {
155+
it('should return paginated assignments ordered by createdAt DESC', async () => {
156+
mockAssignmentRepo.findAndCount.mockResolvedValue([[mockAssignment], 1]);
157+
158+
const result = await service.listAssignments('cohort-1', 'user-1', { page: 1, limit: 10 });
159+
160+
expect(result.data).toEqual([mockAssignment]);
161+
expect(result.total).toBe(1);
162+
expect(result.page).toBe(1);
163+
expect(mockAssignmentRepo.findAndCount).toHaveBeenCalledWith(
164+
expect.objectContaining({
165+
where: { cohortId: 'cohort-1' },
166+
order: { createdAt: 'DESC' },
167+
skip: 0,
168+
take: 10,
169+
}),
170+
);
171+
});
172+
});
173+
174+
describe('clampLimit', () => {
175+
it('should default to DEFAULT_PAGE_SIZE when limit is undefined', () => {
176+
expect(clampLimit(undefined)).toBe(10);
177+
});
178+
179+
it('should cap at MAX_PAGE_SIZE', () => {
180+
expect(clampLimit(200)).toBe(100);
181+
});
182+
183+
it('should enforce minimum of 1', () => {
184+
expect(clampLimit(0)).toBe(1);
185+
});
186+
});
187+
});

src/cohorts/cohorts.service.ts

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import { AddCohortMemberDto } from './dto/add-cohort-member.dto';
1616
import { CreateCohortThreadDto } from './dto/create-cohort-thread.dto';
1717
import { CreateCohortCommentDto } from './dto/create-cohort-comment.dto';
1818
import { CreateCohortAssignmentDto } from './dto/create-cohort-assignment.dto';
19+
import { PaginationQueryDto } from '../common/dto/pagination.dto';
20+
import { OffsetPaginatedResponse } from '../common/interfaces/pagination.interface';
21+
import { buildOffsetResponse, clampLimit } from '../common/utils/pagination.utils';
1922

2023
@Injectable()
2124
export class CohortsService {
@@ -106,9 +109,23 @@ export class CohortsService {
106109
await this.memberRepo.remove(membership);
107110
}
108111

109-
async listMembers(cohortId: string, userId: string): Promise<CohortMember[]> {
112+
async listMembers(
113+
cohortId: string,
114+
userId: string,
115+
query?: PaginationQueryDto,
116+
): Promise<OffsetPaginatedResponse<CohortMember>> {
110117
await this.requireMembership(cohortId, userId);
111-
return this.memberRepo.find({ where: { cohortId }, order: { createdAt: 'ASC' } });
118+
const limit = clampLimit(query?.limit);
119+
const page = query?.page ?? 1;
120+
const skip = (page - 1) * limit;
121+
122+
const [data, total] = await this.memberRepo.findAndCount({
123+
where: { cohortId },
124+
order: { createdAt: 'ASC' },
125+
skip,
126+
take: limit,
127+
});
128+
return buildOffsetResponse(data, total, page, limit);
112129
}
113130

114131
async createThread(
@@ -127,12 +144,23 @@ export class CohortsService {
127144
return this.threadRepo.save(thread);
128145
}
129146

130-
async listThreads(cohortId: string, userId: string): Promise<CohortThread[]> {
147+
async listThreads(
148+
cohortId: string,
149+
userId: string,
150+
query?: PaginationQueryDto,
151+
): Promise<OffsetPaginatedResponse<CohortThread>> {
131152
await this.requireMembership(cohortId, userId);
132-
return this.threadRepo.find({
153+
const limit = clampLimit(query?.limit);
154+
const page = query?.page ?? 1;
155+
const skip = (page - 1) * limit;
156+
157+
const [data, total] = await this.threadRepo.findAndCount({
133158
where: { cohortId },
134159
order: { createdAt: 'DESC' },
160+
skip,
161+
take: limit,
135162
});
163+
return buildOffsetResponse(data, total, page, limit);
136164
}
137165

138166
async getThread(threadId: string, userId: string): Promise<CohortThread> {
@@ -189,9 +217,23 @@ export class CohortsService {
189217
return this.assignmentRepo.save(assignment);
190218
}
191219

192-
async listAssignments(cohortId: string, userId: string): Promise<CohortAssignment[]> {
220+
async listAssignments(
221+
cohortId: string,
222+
userId: string,
223+
query?: PaginationQueryDto,
224+
): Promise<OffsetPaginatedResponse<CohortAssignment>> {
193225
await this.requireMembership(cohortId, userId);
194-
return this.assignmentRepo.find({ where: { cohortId }, order: { createdAt: 'DESC' } });
226+
const limit = clampLimit(query?.limit);
227+
const page = query?.page ?? 1;
228+
const skip = (page - 1) * limit;
229+
230+
const [data, total] = await this.assignmentRepo.findAndCount({
231+
where: { cohortId },
232+
order: { createdAt: 'DESC' },
233+
skip,
234+
take: limit,
235+
});
236+
return buildOffsetResponse(data, total, page, limit);
195237
}
196238

197239
async getAssignment(

src/cohorts/entities/cohort-assignment.entity.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export enum CohortAssignmentStatus {
1515
}
1616

1717
@Entity('cohort_assignments')
18+
@Index(['cohortId', 'createdAt'])
1819
export class CohortAssignment {
1920
@PrimaryGeneratedColumn('uuid')
2021
id: string;

src/cohorts/entities/cohort-member.entity.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
import { Cohort } from './cohort.entity';
1111

1212
@Entity('cohort_members')
13+
@Index(['cohortId', 'createdAt'])
1314
export class CohortMember {
1415
@PrimaryGeneratedColumn('uuid')
1516
id: string;

src/cohorts/entities/cohort-thread.entity.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Cohort } from './cohort.entity';
1212
import { CohortComment } from './cohort-comment.entity';
1313

1414
@Entity('cohort_threads')
15+
@Index(['cohortId', 'createdAt'])
1516
export class CohortThread {
1617
@PrimaryGeneratedColumn('uuid')
1718
id: string;

0 commit comments

Comments
 (0)