From 1133f1bbee627a3f0dc1f1df88a908d00c31b832 Mon Sep 17 00:00:00 2001 From: _david Date: Wed, 2 Sep 2026 18:32:55 +0700 Subject: [PATCH] feat(candidate_profile): add pagination and sort to CV section list endpoints (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/v1/{education|experience|award|certificate|project|reference} now accept optional `page`, `limit`, `sort` query params. Backward compatible by design: omitting `limit` returns the exact same response as before (`data` is the full, unpaginated array). Passing a valid `limit` switches `data` to `{ items, pagination: { page, limit, total, totalPages } }`. `limit` is capped at 100 regardless of what's requested. `sort` accepts a Mongoose sort expression (e.g. `-createdAt`) validated against an allowlist regex (no $, can't smuggle an operator); an invalid value is silently ignored rather than erroring. - services/index.ts: baseFindDocument gains page/limit/sort, applies .sort()/.skip()/.limit() + a parallel countDocuments() only when a valid limit is given - candidate_profile/BaseController.ts: baseGetAll parses page/limit/sort from req.query, validates sort against an allowlist - swagger.config.ts: shared PageParam/LimitParam/SortParam + Pagination schema - 6 CV-section routers (education/experience/award/certificate/project/ reference): wired the new query params into their GET / swagger docs. generalInformation excluded — its GET / returns a single per-candidate document, not a list, so pagination doesn't apply. - tests: baseFindDocument.test.ts, BaseController.test.ts Closes #73 --- .../candidate_profile/BaseController.test.ts | 57 ++++++++++++ .../services/baseFindDocument.test.ts | 93 +++++++++++++++++++ src/candidate_profile/BaseController.ts | 13 +++ src/config/swagger.config.ts | 39 ++++++++ src/routers/api/v1/award.route.ts | 6 +- src/routers/api/v1/certificate.route.ts | 6 +- src/routers/api/v1/education.route.ts | 6 +- src/routers/api/v1/experience.route.ts | 6 +- src/routers/api/v1/project.route.ts | 6 +- src/routers/api/v1/reference.route.ts | 6 +- src/services/index.ts | 48 ++++++++-- 11 files changed, 274 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/candidate_profile/BaseController.test.ts create mode 100644 src/__tests__/services/baseFindDocument.test.ts diff --git a/src/__tests__/candidate_profile/BaseController.test.ts b/src/__tests__/candidate_profile/BaseController.test.ts new file mode 100644 index 0000000..ca50e9d --- /dev/null +++ b/src/__tests__/candidate_profile/BaseController.test.ts @@ -0,0 +1,57 @@ +/** + * Tests for candidate_profile/BaseController.ts's baseGetAll — specifically + * the page/limit/sort query-string parsing added for issue #73. + */ +import { baseGetAll } from '@/candidate_profile/BaseController'; +import * as services from '@/services'; + +jest.mock('@/services'); + +const mockedBaseFindDocument = services.baseFindDocument as jest.MockedFunction; + +function createMocks(query: Record = {}) { + const req: any = { body: { candidateId: 'c1', collection: 'experiences' }, query }; + const json = jest.fn(); + const res: any = { status: jest.fn().mockReturnValue({ json }), json }; + const next = jest.fn(); + return { req, res, next }; +} + +describe('baseGetAll', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedBaseFindDocument.mockResolvedValue({ success: true, message: '', errors: null, data: [] }); + }); + + it('passes page/limit/sort through as numbers/string when present', async () => { + const { req, res, next } = createMocks({ page: '2', limit: '10', sort: '-createdAt' }); + await baseGetAll(req, res, next); + + expect(mockedBaseFindDocument).toHaveBeenCalledWith( + expect.objectContaining({ page: 2, limit: 10, sort: '-createdAt' }), + ); + }); + + it('omits page/limit/sort when the query string has none (backward compatible)', async () => { + const { req, res, next } = createMocks(); + await baseGetAll(req, res, next); + + expect(mockedBaseFindDocument).toHaveBeenCalledWith( + expect.objectContaining({ page: undefined, limit: undefined, sort: undefined }), + ); + }); + + it('silently drops a sort value that could smuggle a Mongo operator', async () => { + const { req, res, next } = createMocks({ sort: '$where' }); + await baseGetAll(req, res, next); + + expect(mockedBaseFindDocument).toHaveBeenCalledWith(expect.objectContaining({ sort: undefined })); + }); + + it('accepts a leading "-" in sort for descending order', async () => { + const { req, res, next } = createMocks({ sort: '-startDate' }); + await baseGetAll(req, res, next); + + expect(mockedBaseFindDocument).toHaveBeenCalledWith(expect.objectContaining({ sort: '-startDate' })); + }); +}); diff --git a/src/__tests__/services/baseFindDocument.test.ts b/src/__tests__/services/baseFindDocument.test.ts new file mode 100644 index 0000000..84e7b8e --- /dev/null +++ b/src/__tests__/services/baseFindDocument.test.ts @@ -0,0 +1,93 @@ +/** + * Tests for services/index.ts's baseFindDocument — specifically the + * pagination/sort support added for issue #73. Uses a fake Mongoose-shaped + * model instead of jest.mock('@/utils/querySafe') since QuerySafe has no + * side effects worth mocking out. + */ +import { baseFindDocument } from '@/services'; + +function createFakeModel(docs: Record[]) { + const query: any = { + sort: jest.fn(), + skip: jest.fn(), + limit: jest.fn(), + exec: jest.fn().mockResolvedValue(docs), + }; + // chainable: each call returns the same query object + query.sort.mockReturnValue(query); + query.skip.mockReturnValue(query); + query.limit.mockReturnValue(query); + + return { + find: jest.fn().mockReturnValue(query), + findOne: jest.fn().mockReturnValue({ exec: jest.fn().mockResolvedValue(docs[0] ?? null) }), + countDocuments: jest.fn().mockResolvedValue(docs.length), + __query: query, + }; +} + +describe('baseFindDocument', () => { + it('fails fast when fields is empty', async () => { + const model = createFakeModel([]); + const result = await baseFindDocument({ model, fields: {}, findOne: false }); + expect(result.success).toBe(false); + expect(model.find).not.toHaveBeenCalled(); + }); + + it('findOne: true returns a single document via MODEL.findOne, untouched by pagination', async () => { + const model = createFakeModel([{ _id: '1', candidateId: 'c1' }]); + const result = await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: true }); + + expect(model.findOne).toHaveBeenCalledWith({ candidateId: 'c1' }); + expect(result).toEqual({ success: true, message: '', errors: null, data: { _id: '1', candidateId: 'c1' } }); + }); + + it('findOne: false, no limit -> returns the full array unchanged (backward compatible)', async () => { + const docs = [{ _id: '1' }, { _id: '2' }]; + const model = createFakeModel(docs); + + const result = await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: false }); + + expect(model.find).toHaveBeenCalledWith({ candidateId: 'c1' }); + expect(model.__query.skip).not.toHaveBeenCalled(); + expect(model.__query.limit).not.toHaveBeenCalled(); + expect(model.countDocuments).not.toHaveBeenCalled(); + expect(result.data).toEqual(docs); + }); + + it('findOne: false, with a valid limit -> paginates and wraps data as { items, pagination }', async () => { + const docs = [{ _id: '1' }, { _id: '2' }]; + const model = createFakeModel(docs); + + const result = await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: false, page: 2, limit: 2 }); + + expect(model.__query.skip).toHaveBeenCalledWith(2); // (page 2 - 1) * limit 2 + expect(model.__query.limit).toHaveBeenCalledWith(2); + expect(model.countDocuments).toHaveBeenCalledWith({ candidateId: 'c1' }); + expect(result.data).toEqual({ + items: docs, + pagination: { page: 2, limit: 2, total: 2, totalPages: 1 }, + }); + }); + + it('clamps limit to the max page size', async () => { + const model = createFakeModel([]); + await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: false, limit: 9999 }); + + expect(model.__query.limit).toHaveBeenCalledWith(100); + }); + + it('defaults page to 1 when page is missing or invalid', async () => { + const model = createFakeModel([]); + await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: false, limit: 10, page: 0 }); + + expect(model.__query.skip).toHaveBeenCalledWith(0); + }); + + it('applies sort when given, with or without pagination', async () => { + const model = createFakeModel([]); + await baseFindDocument({ model, fields: { candidateId: 'c1' }, findOne: false, sort: '-createdAt' }); + + expect(model.__query.sort).toHaveBeenCalledWith('-createdAt'); + }); +}); diff --git a/src/candidate_profile/BaseController.ts b/src/candidate_profile/BaseController.ts index ccb8efc..55182d4 100644 --- a/src/candidate_profile/BaseController.ts +++ b/src/candidate_profile/BaseController.ts @@ -14,6 +14,11 @@ interface baseProp { findOne?: boolean; } +// Field name used to sort by — no `$`, so this can't smuggle a Mongo +// operator into `.sort()`, and it can only ever reorder rows, never widen +// which rows come back. A leading `-` (Mongoose convention) means desc. +const SORT_FIELD_REGEX = /^-?[a-zA-Z0-9_.]+$/; + const modelObject: { [key: string]: any } = { generalInformation: MODELS.generalInformation, experiences: MODELS.Experience, @@ -30,12 +35,20 @@ export const baseGetAll = async (req: Request, res: Response, next: NextFunction if (!candidateId || !collection || !modelObject[collection]) return formatReturn(res, { statusCode: StatusCodes.NOT_FOUND, data: null, message: t('common.notFoundData', (req as any).lang) }); + // Optional pagination/sort (issue #73). Omitting page/limit keeps the + // pre-existing "return everything" behavior (`data` stays a plain + // array) — this is purely additive, no existing caller is affected. + const { page, limit, sort } = req.query as Record; + try { const _result = await baseFindDocument({ fields: { candidateId: candidateId }, model: modelObject[collection], findOne: false, lang: (req as any).lang, + page: page !== undefined ? parseInt(page, 10) : undefined, + limit: limit !== undefined ? parseInt(limit, 10) : undefined, + sort: sort && SORT_FIELD_REGEX.test(sort) ? sort : undefined, }); return formatReturn(res, { ..._result }); } catch (err) { diff --git a/src/config/swagger.config.ts b/src/config/swagger.config.ts index f5d1152..02207ec 100644 --- a/src/config/swagger.config.ts +++ b/src/config/swagger.config.ts @@ -28,6 +28,33 @@ const options: swaggerJsdoc.Options = { bearerFormat: 'JWT', }, }, + parameters: { + // Pagination (issue #73) — shared by every CV-section `GET /` list + // endpoint via BaseController.baseGetAll. All three are optional; + // omitting `limit` returns the full, unpaginated array exactly as + // before (see ApiResponse vs ApiResponsePaginated). + PageParam: { + in: 'query', + name: 'page', + required: false, + schema: { type: 'integer', minimum: 1, default: 1 }, + description: '1-indexed page number. Ignored unless `limit` is also given.', + }, + LimitParam: { + in: 'query', + name: 'limit', + required: false, + schema: { type: 'integer', minimum: 1, maximum: 100 }, + description: 'Page size (max 100). Providing this switches the response `data` shape from an array to `{ items, pagination }`.', + }, + SortParam: { + in: 'query', + name: 'sort', + required: false, + schema: { type: 'string' }, + description: "Mongoose sort expression, e.g. `startDate` or `-createdAt` for descending. Invalid values are silently ignored.", + }, + }, schemas: { ApiResponse: { type: 'object', @@ -38,6 +65,18 @@ const options: swaggerJsdoc.Options = { data: { nullable: true }, }, }, + // Shape of `data` when a `GET /` list endpoint is called with + // `?limit=` (issue #73) — otherwise `data` stays a plain array, + // as documented on ApiResponse. + Pagination: { + type: 'object', + properties: { + page: { type: 'integer' }, + limit: { type: 'integer' }, + total: { type: 'integer' }, + totalPages: { type: 'integer' }, + }, + }, SocialMedia: { type: 'object', properties: { diff --git a/src/routers/api/v1/award.route.ts b/src/routers/api/v1/award.route.ts index a7b973f..53b1fd6 100644 --- a/src/routers/api/v1/award.route.ts +++ b/src/routers/api/v1/award.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all awards for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of awards + * description: List of awards. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/routers/api/v1/certificate.route.ts b/src/routers/api/v1/certificate.route.ts index 084b365..b419477 100644 --- a/src/routers/api/v1/certificate.route.ts +++ b/src/routers/api/v1/certificate.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all certificates for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of certificates + * description: List of certificates. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/routers/api/v1/education.route.ts b/src/routers/api/v1/education.route.ts index 1773237..8788bdd 100644 --- a/src/routers/api/v1/education.route.ts +++ b/src/routers/api/v1/education.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all education entries for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of education entries + * description: List of education entries. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/routers/api/v1/experience.route.ts b/src/routers/api/v1/experience.route.ts index cd30461..302ea9c 100644 --- a/src/routers/api/v1/experience.route.ts +++ b/src/routers/api/v1/experience.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all work experience entries for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of experience entries + * description: List of experience entries. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/routers/api/v1/project.route.ts b/src/routers/api/v1/project.route.ts index c37d655..95513c6 100644 --- a/src/routers/api/v1/project.route.ts +++ b/src/routers/api/v1/project.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all projects for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of projects + * description: List of projects. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/routers/api/v1/reference.route.ts b/src/routers/api/v1/reference.route.ts index 6590968..7699702 100644 --- a/src/routers/api/v1/reference.route.ts +++ b/src/routers/api/v1/reference.route.ts @@ -19,9 +19,13 @@ const router = express.Router(); * summary: List all references for the authenticated candidate * security: * - bearerAuth: [] + * parameters: + * - $ref: '#/components/parameters/PageParam' + * - $ref: '#/components/parameters/LimitParam' + * - $ref: '#/components/parameters/SortParam' * responses: * 200: - * description: List of references + * description: List of references. Passing `limit` switches `data` to `{ items, pagination }` instead of a bare array. * content: * application/json: * schema: diff --git a/src/services/index.ts b/src/services/index.ts index 8c0ae78..8c6b15f 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -12,8 +12,15 @@ interface baseProp { fields: { _id?: string; candidateId?: string }; findOne?: boolean; lang?: string; + page?: number; + limit?: number; + sort?: string; } +// Pagination (issue #73) hard cap — a caller cannot request more than this +// many documents per page regardless of what `limit` it passes. +const MAX_PAGE_LIMIT = 100; + const formatReturn = (props: BaseReturn) => { const { success = false, message = '', errors = null, data = null } = props; return { @@ -37,21 +44,50 @@ export const formatReturnFailed = (props: string | BaseReturn) => { }; export const baseFindDocument = async (props: baseProp) => { - const { model: MODEL, fields = { _id: '' }, findOne = true, lang = DEFAULT_LANG } = props; + const { model: MODEL, fields = { _id: '' }, findOne = true, lang = DEFAULT_LANG, page, limit, sort } = props; if (!MODEL || !fields || !Object.keys(fields).length) return formatReturnFailed(t('common.notFoundData', lang)); - let find; const idQuerySafe = (await import('@/utils/querySafe')).idQuerySafe; const safeFields = idQuerySafe.safeQuery({}, fields); + if (findOne) { - find = await MODEL.findOne(safeFields).exec(); - } else { - find = await MODEL.find(safeFields).exec(); + const find = await MODEL.findOne(safeFields).exec(); + return formatReturn({ success: true, data: find, message: '', errors: null }); } + + let query = MODEL.find(safeFields); + if (sort) query = query.sort(sort); + + /** + * Pagination (issue #73) is opt-in: it only kicks in when the caller + * passes a valid positive `limit`. No `limit` -> exactly the old + * behavior (`data` is the full, unpaginated array), so every existing + * caller of baseGetAll keeps working unchanged. + */ + const hasPagination = Number.isInteger(limit) && (limit as number) > 0; + if (!hasPagination) { + const find = await query.exec(); + return formatReturn({ success: true, data: find, message: '', errors: null }); + } + + const safeLimit = Math.min(limit as number, MAX_PAGE_LIMIT); + const safePage = Number.isInteger(page) && (page as number) > 0 ? (page as number) : 1; + const skip = (safePage - 1) * safeLimit; + + const [items, total] = await Promise.all([query.skip(skip).limit(safeLimit).exec(), MODEL.countDocuments(safeFields)]); + return formatReturn({ success: true, - data: find, + data: { + items, + pagination: { + page: safePage, + limit: safeLimit, + total, + totalPages: Math.max(Math.ceil(total / safeLimit), 1), + }, + }, message: '', errors: null, });