feat(vendors): add pagination and limit validation to GET/vendors#85
feat(vendors): add pagination and limit validation to GET/vendors#85GoluScriptMage wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds query-parameter pagination to the GET /vendors endpoint to avoid returning the full vendors table as it grows, and standardizes the response to include pagination metadata (including total count).
Changes:
- Introduces
page/limitquery validation viaVendorListQueryDto(default page=1, limit=20; max limit=100). - Updates
VendorsService.getAllto fetch a scoped range and return total count using Supabasecount: 'exact'. - Updates
GET /vendorscontroller route and Swagger metadata to reflect the paginated response shape.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/modules/vendors/vendors.service.ts | Implements offset/limit paging via Supabase .range(...) and returns a paginated response envelope. |
| src/modules/vendors/vendors.controller.ts | Binds query DTO for pagination inputs and updates Swagger response contract. |
| src/modules/vendors/dto/vendor-list-response.dto.ts | Adds a paginated list response DTO (data + pagination meta + message). |
| src/modules/vendors/dto/vendor-list-query.dto.ts | Adds validation/transforms for type, page, and limit query parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (error) { | ||
| this.logger.error(`Failed to list vendors: ${error.message}`); | ||
| throw new Error('Failed to list vendors.'); | ||
| } |
| minimum: 1, | ||
| }) | ||
| @IsOptional() | ||
| @Transform(({ value }) => parseInt(value, 10)) |
| async getAll( | ||
| page: number = 1, | ||
| limit: number = 20, | ||
| type?: VendorType, | ||
| ): Promise<VendorListResponseDto> { | ||
| const offset = (page - 1) * limit; | ||
| const client = this.supabaseService.getClient(); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
EmeditWeb
left a comment
There was a problem hiding this comment.
Review verdict: ✅ APPROVE with a minor correction
Clean, standards-compliant pagination. Proper class-validator constraints (@Min, @Max(100), @IsInt, @IsOptional), Swagger decorators (@ApiPropertyOptional, @ApiQuery, response DTO with PaginationMetaDto), correct controller→service delegation with offset = (page - 1) * limit, and no any.
Minor: the page transform is @Transform(({ value }) => parseInt(value, 10)) with no undefined-guard, whereas limit uses @Transform(({ value }) => (value === undefined ? 20 : Number(value))). If page is omitted, parseInt(undefined, 10) is NaN, which would make offset NaN. Align page with the limit pattern (guard undefined, or rely consistently on the default). Trivial fix; otherwise approve.
🔗 Related Issue
endpoint. This prevents performance degradation when the table grows to
hundreds of vendors by replacing the full table scan with scoped database
ranges.
page=1, limit=20, max limit=100).
- [x] Created
VendorListResponseDtoto format responses with standardpagination metadata.
- [x] Updated
VendorsService.getAllto map ranges (.range(offset, offset + limit - 1)) and retrieve total count from Supabase.- [x] Updated
VendorsController.listto bind query decorators and generateSwagger documentation.
completes with zero errors).