Skip to content

Commit

Permalink
address most of the comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sugat009 committed Dec 30, 2024
1 parent 91a0125 commit 49553b7
Show file tree
Hide file tree
Showing 11 changed files with 132 additions and 37 deletions.
5 changes: 2 additions & 3 deletions shared-libs/cht-datasource/src/local/contact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ export namespace v1 {
return false;
}

const contactTypesIds = getContactTypes(settings);
if (!contactTypesIds.includes(doc.type as string)) {
if (!contactTypeUtils.isContact(settings.getAll(), doc)) {
logger.warn(`Document [${doc._id}] is not a valid contact.`);
return false;
}
Expand All @@ -52,7 +51,7 @@ export namespace v1 {
export const getWithLineage = ({ medicDb, settings }: LocalDataContext) => {
const getLineageDocs = getLineageDocsById(medicDb);
const getMedicDocsById = getDocsByIds(medicDb);
return async (identifier: UuidQualifier): Promise<Nullable<ContactType.v1.Contact>> => {
return async (identifier: UuidQualifier): Promise<Nullable<ContactType.v1.ContactWithLineage>> => {
const [contact, ...lineageContacts] = await getLineageDocs(identifier.uuid);
if (!isContact(settings)(contact, identifier.uuid)) {
return null;
Expand Down
8 changes: 4 additions & 4 deletions shared-libs/cht-datasource/src/local/libs/doc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,17 @@ export const queryDocsByRange = (
) => async (
startkey: unknown,
endkey: unknown,
limit: Nullable<number> = null,
skip: Nullable<number> = null
limit?: number,
skip = 0
): Promise<Nullable<Doc>[]> => queryDocs(
db,
view,
{
include_docs: true,
startkey,
endkey,
...(limit !== null && { limit }),
...(skip !== null && { skip })
limit,
skip,
}
);

Expand Down
7 changes: 2 additions & 5 deletions shared-libs/cht-datasource/src/local/person.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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';
import { validateCursor } from './libs/core';

/** @internal */
export namespace v1 {
Expand Down Expand Up @@ -78,11 +79,7 @@ export namespace v1 {
throw new InvalidArgumentError(`Invalid contact type [${personType.contactType}].`);
}

// Adding a number skip variable here so as not to confuse ourselves
const skip = Number(cursor);
if (isNaN(skip) || skip < 0 || !Number.isInteger(skip)) {
throw new InvalidArgumentError(`Invalid cursor token: [${String(cursor)}].`);
}
const skip = validateCursor(cursor);

const getDocsByPageWithPersonType = (
limit: number,
Expand Down
7 changes: 2 additions & 5 deletions shared-libs/cht-datasource/src/local/place.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as Contact from '../contact';
import logger from '@medic/logger';
import { getLineageDocsById, getPrimaryContactIds, hydrateLineage, hydratePrimaryContact } from './libs/lineage';
import { InvalidArgumentError } from '../libs/error';
import { validateCursor } from './libs/core';

/** @internal */
export namespace v1 {
Expand Down Expand Up @@ -77,11 +78,7 @@ export namespace v1 {
throw new InvalidArgumentError(`Invalid contact type [${placeType.contactType}].`);
}

// Adding a number skip variable here so as not to confuse ourselves
const skip = Number(cursor);
if (isNaN(skip) || skip < 0 || !Number.isInteger(skip)) {
throw new InvalidArgumentError(`Invalid cursor token: [${String(cursor)}].`);
}
const skip = validateCursor(cursor);

const getDocsByPageWithPlaceType = (
limit: number,
Expand Down
4 changes: 2 additions & 2 deletions shared-libs/cht-datasource/src/remote/report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Nullable, Page } from '../libs/core';
export namespace v1 {
const getReport = (remoteContext: RemoteDataContext) => getResource(remoteContext, 'api/v1/report');

const getReports = (remoteContext: RemoteDataContext) => getResources(remoteContext, 'api/v1/report/uuid');
const getReportUuids = (remoteContext: RemoteDataContext) => getResources(remoteContext, 'api/v1/report/uuid');

/** @internal */
export const get = (remoteContext: RemoteDataContext) => (
Expand All @@ -25,6 +25,6 @@ export namespace v1 {
freetext: qualifier.freetext,
...(cursor ? { cursor } : {}),
};
return getReports(remoteContext)(queryParams);
return getReportUuids(remoteContext)(queryParams);
};
}
27 changes: 14 additions & 13 deletions shared-libs/cht-datasource/test/local/contact.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ describe('local contact', () => {
let warn: SinonStub;
let debug: SinonStub;
let getContactTypes: SinonStub;
let isContact: SinonStub;

beforeEach(() => {
settingsGetAll = sinon.stub();
Expand All @@ -25,6 +26,7 @@ describe('local contact', () => {
warn = sinon.stub(logger, 'warn');
debug = sinon.stub(logger, 'debug');
getContactTypes = sinon.stub(contactTypeUtils, 'getContactTypes');
isContact = sinon.stub(contactTypeUtils, 'isContact');
});

afterEach(() => sinon.restore());
Expand All @@ -34,7 +36,6 @@ describe('local contact', () => {

describe('get', () => {
const identifier = { uuid: 'uuid' } as const;
const contactTypes = [{ id: 'person' }, { id: 'place' }];
let getDocByIdOuter: SinonStub;
let getDocByIdInner: SinonStub;

Expand All @@ -47,29 +48,29 @@ describe('local contact', () => {
const doc = { type: 'person' };
getDocByIdInner.resolves(doc);
settingsGetAll.returns(settings);
getContactTypes.returns(contactTypes);
isContact.returns(true);

const result = await Contact.v1.get(localContext)(identifier);

expect(result).to.equal(doc);
expect(getDocByIdOuter.calledOnceWithExactly(localContext.medicDb)).to.be.true;
expect(getDocByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.calledOnceWithExactly(settings)).to.be.true;
expect(isContact.calledOnceWithExactly(settingsGetAll(), doc)).to.be.true;
expect(warn.notCalled).to.be.true;
});

it('returns null if the identified doc does not have a contact type', async () => {
const doc = { type: 'not-contact', _id: '_id' };
getDocByIdInner.resolves(doc);
settingsGetAll.returns(settings);
getContactTypes.returns(contactTypes);
isContact.returns(false);

const result = await Contact.v1.get(localContext)(identifier);

expect(result).to.be.null;
expect(getDocByIdOuter.calledOnceWithExactly(localContext.medicDb)).to.be.true;
expect(getDocByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.calledOnceWithExactly(settings)).to.be.true;
expect(isContact.calledOnceWithExactly(settingsGetAll(), doc)).to.be.true;
expect(warn.calledOnceWithExactly(`Document [${doc._id}] is not a valid contact.`)).to.be.true;
});

Expand All @@ -82,14 +83,13 @@ describe('local contact', () => {
expect(getDocByIdOuter.calledOnceWithExactly(localContext.medicDb)).to.be.true;
expect(getDocByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(settingsGetAll.notCalled).to.be.true;
expect(getContactTypes.notCalled).to.be.true;
expect(isContact.notCalled).to.be.true;
expect(warn.calledOnceWithExactly(`No contact found for identifier [${identifier.uuid}].`)).to.be.true;
});
});

describe('getWithLineage', () => {
const identifier = { uuid: 'uuid' } as const;
const validContactTypes = [{ id: 'person' }, { id: 'place' }];
let getLineageDocsByIdInner: SinonStub;
let getLineageDocsByIdOuter: SinonStub;
let getDocsByIdsInner: SinonStub;
Expand Down Expand Up @@ -131,7 +131,7 @@ describe('local contact', () => {
const contact0 = { _id: 'contact0', _rev: 'rev' };
const contact1 = { _id: 'contact1', _rev: 'rev' };
getLineageDocsByIdInner.resolves([person, place0, place1, place2]);
getContactTypes.returns(validContactTypes);
isContact.returns(true);
settingsGetAll.returns(settings);
getPrimaryContactIds.returns([contact0._id, contact1._id, person._id]);
getDocsByIdsInner.resolves([contact0, contact1]);
Expand All @@ -149,7 +149,7 @@ describe('local contact', () => {

expect(result).to.equal(copiedPerson);
expect(getLineageDocsByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.calledOnceWithExactly(settings)).to.be.true;
expect(isContact.calledOnceWithExactly(settingsGetAll(), person)).to.be.true;
expect(warn.notCalled).to.be.true;
expect(debug.notCalled).to.be.true;
expect(getPrimaryContactIds.calledOnceWithExactly([person, place0, place1, place2])).to.be.true;
Expand All @@ -171,6 +171,7 @@ describe('local contact', () => {
expect(result).to.be.null;
expect(getLineageDocsByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.notCalled).to.be.true;
expect(isContact.notCalled).to.be.true;
expect(warn.calledOnceWithExactly(`No contact found for identifier [${identifier.uuid}].`)).to.be.true;
expect(debug.notCalled).to.be.true;
expect(getPrimaryContactIds.notCalled).to.be.true;
Expand All @@ -187,14 +188,14 @@ describe('local contact', () => {
const place1 = { _id: 'place1', _rev: 'rev' };
const place2 = { _id: 'place2', _rev: 'rev' };
getLineageDocsByIdInner.resolves([person, place0, place1, place2]);
getContactTypes.returns(validContactTypes);
isContact.returns(false);
settingsGetAll.returns(settings);

const result = await Contact.v1.getWithLineage(localContext)(identifier);

expect(result).to.be.null;
expect(getLineageDocsByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.calledOnceWithExactly(settings)).to.be.true;
expect(isContact.calledOnceWithExactly(settingsGetAll(), person)).to.be.true;
expect(warn.calledOnceWithExactly(`Document [${identifier.uuid}] is not a valid contact.`)).to.be.true;
expect(debug.notCalled).to.be.true;
expect(getPrimaryContactIds.notCalled).to.be.true;
Expand All @@ -208,14 +209,14 @@ describe('local contact', () => {
it('returns a contact if no lineage is found', async () => {
const person = { type: 'person', _id: 'uuid', _rev: 'rev' };
getLineageDocsByIdInner.resolves([person]);
getContactTypes.returns(validContactTypes);
isContact.returns(true);
settingsGetAll.returns(settings);

const result = await Contact.v1.getWithLineage(localContext)(identifier);

expect(result).to.equal(person);
expect(getLineageDocsByIdInner.calledOnceWithExactly(identifier.uuid)).to.be.true;
expect(getContactTypes.calledOnceWithExactly(settings)).to.be.true;
expect(isContact.calledOnceWithExactly(settingsGetAll(), person)).to.be.true;
expect(warn.notCalled).to.be.true;
expect(debug.calledOnceWithExactly(`No lineage contacts found for person [${identifier.uuid}].`)).to.be.true;
expect(getPrimaryContactIds.notCalled).to.be.true;
Expand Down
54 changes: 54 additions & 0 deletions shared-libs/cht-datasource/test/local/libs/core.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { expect } from 'chai';
import { validateCursor } from '../../../src/local/libs/core';
import { InvalidArgumentError } from '../../../src';

describe('validateCursor', () => {
it('should return the numeric value when given a valid numeric string', () => {
expect(validateCursor('0')).to.equal(0);
expect(validateCursor('1')).to.equal(1);
expect(validateCursor('100')).to.equal(100);
});

it('should accept null and treat it as 0', () => {
expect(validateCursor(null)).to.equal(0);
});

it('should throw InvalidArgumentError for negative numbers', () => {
expect(() => validateCursor('-1')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [-1].'
);
expect(() => validateCursor('-100')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [-100].'
);
});

it('should throw InvalidArgumentError for non-integer numbers', () => {
expect(() => validateCursor('1.5')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [1.5].'
);
expect(() => validateCursor('0.1')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [0.1].'
);
});

it('should throw InvalidArgumentError for non-numeric strings', () => {
expect(() => validateCursor('abc')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [abc].'
);
expect(() => validateCursor('123abc')).to.throw(
InvalidArgumentError,
'Invalid cursor token: [123abc].'
);
});

it('should handle string numbers with leading zeros', () => {
expect(validateCursor('00')).to.equal(0);
expect(validateCursor('01')).to.equal(1);
expect(validateCursor('000100')).to.equal(100);
});
});
19 changes: 14 additions & 5 deletions shared-libs/cht-datasource/test/local/libs/doc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ describe('local doc lib', () => {
});

describe('queryDocsByRange', () => {
const limit = 3;
const skip = 2;

it('returns lineage docs for the given id', async () => {
const doc0 = { _id: 'doc0' };
const doc1 = { _id: 'doc1' };
Expand All @@ -170,7 +173,9 @@ describe('local doc lib', () => {
expect(dbQuery.calledOnceWithExactly('medic-client/docs_by_id_lineage', {
include_docs: true,
startkey: doc0._id,
endkey: doc1._id
endkey: doc1._id,
limit: undefined,
skip: 0
})).to.be.true;
expect(isDoc.args).to.deep.equal([[doc0], [doc1], [doc2]]);
});
Expand All @@ -187,13 +192,15 @@ describe('local doc lib', () => {
});
isDoc.returns(true);

const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc2._id);
const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc2._id, limit, skip);

expect(result).to.deep.equal([doc0, null, doc2]);
expect(dbQuery.calledOnceWithExactly('medic-client/docs_by_id_lineage', {
startkey: doc0._id,
endkey: doc2._id,
include_docs: true
include_docs: true,
limit,
skip,
})).to.be.true;
expect(isDoc.args).to.deep.equal([[doc0], [null], [doc2]]);
});
Expand All @@ -205,13 +212,15 @@ describe('local doc lib', () => {
});
isDoc.returns(false);

const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc0._id);
const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc0._id, limit, skip);

expect(result).to.deep.equal([null]);
expect(dbQuery.calledOnceWithExactly('medic-client/docs_by_id_lineage', {
startkey: doc0._id,
endkey: doc0._id,
include_docs: true
include_docs: true,
limit,
skip
})).to.be.true;
expect(isDoc.calledOnceWithExactly(doc0)).to.be.true;
});
Expand Down
1 change: 1 addition & 0 deletions shared-libs/contact-types-utils/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export function getLeafPlaceTypes(config: Record<string, unknown>): Record<strin
export function getContactType(config: Record<string, unknown>, contact: Record<string, unknown>): Record<string, unknown> | undefined;
export function isPerson(config: Record<string, unknown>, contact: Record<string, unknown>): boolean;
export function isPlace(config: Record<string, unknown>, contact: Record<string, unknown>): boolean;
export function isContact(config: Record<string, unknown>, contact: Record<string, unknown>): boolean;
export function isHardcodedType(type: string): boolean;
export declare const HARDCODED_TYPES: string[];
export function getContactTypes(config?: Record<string, unknown>): Record<string, unknown>[];
Expand Down
5 changes: 5 additions & 0 deletions shared-libs/contact-types-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ const isPlace = (config, contact) => {
return isPlaceType(type);
};

const isContact = (config, contact) => {
return isPlace(config, contact) || isPerson(config, contact);
};

const isSameContactType = (contacts) => {
const contactTypes = new Set(contacts.map(contact => getTypeId(contact)));
return contactTypes.size === 1;
Expand Down Expand Up @@ -98,6 +102,7 @@ module.exports = {
getContactType,
isPerson,
isPlace,
isContact,
isSameContactType,
isHardcodedType,
HARDCODED_TYPES,
Expand Down
Loading

0 comments on commit 49553b7

Please sign in to comment.