diff --git a/src/lib/hierarchy-operations/lineage-constraints.js b/src/lib/hierarchy-operations/lineage-constraints.js index e2079504..4f3da28a 100644 --- a/src/lib/hierarchy-operations/lineage-constraints.js +++ b/src/lib/hierarchy-operations/lineage-constraints.js @@ -9,10 +9,8 @@ module.exports = async (db, options) => { return { assertNoPrimaryContactViolations: async (sourceDoc, destinationDoc, descendantDocs) => { - const invalidPrimaryContactDoc = await getPrimaryContactViolations(db, sourceDoc, destinationDoc, descendantDocs); - if (invalidPrimaryContactDoc) { - throw Error(`Cannot remove contact '${invalidPrimaryContactDoc?.name}' (${invalidPrimaryContactDoc?._id}) from the hierarchy for which they are a primary contact.`); - } + await assertOnPrimaryContactRemoval(db, sourceDoc, destinationDoc, descendantDocs); + await assertSourcePrimaryContactType(db, contactTypeInfo, sourceDoc); }, assertNoHierarchyErrors: (sourceDocs, destinationDoc) => { @@ -43,13 +41,16 @@ module.exports = async (db, options) => { }); }, - isPlace: (contact) => { - const contactType = getContactType(contact); - return !contactTypeInfo[contactType]?.person; - }, + isPlace: (contact) => isPlace(contactTypeInfo, contact), }; }; +function isPlace(contactTypeInfo, contact) { + const contactType = getContactType(contact); + const isPerson = contactTypeInfo[contactType]?.person || false; + return !isPerson; +} + /* Enforce the list of allowed parents for each contact type Ensure we are not creating a circular hierarchy @@ -127,11 +128,11 @@ A place's primary contact must be a descendant of that place. 1. Check to see which part of the contact's lineage will be removed 2. For each removed part of the contact's lineage, confirm that place's primary contact isn't being removed. */ -const getPrimaryContactViolations = async (db, contactDoc, destinationDoc, descendantDocs) => { - const contactsLineageIds = lineageManipulation.pluckIdsFromLineage(contactDoc?.parent); - const parentsLineageIds = lineageManipulation.pluckIdsFromLineage(destinationDoc); +async function assertOnPrimaryContactRemoval(db, sourceDoc, destinationDoc, descendantDocs) { + const sourceLineageIds = lineageManipulation.pluckIdsFromLineage(sourceDoc?.parent); + const destinationLineageIds = lineageManipulation.pluckIdsFromLineage(destinationDoc); - const docIdsRemovedFromContactLineage = contactsLineageIds.filter(value => !parentsLineageIds.includes(value)); + const docIdsRemovedFromContactLineage = sourceLineageIds.filter(value => !destinationLineageIds.includes(value)); const docsRemovedFromContactLineage = await db.allDocs({ keys: docIdsRemovedFromContactLineage, include_docs: true, @@ -141,10 +142,32 @@ const getPrimaryContactViolations = async (db, contactDoc, destinationDoc, desce .map(row => row?.doc?.contact?._id) .filter(Boolean); - return descendantDocs.find(descendant => primaryContactIds.some(primaryId => descendant._id === primaryId)); -}; + const invalidPrimaryContactDoc = descendantDocs.find(descendant => primaryContactIds.some(primaryId => descendant._id === primaryId)); + if (invalidPrimaryContactDoc) { + throw Error(`Cannot remove contact '${invalidPrimaryContactDoc?.name}' (${invalidPrimaryContactDoc?._id}) from the hierarchy for which they are a primary contact.`); + } +} -const getContactType = doc => doc?.type === 'contact' ? doc?.contact_type : doc?.type; +async function assertSourcePrimaryContactType(db, contactTypeInfo, sourceDoc) { + const sourcePrimaryContactId = getPrimaryContactId(sourceDoc); + if (!sourcePrimaryContactId) { + return; + } + + const sourcePrimaryContactDoc = await db.get(sourcePrimaryContactId); + const primaryContactIsPlace = isPlace(contactTypeInfo, sourcePrimaryContactDoc); + if (primaryContactIsPlace) { + throw Error(`Source "${sourceDoc._id}" has primary contact "${sourcePrimaryContactId}" which is of type place`); + } +} + +function getContactType(doc) { + return doc?.type === 'contact' ? doc?.contact_type : doc?.type; +} + +function getPrimaryContactId(doc) { + return typeof doc?.contact === 'string' ? doc.contact : doc?.contact?._id; +} async function fetchContactTypeInfo(db) { try { diff --git a/test/lib/hierarchy-operations/hierarchy-operations.spec.js b/test/lib/hierarchy-operations/hierarchy-operations.spec.js index 770d2589..3d2c7243 100644 --- a/test/lib/hierarchy-operations/hierarchy-operations.spec.js +++ b/test/lib/hierarchy-operations/hierarchy-operations.spec.js @@ -152,7 +152,10 @@ describe('hierarchy-operations', () => { it('move health_center_1 to root', async () => { sinon.spy(pouchDb, 'query'); - await updateHierarchyRules([{ id: 'health_center', parents: [] }]); + await updateHierarchyRules([ + { id: 'health_center', parents: [] }, + { id: 'person', parents: [], person: true }, + ]); await HierarchyOperations(pouchDb).move(['health_center_1'], 'root'); @@ -205,7 +208,10 @@ describe('hierarchy-operations', () => { }); it('move district_1 from root', async () => { - await updateHierarchyRules([{ id: 'district_hospital', parents: ['district_hospital'] }]); + await updateHierarchyRules([ + { id: 'district_hospital', parents: ['district_hospital'] }, + { id: 'person', parents: [], person: true }, + ]); await HierarchyOperations(pouchDb).move(['district_1'], 'district_2'); @@ -261,6 +267,7 @@ describe('hierarchy-operations', () => { await updateHierarchyRules([ { id: 'county', parents: [] }, { id: 'district_hospital', parents: ['county'] }, + { id: 'person', parents: [], person: true }, ]); await HierarchyOperations(pouchDb).move(['district_1'], 'county_1'); @@ -905,6 +912,16 @@ describe('hierarchy-operations', () => { } }); }); + + it('--merge-primary-contacts errors if primary contact is a place', async () => { + await upsert('district_2', { + type: 'district_hospital', + contact: 'health_center_2', + }); + + const actual = HierarchyOperations(pouchDb, { mergePrimaryContacts: true }).merge(['district_2'], 'district_1'); + await expect(actual).to.eventually.be.rejectedWith('"health_center_2" which is of type place'); + }); }); describe('delete', () => { diff --git a/test/lib/hierarchy-operations/lineage-constraints.spec.js b/test/lib/hierarchy-operations/lineage-constraints.spec.js index 85753992..08c2b6c8 100644 --- a/test/lib/hierarchy-operations/lineage-constraints.spec.js +++ b/test/lib/hierarchy-operations/lineage-constraints.spec.js @@ -100,8 +100,8 @@ describe('lineage constriants', () => { }); }); - describe('getPrimaryContactViolations', () => { - const assertNoHierarchyErrors = lineageConstraints.__get__('getPrimaryContactViolations'); + describe('assertOnPrimaryContactRemoval', () => { + const assertOnPrimaryContactRemoval = lineageConstraints.__get__('assertOnPrimaryContactRemoval'); describe('on memory pouchdb', async () => { let pouchDb, scenarioCount = 0; @@ -131,21 +131,21 @@ describe('lineage constriants', () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_2'); - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); - expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); + const actual = assertOnPrimaryContactRemoval(pouchDb, contactDoc, parentDoc, [contactDoc]); + expect(actual).to.eventually.be.rejectedWith(`clinic_1_contact) from the hierarchy`); }); it('cannot move clinic_1_contact to root', async () => { const contactDoc = await pouchDb.get('clinic_1_contact'); - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, undefined, [contactDoc]); - expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); + const actual = assertOnPrimaryContactRemoval(pouchDb, contactDoc, undefined, [contactDoc]); + expect(actual).to.eventually.be.rejectedWith(`clinic_1_contact) from the hierarchy`); }); it('can move clinic_1_contact to clinic_1', async () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_1'); - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await assertOnPrimaryContactRemoval(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); @@ -154,7 +154,7 @@ describe('lineage constriants', () => { const parentDoc = await pouchDb.get('district_1'); const descendants = await Promise.all(['health_center_2_contact', 'clinic_2', 'clinic_2_contact', 'patient_2'].map(id => pouchDb.get(id))); - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); + const doc = await assertOnPrimaryContactRemoval(pouchDb, contactDoc, parentDoc, descendants); expect(doc).to.be.undefined; }); @@ -167,8 +167,8 @@ describe('lineage constriants', () => { const parentDoc = await pouchDb.get('district_2'); const descendants = await Promise.all(['health_center_1_contact', 'clinic_1', 'clinic_1_contact', 'patient_1'].map(id => pouchDb.get(id))); - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); - expect(doc).to.deep.include({ _id: 'patient_1' }); + const actual = assertOnPrimaryContactRemoval(pouchDb, contactDoc, parentDoc, [contactDoc]); + expect(actual).to.eventually.be.rejectedWith(`patient_1) from the hierarchy`); }); // It is possible that one or more parents will not be found. Since these parents are being removed, do not throw @@ -178,7 +178,7 @@ describe('lineage constriants', () => { contactDoc.parent._id = 'dne'; - const doc = await assertNoHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await assertOnPrimaryContactRemoval(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); });