From 20ee6e5c627b19c1c9a03f25142bc698f85062bf Mon Sep 17 00:00:00 2001 From: Sugat Bajracharya <30311933+sugat009@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:23:11 +0545 Subject: [PATCH] feat(#9241): create REST API endpoint for getting people (#9295) --- api/src/controllers/person.js | 27 +++- api/src/routing.js | 1 + api/src/server-utils.js | 6 + api/tests/mocha/controllers/person.spec.js | 110 +++++++++++++++- shared-libs/cht-datasource/src/index.ts | 1 + shared-libs/cht-datasource/src/libs/error.ts | 15 +++ .../cht-datasource/src/local/person.ts | 3 +- shared-libs/cht-datasource/src/person.ts | 9 +- shared-libs/cht-datasource/src/qualifier.ts | 5 +- .../cht-datasource/src/remote/person.ts | 2 +- .../cht-datasource/test/local/person.spec.ts | 2 +- .../cht-datasource/test/person.spec.ts | 4 +- .../cht-datasource/test/qualifier.spec.ts | 2 +- .../cht-datasource/test/remote/person.spec.ts | 5 +- .../api/controllers/person.spec.js | 121 +++++++++++++++++- 15 files changed, 289 insertions(+), 24 deletions(-) create mode 100644 shared-libs/cht-datasource/src/libs/error.ts diff --git a/api/src/controllers/person.js b/api/src/controllers/person.js index d8712a2163c..1f39f3707d3 100644 --- a/api/src/controllers/person.js +++ b/api/src/controllers/person.js @@ -8,20 +8,35 @@ const getPerson = ({ with_lineage }) => ctx.bind( ? Person.v1.getWithLineage : Person.v1.get ); +const getPageByType = () => ctx.bind(Person.v1.getPage); + +const checkUserPermissions = async (req) => { + const userCtx = await auth.getUserCtx(req); + if (!auth.isOnlineOnly(userCtx) || !auth.hasAllPermissions(userCtx, 'can_view_contacts')) { + return Promise.reject({ code: 403, message: 'Insufficient privileges' }); + } +}; module.exports = { v1: { get: serverUtils.doOrError(async (req, res) => { - const userCtx = await auth.getUserCtx(req); - if (!auth.isOnlineOnly(userCtx) || !auth.hasAllPermissions(userCtx, 'can_view_contacts')) { - return Promise.reject({ code: 403, message: 'Insufficient privileges' }); - } + await checkUserPermissions(req); const { uuid } = req.params; const person = await getPerson(req.query)(Qualifier.byUuid(uuid)); if (!person) { return serverUtils.error({ status: 404, message: 'Person not found' }, req, res); } return res.json(person); - }) - } + }), + getAll: serverUtils.doOrError(async (req, res) => { + await checkUserPermissions(req); + + const personType = Qualifier.byContactType(req.query.personType); + const limit = Number(req.query.limit) || 100; + + const docs = await getPageByType()( personType, req.query.cursor, limit ); + + return res.json(docs); + }), + }, }; diff --git a/api/src/routing.js b/api/src/routing.js index 1bebdcb1ff7..e192d40840d 100644 --- a/api/src/routing.js +++ b/api/src/routing.js @@ -481,6 +481,7 @@ app.postJson('/api/v1/people', function(req, res) { .catch(err => serverUtils.error(err, req, res)); }); +app.get('/api/v1/person', person.v1.getAll); app.get('/api/v1/person/:uuid', person.v1.get); app.postJson('/api/v1/bulk-delete', bulkDocs.bulkDelete); diff --git a/api/src/server-utils.js b/api/src/server-utils.js index a107431c8c1..e92f37df75e 100644 --- a/api/src/server-utils.js +++ b/api/src/server-utils.js @@ -5,6 +5,7 @@ const isClientHuman = require('./is-client-human'); const logger = require('@medic/logger'); const MEDIC_BASIC_AUTH = 'Basic realm="Medic Web Services"'; const cookie = require('./services/cookie'); +const {InvalidArgumentError} = require('@medic/cht-datasource'); const wantsJSON = req => req.accepts(['text', 'json']) === 'json'; @@ -63,6 +64,11 @@ module.exports = { logger.warn(`Non-numeric error code: ${code}`); code = 500; } + + if (err instanceof InvalidArgumentError) { + code = 400; + } + if (code === 401) { return module.exports.notLoggedIn(req, res, showPrompt); } diff --git a/api/tests/mocha/controllers/person.spec.js b/api/tests/mocha/controllers/person.spec.js index 8d841e03793..6ee1e4711ca 100644 --- a/api/tests/mocha/controllers/person.spec.js +++ b/api/tests/mocha/controllers/person.spec.js @@ -1,6 +1,6 @@ const sinon = require('sinon'); const { expect } = require('chai'); -const { Person, Qualifier } = require('@medic/cht-datasource'); +const { Person, Qualifier, InvalidArgumentError } = require('@medic/cht-datasource'); const auth = require('../../../src/auth'); const controller = require('../../../src/controllers/person'); const dataContext = require('../../../src/services/data-context'); @@ -154,5 +154,113 @@ describe('Person Controller', () => { expect(serverUtilsError.calledOnceWithExactly(error, req, res)).to.be.true; }); }); + + describe('getPageByType', () => { + let personGetPageByType; + let qualifierByContactType; + const personType = 'person'; + const invalidPersonType = 'invalidPerson'; + const personTypeQualifier = { contactType: personType }; + const person = { name: 'John Doe' }; + const limit = 100; + const skip = 0; + const people = Array.from({ length: 3 }, () => ({ ...person })); + + beforeEach(() => { + req = { + query: { + personType, + limit, + skip + } + }; + personGetPageByType = sinon.stub(); + qualifierByContactType = sinon.stub(Qualifier, 'byContactType'); + dataContextBind.withArgs(Person.v1.getPage).returns(personGetPageByType); + qualifierByContactType.returns(personTypeQualifier); + }); + + afterEach(() => { + expect(getUserCtx.calledOnceWithExactly(req)).to.be.true; + expect(isOnlineOnly.calledOnceWithExactly(userCtx)).to.be.true; + }); + + it('returns a page of people with correct query params', async () => { + isOnlineOnly.returns(true); + hasAllPermissions.returns(true); + personGetPageByType.resolves(people); + + await controller.v1.getAll(req, res); + + expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true; + expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true; + expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true; + expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true; + expect(res.json.calledOnceWithExactly(people)).to.be.true; + expect(serverUtilsError.notCalled).to.be.true; + }); + + it('returns error if user does not have can_view_contacts permission', async () => { + const error = { code: 403, message: 'Insufficient privileges' }; + isOnlineOnly.returns(true); + hasAllPermissions.returns(false); + + await controller.v1.getAll(req, res); + + expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true; + expect(dataContextBind.notCalled).to.be.true; + expect(qualifierByContactType.notCalled).to.be.true; + expect(personGetPageByType.notCalled).to.be.true; + expect(res.json.notCalled).to.be.true; + expect(serverUtilsError.calledOnceWithExactly(error, req, res)).to.be.true; + }); + + it('returns error if not an online user', async () => { + const error = { code: 403, message: 'Insufficient privileges' }; + isOnlineOnly.returns(false); + + await controller.v1.getAll(req, res); + + expect(hasAllPermissions.notCalled).to.be.true; + expect(dataContextBind.notCalled).to.be.true; + expect(qualifierByContactType.notCalled).to.be.true; + expect(personGetPageByType.notCalled).to.be.true; + expect(res.json.notCalled).to.be.true; + expect(serverUtilsError.calledOnceWithExactly(error, req, res)).to.be.true; + }); + + it('returns 400 error when argument is invalid', async () => { + const errorMessage = `Invalid person type: [${invalidPersonType}]`; + const errorPayload = { status: 400, message: errorMessage }; + isOnlineOnly.returns(true); + hasAllPermissions.returns(true); + personGetPageByType.throws(new InvalidArgumentError(errorMessage)); + + await controller.v1.getAll(req, res); + + expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true; + expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true; + expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true; + expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true; + expect(res.json.notCalled).to.be.true; + expect(serverUtilsError.calledOnceWithExactly(errorPayload, req, res)).to.be.true; + }); + + it('rethrows error in case of other errors', async () => { + const err = new Error('error'); + isOnlineOnly.returns(true); + hasAllPermissions.returns(true); + personGetPageByType.throws(err); + + await controller.v1.getAll(req, res); + + expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true; + expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true; + expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true; + expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true; + expect(res.json.notCalled).to.be.true; + expect(serverUtilsError.calledOnceWithExactly(err, req, res)).to.be.true; + }); + }); }); }); diff --git a/shared-libs/cht-datasource/src/index.ts b/shared-libs/cht-datasource/src/index.ts index a88f3f34d64..c94c4485977 100644 --- a/shared-libs/cht-datasource/src/index.ts +++ b/shared-libs/cht-datasource/src/index.ts @@ -36,6 +36,7 @@ export { Nullable, NonEmptyArray } from './libs/core'; export { DataContext } from './libs/data-context'; export { getLocalDataContext } from './local'; export { getRemoteDataContext } from './remote'; +export { InvalidArgumentError } from './libs/error'; export * as Person from './person'; export * as Place from './place'; export * as Qualifier from './qualifier'; diff --git a/shared-libs/cht-datasource/src/libs/error.ts b/shared-libs/cht-datasource/src/libs/error.ts new file mode 100644 index 00000000000..38168373744 --- /dev/null +++ b/shared-libs/cht-datasource/src/libs/error.ts @@ -0,0 +1,15 @@ +/** + * Represents an error that occurs when an invalid argument is provided. + * This error is typically thrown when a function or method receives an argument + * that doesn't meet the expected criteria or constraints. + */ +export class InvalidArgumentError extends Error { + /** + * Constructor + * @param message a descriptive error message why the error was raised + */ + constructor(message: string) { + super(message); + this.name = 'InvalidArgumentError'; + } +} diff --git a/shared-libs/cht-datasource/src/local/person.ts b/shared-libs/cht-datasource/src/local/person.ts index 265a5bca446..359b5736271 100644 --- a/shared-libs/cht-datasource/src/local/person.ts +++ b/shared-libs/cht-datasource/src/local/person.ts @@ -7,6 +7,7 @@ import { getDocById, getDocsByIds, queryDocsByKey } from './libs/doc'; import { LocalDataContext, SettingsService } from './libs/data-context'; import logger from '@medic/logger'; import { getLineageDocsById, getPrimaryContactIds, hydrateLineage, hydratePrimaryContact } from './libs/lineage'; +import {InvalidArgumentError} from '../libs/error'; /** @internal */ export namespace v1 { @@ -70,7 +71,7 @@ export namespace v1 { const personTypesIds = personTypes.map((item) => item.id); if (!personTypesIds.includes(personType.contactType)) { - throw new Error(`Invalid person type: ${personType.contactType}`); + throw new InvalidArgumentError(`Invalid contact type [${personType.contactType}]`); } // Adding a number skip variable here so as not to confuse ourselves diff --git a/shared-libs/cht-datasource/src/person.ts b/shared-libs/cht-datasource/src/person.ts index 6963b7f5211..e8da245e04a 100644 --- a/shared-libs/cht-datasource/src/person.ts +++ b/shared-libs/cht-datasource/src/person.ts @@ -6,6 +6,7 @@ import * as Local from './local'; import * as Place from './place'; import { LocalDataContext } from './local/libs/data-context'; import { RemoteDataContext } from './remote/libs/data-context'; +import { InvalidArgumentError } from './libs/error'; import { Page } from './libs/core'; /** */ @@ -29,7 +30,7 @@ export namespace v1 { const assertPersonQualifier: (qualifier: unknown) => asserts qualifier is UuidQualifier = (qualifier: unknown) => { if (!isUuidQualifier(qualifier)) { - throw new Error(`Invalid identifier [${JSON.stringify(qualifier)}].`); + throw new InvalidArgumentError(`Invalid identifier [${JSON.stringify(qualifier)}].`); } }; @@ -37,19 +38,19 @@ export namespace v1 { qualifier: unknown ) => { if (!isContactTypeQualifier(qualifier)) { - throw new Error(`Invalid type [${JSON.stringify(qualifier)}].`); + throw new InvalidArgumentError(`Invalid contact type [${JSON.stringify(qualifier)}].`); } }; const assertLimit = (limit: unknown) => { if (typeof limit !== 'number' || !Number.isInteger(limit) || limit <= 0) { - throw new Error(`The limit must be a positive number: [${String(limit)}]`); + throw new InvalidArgumentError(`The limit must be a positive number: [${String(limit)}]`); } }; const assertCursor = (cursor: unknown) => { if (typeof cursor !== 'string' || Number(cursor) < 0) { - throw new Error(`The cursor must be a stringified non-negative number: [${String(cursor)}]`); + throw new InvalidArgumentError(`The cursor must be a stringified non-negative number: [${String(cursor)}]`); } }; diff --git a/shared-libs/cht-datasource/src/qualifier.ts b/shared-libs/cht-datasource/src/qualifier.ts index 5ead8fccb15..9f883edf63d 100644 --- a/shared-libs/cht-datasource/src/qualifier.ts +++ b/shared-libs/cht-datasource/src/qualifier.ts @@ -1,4 +1,5 @@ import { isString, hasField, isRecord } from './libs/core'; +import { InvalidArgumentError } from './libs/error'; /** * A qualifier that identifies an entity by its UUID. @@ -13,7 +14,7 @@ export type UuidQualifier = Readonly<{ uuid: string }>; */ export const byUuid = (uuid: string): UuidQualifier => { if (!isString(uuid) || uuid.length === 0) { - throw new Error(`Invalid UUID [${JSON.stringify(uuid)}].`); + throw new InvalidArgumentError(`Invalid UUID [${JSON.stringify(uuid)}].`); } return { uuid }; }; @@ -41,7 +42,7 @@ export type ContactTypeQualifier = Readonly<{ contactType: string }>; */ export const byContactType = (contactType: string): ContactTypeQualifier => { if (!isString(contactType) || contactType.length === 0) { - throw new Error(`Invalid ContactType [${JSON.stringify(contactType)}].`); + throw new InvalidArgumentError(`Invalid contact type [${JSON.stringify(contactType)}].`); } return { contactType }; diff --git a/shared-libs/cht-datasource/src/remote/person.ts b/shared-libs/cht-datasource/src/remote/person.ts index 120d357644b..9e331c5bdff 100644 --- a/shared-libs/cht-datasource/src/remote/person.ts +++ b/shared-libs/cht-datasource/src/remote/person.ts @@ -28,6 +28,6 @@ export namespace v1 { cursor: string, limit: number, ): Promise> => getPeople(remoteContext)( - {'limit': limit.toString(), 'contactType': personType.contactType, cursor} + {'limit': limit.toString(), 'personType': personType.contactType, cursor} ); } diff --git a/shared-libs/cht-datasource/test/local/person.spec.ts b/shared-libs/cht-datasource/test/local/person.spec.ts index 89f4065b1db..46c2c865924 100644 --- a/shared-libs/cht-datasource/test/local/person.spec.ts +++ b/shared-libs/cht-datasource/test/local/person.spec.ts @@ -270,7 +270,7 @@ describe('local person', () => { it('throws an error if person identifier is invalid/does not exist', async () => { await expect(Person.v1.getPage(localContext)(invalidPersonTypeQualifier, cursor, limit)).to.be.rejectedWith( - `Invalid person type: ${invalidPersonTypeQualifier.contactType}` + `Invalid contact type [${invalidPersonTypeQualifier.contactType}]` ); expect(settingsGetAll.calledOnce).to.be.true; diff --git a/shared-libs/cht-datasource/test/person.spec.ts b/shared-libs/cht-datasource/test/person.spec.ts index acca2a0658f..a3d1c5a170e 100644 --- a/shared-libs/cht-datasource/test/person.spec.ts +++ b/shared-libs/cht-datasource/test/person.spec.ts @@ -171,7 +171,7 @@ describe('person', () => { isContactTypeQualifier.returns(false); await expect(Person.v1.getPage(dataContext)(invalidQualifier, cursor, limit)) - .to.be.rejectedWith(`Invalid type [${JSON.stringify(invalidQualifier)}].`); + .to.be.rejectedWith(`Invalid contact type [${JSON.stringify(invalidQualifier)}].`); expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true; expect(adapt.calledOnceWithExactly(dataContext, Local.Person.v1.getPage, Remote.Person.v1.getPage)).to.be.true; @@ -276,7 +276,7 @@ describe('person', () => { isContactTypeQualifier.returns(false); expect(() => Person.v1.getAll(dataContext)(personTypeQualifier)) - .to.throw(`Invalid type [${JSON.stringify(personTypeQualifier)}]`); + .to.throw(`Invalid contact type [${JSON.stringify(personTypeQualifier)}]`); expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true; expect(personGetPage.notCalled).to.be.true; expect(isContactTypeQualifier.calledOnceWithExactly(personTypeQualifier)).to.be.true; diff --git a/shared-libs/cht-datasource/test/qualifier.spec.ts b/shared-libs/cht-datasource/test/qualifier.spec.ts index 60146d3eecb..49a06da641c 100644 --- a/shared-libs/cht-datasource/test/qualifier.spec.ts +++ b/shared-libs/cht-datasource/test/qualifier.spec.ts @@ -44,7 +44,7 @@ describe('qualifier', () => { ].forEach(contactType => { it(`throws an error for ${JSON.stringify(contactType)}`, () => { expect(() => byContactType(contactType as string)).to.throw( - `Invalid ContactType [${JSON.stringify(contactType)}].` + `Invalid contact type [${JSON.stringify(contactType)}].` ); }); }); diff --git a/shared-libs/cht-datasource/test/remote/person.spec.ts b/shared-libs/cht-datasource/test/remote/person.spec.ts index d578f27402a..00d77647f40 100644 --- a/shared-libs/cht-datasource/test/remote/person.spec.ts +++ b/shared-libs/cht-datasource/test/remote/person.spec.ts @@ -72,11 +72,12 @@ describe('remote person', () => { describe('getPage', () => { const limit = 3; const cursor = '1'; - const personTypeQualifier = {contactType: 'person'}; + const personType = 'person'; + const personTypeQualifier = {contactType: personType}; const queryParam = { limit: limit.toString(), + personType, cursor, - ...personTypeQualifier }; it('returns people', async () => { diff --git a/tests/integration/api/controllers/person.spec.js b/tests/integration/api/controllers/person.spec.js index aefdd14fac6..8587335716a 100644 --- a/tests/integration/api/controllers/person.spec.js +++ b/tests/integration/api/controllers/person.spec.js @@ -2,13 +2,16 @@ const utils = require('@utils'); const placeFactory = require('@factories/cht/contacts/place'); const personFactory = require('@factories/cht/contacts/person'); const { getRemoteDataContext, Person, Qualifier } = require('@medic/cht-datasource'); +const chai = require('chai'); const { expect } = require('chai'); +const deepEqualInAnyOrder = require('deep-equal-in-any-order'); const userFactory = require('@factories/cht/users/users'); +chai.use(deepEqualInAnyOrder); describe('Person API', () => { const contact0 = utils.deepFreeze(personFactory.build({ name: 'contact0', role: 'chw' })); - const contact1 = utils.deepFreeze(personFactory.build({ name: 'contact0', role: 'chw_supervisor' })); - const contact2 = utils.deepFreeze(personFactory.build({ name: 'contact0', role: 'program_officer' })); + const contact1 = utils.deepFreeze(personFactory.build({ name: 'contact1', role: 'chw_supervisor' })); + const contact2 = utils.deepFreeze(personFactory.build({ name: 'contact2', role: 'program_officer' })); const placeMap = utils.deepFreeze(placeFactory.generateHierarchy()); const place0 = utils.deepFreeze({ ...placeMap.get('clinic'), contact: { _id: contact0._id } }); const place1 = utils.deepFreeze({ ...placeMap.get('health_center'), contact: { _id: contact1._id } }); @@ -46,10 +49,15 @@ describe('Person API', () => { }, roles: ['chw'] })); + const allDocItems = [contact0, contact1, contact2, place0, place1, place2, patient]; const dataContext = getRemoteDataContext(utils.getOrigin()); + const e2eTestUser = { + '_id': 'e2e_contact_test_id', + 'type': 'person', + }; before(async () => { - await utils.saveDocs([contact0, contact1, contact2, place0, place1, place2, patient]); + await utils.saveDocs(allDocItems); await utils.createUsers([userNoPerms, offlineUser]); }); @@ -104,4 +112,111 @@ describe('Person API', () => { }); }); }); + + describe('GET /api/v1/person', async () => { + const getPage = Person.v1.getPage(dataContext); + const limit = 100; + const cursor = '0'; + const personType = 'person'; + const invalidContactType = 'invalidPerson'; + const onlineUserPlaceHierarchy = { + parent: { + _id: place1._id, + parent: { + _id: place2._id, + } + } + }; + const offlineUserPlaceHierarchy = { + parent: { + _id: place0._id, + ...onlineUserPlaceHierarchy + } + }; + + it('returns a page of people', async () => { + const expectedPeople = [ + contact0, + contact1, + contact2, + patient, + e2eTestUser, + { + type: personType, + ...userNoPerms.contact, + ...onlineUserPlaceHierarchy + }, + { + type: personType, + ...offlineUser.contact, + ...offlineUserPlaceHierarchy + } + ]; + const responsePage = await getPage(Qualifier.byContactType(personType), cursor, limit); + const responsePeople = responsePage.data; + const responseCursor = responsePage.cursor; + + expect(responsePeople).excludingEvery(['_rev', 'reported_date']).to.deep.equalInAnyOrder(expectedPeople); + expect(responseCursor).to.be.equal('-1'); + }); + + it(`throws error when user does not have can_view_contacts permission`, async () => { + const opts = { + path: `/api/v1/person`, + auth: { username: userNoPerms.username, password: userNoPerms.password }, + }; + await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); + }); + + it(`throws error when user is not an online user`, async () => { + const opts = { + path: `/api/v1/person`, + auth: { username: offlineUser.username, password: offlineUser.password }, + }; + await expect(utils.request(opts)).to.be.rejectedWith('403 - {"code":403,"error":"Insufficient privileges"}'); + }); + + it('throws 400 error when personType is invalid', async () => { + const queryParams = { + 'personType': invalidContactType + }; + const queryString = new URLSearchParams(queryParams).toString(); + const opts = { + path: `/api/v1/person?${queryString}`, + }; + + await expect(utils.request(opts)) + .to.be.rejectedWith(`400 - {"code":400,"error":"Invalid contact type [${invalidContactType}]"}`); + }); + + it('throws 400 error when limit is invalid', async () => { + const queryParams = { + personType, + limit: -1 + }; + const queryString = new URLSearchParams(queryParams).toString(); + const opts = { + path: `/api/v1/person?${queryString}`, + }; + + await expect(utils.request(opts)) + .to.be.rejectedWith(`400 - {"code":400,"error":"The limit must be a positive number: [${-1}]"}`); + }); + + it('throws 400 error when cursor is invalid', async () => { + const queryParams = { + personType, + cursor: '-1' + }; + const queryString = new URLSearchParams(queryParams).toString(); + const opts = { + path: `/api/v1/person?${queryString}`, + }; + + await expect(utils.request(opts)) + .to.be.rejectedWith( + `400 - {"code":400,"error":"The cursor must be a stringified non-negative number: [${-1}]"}` + ); + }); + }); });