From e00c1dcc5ede3e2dea8d8f5e609457e2849fd608 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Thu, 18 May 2023 22:36:02 -0700 Subject: [PATCH 1/2] Fixes #415 - Paging arguments limit/skip --- src/helpers/index.js | 8 ++++++-- src/routers/domain.js | 2 +- src/services/history.js | 4 +--- tests/services/history.test.js | 22 +++++++++++++++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/helpers/index.js b/src/helpers/index.js index 922b1a6..3aa37d2 100644 --- a/src/helpers/index.js +++ b/src/helpers/index.js @@ -88,10 +88,14 @@ export function sortBy(args) { } export function validatePagingArgs(args) { - if (args.limit && !Number.isInteger(args.limit)) + if (args.limit && !Number.isInteger(parseInt(args.limit))) + return false; + else if (parseInt(args.limit) < 1) return false; - if (args.skip && !Number.isInteger(args.skip)) + if (args.skip && !Number.isInteger(parseInt(args.skip))) + return false; + else if (parseInt(args.skip) < 0) return false; if (args.sortBy) { diff --git a/src/routers/domain.js b/src/routers/domain.js index 018200d..7fed89d 100644 --- a/src/routers/domain.js +++ b/src/routers/domain.js @@ -60,7 +60,7 @@ router.get('/domain/history/:id', auth, [ const domain = await Services.getDomainById(req.params.id); const query = 'oldValue newValue updatedBy date -_id'; - const history = await getHistory(query, domain._id, undefined, req.query); + const history = await getHistory(query, domain._id, domain._id, req.query); await verifyOwnership(req.admin, domain, domain._id, ActionTypes.READ, RouterTypes.DOMAIN); diff --git a/src/services/history.js b/src/services/history.js index 5d20ce1..80b9e82 100644 --- a/src/services/history.js +++ b/src/services/history.js @@ -3,12 +3,10 @@ import { sortBy, validatePagingArgs } from '../helpers'; import { BadRequestError } from '../exceptions'; export async function getHistory(query, domainId, elementId, pagingArgs = {}) { - const findQuery = elementId ? { domainId, elementId } : { domainId }; - if (!validatePagingArgs(pagingArgs)) throw new BadRequestError('Invalid paging args'); - return History.find(findQuery) + return History.find({ domainId, elementId }) .select(query) .sort(sortBy(pagingArgs)) .limit(parseInt(pagingArgs.limit || 10)) diff --git a/tests/services/history.test.js b/tests/services/history.test.js index 45f49e7..6e32922 100644 --- a/tests/services/history.test.js +++ b/tests/services/history.test.js @@ -60,10 +60,30 @@ describe('Testing history services', () => { await expect(call()).rejects.toThrowError('Invalid paging args'); }); + test('HISTORY_SERVICE - Should NOT get history - invalid paging args - limit not number', async () => { + const call = async () => { + await getHistory('elementId', domainId, element1Id, { + limit: 'a' + }); + }; + + await expect(call()).rejects.toThrowError('Invalid paging args'); + }); + test('HISTORY_SERVICE - Should NOT get history - invalid paging args - skip', async () => { const call = async () => { await getHistory('elementId', domainId, element1Id, { - skip: '0' + skip: '-1' + }); + }; + + await expect(call()).rejects.toThrowError('Invalid paging args'); + }); + + test('HISTORY_SERVICE - Should NOT get history - invalid paging args - skip not number', async () => { + const call = async () => { + await getHistory('elementId', domainId, element1Id, { + skip: 'a' }); }; From 958bd01b7d9277f080e02640ae3b1dd8283883a6 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Thu, 18 May 2023 22:45:01 -0700 Subject: [PATCH 2/2] Fixed code smells --- src/helpers/index.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/helpers/index.js b/src/helpers/index.js index 3aa37d2..13dfacf 100644 --- a/src/helpers/index.js +++ b/src/helpers/index.js @@ -88,14 +88,12 @@ export function sortBy(args) { } export function validatePagingArgs(args) { - if (args.limit && !Number.isInteger(parseInt(args.limit))) - return false; - else if (parseInt(args.limit) < 1) + if (args.limit && !Number.isInteger(parseInt(args.limit)) || + parseInt(args.limit) < 1) return false; - if (args.skip && !Number.isInteger(parseInt(args.skip))) - return false; - else if (parseInt(args.skip) < 0) + if (args.skip && !Number.isInteger(parseInt(args.skip)) || + parseInt(args.skip) < 0) return false; if (args.sortBy) {