From a3173a2a7c862f5492b2f3a50452e81ef3fb2da6 Mon Sep 17 00:00:00 2001 From: ghzhost Date: Sun, 16 Aug 2026 01:48:51 +0000 Subject: [PATCH] fix(api): add limit/offset pagination and caps to list endpoints (#63) --- src/bounties/bounties-pagination.spec.ts | 94 +++++++++++++++++++ src/bounties/bounties.controller.ts | 8 +- src/bounties/bounties.service.ts | 29 +++++- src/bounties/dto/list-bounties.dto.ts | 11 +++ src/common/dto/pagination.dto.ts | 55 +++++++++++ .../maintenance-pool.controller.ts | 8 +- .../maintenance-pool.service.ts | 25 ++++- src/milestones/milestones.controller.ts | 8 +- src/milestones/milestones.service.ts | 23 ++++- src/users/users-pagination.spec.ts | 59 ++++++++++++ src/users/users.controller.ts | 8 +- src/users/users.service.ts | 23 ++++- 12 files changed, 330 insertions(+), 21 deletions(-) create mode 100644 src/bounties/bounties-pagination.spec.ts create mode 100644 src/bounties/dto/list-bounties.dto.ts create mode 100644 src/common/dto/pagination.dto.ts create mode 100644 src/users/users-pagination.spec.ts diff --git a/src/bounties/bounties-pagination.spec.ts b/src/bounties/bounties-pagination.spec.ts new file mode 100644 index 0000000..ee1b785 --- /dev/null +++ b/src/bounties/bounties-pagination.spec.ts @@ -0,0 +1,94 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { BountiesService } from './bounties.service'; +import { EscrowService } from '../escrow/escrow.service'; +import { Bounty, Team, User } from '../common/entities'; +import { AssetType, BountyDifficulty, BountyStatus } from '../common/enums'; +import { DEFAULT_PAGE_LIMIT, MAX_PAGE_LIMIT } from '../common/dto/pagination.dto'; + +describe('BountiesService list pagination', () => { + let service: BountiesService; + let bountyRepo: { findAndCount: jest.Mock }; + + beforeEach(async () => { + bountyRepo = { + findAndCount: jest.fn(), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + BountiesService, + { provide: getRepositoryToken(Bounty), useValue: bountyRepo }, + { provide: getRepositoryToken(User), useValue: {} }, + { provide: getRepositoryToken(Team), useValue: {} }, + { provide: EscrowService, useValue: {} }, + ], + }).compile(); + + service = module.get(BountiesService); + }); + + it('defaults limit to DEFAULT_PAGE_LIMIT (20) and offset to 0', async () => { + const fakeBounties = Array.from({ length: 20 }, (_, i) => ({ + id: `bounty-${i}`, + status: BountyStatus.OPEN, + })) as Bounty[]; + + bountyRepo.findAndCount.mockResolvedValue([fakeBounties, 45]); + + const result = await service.list(); + + expect(bountyRepo.findAndCount).toHaveBeenCalledWith({ + where: {}, + take: DEFAULT_PAGE_LIMIT, + skip: 0, + order: { createdAt: 'DESC' }, + }); + + expect(result.data.length).toBe(20); + expect(result.total).toBe(45); + expect(result.limit).toBe(20); + expect(result.offset).toBe(0); + expect(result.hasMore).toBe(true); + }); + + it('enforces MAX_PAGE_LIMIT when caller passes a higher limit', async () => { + bountyRepo.findAndCount.mockResolvedValue([[], 0]); + + await service.list({ limit: 5000, offset: 10 }); + + expect(bountyRepo.findAndCount).toHaveBeenCalledWith({ + where: {}, + take: MAX_PAGE_LIMIT, + skip: 10, + order: { createdAt: 'DESC' }, + }); + }); + + it('filters by status when provided in query', async () => { + bountyRepo.findAndCount.mockResolvedValue([[], 0]); + + await service.list({ status: BountyStatus.FUNDED, limit: 10, offset: 5 }); + + expect(bountyRepo.findAndCount).toHaveBeenCalledWith({ + where: { status: BountyStatus.FUNDED }, + take: 10, + skip: 5, + order: { createdAt: 'DESC' }, + }); + }); + + it('correctly calculates hasMore = false when at end of list', async () => { + const fakeBounties = Array.from({ length: 5 }, (_, i) => ({ + id: `bounty-${i}`, + status: BountyStatus.OPEN, + })) as Bounty[]; + + bountyRepo.findAndCount.mockResolvedValue([fakeBounties, 25]); + + const result = await service.list({ limit: 20, offset: 20 }); + + expect(result.hasMore).toBe(false); + expect(result.total).toBe(25); + }); +}); diff --git a/src/bounties/bounties.controller.ts b/src/bounties/bounties.controller.ts index 173df3d..5773579 100644 --- a/src/bounties/bounties.controller.ts +++ b/src/bounties/bounties.controller.ts @@ -4,8 +4,10 @@ import { IsString } from 'class-validator'; import { BountiesService } from './bounties.service'; import { CreateBountyDto } from './dto/create-bounty.dto'; import { ClaimBountyDto } from './dto/claim-bounty.dto'; -import { BountyStatus } from '../common/enums'; +import { ListBountiesDto } from './dto/list-bounties.dto'; import { Idempotent } from '../common/idempotency/idempotent.decorator'; +import { PaginatedResponse } from '../common/dto/pagination.dto'; +import { Bounty } from '../common/entities'; class FundBountyDto { @IsString() @@ -23,8 +25,8 @@ export class BountiesController { } @Get() - list(@Query('status') status?: BountyStatus) { - return this.bountiesService.list(status); + list(@Query() query: ListBountiesDto): Promise> { + return this.bountiesService.list(query); } @Get(':id') diff --git a/src/bounties/bounties.service.ts b/src/bounties/bounties.service.ts index 7bf75f8..f869eb0 100644 --- a/src/bounties/bounties.service.ts +++ b/src/bounties/bounties.service.ts @@ -1,11 +1,18 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { FindOptionsWhere, Repository } from 'typeorm'; import { Bounty, Team, User } from '../common/entities'; import { BountyStatus } from '../common/enums'; import { assertTransition } from './bounty-state-machine'; import { EscrowService } from '../escrow/escrow.service'; import { CreateBountyDto } from './dto/create-bounty.dto'; +import { ListBountiesDto } from './dto/list-bounties.dto'; +import { + DEFAULT_PAGE_LIMIT, + MAX_PAGE_LIMIT, + PaginatedResponse, + buildPaginatedResponse, +} from '../common/dto/pagination.dto'; @Injectable() export class BountiesService { @@ -171,7 +178,23 @@ export class BountiesService { return overdue.length; } - async list(status?: BountyStatus): Promise { - return this.bountyRepo.find({ where: status ? { status } : {} }); + async list(query?: ListBountiesDto): Promise> { + const limit = Math.min( + Math.max(Number(query?.limit) || DEFAULT_PAGE_LIMIT, 1), + MAX_PAGE_LIMIT, + ); + const offset = Math.max(Number(query?.offset) || 0, 0); + const where: FindOptionsWhere = query?.status + ? { status: query.status } + : {}; + + const [data, total] = await this.bountyRepo.findAndCount({ + where, + take: limit, + skip: offset, + order: { createdAt: 'DESC' }, + }); + + return buildPaginatedResponse(data, total, limit, offset); } } diff --git a/src/bounties/dto/list-bounties.dto.ts b/src/bounties/dto/list-bounties.dto.ts new file mode 100644 index 0000000..cdbe50f --- /dev/null +++ b/src/bounties/dto/list-bounties.dto.ts @@ -0,0 +1,11 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { IsEnum, IsOptional } from 'class-validator'; +import { BountyStatus } from '../../common/enums'; +import { PaginationQueryDto } from '../../common/dto/pagination.dto'; + +export class ListBountiesDto extends PaginationQueryDto { + @ApiPropertyOptional({ enum: BountyStatus, description: 'Filter bounties by status' }) + @IsOptional() + @IsEnum(BountyStatus) + status?: BountyStatus; +} diff --git a/src/common/dto/pagination.dto.ts b/src/common/dto/pagination.dto.ts new file mode 100644 index 0000000..1f49b39 --- /dev/null +++ b/src/common/dto/pagination.dto.ts @@ -0,0 +1,55 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { Type } from 'class-transformer'; +import { IsInt, IsOptional, Max, Min } from 'class-validator'; + +export const DEFAULT_PAGE_LIMIT = 20; +export const MAX_PAGE_LIMIT = 100; + +export class PaginationQueryDto { + @ApiPropertyOptional({ + description: `Number of records to return (default: ${DEFAULT_PAGE_LIMIT}, max: ${MAX_PAGE_LIMIT})`, + default: DEFAULT_PAGE_LIMIT, + minimum: 1, + maximum: MAX_PAGE_LIMIT, + }) + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(MAX_PAGE_LIMIT) + limit: number = DEFAULT_PAGE_LIMIT; + + @ApiPropertyOptional({ + description: 'Number of records to skip (default: 0)', + default: 0, + minimum: 0, + }) + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(0) + offset: number = 0; +} + +export interface PaginatedResponse { + data: T[]; + total: number; + limit: number; + offset: number; + hasMore: boolean; +} + +export function buildPaginatedResponse( + data: T[], + total: number, + limit: number = DEFAULT_PAGE_LIMIT, + offset: number = 0, +): PaginatedResponse { + return { + data, + total, + limit, + offset, + hasMore: offset + data.length < total, + }; +} diff --git a/src/maintenance-pool/maintenance-pool.controller.ts b/src/maintenance-pool/maintenance-pool.controller.ts index 3582585..d0ba029 100644 --- a/src/maintenance-pool/maintenance-pool.controller.ts +++ b/src/maintenance-pool/maintenance-pool.controller.ts @@ -1,10 +1,12 @@ -import { Body, Controller, Get, Param, Post } from '@nestjs/common'; +import { Body, Controller, Get, Param, Post, Query } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { IsOptional, IsString, IsUUID } from 'class-validator'; import { MaintenancePoolService } from './maintenance-pool.service'; import { CreatePoolDto } from './dto/create-pool.dto'; import { IsMoneyAmount } from '../common/validators/money.validator'; import { Idempotent } from '../common/idempotency/idempotent.decorator'; +import { PaginationQueryDto, PaginatedResponse } from '../common/dto/pagination.dto'; +import { MaintenancePool } from '../common/entities'; class DepositDto { @IsMoneyAmount() @@ -37,8 +39,8 @@ export class MaintenancePoolController { } @Get() - list() { - return this.poolService.list(); + list(@Query() query: PaginationQueryDto): Promise> { + return this.poolService.list(query); } @Get(':id') diff --git a/src/maintenance-pool/maintenance-pool.service.ts b/src/maintenance-pool/maintenance-pool.service.ts index 258603a..f4ff0ca 100644 --- a/src/maintenance-pool/maintenance-pool.service.ts +++ b/src/maintenance-pool/maintenance-pool.service.ts @@ -9,6 +9,13 @@ import { MaintenancePool } from '../common/entities'; import { MaintenancePoolStatus } from '../common/enums'; import { EscrowService } from '../escrow/escrow.service'; import { CreatePoolDto } from './dto/create-pool.dto'; +import { + DEFAULT_PAGE_LIMIT, + MAX_PAGE_LIMIT, + PaginationQueryDto, + PaginatedResponse, + buildPaginatedResponse, +} from '../common/dto/pagination.dto'; /** * Recurring maintenance pool: sponsors make monthly deposits into a shared @@ -106,7 +113,21 @@ export class MaintenancePoolService { return payment; } - async list(): Promise { - return this.poolRepo.find(); + async list( + query?: PaginationQueryDto, + ): Promise> { + const limit = Math.min( + Math.max(Number(query?.limit) || DEFAULT_PAGE_LIMIT, 1), + MAX_PAGE_LIMIT, + ); + const offset = Math.max(Number(query?.offset) || 0, 0); + + const [data, total] = await this.poolRepo.findAndCount({ + take: limit, + skip: offset, + order: { createdAt: 'DESC' }, + }); + + return buildPaginatedResponse(data, total, limit, offset); } } diff --git a/src/milestones/milestones.controller.ts b/src/milestones/milestones.controller.ts index 0da2760..f55aa79 100644 --- a/src/milestones/milestones.controller.ts +++ b/src/milestones/milestones.controller.ts @@ -1,9 +1,11 @@ -import { Body, Controller, Get, Param, Post } from '@nestjs/common'; +import { Body, Controller, Get, Param, Post, Query } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; import { IsOptional, IsString, IsUUID } from 'class-validator'; import { MilestonesService } from './milestones.service'; import { CreateMilestoneDto } from './dto/create-milestone.dto'; import { Idempotent } from '../common/idempotency/idempotent.decorator'; +import { PaginationQueryDto, PaginatedResponse } from '../common/dto/pagination.dto'; +import { Milestone } from '../common/entities'; class FundMilestoneDto { @IsString() @@ -30,8 +32,8 @@ export class MilestonesController { } @Get() - list() { - return this.milestonesService.list(); + list(@Query() query: PaginationQueryDto): Promise> { + return this.milestonesService.list(query); } @Get(':id') diff --git a/src/milestones/milestones.service.ts b/src/milestones/milestones.service.ts index 43bb7fb..8ac964e 100644 --- a/src/milestones/milestones.service.ts +++ b/src/milestones/milestones.service.ts @@ -9,6 +9,13 @@ import { Issue, Milestone } from '../common/entities'; import { MilestoneStatus } from '../common/enums'; import { EscrowService } from '../escrow/escrow.service'; import { CreateMilestoneDto } from './dto/create-milestone.dto'; +import { + DEFAULT_PAGE_LIMIT, + MAX_PAGE_LIMIT, + PaginationQueryDto, + PaginatedResponse, + buildPaginatedResponse, +} from '../common/dto/pagination.dto'; @Injectable() export class MilestonesService { @@ -128,7 +135,19 @@ export class MilestonesService { return payment; } - async list(): Promise { - return this.milestoneRepo.find(); + async list(query?: PaginationQueryDto): Promise> { + const limit = Math.min( + Math.max(Number(query?.limit) || DEFAULT_PAGE_LIMIT, 1), + MAX_PAGE_LIMIT, + ); + const offset = Math.max(Number(query?.offset) || 0, 0); + + const [data, total] = await this.milestoneRepo.findAndCount({ + take: limit, + skip: offset, + order: { createdAt: 'DESC' }, + }); + + return buildPaginatedResponse(data, total, limit, offset); } } diff --git a/src/users/users-pagination.spec.ts b/src/users/users-pagination.spec.ts new file mode 100644 index 0000000..4eb7a2d --- /dev/null +++ b/src/users/users-pagination.spec.ts @@ -0,0 +1,59 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { UsersService } from './users.service'; +import { GithubAccount, User } from '../common/entities'; +import { DEFAULT_PAGE_LIMIT, MAX_PAGE_LIMIT } from '../common/dto/pagination.dto'; + +describe('UsersService list pagination', () => { + let service: UsersService; + let userRepo: { findAndCount: jest.Mock }; + + beforeEach(async () => { + userRepo = { + findAndCount: jest.fn(), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + UsersService, + { provide: getRepositoryToken(User), useValue: userRepo }, + { provide: getRepositoryToken(GithubAccount), useValue: {} }, + ], + }).compile(); + + service = module.get(UsersService); + }); + + it('paginates users list with defaults and caps limit', async () => { + const fakeUsers = Array.from({ length: 15 }, (_, i) => ({ + id: `user-${i}`, + username: `user_${i}`, + })) as User[]; + + userRepo.findAndCount.mockResolvedValue([fakeUsers, 100]); + + const result = await service.list({ limit: 15, offset: 0 }); + + expect(userRepo.findAndCount).toHaveBeenCalledWith({ + take: 15, + skip: 0, + order: { createdAt: 'DESC' }, + }); + + expect(result.data.length).toBe(15); + expect(result.total).toBe(100); + expect(result.hasMore).toBe(true); + }); + + it('caps max limit to MAX_PAGE_LIMIT (100)', async () => { + userRepo.findAndCount.mockResolvedValue([[], 0]); + + await service.list({ limit: 9999, offset: 0 }); + + expect(userRepo.findAndCount).toHaveBeenCalledWith({ + take: MAX_PAGE_LIMIT, + skip: 0, + order: { createdAt: 'DESC' }, + }); + }); +}); diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 37a22bb..908eb58 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -1,8 +1,10 @@ -import { Body, Controller, Get, Param, Patch, UseGuards } from '@nestjs/common'; +import { Body, Controller, Get, Param, Patch, Query, UseGuards } from '@nestjs/common'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; import { IsString } from 'class-validator'; import { UsersService } from './users.service'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; +import { PaginationQueryDto, PaginatedResponse } from '../common/dto/pagination.dto'; +import { User } from '../common/entities'; class SetStellarAddressDto { @IsString() @@ -15,8 +17,8 @@ export class UsersController { constructor(private readonly usersService: UsersService) {} @Get() - list() { - return this.usersService.list(); + list(@Query() query: PaginationQueryDto): Promise> { + return this.usersService.list(query); } @Get(':id') diff --git a/src/users/users.service.ts b/src/users/users.service.ts index faf55c7..cf3f499 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -3,6 +3,13 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { GithubAccount, User } from '../common/entities'; import { UserRole } from '../common/enums'; +import { + DEFAULT_PAGE_LIMIT, + MAX_PAGE_LIMIT, + PaginationQueryDto, + PaginatedResponse, + buildPaginatedResponse, +} from '../common/dto/pagination.dto'; export interface UpsertFromGithubInput { githubId: string; @@ -98,7 +105,19 @@ export class UsersService { return this.userRepo.save(user); } - async list(): Promise { - return this.userRepo.find(); + async list(query?: PaginationQueryDto): Promise> { + const limit = Math.min( + Math.max(Number(query?.limit) || DEFAULT_PAGE_LIMIT, 1), + MAX_PAGE_LIMIT, + ); + const offset = Math.max(Number(query?.offset) || 0, 0); + + const [data, total] = await this.userRepo.findAndCount({ + take: limit, + skip: offset, + order: { createdAt: 'DESC' }, + }); + + return buildPaginatedResponse(data, total, limit, offset); } }