From d16df6d73fe1e80a97c820cbe9af80bdecd2ccb4 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Wed, 6 Nov 2024 17:40:46 -0700 Subject: [PATCH 01/21] Testing this common library going to get weird --- .gitignore | 1 + src/fn/merge-contacts.js | 200 +++++++++++++++++++++++++++++++++++++++ src/fn/move-contacts.js | 129 +++---------------------- src/lib/mm-shared.js | 153 ++++++++++++++++++++++++++++++ 4 files changed, 365 insertions(+), 118 deletions(-) create mode 100644 src/fn/merge-contacts.js create mode 100644 src/lib/mm-shared.js diff --git a/.gitignore b/.gitignore index 39c909fa0..e1a85a64d 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ upload-docs.*.log.json /.vscode/ /.idea/ /.settings/ +/json_docs/ *.swp coverage .nyc_output diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js new file mode 100644 index 000000000..963ae6aab --- /dev/null +++ b/src/fn/merge-contacts.js @@ -0,0 +1,200 @@ +const minimist = require('minimist'); +const path = require('path'); + +const environment = require('../lib/environment'); +const lineageManipulation = require('../lib/lineage-manipulation'); +const lineageConstraints = require('../lib/lineage-constraints'); +const pouch = require('../lib/db'); +const { trace, info } = require('../lib/log'); + +const { + BATCH_SIZE, + prepareDocumentDirectory, + prettyPrintDocument, + replaceLineageInAncestors, + bold, + writeDocumentToDisk, + fetch, +} = require('../lib/mm-shared'); + +module.exports = { + requiresInstance: true, + execute: () => { + const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); + const db = pouch(); + prepareDocumentDirectory(args); + return updateLineagesAndStage(args, db); + } +}; + +const updateLineagesAndStage = async (options, db) => { + trace(`Fetching contact details: ${options.winnerId}`); + const winnerDoc = await fetch.contact(db, options.winnerId); + + const constraints = await lineageConstraints(db, winnerDoc); + const loserDocs = await fetch.contactList(db, options.loserIds); + await validateContacts(loserDocs, constraints); + + let affectedContactCount = 0, affectedReportCount = 0; + const replacementLineage = lineageManipulation.createLineageFromDoc(winnerDoc); + for (let loserId of options.loserIds) { + const contactDoc = loserDocs[loserId]; + const descendantsAndSelf = await fetch.descendantsOf(db, loserId); + + const self = descendantsAndSelf.find(d => d._id === loserId); + writeDocumentToDisk(options, { + _id: self._id, + _rev: self._rev, + _deleted: true, + }); + + // Check that primary contact is not removed from areas where they are required + const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); + if (invalidPrimaryContactDoc) { + throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + } + + trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); + const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, loserId); + + const ancestors = await fetch.ancestorsOf(db, contactDoc); + trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); + const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); + + minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); + + const movedReportsCount = await moveReports(db, descendantsAndSelf, options, options.winnerId, loserId); + trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); + + affectedContactCount += updatedDescendants.length + updatedAncestors.length; + affectedReportCount += movedReportsCount; + + info(`Staged updates to ${prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + } + + info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); +}; + +/* +Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) +Confirms the list of contacts are possible to move +*/ +const validateContacts = async (contactDocs, constraints) => { + Object.values(contactDocs).forEach(doc => { + const hierarchyError = constraints.getHierarchyErrors(doc); + if (hierarchyError) { + throw Error(`Hierarchy Constraints: ${hierarchyError}`); + } + }); + + /* + It is nice that the tool can move lists of contacts as one operation, but strange things happen when two loserIds are in the same lineage. + For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. + */ + const loserIds = Object.keys(contactDocs); + Object.values(contactDocs) + .forEach(doc => { + const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; + const violatingParentId = parentIdsOfDoc.find(winnerId => loserIds.includes(winnerId)); + if (violatingParentId) { + throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); + } + }); +}; + +// Parses extraArgs and asserts if required parameters are not present +const parseExtraArgs = (projectDir, extraArgs = []) => { + const args = minimist(extraArgs, { boolean: true }); + + const loserIds = (args.losers || args.loser || '') + .split(',') + .filter(Boolean); + + if (loserIds.length === 0) { + usage(); + throw Error(`Action "merge-contacts" is missing required list of contacts ${bold('--losers')} to be merged into the winner`); + } + + if (!args.winner) { + usage(); + throw Error(`Action "merge-contacts" is missing required parameter ${bold('--winner')}`); + } + + return { + winnerId: args.winner, + loserIds, + docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'), + force: !!args.force, + }; +}; + +const usage = () => { + info(` +${bold('cht-conf\'s merge-contacts action')} +When combined with 'upload-docs' this action merges multiple contacts and all their associated data into one. + +${bold('USAGE')} +cht --local merge-contacts -- --winner= --losers=, + +${bold('OPTIONS')} +--winner= + Specifies the ID of the contact that should have all other contact data merged into it. + +--losers=, + A comma delimited list of IDs of contacts which will be deleted and all of their data will be merged into the winner contact. + +--docDirectoryPath= + Specifies the folder used to store the documents representing the changes in hierarchy. +`); +}; + +const moveReports = async (db, descendantsAndSelf, writeOptions, winnerId, loserId) => { + let skip = 0; + let reportDocsBatch; + do { + info(`Processing ${skip} to ${skip + BATCH_SIZE} report docs`); + reportDocsBatch = await fetch.reportsCreatedFor(db, loserId, skip); + + reportDocsBatch.forEach(report => { + const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; + for (const subjectId of subjectIds) { + if (report[subjectId]) { + report[subjectId] = winnerId; + } + + if (report.fields[subjectId]) { + report.fields[subjectId] = winnerId; + } + } + + writeDocumentToDisk(writeOptions, report); + }); + + skip += reportDocsBatch.length; + } while (reportDocsBatch.length >= BATCH_SIZE); + + return skip; +}; + +const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { + docs.forEach(doc => { + lineageManipulation.minifyLineagesInDoc(doc); + writeDocumentToDisk(parsedArgs, doc); + }); +}; + +const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contactId) => descendantsAndSelf.reduce((agg, doc) => { + // skip top-level because it is now being deleted + if (doc._id === contactId) { + return agg; + } + + const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, contactId); + + // TODO: seems wrong + const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, contactId); + if (parentWasUpdated || contactWasUpdated) { + agg.push(doc); + } + return agg; +}, []); diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 6b29e3b03..e0c9ff24f 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -1,16 +1,21 @@ const minimist = require('minimist'); const path = require('path'); -const userPrompt = require('../lib/user-prompt'); const environment = require('../lib/environment'); -const fs = require('../lib/sync-fs'); const lineageManipulation = require('../lib/lineage-manipulation'); const lineageConstraints = require('../lib/lineage-constraints'); const pouch = require('../lib/db'); -const { warn, trace, info } = require('../lib/log'); - -const HIERARCHY_ROOT = 'root'; -const BATCH_SIZE = 10000; +const { trace, info } = require('../lib/log'); + +const { + HIERARCHY_ROOT, + BATCH_SIZE, + prepareDocumentDirectory, + prettyPrintDocument, + replaceLineageInAncestors, + bold, + fetch, +} = require('../lib/mm-shared'); module.exports = { requiresInstance: true, @@ -22,7 +27,6 @@ module.exports = { } }; -const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; const updateLineagesAndStage = async (options, db) => { trace(`Fetching contact details for parent: ${options.parentId}`); const parentDoc = await fetch.contact(db, options.parentId); @@ -117,21 +121,7 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { }; }; -const prepareDocumentDirectory = ({ docDirectoryPath, force }) => { - if (!fs.exists(docDirectoryPath)) { - fs.mkdir(docDirectoryPath); - } else if (!force && fs.recurseFiles(docDirectoryPath).length > 0) { - warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); - if(userPrompt.keyInYN()) { - fs.deleteFilesInFolder(docDirectoryPath); - } else { - throw new Error('User aborted execution.'); - } - } -}; - const usage = () => { - const bold = text => `\x1b[1m${text}\x1b[0m`; info(` ${bold('cht-conf\'s move-contacts action')} When combined with 'upload-docs' this action effectively moves a contact from one place in the hierarchy to another. @@ -176,92 +166,6 @@ const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { }); }; -const writeDocumentToDisk = ({ docDirectoryPath }, doc) => { - const destinationPath = path.join(docDirectoryPath, `${doc._id}.doc.json`); - if (fs.exists(destinationPath)) { - warn(`File at ${destinationPath} already exists and is being overwritten.`); - } - - trace(`Writing updated document to ${destinationPath}`); - fs.writeJson(destinationPath, doc); -}; - -const fetch = { - /* - Fetches all of the documents associated with the "contactIds" and confirms they exist. - */ - contactList: async (db, ids) => { - const contactDocs = await db.allDocs({ - keys: ids, - include_docs: true, - }); - - const missingContactErrors = contactDocs.rows.filter(row => !row.doc).map(row => `Contact with id '${row.key}' could not be found.`); - if (missingContactErrors.length > 0) { - throw Error(missingContactErrors); - } - - return contactDocs.rows.reduce((agg, curr) => Object.assign(agg, { [curr.doc._id]: curr.doc }), {}); - }, - - contact: async (db, id) => { - try { - if (id === HIERARCHY_ROOT) { - return undefined; - } - - return await db.get(id); - } catch (err) { - if (err.name !== 'not_found') { - throw err; - } - - throw Error(`Contact with id '${id}' could not be found`); - } - }, - - /* - Given a contact's id, obtain the documents of all descendant contacts - */ - descendantsOf: async (db, contactId) => { - const descendantDocs = await db.query('medic/contacts_by_depth', { - key: [contactId], - include_docs: true, - }); - - return descendantDocs.rows - .map(row => row.doc) - /* We should not move or update tombstone documents */ - .filter(doc => doc && doc.type !== 'tombstone'); - }, - - reportsCreatedBy: async (db, contactIds, skip) => { - const reports = await db.query('medic-client/reports_by_freetext', { - keys: contactIds.map(id => [`contact:${id}`]), - include_docs: true, - limit: BATCH_SIZE, - skip: skip, - }); - - return reports.rows.map(row => row.doc); - }, - - ancestorsOf: async (db, contactDoc) => { - const ancestorIds = lineageManipulation.pluckIdsFromLineage(contactDoc.parent); - const ancestors = await db.allDocs({ - keys: ancestorIds, - include_docs: true, - }); - - const ancestorIdsNotFound = ancestors.rows.filter(ancestor => !ancestor.doc).map(ancestor => ancestor.key); - if (ancestorIdsNotFound.length > 0) { - throw Error(`Contact '${prettyPrintDocument(contactDoc)} has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); - } - - return ancestors.rows.map(ancestor => ancestor.doc); - }, -}; - const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage)) { agg.push(doc); @@ -278,14 +182,3 @@ const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contac } return agg; }, []); - -const replaceLineageInAncestors = (descendantsAndSelf, ancestors) => ancestors.reduce((agg, ancestor) => { - let result = agg; - const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); - if (primaryContact) { - ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); - result = [ancestor, ...result]; - } - - return result; -}, []); diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js new file mode 100644 index 000000000..bd324a13f --- /dev/null +++ b/src/lib/mm-shared.js @@ -0,0 +1,153 @@ +const path = require('path'); + +const userPrompt = require('./user-prompt'); +const fs = require('./sync-fs'); +const { warn, trace } = require('./log'); +const lineageManipulation = require('./lineage-manipulation'); + +const HIERARCHY_ROOT = 'root'; +const BATCH_SIZE = 10000; + +const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; + +const prepareDocumentDirectory = ({ docDirectoryPath, force }) => { + if (!fs.exists(docDirectoryPath)) { + fs.mkdir(docDirectoryPath); + } else if (!force && fs.recurseFiles(docDirectoryPath).length > 0) { + warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); + if(userPrompt.keyInYN()) { + fs.deleteFilesInFolder(docDirectoryPath); + } else { + throw new Error('User aborted execution.'); + } + } +}; + +const writeDocumentToDisk = ({ docDirectoryPath }, doc) => { + const destinationPath = path.join(docDirectoryPath, `${doc._id}.doc.json`); + if (fs.exists(destinationPath)) { + warn(`File at ${destinationPath} already exists and is being overwritten.`); + } + + trace(`Writing updated document to ${destinationPath}`); + fs.writeJson(destinationPath, doc); +}; + +const replaceLineageInAncestors = (descendantsAndSelf, ancestors) => ancestors.reduce((agg, ancestor) => { + let result = agg; + const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); + if (primaryContact) { + ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); + result = [ancestor, ...result]; + } + + return result; +}, []); + + +const fetch = { + /* + Fetches all of the documents associated with the "contactIds" and confirms they exist. + */ + contactList: async (db, ids) => { + const contactDocs = await db.allDocs({ + keys: ids, + include_docs: true, + }); + + const missingContactErrors = contactDocs.rows.filter(row => !row.doc).map(row => `Contact with id '${row.key}' could not be found.`); + if (missingContactErrors.length > 0) { + throw Error(missingContactErrors); + } + + return contactDocs.rows.reduce((agg, curr) => Object.assign(agg, { [curr.doc._id]: curr.doc }), {}); + }, + + contact: async (db, id) => { + try { + if (id === HIERARCHY_ROOT) { + return undefined; + } + + return await db.get(id); + } catch (err) { + if (err.name !== 'not_found') { + throw err; + } + + throw Error(`Contact with id '${id}' could not be found`); + } + }, + + /* + Given a contact's id, obtain the documents of all descendant contacts + */ + descendantsOf: async (db, contactId) => { + const descendantDocs = await db.query('medic/contacts_by_depth', { + key: [contactId], + include_docs: true, + }); + + return descendantDocs.rows + .map(row => row.doc) + /* We should not move or update tombstone documents */ + .filter(doc => doc && doc.type !== 'tombstone'); + }, + + reportsCreatedBy: async (db, contactIds, skip) => { + const reports = await db.query('medic-client/reports_by_freetext', { + keys: contactIds.map(id => [`contact:${id}`]), + include_docs: true, + limit: BATCH_SIZE, + skip, + }); + + return reports.rows.map(row => row.doc); + }, + + reportsCreatedFor: async (db, contactId, skip) => { + // TODO is this the right way? + const reports = await db.query('medic-client/reports_by_freetext', { + keys: [ + [`patient_id:${contactId}`], + [`patient_uuid:${contactId}`], + [`place_id:${contactId}`], + [`place_uuid:${contactId}`], + ], + include_docs: true, + limit: BATCH_SIZE, + skip, + }); + + return reports.rows.map(row => row.doc); + }, + + ancestorsOf: async (db, contactDoc) => { + const ancestorIds = lineageManipulation.pluckIdsFromLineage(contactDoc.parent); + const ancestors = await db.allDocs({ + keys: ancestorIds, + include_docs: true, + }); + + const ancestorIdsNotFound = ancestors.rows.filter(ancestor => !ancestor.doc).map(ancestor => ancestor.key); + if (ancestorIdsNotFound.length > 0) { + throw Error(`Contact '${prettyPrintDocument(contactDoc)} has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); + } + + return ancestors.rows.map(ancestor => ancestor.doc); + }, +}; + +const bold = text => `\x1b[1m${text}\x1b[0m`; + +module.exports = { + HIERARCHY_ROOT, + BATCH_SIZE, + bold, + prepareDocumentDirectory, + prettyPrintDocument, + minifyLineageAndWriteToDisk, + replaceLineageInAncestors, + writeDocumentToDisk, + fetch, +}; From 3e1827ede9c5838f8bae86958d21d8904e57aaf8 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Wed, 6 Nov 2024 22:28:19 -0700 Subject: [PATCH 02/21] Move-Contacts tests passing again --- src/fn/move-contacts.js | 46 +++++++++++-------------- src/lib/mm-shared.js | 1 - test/fn/mm-shared.spec.js | 50 +++++++++++++++++++++++++++ test/fn/move-contacts.spec.js | 63 ++++++++--------------------------- 4 files changed, 82 insertions(+), 78 deletions(-) create mode 100644 test/fn/mm-shared.spec.js diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index e0c9ff24f..342ef944b 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -7,52 +7,44 @@ const lineageConstraints = require('../lib/lineage-constraints'); const pouch = require('../lib/db'); const { trace, info } = require('../lib/log'); -const { - HIERARCHY_ROOT, - BATCH_SIZE, - prepareDocumentDirectory, - prettyPrintDocument, - replaceLineageInAncestors, - bold, - fetch, -} = require('../lib/mm-shared'); +const Shared = require('../lib/mm-shared'); module.exports = { requiresInstance: true, execute: () => { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); - prepareDocumentDirectory(args); + Shared.prepareDocumentDirectory(args); return updateLineagesAndStage(args, db); } }; const updateLineagesAndStage = async (options, db) => { trace(`Fetching contact details for parent: ${options.parentId}`); - const parentDoc = await fetch.contact(db, options.parentId); + const parentDoc = await Shared.fetch.contact(db, options.parentId); const constraints = await lineageConstraints(db, parentDoc); - const contactDocs = await fetch.contactList(db, options.contactIds); + const contactDocs = await Shared.fetch.contactList(db, options.contactIds); await validateContacts(contactDocs, constraints); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(parentDoc); for (let contactId of options.contactIds) { const contactDoc = contactDocs[contactId]; - const descendantsAndSelf = await fetch.descendantsOf(db, contactId); + const descendantsAndSelf = await Shared.fetch.descendantsOf(db, contactId); // Check that primary contact is not removed from areas where they are required const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); if (invalidPrimaryContactDoc) { - throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + throw Error(`Cannot remove contact ${Shared.prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); } - trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); + trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${Shared.prettyPrintDocument(contactDoc)}.`); const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, contactId); - const ancestors = await fetch.ancestorsOf(db, contactDoc); - trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); + const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); + trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${Shared.prettyPrintDocument(contactDoc)}.`); + const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); @@ -62,7 +54,7 @@ const updateLineagesAndStage = async (options, db) => { affectedContactCount += updatedDescendants.length + updatedAncestors.length; affectedReportCount += movedReportsCount; - info(`Staged updates to ${prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + info(`Staged updates to ${Shared.prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); } info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); @@ -123,18 +115,18 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { const usage = () => { info(` -${bold('cht-conf\'s move-contacts action')} +${Shared.bold('cht-conf\'s move-contacts action')} When combined with 'upload-docs' this action effectively moves a contact from one place in the hierarchy to another. -${bold('USAGE')} +${Shared.bold('USAGE')} cht --local move-contacts -- --contacts=, --parent= -${bold('OPTIONS')} +${Shared.bold('OPTIONS')} --contacts=, A comma delimited list of ids of contacts to be moved. --parent= - Specifies the ID of the new parent. Use '${HIERARCHY_ROOT}' to identify the top of the hierarchy (no parent). + Specifies the ID of the new parent. Use '${Shared.HIERARCHY_ROOT}' to identify the top of the hierarchy (no parent). --docDirectoryPath= Specifies the folder used to store the documents representing the changes in hierarchy. @@ -147,14 +139,14 @@ const moveReports = async (db, descendantsAndSelf, writeOptions, replacementLine let skip = 0; let reportDocsBatch; do { - info(`Processing ${skip} to ${skip + BATCH_SIZE} report docs`); - reportDocsBatch = await fetch.reportsCreatedBy(db, contactIds, skip); + info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); + reportDocsBatch = await Shared.fetch.reportsCreatedBy(db, contactIds, skip); const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, contactId); minifyLineageAndWriteToDisk(updatedReports, writeOptions); skip += reportDocsBatch.length; - } while (reportDocsBatch.length >= BATCH_SIZE); + } while (reportDocsBatch.length >= Shared.BATCH_SIZE); return skip; }; @@ -162,7 +154,7 @@ const moveReports = async (db, descendantsAndSelf, writeOptions, replacementLine const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { docs.forEach(doc => { lineageManipulation.minifyLineagesInDoc(doc); - writeDocumentToDisk(parsedArgs, doc); + Shared.writeDocumentToDisk(parsedArgs, doc); }); }; diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js index bd324a13f..a9b8c56e3 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/mm-shared.js @@ -146,7 +146,6 @@ module.exports = { bold, prepareDocumentDirectory, prettyPrintDocument, - minifyLineageAndWriteToDisk, replaceLineageInAncestors, writeDocumentToDisk, fetch, diff --git a/test/fn/mm-shared.spec.js b/test/fn/mm-shared.spec.js new file mode 100644 index 000000000..8902613cd --- /dev/null +++ b/test/fn/mm-shared.spec.js @@ -0,0 +1,50 @@ +const { assert } = require('chai'); +const rewire = require('rewire'); +const sinon = require('sinon'); + +const environment = require('../../src/lib/environment'); +const fs = require('../../src/lib/sync-fs'); +const Shared = rewire('../../src/lib/mm-shared'); +const userPrompt = rewire('../../src/lib/user-prompt'); + + +describe('mm-shared', () => { + let readline; + + let docOnj = { docDirectoryPath: '/test/path/for/testing ', force: false }; + beforeEach(() => { + readline = { keyInYN: sinon.stub() }; + userPrompt.__set__('readline', readline); + Shared.__set__('userPrompt', userPrompt); + sinon.stub(fs, 'exists').returns(true); + sinon.stub(fs, 'recurseFiles').returns(Array(20)); + sinon.stub(fs, 'deleteFilesInFolder').returns(true); + }); + afterEach(() => { + sinon.restore(); + }); + + it('does not delete files in directory when user presses n', () => { + readline.keyInYN.returns(false); + sinon.stub(environment, 'force').get(() => false); + try { + Shared.prepareDocumentDirectory(docOnj); + assert.fail('Expected error to be thrown'); + } catch(e) { + assert.equal(fs.deleteFilesInFolder.callCount, 0); + } + }); + + it('deletes files in directory when user presses y', () => { + readline.keyInYN.returns(true); + sinon.stub(environment, 'force').get(() => false); + Shared.prepareDocumentDirectory(docOnj); + assert.equal(fs.deleteFilesInFolder.callCount, 1); + }); + + it('deletes files in directory when force is set', () => { + sinon.stub(environment, 'force').get(() => true); + Shared.prepareDocumentDirectory(docOnj); + assert.equal(fs.deleteFilesInFolder.callCount, 1); + }); +}); diff --git a/test/fn/move-contacts.spec.js b/test/fn/move-contacts.spec.js index a7f471282..1d27c6a3b 100644 --- a/test/fn/move-contacts.spec.js +++ b/test/fn/move-contacts.spec.js @@ -4,12 +4,15 @@ const sinon = require('sinon'); const fs = require('../../src/lib/sync-fs'); const environment = require('../../src/lib/environment'); +const Shared = rewire('../../src/lib/mm-shared'); + const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); const moveContactsModule = rewire('../../src/fn/move-contacts'); -moveContactsModule.__set__('prepareDocumentDirectory', () => {}); +moveContactsModule.__set__('Shared', Shared); + const updateLineagesAndStage = moveContactsModule.__get__('updateLineagesAndStage'); const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); @@ -80,7 +83,8 @@ describe('move-contacts', () => { views: { contacts_by_depth }, }); - moveContactsModule.__set__('writeDocumentToDisk', (docDirectoryPath, doc) => writtenDocs.push(doc)); + Shared.writeDocumentToDisk = (docDirectoryPath, doc) => writtenDocs.push(doc); + Shared.prepareDocumentDirectory = () => {}; writtenDocs.length = 0; }); @@ -549,53 +553,9 @@ describe('move-contacts', () => { }); }); - let readline; - describe('prepareDocumentDirectory', () => { - const moveContacts = rewire('../../src/fn/move-contacts'); - const userPrompt = rewire('../../src/lib/user-prompt'); - const prepareDocDir = moveContacts.__get__('prepareDocumentDirectory'); - let docOnj = { docDirectoryPath: '/test/path/for/testing ', force: false }; - beforeEach(() => { - readline = { keyInYN: sinon.stub() }; - userPrompt.__set__('readline', readline); - moveContacts.__set__('userPrompt', userPrompt); - sinon.stub(fs, 'exists').returns(true); - sinon.stub(fs, 'recurseFiles').returns(Array(20)); - sinon.stub(fs, 'deleteFilesInFolder').returns(true); - }); - afterEach(() => { - sinon.restore(); - }); - - it('does not delete files in directory when user presses n', () => { - readline.keyInYN.returns(false); - sinon.stub(environment, 'force').get(() => false); - try { - prepareDocDir(docOnj); - assert.fail('Expected error to be thrown'); - } catch(e) { - assert.equal(fs.deleteFilesInFolder.callCount, 0); - } - }); - - it('deletes files in directory when user presses y', () => { - readline.keyInYN.returns(true); - sinon.stub(environment, 'force').get(() => false); - prepareDocDir(docOnj); - assert.equal(fs.deleteFilesInFolder.callCount, 1); - }); - - it('deletes files in directory when force is set', () => { - sinon.stub(environment, 'force').get(() => true); - prepareDocDir(docOnj); - assert.equal(fs.deleteFilesInFolder.callCount, 1); - }); - }); - describe('batching works as expected', () => { - let defaultBatchSize; + const initialBatchSize = Shared.BATCH_SIZE; beforeEach(async () => { - defaultBatchSize = moveContactsModule.__get__('BATCH_SIZE'); await mockReport(pouchDb, { id: 'report_2', creatorId: 'health_center_1_contact', @@ -613,11 +573,13 @@ describe('move-contacts', () => { }); afterEach(() => { - moveContactsModule.__set__('BATCH_SIZE', defaultBatchSize); + Shared.BATCH_SIZE = initialBatchSize; + Shared.__set__('BATCH_SIZE', initialBatchSize); }); it('move health_center_1 to district_2 in batches of 1', async () => { - moveContactsModule.__set__('BATCH_SIZE', 1); + Shared.__set__('BATCH_SIZE', 1); + Shared.BATCH_SIZE = 1; sinon.spy(pouchDb, 'query'); await updateLineagesAndStage({ @@ -692,7 +654,8 @@ describe('move-contacts', () => { }); it('should health_center_1 to district_1 in batches of 2', async () => { - moveContactsModule.__set__('BATCH_SIZE', 2); + Shared.__set__('BATCH_SIZE', 2); + Shared.BATCH_SIZE = 2; sinon.spy(pouchDb, 'query'); await updateLineagesAndStage({ From d914e64fd436955edc6a2f367773cd456f245835 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 12 Nov 2024 16:09:25 -0700 Subject: [PATCH 03/21] First test passing for merge --- package-lock.json | 1 + package.json | 1 + src/fn/merge-contacts.js | 108 +++++++++--------- src/fn/move-contacts.js | 12 +- src/lib/lineage-constraints.js | 31 ++++- src/lib/lineage-manipulation.js | 59 ++++++---- src/lib/mm-shared.js | 14 ++- test/fn/merge-contacts.spec.js | 158 ++++++++++++++++++++++++++ test/lib/lineage-constraints.spec.js | 36 +++--- test/lib/lineage-manipulation.spec.js | 18 +-- test/mock-hierarchies.js | 7 +- 11 files changed, 329 insertions(+), 116 deletions(-) create mode 100644 test/fn/merge-contacts.spec.js diff --git a/package-lock.json b/package-lock.json index 60ff114c6..09d2cfac7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,6 +24,7 @@ "json-diff": "^1.0.6", "json-stringify-safe": "^5.0.1", "json2csv": "^4.5.4", + "lodash": "^4.17.21", "mime-types": "^2.1.35", "minimist": "^1.2.8", "mkdirp": "^3.0.1", diff --git a/package.json b/package.json index c06ae5d57..1f5a35ba2 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "json-diff": "^1.0.6", "json-stringify-safe": "^5.0.1", "json2csv": "^4.5.4", + "lodash": "^4.17.21", "mime-types": "^2.1.35", "minimist": "^1.2.8", "mkdirp": "^3.0.1", diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 963ae6aab..0284f68c5 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -7,47 +7,40 @@ const lineageConstraints = require('../lib/lineage-constraints'); const pouch = require('../lib/db'); const { trace, info } = require('../lib/log'); -const { - BATCH_SIZE, - prepareDocumentDirectory, - prettyPrintDocument, - replaceLineageInAncestors, - bold, - writeDocumentToDisk, - fetch, -} = require('../lib/mm-shared'); +const Shared = require('../lib/mm-shared'); module.exports = { requiresInstance: true, execute: () => { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); - prepareDocumentDirectory(args); - return updateLineagesAndStage(args, db); + Shared.prepareDocumentDirectory(args); + return mergeContacts(args, db); } }; -const updateLineagesAndStage = async (options, db) => { +const mergeContacts = async (options, db) => { trace(`Fetching contact details: ${options.winnerId}`); - const winnerDoc = await fetch.contact(db, options.winnerId); + const winnerDoc = await Shared.fetch.contact(db, options.winnerId); const constraints = await lineageConstraints(db, winnerDoc); - const loserDocs = await fetch.contactList(db, options.loserIds); + const loserDocs = await Shared.fetch.contactList(db, options.loserIds); await validateContacts(loserDocs, constraints); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(winnerDoc); for (let loserId of options.loserIds) { const contactDoc = loserDocs[loserId]; - const descendantsAndSelf = await fetch.descendantsOf(db, loserId); + const descendantsAndSelf = await Shared.fetch.descendantsOf(db, loserId); const self = descendantsAndSelf.find(d => d._id === loserId); - writeDocumentToDisk(options, { + Shared.writeDocumentToDisk(options, { _id: self._id, _rev: self._rev, _deleted: true, }); + const { prettyPrintDocument } = Shared; // Check that primary contact is not removed from areas where they are required const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); if (invalidPrimaryContactDoc) { @@ -57,13 +50,13 @@ const updateLineagesAndStage = async (options, db) => { trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, loserId); - const ancestors = await fetch.ancestorsOf(db, contactDoc); + const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); + const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); - const movedReportsCount = await moveReports(db, descendantsAndSelf, options, options.winnerId, loserId); + const movedReportsCount = await reassignReportSubjects(db, descendantsAndSelf, options, replacementLineage, loserId); trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); affectedContactCount += updatedDescendants.length + updatedAncestors.length; @@ -79,27 +72,13 @@ const updateLineagesAndStage = async (options, db) => { Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) Confirms the list of contacts are possible to move */ -const validateContacts = async (contactDocs, constraints) => { - Object.values(contactDocs).forEach(doc => { - const hierarchyError = constraints.getHierarchyErrors(doc); +const validateContacts = async (loserDocs, constraints) => { + Object.values(loserDocs).forEach(doc => { + const hierarchyError = constraints.getMergeContactHierarchyViolations(doc); if (hierarchyError) { throw Error(`Hierarchy Constraints: ${hierarchyError}`); } }); - - /* - It is nice that the tool can move lists of contacts as one operation, but strange things happen when two loserIds are in the same lineage. - For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. - */ - const loserIds = Object.keys(contactDocs); - Object.values(contactDocs) - .forEach(doc => { - const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; - const violatingParentId = parentIdsOfDoc.find(winnerId => loserIds.includes(winnerId)); - if (violatingParentId) { - throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); - } - }); }; // Parses extraArgs and asserts if required parameters are not present @@ -112,12 +91,12 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { if (loserIds.length === 0) { usage(); - throw Error(`Action "merge-contacts" is missing required list of contacts ${bold('--losers')} to be merged into the winner`); + throw Error(`Action "merge-contacts" is missing required list of contacts ${Shared.bold('--losers')} to be merged into the winner`); } if (!args.winner) { usage(); - throw Error(`Action "merge-contacts" is missing required parameter ${bold('--winner')}`); + throw Error(`Action "merge-contacts" is missing required parameter ${Shared.bold('--winner')}`); } return { @@ -130,13 +109,13 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { const usage = () => { info(` -${bold('cht-conf\'s merge-contacts action')} +${Shared.bold('cht-conf\'s merge-contacts action')} When combined with 'upload-docs' this action merges multiple contacts and all their associated data into one. -${bold('USAGE')} +${Shared.bold('USAGE')} cht --local merge-contacts -- --winner= --losers=, -${bold('OPTIONS')} +${Shared.bold('OPTIONS')} --winner= Specifies the ID of the contact that should have all other contact data merged into it. @@ -148,38 +127,61 @@ ${bold('OPTIONS')} `); }; -const moveReports = async (db, descendantsAndSelf, writeOptions, winnerId, loserId) => { +const reassignReportSubjects = async (db, descendantsAndSelf, writeOptions, replacementLineage, loserId) => { + const descendantIds = descendantsAndSelf.map(contact => contact._id); + const winnerId = writeOptions.winnerId; + let skip = 0; let reportDocsBatch; do { - info(`Processing ${skip} to ${skip + BATCH_SIZE} report docs`); - reportDocsBatch = await fetch.reportsCreatedFor(db, loserId, skip); + info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); + reportDocsBatch = await Shared.fetch.reportsCreatedByOrFor(db, descendantIds, loserId, skip); + + const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, loserId); reportDocsBatch.forEach(report => { + let updated = false; const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; for (const subjectId of subjectIds) { - if (report[subjectId]) { + if (report[subjectId] === loserId) { report[subjectId] = winnerId; + updated = true; } - if (report.fields[subjectId]) { + if (report.fields[subjectId] === loserId) { report.fields[subjectId] = winnerId; + updated = true; } - } - writeDocumentToDisk(writeOptions, report); + if (updated) { + const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); + if (!isAlreadyUpdated) { + updatedReports.push(report); + } + } + } }); + minifyLineageAndWriteToDisk(updatedReports, writeOptions); + skip += reportDocsBatch.length; - } while (reportDocsBatch.length >= BATCH_SIZE); + } while (reportDocsBatch.length >= Shared.BATCH_SIZE); return skip; }; +// Shared? +const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { + if (lineageManipulation.replaceLineageAt(doc, 'contact', replaceWith, startingFromIdInLineage)) { + agg.push(doc); + } + return agg; +}, []); + const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { docs.forEach(doc => { lineageManipulation.minifyLineagesInDoc(doc); - writeDocumentToDisk(parsedArgs, doc); + Shared.writeDocumentToDisk(parsedArgs, doc); }); }; @@ -189,10 +191,8 @@ const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contac return agg; } - const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, contactId); - - // TODO: seems wrong - const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, contactId); + const parentWasUpdated = lineageManipulation.replaceLineageAt(doc, 'parent', replacementLineage, contactId); + const contactWasUpdated = lineageManipulation.replaceLineageAt(doc, 'contact', replacementLineage, contactId); if (parentWasUpdated || contactWasUpdated) { agg.push(doc); } diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 342ef944b..66c160863 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -66,7 +66,7 @@ Confirms the list of contacts are possible to move */ const validateContacts = async (contactDocs, constraints) => { Object.values(contactDocs).forEach(doc => { - const hierarchyError = constraints.getHierarchyErrors(doc); + const hierarchyError = constraints.getMoveContactHierarchyViolations(doc); if (hierarchyError) { throw Error(`Hierarchy Constraints: ${hierarchyError}`); } @@ -142,8 +142,8 @@ const moveReports = async (db, descendantsAndSelf, writeOptions, replacementLine info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); reportDocsBatch = await Shared.fetch.reportsCreatedBy(db, contactIds, skip); - const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, contactId); - minifyLineageAndWriteToDisk(updatedReports, writeOptions); + const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, contactId); + minifyLineageAndWriteToDisk(updatedReports, writeOptions); skip += reportDocsBatch.length; } while (reportDocsBatch.length >= Shared.BATCH_SIZE); @@ -159,7 +159,7 @@ const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { }; const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage)) { + if (lineageManipulation.replaceLineageAfter(doc, 'contact', replaceWith, startingFromIdInLineage)) { agg.push(doc); } return agg; @@ -167,8 +167,8 @@ const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, start const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contactId) => descendantsAndSelf.reduce((agg, doc) => { const startingFromIdInLineage = doc._id === contactId ? undefined : contactId; - const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage); - const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, contactId); + const parentWasUpdated = lineageManipulation.replaceLineageAfter(doc, 'parent', replacementLineage, startingFromIdInLineage); + const contactWasUpdated = lineageManipulation.replaceLineageAfter(doc, 'contact', replacementLineage, contactId); if (parentWasUpdated || contactWasUpdated) { agg.push(doc); } diff --git a/src/lib/lineage-constraints.js b/src/lib/lineage-constraints.js index c0eb59647..c3042ee29 100644 --- a/src/lib/lineage-constraints.js +++ b/src/lib/lineage-constraints.js @@ -32,8 +32,9 @@ const lineageConstraints = async (repository, parentDoc) => { } return { - getHierarchyErrors: contactDoc => getHierarchyViolations(mapTypeToAllowedParents, contactDoc, parentDoc), getPrimaryContactViolations: (contactDoc, descendantDocs) => getPrimaryContactViolations(repository, contactDoc, parentDoc, descendantDocs), + getMoveContactHierarchyViolations: contactDoc => getMoveContactHierarchyViolations(mapTypeToAllowedParents, contactDoc, parentDoc), + getMergeContactHierarchyViolations: contactDoc => getMergeContactHierarchyViolations(contactDoc, parentDoc), }; }; @@ -41,7 +42,8 @@ const lineageConstraints = async (repository, parentDoc) => { Enforce the list of allowed parents for each contact type Ensure we are not creating a circular hierarchy */ -const getHierarchyViolations = (mapTypeToAllowedParents, contactDoc, parentDoc) => { +const getMoveContactHierarchyViolations = (mapTypeToAllowedParents, contactDoc, parentDoc) => { + // TODO reuse this code const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); const contactType = getContactType(contactDoc); const parentType = getContactType(parentDoc); @@ -63,6 +65,31 @@ const getHierarchyViolations = (mapTypeToAllowedParents, contactDoc, parentDoc) } }; +/* +Enforce the list of allowed parents for each contact type +Ensure we are not creating a circular hierarchy +*/ +const getMergeContactHierarchyViolations = (loserDoc, winnerDoc) => { + const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); + const loserContactType = getContactType(loserDoc); + const winnerContactType = getContactType(winnerDoc); + if (!loserContactType) { + return 'contact required attribute "type" is undefined'; + } + + if (winnerDoc && !winnerContactType) { + return `winner contact "${winnerDoc._id}" required attribute "type" is undefined`; + } + + if (loserContactType !== winnerContactType) { + return `contact "${loserDoc._id}" must have same contact type as "${winnerContactType}". Former is "${loserContactType}" while later is "${winnerContactType}".`; + } + + if (loserDoc._id === winnerDoc._id) { + return `Cannot merge contact with self`; + } +}; + /* A place's primary contact must be a descendant of that place. diff --git a/src/lib/lineage-manipulation.js b/src/lib/lineage-manipulation.js index e87eb7107..001c637dc 100644 --- a/src/lib/lineage-manipulation.js +++ b/src/lib/lineage-manipulation.js @@ -5,32 +5,17 @@ Given a doc, replace the lineage information therein with "replaceWith" startingFromIdInLineage (optional) - Will result in a partial replacement of the lineage. Only the part of the lineage "after" the parent with _id=startingFromIdInLineage will be replaced by "replaceWith" */ -const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { - const handleReplacement = (replaceInDoc, docAttr, replaceWith) => { - if (!replaceWith) { - const lineageWasDeleted = !!replaceInDoc[docAttr]; - replaceInDoc[docAttr] = undefined; - return lineageWasDeleted; - } else if (replaceInDoc[docAttr]) { - replaceInDoc[docAttr]._id = replaceWith._id; - replaceInDoc[docAttr].parent = replaceWith.parent; - } else { - replaceInDoc[docAttr] = replaceWith; - } - - return true; - }; - +const replaceLineageAfter = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { // Replace the full lineage if (!startingFromIdInLineage) { - return handleReplacement(doc, lineageAttributeName, replaceWith); + return _doReplaceInLineage(doc, lineageAttributeName, replaceWith); } // Replace part of a lineage let currentParent = doc[lineageAttributeName]; while (currentParent) { if (currentParent._id === startingFromIdInLineage) { - return handleReplacement(currentParent, 'parent', replaceWith); + return _doReplaceInLineage(currentParent, 'parent', replaceWith); } currentParent = currentParent.parent; } @@ -38,6 +23,41 @@ const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdIn return false; }; +const replaceLineageAt = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { + if (!replaceWith || !startingFromIdInLineage) { + throw Error('replaceWith and startingFromIdInLineage must be defined'); + } + + // Replace part of a lineage + let currentElement = doc; + let currentAttributeName = lineageAttributeName; + while (currentElement) { + if (currentElement[currentAttributeName]?._id === startingFromIdInLineage) { + return _doReplaceInLineage(currentElement, currentAttributeName, replaceWith); + } + + currentElement = currentElement[currentAttributeName]; + currentAttributeName = 'parent'; + } + + return false; +}; + +const _doReplaceInLineage = (replaceInDoc, lineageAttributeName, replaceWith) => { + if (!replaceWith) { + const lineageWasDeleted = !!replaceInDoc[lineageAttributeName]; + replaceInDoc[lineageAttributeName] = undefined; + return lineageWasDeleted; + } else if (replaceInDoc[lineageAttributeName]) { + replaceInDoc[lineageAttributeName]._id = replaceWith._id; + replaceInDoc[lineageAttributeName].parent = replaceWith.parent; + } else { + replaceInDoc[lineageAttributeName] = replaceWith; + } + + return true; +}; + /* Function borrowed from shared-lib/lineage */ @@ -103,5 +123,6 @@ module.exports = { createLineageFromDoc, minifyLineagesInDoc, pluckIdsFromLineage, - replaceLineage, + replaceLineageAfter, + replaceLineageAt, }; diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js index a9b8c56e3..6783e2d8b 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/mm-shared.js @@ -1,3 +1,4 @@ +const _ = require('lodash'); const path = require('path'); const userPrompt = require('./user-prompt'); @@ -105,21 +106,22 @@ const fetch = { return reports.rows.map(row => row.doc); }, - reportsCreatedFor: async (db, contactId, skip) => { + reportsCreatedByOrFor: async (db, descendantIds, loserId, skip) => { // TODO is this the right way? const reports = await db.query('medic-client/reports_by_freetext', { keys: [ - [`patient_id:${contactId}`], - [`patient_uuid:${contactId}`], - [`place_id:${contactId}`], - [`place_uuid:${contactId}`], + ...descendantIds.map(descendantId => [`contact:${descendantId}`]), + [`patient_id:${loserId}`], + [`patient_uuid:${loserId}`], + [`place_id:${loserId}`], + [`place_uuid:${loserId}`], ], include_docs: true, limit: BATCH_SIZE, skip, }); - return reports.rows.map(row => row.doc); + return _.uniqBy(reports.rows.map(row => row.doc), '_id'); }, ancestorsOf: async (db, contactDoc) => { diff --git a/test/fn/merge-contacts.spec.js b/test/fn/merge-contacts.spec.js new file mode 100644 index 000000000..93da88fbf --- /dev/null +++ b/test/fn/merge-contacts.spec.js @@ -0,0 +1,158 @@ +const { assert, expect } = require('chai'); +const rewire = require('rewire'); +const sinon = require('sinon'); + +const Shared = rewire('../../src/lib/mm-shared'); + +const PouchDB = require('pouchdb-core'); +PouchDB.plugin(require('pouchdb-adapter-memory')); +PouchDB.plugin(require('pouchdb-mapreduce')); + +const mergeContactsModule = rewire('../../src/fn/merge-contacts'); +mergeContactsModule.__set__('Shared', Shared); + +const mergeContacts = mergeContactsModule.__get__('mergeContacts'); +const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); + +const contacts_by_depth = { + // eslint-disable-next-line quotes + map: "function(doc) {\n if (doc.type === 'tombstone' && doc.tombstone) {\n doc = doc.tombstone;\n }\n if (['contact', 'person', 'clinic', 'health_center', 'district_hospital'].indexOf(doc.type) !== -1) {\n var value = doc.patient_id || doc.place_id;\n var parent = doc;\n var depth = 0;\n while (parent) {\n if (parent._id) {\n emit([parent._id], value);\n emit([parent._id, depth], value);\n }\n depth++;\n parent = parent.parent;\n }\n }\n}", +}; + +const reports_by_freetext = { + // eslint-disable-next-line quotes + map: "function(doc) {\n var skip = [ '_id', '_rev', 'type', 'refid', 'content' ];\n\n var usedKeys = [];\n var emitMaybe = function(key, value) {\n if (usedKeys.indexOf(key) === -1 && // Not already used\n key.length > 2 // Not too short\n ) {\n usedKeys.push(key);\n emit([key], value);\n }\n };\n\n var emitField = function(key, value, reportedDate) {\n if (!key || !value) {\n return;\n }\n key = key.toLowerCase();\n if (skip.indexOf(key) !== -1 || /_date$/.test(key)) {\n return;\n }\n if (typeof value === 'string') {\n value = value.toLowerCase();\n value.split(/\\s+/).forEach(function(word) {\n emitMaybe(word, reportedDate);\n });\n }\n if (typeof value === 'number' || typeof value === 'string') {\n emitMaybe(key + ':' + value, reportedDate);\n }\n };\n\n if (doc.type === 'data_record' && doc.form) {\n Object.keys(doc).forEach(function(key) {\n emitField(key, doc[key], doc.reported_date);\n });\n if (doc.fields) {\n Object.keys(doc.fields).forEach(function(key) {\n emitField(key, doc.fields[key], doc.reported_date);\n });\n }\n if (doc.contact && doc.contact._id) {\n emitMaybe('contact:' + doc.contact._id.toLowerCase(), doc.reported_date);\n }\n }\n}" +}; + +describe('merge-contacts', () => { + let pouchDb, scenarioCount = 0; + const writtenDocs = []; + const getWrittenDoc = docId => { + const matches = writtenDocs.filter(doc => doc && doc._id === docId); + if (matches.length === 0) { + return undefined; + } + + // Remove _rev because it makes expectations harder to write + const result = matches[matches.length - 1]; + delete result._rev; + return result; + }; + + beforeEach(async () => { + pouchDb = new PouchDB(`merge-contacts-${scenarioCount++}`); + + await mockHierarchy(pouchDb, { + district_1: {}, + district_2: { + health_center_2: { + clinic_2: { + patient_2: {}, + }, + } + }, + }); + + await pouchDb.put({ _id: 'settings', settings: {} }); + + await pouchDb.put({ + _id: '_design/medic-client', + views: { reports_by_freetext }, + }); + + await pouchDb.put({ + _id: '_design/medic', + views: { contacts_by_depth }, + }); + + Shared.writeDocumentToDisk = (docDirectoryPath, doc) => writtenDocs.push(doc); + Shared.prepareDocumentDirectory = () => {}; + writtenDocs.length = 0; + }); + + afterEach(async () => pouchDb.destroy()); + + it('merge district_2 into district_1', async () => { + // setup + await mockReport(pouchDb, { + id: 'changing_subject_and_contact', + creatorId: 'health_center_2_contact', + patientId: 'district_2' + }); + + await mockReport(pouchDb, { + id: 'changing_contact', + creatorId: 'health_center_2_contact', + patientId: 'patient_2' + }); + + await mockReport(pouchDb, { + id: 'changing_subject', + patientId: 'district_2' + }); + + // action + await mergeContacts({ + loserIds: ['district_2'], + winnerId: 'district_1', + }, pouchDb); + + // assert + expect(getWrittenDoc('district_2')).to.deep.eq({ + _id: 'district_2', + _deleted: true, + }); + + expect(getWrittenDoc('health_center_2')).to.deep.eq({ + _id: 'health_center_2', + type: 'health_center', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + parent: parentsToLineage('district_1'), + }); + + expect(getWrittenDoc('clinic_2')).to.deep.eq({ + _id: 'clinic_2', + type: 'clinic', + contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_1'), + parent: parentsToLineage('health_center_2', 'district_1'), + }); + + expect(getWrittenDoc('patient_2')).to.deep.eq({ + _id: 'patient_2', + type: 'person', + parent: parentsToLineage('clinic_2', 'health_center_2', 'district_1'), + }); + + expect(getWrittenDoc('changing_subject_and_contact')).to.deep.eq({ + _id: 'changing_subject_and_contact', + form: 'foo', + type: 'data_record', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + fields: { + patient_uuid: 'district_1' + } + }); + + expect(getWrittenDoc('changing_contact')).to.deep.eq({ + _id: 'changing_contact', + form: 'foo', + type: 'data_record', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + fields: { + patient_uuid: 'patient_2' + } + }); + + expect(getWrittenDoc('changing_subject')).to.deep.eq({ + _id: 'changing_subject', + form: 'foo', + type: 'data_record', + contact: { + _id: 'dne', + }, + fields: { + patient_uuid: 'district_1' + } + }); + }); +}); diff --git a/test/lib/lineage-constraints.spec.js b/test/lib/lineage-constraints.spec.js index 66c6134d3..bcf574f12 100644 --- a/test/lib/lineage-constraints.spec.js +++ b/test/lib/lineage-constraints.spec.js @@ -11,11 +11,11 @@ const log = require('../../src/lib/log'); log.level = log.LEVEL_INFO; describe('lineage constriants', () => { - describe('getHierarchyErrors', () => { + describe('getMoveContactHierarchyViolations', () => { const scenario = async (contact_types, contactType, parentType) => { const mockDb = { get: () => ({ settings: { contact_types } }) }; - const { getHierarchyErrors } = await lineageConstraints(mockDb, { type: parentType }); - return getHierarchyErrors({ type: contactType }); + const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: parentType }); + return getMoveContactHierarchyViolations({ type: contactType }); }; it('empty rules yields error', async () => expect(await scenario([], 'person', 'health_center')).to.include('unknown type')); @@ -42,22 +42,22 @@ describe('lineage constriants', () => { it('no settings doc requires valid parent type', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getHierarchyErrors } = await lineageConstraints(mockDb, { type: 'dne' }); - const actual = getHierarchyErrors({ type: 'person' }); + const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'dne' }); + const actual = getMoveContactHierarchyViolations({ type: 'person' }); expect(actual).to.include('cannot have parent of type'); }); it('no settings doc requires valid contact type', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getHierarchyErrors } = await lineageConstraints(mockDb, { type: 'clinic' }); - const actual = getHierarchyErrors({ type: 'dne' }); + const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'clinic' }); + const actual = getMoveContactHierarchyViolations({ type: 'dne' }); expect(actual).to.include('unknown type'); }); it('no settings doc yields not defined', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getHierarchyErrors } = await lineageConstraints(mockDb, { type: 'clinic' }); - const actual = getHierarchyErrors({ type: 'person' }); + const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'clinic' }); + const actual = getMoveContactHierarchyViolations({ type: 'person' }); expect(actual).to.be.undefined; }); @@ -68,15 +68,15 @@ describe('lineage constriants', () => { it('can move district_hospital to root', async () => { const mockDb = { get: () => ({ settings: { } }) }; - const { getHierarchyErrors } = await lineageConstraints(mockDb, undefined); - const actual = getHierarchyErrors({ type: 'district_hospital' }); + const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, undefined); + const actual = getMoveContactHierarchyViolations({ type: 'district_hospital' }); expect(actual).to.be.undefined; }); }); }); describe('getPrimaryContactViolations', () => { - const getHierarchyErrors = lineageConstraints.__get__('getPrimaryContactViolations'); + const getMoveContactHierarchyViolations = lineageConstraints.__get__('getPrimaryContactViolations'); describe('on memory pouchdb', async () => { let pouchDb, scenarioCount = 0; @@ -106,13 +106,13 @@ describe('lineage constriants', () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_2'); - const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); }); it('cannot move clinic_1_contact to root', async () => { const contactDoc = await pouchDb.get('clinic_1_contact'); - const doc = await getHierarchyErrors(pouchDb, contactDoc, undefined, [contactDoc]); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, undefined, [contactDoc]); expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); }); @@ -120,7 +120,7 @@ describe('lineage constriants', () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_1'); - const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); @@ -129,7 +129,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 getHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, descendants); expect(doc).to.be.undefined; }); @@ -142,7 +142,7 @@ 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 getHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, descendants); expect(doc).to.deep.include({ _id: 'patient_1' }); }); @@ -153,7 +153,7 @@ describe('lineage constriants', () => { contactDoc.parent._id = 'dne'; - const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); }); diff --git a/test/lib/lineage-manipulation.spec.js b/test/lib/lineage-manipulation.spec.js index 7ad0d6e09..1e8947a6c 100644 --- a/test/lib/lineage-manipulation.spec.js +++ b/test/lib/lineage-manipulation.spec.js @@ -1,18 +1,18 @@ const { expect } = require('chai'); -const { replaceLineage, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../src/lib/lineage-manipulation'); +const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../src/lib/lineage-manipulation'); const log = require('../../src/lib/log'); log.level = log.LEVEL_TRACE; const { parentsToLineage } = require('../mock-hierarchies'); describe('lineage manipulation', () => { - describe('replaceLineage', () => { + describe('replaceLineageAfter', () => { const mockReport = data => Object.assign({ _id: 'r', type: 'data_record', contact: parentsToLineage('parent', 'grandparent') }, data); const mockContact = data => Object.assign({ _id: 'c', type: 'person', parent: parentsToLineage('parent', 'grandparent') }, data); it('replace with empty lineage', () => { const mock = mockReport(); - expect(replaceLineage(mock, 'contact', undefined)).to.be.true; + expect(replaceLineageAfter(mock, 'contact', undefined)).to.be.true; expect(mock).to.deep.eq({ _id: 'r', type: 'data_record', @@ -22,7 +22,7 @@ describe('lineage manipulation', () => { it('replace full lineage', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -34,7 +34,7 @@ describe('lineage manipulation', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -45,12 +45,12 @@ describe('lineage manipulation', () => { it('replace empty with empty', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineage(mock, 'parent', undefined)).to.be.false; + expect(replaceLineageAfter(mock, 'parent', undefined)).to.be.false; }); it('replace lineage starting at contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('new_grandparent'), 'parent')).to.be.true; + expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_grandparent'), 'parent')).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -60,7 +60,7 @@ describe('lineage manipulation', () => { it('replace empty starting at contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', undefined, 'parent')).to.be.true; + expect(replaceLineageAfter(mock, 'parent', undefined, 'parent')).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -70,7 +70,7 @@ describe('lineage manipulation', () => { it('replace starting at non-existant contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('irrelevant'), 'dne')).to.be.false; + expect(replaceLineageAfter(mock, 'parent', parentsToLineage('irrelevant'), 'dne')).to.be.false; }); }); diff --git a/test/mock-hierarchies.js b/test/mock-hierarchies.js index d8a2436b3..6d99d8332 100644 --- a/test/mock-hierarchies.js +++ b/test/mock-hierarchies.js @@ -35,13 +35,16 @@ const mockHierarchy = async (db, hierarchy, existingLineage, depth = 0) => { }; const mockReport = async (db, report) => { - const creatorDoc = await db.get(report.creatorId); + const creatorDoc = report.creatorId && await db.get(report.creatorId); await db.put({ _id: report.id, form: 'foo', type: 'data_record', - contact: buildLineage(report.creatorId, creatorDoc.parent), + contact: buildLineage(report.creatorId || 'dne', creatorDoc?.parent), + fields: { + patient_uuid: report.patientId, + } }); }; From 090895c63377a6c63c9d284d26fd28a0b40b2dca Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 12 Nov 2024 16:18:42 -0700 Subject: [PATCH 04/21] Negative cases --- test/fn/merge-contacts.spec.js | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/test/fn/merge-contacts.spec.js b/test/fn/merge-contacts.spec.js index 93da88fbf..3aa5a98e3 100644 --- a/test/fn/merge-contacts.spec.js +++ b/test/fn/merge-contacts.spec.js @@ -1,9 +1,13 @@ -const { assert, expect } = require('chai'); + +const chai = require('chai'); +const chaiAsPromised = require('chai-as-promised'); const rewire = require('rewire'); -const sinon = require('sinon'); const Shared = rewire('../../src/lib/mm-shared'); +chai.use(chaiAsPromised); +const { expect } = chai; + const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); @@ -155,4 +159,28 @@ describe('merge-contacts', () => { } }); }); + + it('throw if loser does not exist', async () => { + const actual = mergeContacts({ + loserIds: ['dne'], + winnerId: 'district_1', + }, pouchDb); + await expect(actual).to.eventually.rejectedWith('could not be found'); + }); + + it('throw if winner does not exist', async () => { + const actual = mergeContacts({ + loserIds: ['district_1'], + winnerId: 'dne', + }, pouchDb); + await expect(actual).to.eventually.rejectedWith('could not be found'); + }); + + it('throw if loser is winner', async () => { + const actual = mergeContacts({ + loserIds: ['district_1', 'district_2'], + winnerId: 'district_2', + }, pouchDb); + await expect(actual).to.eventually.rejectedWith('merge contact with self'); + }); }); From 3e6168c494c53198423984e2d2d53227b49c40fe Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 12 Nov 2024 16:46:21 -0700 Subject: [PATCH 05/21] Fix move-contacts tests again --- src/fn/merge-contacts.js | 14 +++++++------- src/fn/move-contacts.js | 9 +++++---- src/lib/mm-shared.js | 3 --- test/fn/move-contacts.spec.js | 23 +++++++++++------------ 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 0284f68c5..24513b003 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -40,7 +40,7 @@ const mergeContacts = async (options, db) => { _deleted: true, }); - const { prettyPrintDocument } = Shared; + const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; // Check that primary contact is not removed from areas where they are required const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); if (invalidPrimaryContactDoc) { @@ -56,7 +56,7 @@ const mergeContacts = async (options, db) => { minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); - const movedReportsCount = await reassignReportSubjects(db, descendantsAndSelf, options, replacementLineage, loserId); + const movedReportsCount = await moveReportsAndReassign(db, descendantsAndSelf, options, replacementLineage, loserId); trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); affectedContactCount += updatedDescendants.length + updatedAncestors.length; @@ -89,14 +89,14 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { .split(',') .filter(Boolean); - if (loserIds.length === 0) { + if (!args.winner) { usage(); - throw Error(`Action "merge-contacts" is missing required list of contacts ${Shared.bold('--losers')} to be merged into the winner`); + throw Error(`Action "merge-contacts" is missing required contact ID ${Shared.bold('--winner')}. Other contacts will be merged into this contact.`); } - if (!args.winner) { + if (loserIds.length === 0) { usage(); - throw Error(`Action "merge-contacts" is missing required parameter ${Shared.bold('--winner')}`); + throw Error(`Action "merge-contacts" is missing required contact ID(s) ${Shared.bold('--losers')}. These contacts will be merged into the contact specified by ${Shared.bold('--winner')}`); } return { @@ -127,7 +127,7 @@ ${Shared.bold('OPTIONS')} `); }; -const reassignReportSubjects = async (db, descendantsAndSelf, writeOptions, replacementLineage, loserId) => { +const moveReportsAndReassign = async (db, descendantsAndSelf, writeOptions, replacementLineage, loserId) => { const descendantIds = descendantsAndSelf.map(contact => contact._id); const winnerId = writeOptions.winnerId; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 66c160863..cd4c81b8a 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -29,6 +29,7 @@ const updateLineagesAndStage = async (options, db) => { let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(parentDoc); + const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; for (let contactId of options.contactIds) { const contactDoc = contactDocs[contactId]; const descendantsAndSelf = await Shared.fetch.descendantsOf(db, contactId); @@ -36,14 +37,14 @@ const updateLineagesAndStage = async (options, db) => { // Check that primary contact is not removed from areas where they are required const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); if (invalidPrimaryContactDoc) { - throw Error(`Cannot remove contact ${Shared.prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); } - trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${Shared.prettyPrintDocument(contactDoc)}.`); + trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, contactId); const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); - trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${Shared.prettyPrintDocument(contactDoc)}.`); + trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); @@ -54,7 +55,7 @@ const updateLineagesAndStage = async (options, db) => { affectedContactCount += updatedDescendants.length + updatedAncestors.length; affectedReportCount += movedReportsCount; - info(`Staged updates to ${Shared.prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + info(`Staged updates to ${prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); } info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js index 6783e2d8b..3a61839b2 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/mm-shared.js @@ -9,8 +9,6 @@ const lineageManipulation = require('./lineage-manipulation'); const HIERARCHY_ROOT = 'root'; const BATCH_SIZE = 10000; -const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; - const prepareDocumentDirectory = ({ docDirectoryPath, force }) => { if (!fs.exists(docDirectoryPath)) { fs.mkdir(docDirectoryPath); @@ -147,7 +145,6 @@ module.exports = { BATCH_SIZE, bold, prepareDocumentDirectory, - prettyPrintDocument, replaceLineageInAncestors, writeDocumentToDisk, fetch, diff --git a/test/fn/move-contacts.spec.js b/test/fn/move-contacts.spec.js index 1d27c6a3b..22d845d5e 100644 --- a/test/fn/move-contacts.spec.js +++ b/test/fn/move-contacts.spec.js @@ -126,6 +126,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_2'), }); }); @@ -170,6 +171,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1'), }); @@ -232,6 +234,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_1', 'district_2'), }); }); @@ -283,6 +286,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_1', 'county_1'), }); }); @@ -321,6 +325,7 @@ describe('move-contacts', () => { _id: 'report_focal', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('focal', 'subcounty', 'county'), }); }); @@ -466,18 +471,6 @@ describe('move-contacts', () => { } }); - it('throw if contact_id does not exist', async () => { - try { - await updateLineagesAndStage({ - contactIds: ['dne'], - parentId: 'clinic_1' - }, pouchDb); - assert.fail('should throw'); - } catch (err) { - expect(err.message).to.include('could not be found'); - } - }); - it('throw if contact_id is not a contact', async () => { try { await updateLineagesAndStage({ @@ -617,6 +610,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_2'), }); @@ -624,6 +618,7 @@ describe('move-contacts', () => { _id: 'report_2', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_2'), }); @@ -631,6 +626,7 @@ describe('move-contacts', () => { _id: 'report_3', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_2'), }); @@ -693,6 +689,7 @@ describe('move-contacts', () => { _id: 'report_1', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_1'), }); @@ -700,6 +697,7 @@ describe('move-contacts', () => { _id: 'report_2', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_1'), }); @@ -707,6 +705,7 @@ describe('move-contacts', () => { _id: 'report_3', form: 'foo', type: 'data_record', + fields: {}, contact: parentsToLineage('health_center_1_contact', 'health_center_1', 'district_1'), }); From 2449dd4da8ff17e41e32765bb16ced54ff90e40c Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 13:33:15 -0700 Subject: [PATCH 06/21] Some renaming --- src/fn/merge-contacts.js | 66 ++++++++++++++-------------- src/lib/lineage-constraints.js | 18 ++++---- src/lib/mm-shared.js | 10 ++--- test/fn/merge-contacts.spec.js | 80 +++++++++++++++++++++++++++++----- 4 files changed, 115 insertions(+), 59 deletions(-) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 24513b003..8eba4a977 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -20,20 +20,20 @@ module.exports = { }; const mergeContacts = async (options, db) => { - trace(`Fetching contact details: ${options.winnerId}`); - const winnerDoc = await Shared.fetch.contact(db, options.winnerId); + trace(`Fetching contact details: ${options.keptId}`); + const keptDoc = await Shared.fetch.contact(db, options.keptId); - const constraints = await lineageConstraints(db, winnerDoc); - const loserDocs = await Shared.fetch.contactList(db, options.loserIds); - await validateContacts(loserDocs, constraints); + const constraints = await lineageConstraints(db, keptDoc); + const removedDocs = await Shared.fetch.contactList(db, options.removedIds); + await validateContacts(removedDocs, constraints); let affectedContactCount = 0, affectedReportCount = 0; - const replacementLineage = lineageManipulation.createLineageFromDoc(winnerDoc); - for (let loserId of options.loserIds) { - const contactDoc = loserDocs[loserId]; - const descendantsAndSelf = await Shared.fetch.descendantsOf(db, loserId); + const replacementLineage = lineageManipulation.createLineageFromDoc(keptDoc); + for (let removedId of options.removedIds) { + const contactDoc = removedDocs[removedId]; + const descendantsAndSelf = await Shared.fetch.descendantsOf(db, removedId); - const self = descendantsAndSelf.find(d => d._id === loserId); + const self = descendantsAndSelf.find(d => d._id === removedId); Shared.writeDocumentToDisk(options, { _id: self._id, _rev: self._rev, @@ -48,7 +48,7 @@ const mergeContacts = async (options, db) => { } trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, loserId); + const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, removedId); const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); @@ -56,7 +56,7 @@ const mergeContacts = async (options, db) => { minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); - const movedReportsCount = await moveReportsAndReassign(db, descendantsAndSelf, options, replacementLineage, loserId); + const movedReportsCount = await moveReportsAndReassign(db, descendantsAndSelf, options, replacementLineage, removedId); trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); affectedContactCount += updatedDescendants.length + updatedAncestors.length; @@ -72,8 +72,8 @@ const mergeContacts = async (options, db) => { Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) Confirms the list of contacts are possible to move */ -const validateContacts = async (loserDocs, constraints) => { - Object.values(loserDocs).forEach(doc => { +const validateContacts = async (removedDocs, constraints) => { + Object.values(removedDocs).forEach(doc => { const hierarchyError = constraints.getMergeContactHierarchyViolations(doc); if (hierarchyError) { throw Error(`Hierarchy Constraints: ${hierarchyError}`); @@ -85,23 +85,23 @@ const validateContacts = async (loserDocs, constraints) => { const parseExtraArgs = (projectDir, extraArgs = []) => { const args = minimist(extraArgs, { boolean: true }); - const loserIds = (args.losers || args.loser || '') + const removedIds = (args.removed || '') .split(',') .filter(Boolean); - if (!args.winner) { + if (!args.kept) { usage(); - throw Error(`Action "merge-contacts" is missing required contact ID ${Shared.bold('--winner')}. Other contacts will be merged into this contact.`); + throw Error(`Action "merge-contacts" is missing required contact ID ${Shared.bold('--kept')}. Other contacts will be merged into this contact.`); } - if (loserIds.length === 0) { + if (removedIds.length === 0) { usage(); - throw Error(`Action "merge-contacts" is missing required contact ID(s) ${Shared.bold('--losers')}. These contacts will be merged into the contact specified by ${Shared.bold('--winner')}`); + throw Error(`Action "merge-contacts" is missing required contact ID(s) ${Shared.bold('--removed')}. These contacts will be merged into the contact specified by ${Shared.bold('--kept')}`); } return { - winnerId: args.winner, - loserIds, + keptId: args.kept, + removedIds, docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'), force: !!args.force, }; @@ -113,43 +113,43 @@ ${Shared.bold('cht-conf\'s merge-contacts action')} When combined with 'upload-docs' this action merges multiple contacts and all their associated data into one. ${Shared.bold('USAGE')} -cht --local merge-contacts -- --winner= --losers=, +cht --local merge-contacts -- --kept= --removed=, ${Shared.bold('OPTIONS')} ---winner= +--kept= Specifies the ID of the contact that should have all other contact data merged into it. ---losers=, - A comma delimited list of IDs of contacts which will be deleted and all of their data will be merged into the winner contact. +--removed=, + A comma delimited list of IDs of contacts which will be deleted and all of their data will be merged into the kept contact. --docDirectoryPath= Specifies the folder used to store the documents representing the changes in hierarchy. `); }; -const moveReportsAndReassign = async (db, descendantsAndSelf, writeOptions, replacementLineage, loserId) => { +const moveReportsAndReassign = async (db, descendantsAndSelf, writeOptions, replacementLineage, removedId) => { const descendantIds = descendantsAndSelf.map(contact => contact._id); - const winnerId = writeOptions.winnerId; + const keptId = writeOptions.keptId; let skip = 0; let reportDocsBatch; do { info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); - reportDocsBatch = await Shared.fetch.reportsCreatedByOrFor(db, descendantIds, loserId, skip); + reportDocsBatch = await Shared.fetch.reportsCreatedByOrFor(db, descendantIds, removedId, skip); - const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, loserId); + const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, removedId); reportDocsBatch.forEach(report => { let updated = false; const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; for (const subjectId of subjectIds) { - if (report[subjectId] === loserId) { - report[subjectId] = winnerId; + if (report[subjectId] === removedId) { + report[subjectId] = keptId; updated = true; } - if (report.fields[subjectId] === loserId) { - report.fields[subjectId] = winnerId; + if (report.fields[subjectId] === removedId) { + report.fields[subjectId] = keptId; updated = true; } diff --git a/src/lib/lineage-constraints.js b/src/lib/lineage-constraints.js index c3042ee29..9d64ed499 100644 --- a/src/lib/lineage-constraints.js +++ b/src/lib/lineage-constraints.js @@ -69,23 +69,23 @@ const getMoveContactHierarchyViolations = (mapTypeToAllowedParents, contactDoc, Enforce the list of allowed parents for each contact type Ensure we are not creating a circular hierarchy */ -const getMergeContactHierarchyViolations = (loserDoc, winnerDoc) => { +const getMergeContactHierarchyViolations = (removedDoc, keptDoc) => { const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); - const loserContactType = getContactType(loserDoc); - const winnerContactType = getContactType(winnerDoc); - if (!loserContactType) { + const removedContactType = getContactType(removedDoc); + const keptContactType = getContactType(keptDoc); + if (!removedContactType) { return 'contact required attribute "type" is undefined'; } - if (winnerDoc && !winnerContactType) { - return `winner contact "${winnerDoc._id}" required attribute "type" is undefined`; + if (keptDoc && !keptContactType) { + return `kept contact "${keptDoc._id}" required attribute "type" is undefined`; } - if (loserContactType !== winnerContactType) { - return `contact "${loserDoc._id}" must have same contact type as "${winnerContactType}". Former is "${loserContactType}" while later is "${winnerContactType}".`; + if (removedContactType !== keptContactType) { + return `contact "${removedDoc._id}" must have same contact type as "${keptContactType}". Former is "${removedContactType}" while later is "${keptContactType}".`; } - if (loserDoc._id === winnerDoc._id) { + if (removedDoc._id === keptDoc._id) { return `Cannot merge contact with self`; } }; diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js index 3a61839b2..2bf265043 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/mm-shared.js @@ -104,15 +104,15 @@ const fetch = { return reports.rows.map(row => row.doc); }, - reportsCreatedByOrFor: async (db, descendantIds, loserId, skip) => { + reportsCreatedByOrFor: async (db, descendantIds, removedId, skip) => { // TODO is this the right way? const reports = await db.query('medic-client/reports_by_freetext', { keys: [ ...descendantIds.map(descendantId => [`contact:${descendantId}`]), - [`patient_id:${loserId}`], - [`patient_uuid:${loserId}`], - [`place_id:${loserId}`], - [`place_uuid:${loserId}`], + [`patient_id:${removedId}`], + [`patient_uuid:${removedId}`], + [`place_id:${removedId}`], + [`place_uuid:${removedId}`], ], include_docs: true, limit: BATCH_SIZE, diff --git a/test/fn/merge-contacts.spec.js b/test/fn/merge-contacts.spec.js index 3aa5a98e3..db4278838 100644 --- a/test/fn/merge-contacts.spec.js +++ b/test/fn/merge-contacts.spec.js @@ -42,12 +42,19 @@ describe('merge-contacts', () => { delete result._rev; return result; }; + const expectWrittenDocs = expected => expect(writtenDocs.map(doc => doc._id)).to.have.members(expected); beforeEach(async () => { pouchDb = new PouchDB(`merge-contacts-${scenarioCount++}`); await mockHierarchy(pouchDb, { - district_1: {}, + district_1: { + health_center_1: { + clinic_1: { + patient_1: {}, + }, + } + }, district_2: { health_center_2: { clinic_2: { @@ -97,11 +104,19 @@ describe('merge-contacts', () => { // action await mergeContacts({ - loserIds: ['district_2'], - winnerId: 'district_1', + removedIds: ['district_2'], + keptId: 'district_1', }, pouchDb); // assert + expectWrittenDocs([ + 'district_2', 'district_2_contact', + 'health_center_2', 'health_center_2_contact', + 'clinic_2', 'clinic_2_contact', + 'patient_2', + 'changing_subject_and_contact', 'changing_contact', 'changing_subject' + ]); + expect(getWrittenDoc('district_2')).to.deep.eq({ _id: 'district_2', _deleted: true, @@ -160,26 +175,67 @@ describe('merge-contacts', () => { }); }); - it('throw if loser does not exist', async () => { + it('merge two patients', async () => { + // setup + await mockReport(pouchDb, { + id: 'pat1', + creatorId: 'clinic_1_contact', + patientId: 'patient_1' + }); + + await mockReport(pouchDb, { + id: 'pat2', + creatorId: 'clinic_2_contact', + patientId: 'patient_2' + }); + + // action + await mergeContacts({ + removedIds: ['patient_2'], + keptId: 'patient_1', + }, pouchDb); + + await expectWrittenDocs(['patient_2', 'pat2']); + + expect(getWrittenDoc('patient_2')).to.deep.eq({ + _id: 'patient_2', + _deleted: true, + }); + + expect(getWrittenDoc('pat2')).to.deep.eq({ + _id: 'pat2', + form: 'foo', + type: 'data_record', + // still created by the user in district-2 + contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_2'), + fields: { + patient_uuid: 'patient_1' + } + }); + }); + + xit('write to ancestors', () => {}); + + it('throw if removed does not exist', async () => { const actual = mergeContacts({ - loserIds: ['dne'], - winnerId: 'district_1', + removedIds: ['dne'], + keptId: 'district_1', }, pouchDb); await expect(actual).to.eventually.rejectedWith('could not be found'); }); - it('throw if winner does not exist', async () => { + it('throw if kept does not exist', async () => { const actual = mergeContacts({ - loserIds: ['district_1'], - winnerId: 'dne', + removedIds: ['district_1'], + keptId: 'dne', }, pouchDb); await expect(actual).to.eventually.rejectedWith('could not be found'); }); - it('throw if loser is winner', async () => { + it('throw if removed is kept', async () => { const actual = mergeContacts({ - loserIds: ['district_1', 'district_2'], - winnerId: 'district_2', + removedIds: ['district_1', 'district_2'], + keptId: 'district_2', }, pouchDb); await expect(actual).to.eventually.rejectedWith('merge contact with self'); }); From b5f8c3be29ceab4c3fbf61528983ca734f08e114 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 15:33:47 -0700 Subject: [PATCH 07/21] Refactor to use options --- src/fn/merge-contacts.js | 177 +++++---------------------------- src/fn/move-contacts.js | 132 +++--------------------- src/lib/lineage-constraints.js | 11 +- src/lib/mm-shared.js | 30 ++---- src/lib/move-contacts-lib.js | 169 +++++++++++++++++++++++++++++++ test/fn/merge-contacts.spec.js | 34 ++----- test/fn/move-contacts.spec.js | 109 +++++--------------- 7 files changed, 260 insertions(+), 402 deletions(-) create mode 100644 src/lib/move-contacts-lib.js diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 8eba4a977..615a681a2 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -2,199 +2,70 @@ const minimist = require('minimist'); const path = require('path'); const environment = require('../lib/environment'); -const lineageManipulation = require('../lib/lineage-manipulation'); -const lineageConstraints = require('../lib/lineage-constraints'); const pouch = require('../lib/db'); -const { trace, info } = require('../lib/log'); +const { info } = require('../lib/log'); -const Shared = require('../lib/mm-shared'); +const moveContactsLib = require('../lib/move-contacts-lib'); module.exports = { requiresInstance: true, execute: () => { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); - Shared.prepareDocumentDirectory(args); - return mergeContacts(args, db); - } -}; - -const mergeContacts = async (options, db) => { - trace(`Fetching contact details: ${options.keptId}`); - const keptDoc = await Shared.fetch.contact(db, options.keptId); - - const constraints = await lineageConstraints(db, keptDoc); - const removedDocs = await Shared.fetch.contactList(db, options.removedIds); - await validateContacts(removedDocs, constraints); - - let affectedContactCount = 0, affectedReportCount = 0; - const replacementLineage = lineageManipulation.createLineageFromDoc(keptDoc); - for (let removedId of options.removedIds) { - const contactDoc = removedDocs[removedId]; - const descendantsAndSelf = await Shared.fetch.descendantsOf(db, removedId); - - const self = descendantsAndSelf.find(d => d._id === removedId); - Shared.writeDocumentToDisk(options, { - _id: self._id, - _rev: self._rev, - _deleted: true, - }); - - const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; - // Check that primary contact is not removed from areas where they are required - const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); - if (invalidPrimaryContactDoc) { - throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + const options = { + sourceIds: args.removeIds, + destinationId: args.keepId, + merge: true, + docDirectoryPath: args.docDirectoryPath, + force: args.force, } - - trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, removedId); - - const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); - trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); - - minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); - - const movedReportsCount = await moveReportsAndReassign(db, descendantsAndSelf, options, replacementLineage, removedId); - trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); - - affectedContactCount += updatedDescendants.length + updatedAncestors.length; - affectedReportCount += movedReportsCount; - - info(`Staged updates to ${prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + return moveContactsLib.move(db, options); } - - info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); -}; - -/* -Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) -Confirms the list of contacts are possible to move -*/ -const validateContacts = async (removedDocs, constraints) => { - Object.values(removedDocs).forEach(doc => { - const hierarchyError = constraints.getMergeContactHierarchyViolations(doc); - if (hierarchyError) { - throw Error(`Hierarchy Constraints: ${hierarchyError}`); - } - }); }; // Parses extraArgs and asserts if required parameters are not present const parseExtraArgs = (projectDir, extraArgs = []) => { const args = minimist(extraArgs, { boolean: true }); - const removedIds = (args.removed || '') + const removeIds = (args.remove || '') .split(',') .filter(Boolean); - if (!args.kept) { + if (!args.keep) { usage(); - throw Error(`Action "merge-contacts" is missing required contact ID ${Shared.bold('--kept')}. Other contacts will be merged into this contact.`); + throw Error(`Action "merge-contacts" is missing required contact ID ${bold('--keep')}. Other contacts will be merged into this contact.`); } - if (removedIds.length === 0) { + if (removeIds.length === 0) { usage(); - throw Error(`Action "merge-contacts" is missing required contact ID(s) ${Shared.bold('--removed')}. These contacts will be merged into the contact specified by ${Shared.bold('--kept')}`); + throw Error(`Action "merge-contacts" is missing required contact ID(s) ${bold('--remove')}. These contacts will be merged into the contact specified by ${bold('--keep')}`); } return { - keptId: args.kept, - removedIds, + keepId: args.keep, + removeIds, docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'), force: !!args.force, }; }; +const bold = text => `\x1b[1m${text}\x1b[0m`; const usage = () => { info(` -${Shared.bold('cht-conf\'s merge-contacts action')} +${bold('cht-conf\'s merge-contacts action')} When combined with 'upload-docs' this action merges multiple contacts and all their associated data into one. -${Shared.bold('USAGE')} -cht --local merge-contacts -- --kept= --removed=, +${bold('USAGE')} +cht --local merge-contacts -- --keep= --remove=, -${Shared.bold('OPTIONS')} ---kept= +${bold('OPTIONS')} +--keep= Specifies the ID of the contact that should have all other contact data merged into it. ---removed=, - A comma delimited list of IDs of contacts which will be deleted and all of their data will be merged into the kept contact. +--remove=, + A comma delimited list of IDs of contacts which will be deleted and all of their data will be merged into the keep contact. --docDirectoryPath= Specifies the folder used to store the documents representing the changes in hierarchy. `); }; - -const moveReportsAndReassign = async (db, descendantsAndSelf, writeOptions, replacementLineage, removedId) => { - const descendantIds = descendantsAndSelf.map(contact => contact._id); - const keptId = writeOptions.keptId; - - let skip = 0; - let reportDocsBatch; - do { - info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); - reportDocsBatch = await Shared.fetch.reportsCreatedByOrFor(db, descendantIds, removedId, skip); - - const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, removedId); - - reportDocsBatch.forEach(report => { - let updated = false; - const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; - for (const subjectId of subjectIds) { - if (report[subjectId] === removedId) { - report[subjectId] = keptId; - updated = true; - } - - if (report.fields[subjectId] === removedId) { - report.fields[subjectId] = keptId; - updated = true; - } - - if (updated) { - const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); - if (!isAlreadyUpdated) { - updatedReports.push(report); - } - } - } - }); - - minifyLineageAndWriteToDisk(updatedReports, writeOptions); - - skip += reportDocsBatch.length; - } while (reportDocsBatch.length >= Shared.BATCH_SIZE); - - return skip; -}; - -// Shared? -const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - if (lineageManipulation.replaceLineageAt(doc, 'contact', replaceWith, startingFromIdInLineage)) { - agg.push(doc); - } - return agg; -}, []); - -const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { - docs.forEach(doc => { - lineageManipulation.minifyLineagesInDoc(doc); - Shared.writeDocumentToDisk(parsedArgs, doc); - }); -}; - -const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contactId) => descendantsAndSelf.reduce((agg, doc) => { - // skip top-level because it is now being deleted - if (doc._id === contactId) { - return agg; - } - - const parentWasUpdated = lineageManipulation.replaceLineageAt(doc, 'parent', replacementLineage, contactId); - const contactWasUpdated = lineageManipulation.replaceLineageAt(doc, 'contact', replacementLineage, contactId); - if (parentWasUpdated || contactWasUpdated) { - agg.push(doc); - } - return agg; -}, []); diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index cd4c81b8a..0b5ae2046 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -2,90 +2,25 @@ const minimist = require('minimist'); const path = require('path'); const environment = require('../lib/environment'); -const lineageManipulation = require('../lib/lineage-manipulation'); -const lineageConstraints = require('../lib/lineage-constraints'); const pouch = require('../lib/db'); -const { trace, info } = require('../lib/log'); +const { info } = require('../lib/log'); -const Shared = require('../lib/mm-shared'); +const moveContactsLib = require('../lib/move-contacts-lib'); module.exports = { requiresInstance: true, execute: () => { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); - Shared.prepareDocumentDirectory(args); - return updateLineagesAndStage(args, db); - } -}; - -const updateLineagesAndStage = async (options, db) => { - trace(`Fetching contact details for parent: ${options.parentId}`); - const parentDoc = await Shared.fetch.contact(db, options.parentId); - - const constraints = await lineageConstraints(db, parentDoc); - const contactDocs = await Shared.fetch.contactList(db, options.contactIds); - await validateContacts(contactDocs, constraints); - - let affectedContactCount = 0, affectedReportCount = 0; - const replacementLineage = lineageManipulation.createLineageFromDoc(parentDoc); - const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; - for (let contactId of options.contactIds) { - const contactDoc = contactDocs[contactId]; - const descendantsAndSelf = await Shared.fetch.descendantsOf(db, contactId); - - // Check that primary contact is not removed from areas where they are required - const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(contactDoc, descendantsAndSelf); - if (invalidPrimaryContactDoc) { - throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + const options = { + sourceIds: args.contactIds, + destinationId: args.parentId, + merge: false, + docDirectoryPath: args.docDirectoryPath, + force: args.force, } - - trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, contactId); - - const ancestors = await Shared.fetch.ancestorsOf(db, contactDoc); - trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(contactDoc)}.`); - const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); - - minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors], options); - - const movedReportsCount = await moveReports(db, descendantsAndSelf, options, replacementLineage, contactId); - trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); - - affectedContactCount += updatedDescendants.length + updatedAncestors.length; - affectedReportCount += movedReportsCount; - - info(`Staged updates to ${prettyPrintDocument(contactDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + return moveContactsLib.move(db, options); } - - info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); -}; - -/* -Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) -Confirms the list of contacts are possible to move -*/ -const validateContacts = async (contactDocs, constraints) => { - Object.values(contactDocs).forEach(doc => { - const hierarchyError = constraints.getMoveContactHierarchyViolations(doc); - if (hierarchyError) { - throw Error(`Hierarchy Constraints: ${hierarchyError}`); - } - }); - - /* - It is nice that the tool can move lists of contacts as one operation, but strange things happen when two contactIds are in the same lineage. - For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. - */ - const contactIds = Object.keys(contactDocs); - Object.values(contactDocs) - .forEach(doc => { - const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; - const violatingParentId = parentIdsOfDoc.find(parentId => contactIds.includes(parentId)); - if (violatingParentId) { - throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); - } - }); }; // Parses extraArgs and asserts if required parameters are not present @@ -114,15 +49,16 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { }; }; +const bold = text => `\x1b[1m${text}\x1b[0m`; const usage = () => { info(` -${Shared.bold('cht-conf\'s move-contacts action')} +${bold('cht-conf\'s move-contacts action')} When combined with 'upload-docs' this action effectively moves a contact from one place in the hierarchy to another. -${Shared.bold('USAGE')} +${bold('USAGE')} cht --local move-contacts -- --contacts=, --parent= -${Shared.bold('OPTIONS')} +${bold('OPTIONS')} --contacts=, A comma delimited list of ids of contacts to be moved. @@ -133,45 +69,3 @@ ${Shared.bold('OPTIONS')} Specifies the folder used to store the documents representing the changes in hierarchy. `); }; - -const moveReports = async (db, descendantsAndSelf, writeOptions, replacementLineage, contactId) => { - const contactIds = descendantsAndSelf.map(contact => contact._id); - - let skip = 0; - let reportDocsBatch; - do { - info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); - reportDocsBatch = await Shared.fetch.reportsCreatedBy(db, contactIds, skip); - - const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, contactId); - minifyLineageAndWriteToDisk(updatedReports, writeOptions); - - skip += reportDocsBatch.length; - } while (reportDocsBatch.length >= Shared.BATCH_SIZE); - - return skip; -}; - -const minifyLineageAndWriteToDisk = (docs, parsedArgs) => { - docs.forEach(doc => { - lineageManipulation.minifyLineagesInDoc(doc); - Shared.writeDocumentToDisk(parsedArgs, doc); - }); -}; - -const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - if (lineageManipulation.replaceLineageAfter(doc, 'contact', replaceWith, startingFromIdInLineage)) { - agg.push(doc); - } - return agg; -}, []); - -const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, contactId) => descendantsAndSelf.reduce((agg, doc) => { - const startingFromIdInLineage = doc._id === contactId ? undefined : contactId; - const parentWasUpdated = lineageManipulation.replaceLineageAfter(doc, 'parent', replacementLineage, startingFromIdInLineage); - const contactWasUpdated = lineageManipulation.replaceLineageAfter(doc, 'contact', replacementLineage, contactId); - if (parentWasUpdated || contactWasUpdated) { - agg.push(doc); - } - return agg; -}, []); diff --git a/src/lib/lineage-constraints.js b/src/lib/lineage-constraints.js index 9d64ed499..d5b1cb5a5 100644 --- a/src/lib/lineage-constraints.js +++ b/src/lib/lineage-constraints.js @@ -3,7 +3,7 @@ const { trace } = log; const { pluckIdsFromLineage } = require('./lineage-manipulation'); -const lineageConstraints = async (repository, parentDoc) => { +const lineageConstraints = async (repository, parentDoc, options) => { let mapTypeToAllowedParents; try { const { settings } = await repository.get('settings'); @@ -33,8 +33,13 @@ const lineageConstraints = async (repository, parentDoc) => { return { getPrimaryContactViolations: (contactDoc, descendantDocs) => getPrimaryContactViolations(repository, contactDoc, parentDoc, descendantDocs), - getMoveContactHierarchyViolations: contactDoc => getMoveContactHierarchyViolations(mapTypeToAllowedParents, contactDoc, parentDoc), - getMergeContactHierarchyViolations: contactDoc => getMergeContactHierarchyViolations(contactDoc, parentDoc), + validate: (contactDoc) => { + if (options.merge) { + return getMergeContactHierarchyViolations(contactDoc, parentDoc); + } + + return getMoveContactHierarchyViolations(mapTypeToAllowedParents, contactDoc, parentDoc); + }, }; }; diff --git a/src/lib/mm-shared.js b/src/lib/mm-shared.js index 2bf265043..977c52957 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/mm-shared.js @@ -93,26 +93,19 @@ const fetch = { .filter(doc => doc && doc.type !== 'tombstone'); }, - reportsCreatedBy: async (db, contactIds, skip) => { - const reports = await db.query('medic-client/reports_by_freetext', { - keys: contactIds.map(id => [`contact:${id}`]), - include_docs: true, - limit: BATCH_SIZE, - skip, - }); - - return reports.rows.map(row => row.doc); - }, + reportsCreatedByOrAt: async (db, createdByIds, createdAtId, skip) => { + const createdByKeys = createdByIds.map(descendantId => [`contact:${descendantId}`]); + const createdAtKeys = createdAtId ? [ + [`patient_id:${createdAtId}`], + [`patient_uuid:${createdAtId}`], + [`place_id:${createdAtId}`], + [`place_uuid:${createdAtId}`] + ] : []; - reportsCreatedByOrFor: async (db, descendantIds, removedId, skip) => { - // TODO is this the right way? const reports = await db.query('medic-client/reports_by_freetext', { keys: [ - ...descendantIds.map(descendantId => [`contact:${descendantId}`]), - [`patient_id:${removedId}`], - [`patient_uuid:${removedId}`], - [`place_id:${removedId}`], - [`place_uuid:${removedId}`], + ...createdByKeys, + ...createdAtKeys, ], include_docs: true, limit: BATCH_SIZE, @@ -138,12 +131,9 @@ const fetch = { }, }; -const bold = text => `\x1b[1m${text}\x1b[0m`; - module.exports = { HIERARCHY_ROOT, BATCH_SIZE, - bold, prepareDocumentDirectory, replaceLineageInAncestors, writeDocumentToDisk, diff --git a/src/lib/move-contacts-lib.js b/src/lib/move-contacts-lib.js new file mode 100644 index 000000000..d54b6484d --- /dev/null +++ b/src/lib/move-contacts-lib.js @@ -0,0 +1,169 @@ +const lineageManipulation = require('../lib/lineage-manipulation'); +const lineageConstraints = require('../lib/lineage-constraints'); +const { trace, info } = require('../lib/log'); + +const Shared = require('../lib/mm-shared'); + +module.exports = (options) => { + const move = async (sourceIds, destinationId, db) => { + Shared.prepareDocumentDirectory(options); + trace(`Fetching contact details: ${destinationId}`); + const destinationDoc = await Shared.fetch.contact(db, destinationId); + + const constraints = await lineageConstraints(db, destinationDoc, options); + const sourceDocs = await Shared.fetch.contactList(db, sourceIds); + await validateContacts(sourceDocs, constraints); + + let affectedContactCount = 0, affectedReportCount = 0; + const replacementLineage = lineageManipulation.createLineageFromDoc(destinationDoc); + for (let sourceId of sourceIds) { + const sourceDoc = sourceDocs[sourceId]; + const descendantsAndSelf = await Shared.fetch.descendantsOf(db, sourceId); + + if (options.merge) { + const self = descendantsAndSelf.find(d => d._id === sourceId); + Shared.writeDocumentToDisk(options, { + _id: self._id, + _rev: self._rev, + _deleted: true, + }); + } + + const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; + // Check that primary contact is not removed from areas where they are required + const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(sourceDoc, descendantsAndSelf); + if (invalidPrimaryContactDoc) { + throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); + } + + trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(sourceDoc)}.`); + const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, sourceId); + + const ancestors = await Shared.fetch.ancestorsOf(db, sourceDoc); + trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(sourceDoc)}.`); + const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); + + minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors]); + + const movedReportsCount = await moveReports(db, descendantsAndSelf, replacementLineage, sourceId, destinationId); + trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); + + affectedContactCount += updatedDescendants.length + updatedAncestors.length; + affectedReportCount += movedReportsCount; + + info(`Staged updates to ${prettyPrintDocument(sourceDoc)}. ${updatedDescendants.length} contact(s) and ${movedReportsCount} report(s).`); + } + + info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); + }; + + /* + Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) + Confirms the list of contacts are possible to move + */ + const validateContacts = async (sourceDocs, constraints) => { + Object.values(sourceDocs).forEach(doc => { + const hierarchyError = constraints.validate(doc); + if (hierarchyError) { + throw Error(`Hierarchy Constraints: ${hierarchyError}`); + } + }); + + /* + It is nice that the tool can move lists of contacts as one operation, but strange things happen when two contactIds are in the same lineage. + For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. + */ + const contactIds = Object.keys(sourceDocs); + Object.values(sourceDocs) + .forEach(doc => { + const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; + const violatingParentId = parentIdsOfDoc.find(parentId => contactIds.includes(parentId)); + if (violatingParentId) { + throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); + } + }); + }; + + const moveReports = async (db, descendantsAndSelf, replacementLineage, sourceId, destinationId) => { + const descendantIds = descendantsAndSelf.map(contact => contact._id); + + let skip = 0; + let reportDocsBatch; + do { + info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); + const createdAtId = options.merge && sourceId; + reportDocsBatch = await Shared.fetch.reportsCreatedByOrAt(db, descendantIds, createdAtId, skip); + + const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, sourceId); + + if (options.merge) { + reportDocsBatch.forEach(report => { + let updated = false; + const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; + for (const subjectId of subjectIds) { + if (report[subjectId] === sourceId) { + report[subjectId] = destinationId; + updated = true; + } + + if (report.fields[subjectId] === sourceId) { + report.fields[subjectId] = destinationId; + updated = true; + } + + if (updated) { + const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); + if (!isAlreadyUpdated) { + updatedReports.push(report); + } + } + } + }); + } + + minifyLineageAndWriteToDisk(updatedReports); + + skip += reportDocsBatch.length; + } while (reportDocsBatch.length >= Shared.BATCH_SIZE); + + return skip; + }; + + const minifyLineageAndWriteToDisk = (docs) => { + docs.forEach(doc => { + lineageManipulation.minifyLineagesInDoc(doc); + Shared.writeDocumentToDisk(options, doc); + }); + }; + + const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { + const operation = options.merge ? lineageManipulation.replaceLineageAt : lineageManipulation.replaceLineageAfter; + if (operation(doc, 'contact', replaceWith, startingFromIdInLineage)) { + agg.push(doc); + } + return agg; + }, []); + + const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, destinationId) => descendantsAndSelf.reduce((agg, doc) => { + const startingFromIdInLineage = options.merge ? destinationId : + doc._id === destinationId ? undefined : destinationId; + + // skip top-level because it will be deleted + if (options.merge) { + if (doc._id === destinationId) { + return agg; + } + } + + const lineageOperation = options.merge ? lineageManipulation.replaceLineageAt : lineageManipulation.replaceLineageAfter; + const parentWasUpdated = lineageOperation(doc, 'parent', replacementLineage, startingFromIdInLineage); + const contactWasUpdated = lineageOperation(doc, 'contact', replacementLineage, destinationId); + if (parentWasUpdated || contactWasUpdated) { + agg.push(doc); + } + return agg; + }, []); + + return { move }; +}; + diff --git a/test/fn/merge-contacts.spec.js b/test/fn/merge-contacts.spec.js index db4278838..df4e5e19f 100644 --- a/test/fn/merge-contacts.spec.js +++ b/test/fn/merge-contacts.spec.js @@ -12,10 +12,11 @@ const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const mergeContactsModule = rewire('../../src/fn/merge-contacts'); -mergeContactsModule.__set__('Shared', Shared); +const MoveContactsLib = rewire('../../src/lib/move-contacts-lib'); +MoveContactsLib.__set__('Shared', Shared); + +const move = MoveContactsLib({ merge: true }).move; -const mergeContacts = mergeContactsModule.__get__('mergeContacts'); const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); const contacts_by_depth = { @@ -102,11 +103,8 @@ describe('merge-contacts', () => { patientId: 'district_2' }); - // action - await mergeContacts({ - removedIds: ['district_2'], - keptId: 'district_1', - }, pouchDb); + // action + await move(['district_2'], 'district_1', pouchDb); // assert expectWrittenDocs([ @@ -190,10 +188,7 @@ describe('merge-contacts', () => { }); // action - await mergeContacts({ - removedIds: ['patient_2'], - keptId: 'patient_1', - }, pouchDb); + await move(['patient_2'], 'patient_1', pouchDb); await expectWrittenDocs(['patient_2', 'pat2']); @@ -217,26 +212,17 @@ describe('merge-contacts', () => { xit('write to ancestors', () => {}); it('throw if removed does not exist', async () => { - const actual = mergeContacts({ - removedIds: ['dne'], - keptId: 'district_1', - }, pouchDb); + const actual = move(['dne'], 'district_1', pouchDb); await expect(actual).to.eventually.rejectedWith('could not be found'); }); it('throw if kept does not exist', async () => { - const actual = mergeContacts({ - removedIds: ['district_1'], - keptId: 'dne', - }, pouchDb); + const actual = move(['district_1'], 'dne', pouchDb); await expect(actual).to.eventually.rejectedWith('could not be found'); }); it('throw if removed is kept', async () => { - const actual = mergeContacts({ - removedIds: ['district_1', 'district_2'], - keptId: 'district_2', - }, pouchDb); + const actual = move(['district_1', 'district_2'], 'district_2', pouchDb); await expect(actual).to.eventually.rejectedWith('merge contact with self'); }); }); diff --git a/test/fn/move-contacts.spec.js b/test/fn/move-contacts.spec.js index 22d845d5e..66847ee70 100644 --- a/test/fn/move-contacts.spec.js +++ b/test/fn/move-contacts.spec.js @@ -1,20 +1,18 @@ const { assert, expect } = require('chai'); const rewire = require('rewire'); const sinon = require('sinon'); -const fs = require('../../src/lib/sync-fs'); -const environment = require('../../src/lib/environment'); +const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); const Shared = rewire('../../src/lib/mm-shared'); const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const moveContactsModule = rewire('../../src/fn/move-contacts'); -moveContactsModule.__set__('Shared', Shared); +const MoveContactsLib = rewire('../../src/lib/move-contacts-lib'); +MoveContactsLib.__set__('Shared', Shared); -const updateLineagesAndStage = moveContactsModule.__get__('updateLineagesAndStage'); -const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); +const move = MoveContactsLib({ merge: false }).move; const contacts_by_depth = { // eslint-disable-next-line quotes @@ -91,10 +89,7 @@ describe('move-contacts', () => { afterEach(async () => pouchDb.destroy()); it('move health_center_1 to district_2', async () => { - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'district_2', - }, pouchDb); + await move(['health_center_1'], 'district_2', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -136,10 +131,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: [] }]); - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'root', - }, pouchDb); + await move(['health_center_1'], 'root', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -192,10 +184,7 @@ describe('move-contacts', () => { it('move district_1 from root', async () => { await updateHierarchyRules([{ id: 'district_hospital', parents: ['district_hospital'] }]); - await updateLineagesAndStage({ - contactIds: ['district_1'], - parentId: 'district_2', - }, pouchDb); + await move(['district_1'], 'district_2', pouchDb); expect(getWrittenDoc('district_1')).to.deep.eq({ _id: 'district_1', @@ -251,10 +240,7 @@ describe('move-contacts', () => { { id: 'district_hospital', parents: ['county'] }, ]); - await updateLineagesAndStage({ - contactIds: ['district_1'], - parentId: 'county_1', - }, pouchDb); + await move(['district_1'], 'county_1', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -309,10 +295,7 @@ describe('move-contacts', () => { creatorId: 'focal', }); - await updateLineagesAndStage({ - contactIds: ['focal'], - parentId: 'subcounty', - }, pouchDb); + await move(['focal'], 'subcounty', pouchDb); expect(getWrittenDoc('focal')).to.deep.eq({ _id: 'focal', @@ -357,10 +340,7 @@ describe('move-contacts', () => { parent: parentsToLineage(), }); - await updateLineagesAndStage({ - contactIds: ['t_patient_1'], - parentId: 't_clinic_2', - }, pouchDb); + await move(['t_patient_1'], 't_clinic_2', pouchDb); expect(getWrittenDoc('t_health_center_1')).to.deep.eq({ _id: 't_health_center_1', @@ -381,10 +361,7 @@ describe('move-contacts', () => { // We don't want lineage { id, parent: '' } to result from district_hospitals which have parent: '' it('district_hospital with empty string parent is not preserved', async () => { await upsert('district_2', { parent: '', type: 'district_hospital' }); - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'district_2', - }, pouchDb); + await move(['health_center_1'], 'district_2', pouchDb); expect(getWrittenDoc('health_center_1')).to.deep.eq({ _id: 'health_center_1', @@ -412,11 +389,7 @@ describe('move-contacts', () => { await upsert('clinic_1', clinic); await upsert('patient_1', patient); - await updateLineagesAndStage({ - contactIds: ['clinic_1'], - parentId: 'district_2', - }, pouchDb); - + await move(['clinic_1'], 'district_2', pouchDb); expect(getWrittenDoc('clinic_1')).to.deep.eq({ _id: 'clinic_1', @@ -437,10 +410,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: ['clinic'] }]); try { - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'clinic_1', - }, pouchDb); + await move(['health_center_1'], 'clinic_1', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('circular'); @@ -449,10 +419,7 @@ describe('move-contacts', () => { it('throw if parent does not exist', async () => { try { - await updateLineagesAndStage({ - contactIds: ['clinic_1'], - parentId: 'dne_parent_id' - }, pouchDb); + await move(['clinic_1'], 'dne_parent_id', pouchDb); assert.fail('should throw when parent is not defined'); } catch (err) { expect(err.message).to.include('could not be found'); @@ -461,10 +428,7 @@ describe('move-contacts', () => { it('throw when altering same lineage', async () => { try { - await updateLineagesAndStage({ - contactIds: ['patient_1', 'health_center_1'], - parentId: 'district_2', - }, pouchDb); + await move(['patient_1', 'health_center_1'], 'district_2', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('same lineage'); @@ -473,10 +437,7 @@ describe('move-contacts', () => { it('throw if contact_id is not a contact', async () => { try { - await updateLineagesAndStage({ - contactIds: ['report_1'], - parentId: 'clinic_1' - }, pouchDb); + await move(['report_1'], 'clinic_1', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('unknown type'); @@ -485,11 +446,7 @@ describe('move-contacts', () => { it('throw if moving primary contact of parent', async () => { try { - await updateLineagesAndStage({ - contactIds: ['clinic_1_contact'], - parentId: 'district_1' - }, pouchDb); - + await move(['clinic_1_contact'], 'district_1', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('primary contact'); @@ -499,11 +456,7 @@ describe('move-contacts', () => { it('throw if setting parent to self', async () => { await updateHierarchyRules([{ id: 'clinic', parents: ['clinic'] }]); try { - await updateLineagesAndStage({ - contactIds: ['clinic_1'], - parentId: 'clinic_1' - }, pouchDb); - + await move(['clinic_1'], 'clinic_1', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('circular'); @@ -514,19 +467,15 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'district_hospital', parents: [] }]); try { - await updateLineagesAndStage({ - contactIds: ['district_1'], - parentId: 'district_2', - }, pouchDb); - + await move(['district_1'], 'district_2', pouchDb); assert.fail('Expected error'); } catch (err) { expect(err.message).to.include('parent of type'); } }); - describe('parseExtraArgs', () => { - const parseExtraArgs = moveContactsModule.__get__('parseExtraArgs'); + xdescribe('parseExtraArgs', () => { + // const parseExtraArgs = MoveContactsLib.__get__('parseExtraArgs'); it('undefined arguments', () => { expect(() => parseExtraArgs(__dirname, undefined)).to.throw('required list of contacts'); }); @@ -538,8 +487,8 @@ describe('move-contacts', () => { it('contacts and parents', () => { const args = ['--contacts=food,is,tasty', '--parent=bar', '--docDirectoryPath=/', '--force=hi']; expect(parseExtraArgs(__dirname, args)).to.deep.eq({ - contactIds: ['food', 'is', 'tasty'], - parentId: 'bar', + sourceIds: ['food', 'is', 'tasty'], + destinationId: 'bar', force: true, docDirectoryPath: '/', }); @@ -575,11 +524,8 @@ describe('move-contacts', () => { Shared.BATCH_SIZE = 1; sinon.spy(pouchDb, 'query'); - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'district_2', - }, pouchDb); - + await move(['health_center_1'], 'district_2', pouchDb); + expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', type: 'person', @@ -654,10 +600,7 @@ describe('move-contacts', () => { Shared.BATCH_SIZE = 2; sinon.spy(pouchDb, 'query'); - await updateLineagesAndStage({ - contactIds: ['health_center_1'], - parentId: 'district_1', - }, pouchDb); + await move(['health_center_1'], 'district_1', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', From 1273fb620ceb39d552d57ec686f1ce5157ef5c8e Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 15:44:53 -0700 Subject: [PATCH 08/21] Move folder structure --- src/fn/merge-contacts.js | 8 +++----- src/fn/move-contacts.js | 8 +++----- src/lib/{ => move-contacts}/lineage-constraints.js | 2 +- src/lib/{ => move-contacts}/lineage-manipulation.js | 0 src/lib/{ => move-contacts}/mm-shared.js | 6 +++--- src/lib/{ => move-contacts}/move-contacts-lib.js | 8 ++++---- test/lib/{ => move-contacts}/lineage-constraints.spec.js | 6 +++--- test/lib/{ => move-contacts}/lineage-manipulation.spec.js | 6 +++--- test/{fn => lib/move-contacts}/merge-contacts.spec.js | 6 +++--- test/{fn => lib/move-contacts}/mm-shared.spec.js | 8 ++++---- test/{fn => lib/move-contacts}/move-contacts.spec.js | 6 +++--- 11 files changed, 30 insertions(+), 34 deletions(-) rename src/lib/{ => move-contacts}/lineage-constraints.js (99%) rename src/lib/{ => move-contacts}/lineage-manipulation.js (100%) rename src/lib/{ => move-contacts}/mm-shared.js (97%) rename src/lib/{ => move-contacts}/move-contacts-lib.js (96%) rename test/lib/{ => move-contacts}/lineage-constraints.spec.js (97%) rename test/lib/{ => move-contacts}/lineage-manipulation.spec.js (95%) rename test/{fn => lib/move-contacts}/merge-contacts.spec.js (97%) rename test/{fn => lib/move-contacts}/mm-shared.spec.js (85%) rename test/{fn => lib/move-contacts}/move-contacts.spec.js (99%) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 615a681a2..13c3e9a19 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -5,7 +5,7 @@ const environment = require('../lib/environment'); const pouch = require('../lib/db'); const { info } = require('../lib/log'); -const moveContactsLib = require('../lib/move-contacts-lib'); +const MoveContactsLib = require('../lib/move-contacts/move-contacts-lib'); module.exports = { requiresInstance: true, @@ -13,13 +13,11 @@ module.exports = { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); const options = { - sourceIds: args.removeIds, - destinationId: args.keepId, merge: true, docDirectoryPath: args.docDirectoryPath, force: args.force, - } - return moveContactsLib.move(db, options); + }; + return MoveContactsLib(options).move(args.removeIds, args.keepId, db); } }; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 0b5ae2046..7e1e68a56 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -5,7 +5,7 @@ const environment = require('../lib/environment'); const pouch = require('../lib/db'); const { info } = require('../lib/log'); -const moveContactsLib = require('../lib/move-contacts-lib'); +const MoveContactsLib = require('../lib/move-contacts/move-contacts-lib'); module.exports = { requiresInstance: true, @@ -13,13 +13,11 @@ module.exports = { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); const options = { - sourceIds: args.contactIds, - destinationId: args.parentId, merge: false, docDirectoryPath: args.docDirectoryPath, force: args.force, - } - return moveContactsLib.move(db, options); + }; + return MoveContactsLib(options).move(args.contactIds, args.parentId, db); } }; diff --git a/src/lib/lineage-constraints.js b/src/lib/move-contacts/lineage-constraints.js similarity index 99% rename from src/lib/lineage-constraints.js rename to src/lib/move-contacts/lineage-constraints.js index d5b1cb5a5..ff7b0992f 100644 --- a/src/lib/lineage-constraints.js +++ b/src/lib/move-contacts/lineage-constraints.js @@ -1,4 +1,4 @@ -const log = require('./log'); +const log = require('../log'); const { trace } = log; const { pluckIdsFromLineage } = require('./lineage-manipulation'); diff --git a/src/lib/lineage-manipulation.js b/src/lib/move-contacts/lineage-manipulation.js similarity index 100% rename from src/lib/lineage-manipulation.js rename to src/lib/move-contacts/lineage-manipulation.js diff --git a/src/lib/mm-shared.js b/src/lib/move-contacts/mm-shared.js similarity index 97% rename from src/lib/mm-shared.js rename to src/lib/move-contacts/mm-shared.js index 977c52957..37f6f7fc7 100644 --- a/src/lib/mm-shared.js +++ b/src/lib/move-contacts/mm-shared.js @@ -1,9 +1,9 @@ const _ = require('lodash'); const path = require('path'); -const userPrompt = require('./user-prompt'); -const fs = require('./sync-fs'); -const { warn, trace } = require('./log'); +const userPrompt = require('../user-prompt'); +const fs = require('../sync-fs'); +const { warn, trace } = require('../log'); const lineageManipulation = require('./lineage-manipulation'); const HIERARCHY_ROOT = 'root'; diff --git a/src/lib/move-contacts-lib.js b/src/lib/move-contacts/move-contacts-lib.js similarity index 96% rename from src/lib/move-contacts-lib.js rename to src/lib/move-contacts/move-contacts-lib.js index d54b6484d..5391d3459 100644 --- a/src/lib/move-contacts-lib.js +++ b/src/lib/move-contacts/move-contacts-lib.js @@ -1,8 +1,8 @@ -const lineageManipulation = require('../lib/lineage-manipulation'); -const lineageConstraints = require('../lib/lineage-constraints'); -const { trace, info } = require('../lib/log'); +const lineageManipulation = require('./lineage-manipulation'); +const lineageConstraints = require('./lineage-constraints'); +const { trace, info } = require('../log'); -const Shared = require('../lib/mm-shared'); +const Shared = require('./mm-shared'); module.exports = (options) => { const move = async (sourceIds, destinationId, db) => { diff --git a/test/lib/lineage-constraints.spec.js b/test/lib/move-contacts/lineage-constraints.spec.js similarity index 97% rename from test/lib/lineage-constraints.spec.js rename to test/lib/move-contacts/lineage-constraints.spec.js index bcf574f12..52e612c61 100644 --- a/test/lib/lineage-constraints.spec.js +++ b/test/lib/move-contacts/lineage-constraints.spec.js @@ -4,10 +4,10 @@ const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const { mockHierarchy } = require('../mock-hierarchies'); +const { mockHierarchy } = require('../../mock-hierarchies'); -const lineageConstraints = rewire('../../src/lib/lineage-constraints'); -const log = require('../../src/lib/log'); +const lineageConstraints = rewire('../../../src/lib/move-contacts/lineage-constraints'); +const log = require('../../../src/lib/log'); log.level = log.LEVEL_INFO; describe('lineage constriants', () => { diff --git a/test/lib/lineage-manipulation.spec.js b/test/lib/move-contacts/lineage-manipulation.spec.js similarity index 95% rename from test/lib/lineage-manipulation.spec.js rename to test/lib/move-contacts/lineage-manipulation.spec.js index 1e8947a6c..1a4fc467a 100644 --- a/test/lib/lineage-manipulation.spec.js +++ b/test/lib/move-contacts/lineage-manipulation.spec.js @@ -1,9 +1,9 @@ const { expect } = require('chai'); -const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../src/lib/lineage-manipulation'); -const log = require('../../src/lib/log'); +const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../../src/lib/move-contacts/lineage-manipulation'); +const log = require('../../../src/lib/log'); log.level = log.LEVEL_TRACE; -const { parentsToLineage } = require('../mock-hierarchies'); +const { parentsToLineage } = require('../../mock-hierarchies'); describe('lineage manipulation', () => { describe('replaceLineageAfter', () => { diff --git a/test/fn/merge-contacts.spec.js b/test/lib/move-contacts/merge-contacts.spec.js similarity index 97% rename from test/fn/merge-contacts.spec.js rename to test/lib/move-contacts/merge-contacts.spec.js index df4e5e19f..2d17eddcf 100644 --- a/test/fn/merge-contacts.spec.js +++ b/test/lib/move-contacts/merge-contacts.spec.js @@ -3,7 +3,7 @@ const chai = require('chai'); const chaiAsPromised = require('chai-as-promised'); const rewire = require('rewire'); -const Shared = rewire('../../src/lib/mm-shared'); +const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); chai.use(chaiAsPromised); const { expect } = chai; @@ -12,12 +12,12 @@ const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const MoveContactsLib = rewire('../../src/lib/move-contacts-lib'); +const MoveContactsLib = rewire('../../../src/lib/move-contacts/move-contacts-lib'); MoveContactsLib.__set__('Shared', Shared); const move = MoveContactsLib({ merge: true }).move; -const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); +const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); const contacts_by_depth = { // eslint-disable-next-line quotes diff --git a/test/fn/mm-shared.spec.js b/test/lib/move-contacts/mm-shared.spec.js similarity index 85% rename from test/fn/mm-shared.spec.js rename to test/lib/move-contacts/mm-shared.spec.js index 8902613cd..30cb03b61 100644 --- a/test/fn/mm-shared.spec.js +++ b/test/lib/move-contacts/mm-shared.spec.js @@ -2,10 +2,10 @@ const { assert } = require('chai'); const rewire = require('rewire'); const sinon = require('sinon'); -const environment = require('../../src/lib/environment'); -const fs = require('../../src/lib/sync-fs'); -const Shared = rewire('../../src/lib/mm-shared'); -const userPrompt = rewire('../../src/lib/user-prompt'); +const environment = require('../../../src/lib/environment'); +const fs = require('../../../src/lib/sync-fs'); +const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); +const userPrompt = rewire('../../../src/lib/user-prompt'); describe('mm-shared', () => { diff --git a/test/fn/move-contacts.spec.js b/test/lib/move-contacts/move-contacts.spec.js similarity index 99% rename from test/fn/move-contacts.spec.js rename to test/lib/move-contacts/move-contacts.spec.js index 66847ee70..a15a4170a 100644 --- a/test/fn/move-contacts.spec.js +++ b/test/lib/move-contacts/move-contacts.spec.js @@ -2,14 +2,14 @@ const { assert, expect } = require('chai'); const rewire = require('rewire'); const sinon = require('sinon'); -const { mockReport, mockHierarchy, parentsToLineage } = require('../mock-hierarchies'); -const Shared = rewire('../../src/lib/mm-shared'); +const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); +const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const MoveContactsLib = rewire('../../src/lib/move-contacts-lib'); +const MoveContactsLib = rewire('../../../src/lib/move-contacts/move-contacts-lib'); MoveContactsLib.__set__('Shared', Shared); const move = MoveContactsLib({ merge: false }).move; From 25ad23087615fe18c7194ada770f767d3aff214b Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 16:30:11 -0700 Subject: [PATCH 09/21] Lineage Constraints --- src/lib/move-contacts/lineage-constraints.js | 181 +++++++++++------- src/lib/move-contacts/move-contacts-lib.js | 35 +--- .../move-contacts/lineage-constraints.spec.js | 64 ++++--- test/lib/move-contacts/merge-contacts.spec.js | 2 +- 4 files changed, 148 insertions(+), 134 deletions(-) diff --git a/src/lib/move-contacts/lineage-constraints.js b/src/lib/move-contacts/lineage-constraints.js index ff7b0992f..91d2845d1 100644 --- a/src/lib/move-contacts/lineage-constraints.js +++ b/src/lib/move-contacts/lineage-constraints.js @@ -1,45 +1,44 @@ const log = require('../log'); const { trace } = log; -const { pluckIdsFromLineage } = require('./lineage-manipulation'); +const lineageManipulation = require('./lineage-manipulation'); -const lineageConstraints = async (repository, parentDoc, options) => { - let mapTypeToAllowedParents; - try { - const { settings } = await repository.get('settings'); - const { contact_types } = settings; +module.exports = async (db, options = {}) => { + const mapTypeToAllowedParents = await fetchAllowedParents(db); - if (Array.isArray(contact_types)) { - trace('Found app_settings.contact_types. Configurable hierarchy constraints will be enforced.'); - mapTypeToAllowedParents = contact_types - .filter(rule => rule) - .reduce((agg, curr) => Object.assign(agg, { [curr.id]: curr.parents }), {}); + const getHierarchyErrors = (sourceDoc, destinationDoc) => { + if (options.merge) { + return getMergeViolations(sourceDoc, destinationDoc); } - } catch (err) { - if (err.name !== 'not_found') { - throw err; - } - } - if (!mapTypeToAllowedParents) { - trace('Default hierarchy constraints will be enforced.'); - mapTypeToAllowedParents = { - district_hospital: [], - health_center: ['district_hospital'], - clinic: ['health_center'], - person: ['district_hospital', 'health_center', 'clinic'], - }; - } + return getMovingViolations(mapTypeToAllowedParents, sourceDoc, destinationDoc); + }; return { - getPrimaryContactViolations: (contactDoc, descendantDocs) => getPrimaryContactViolations(repository, contactDoc, parentDoc, descendantDocs), - validate: (contactDoc) => { - if (options.merge) { - return getMergeContactHierarchyViolations(contactDoc, parentDoc); - } - - return getMoveContactHierarchyViolations(mapTypeToAllowedParents, contactDoc, parentDoc); - }, + getPrimaryContactViolations: (sourceDoc, destinationDoc, descendantDocs) => getPrimaryContactViolations(db, sourceDoc, destinationDoc, descendantDocs), + getHierarchyErrors, + assertHierarchyErrors: (sourceDocs, destinationDoc) => { + sourceDocs.forEach(sourceDoc => { + const hierarchyError = getHierarchyErrors(sourceDoc, destinationDoc); + if (hierarchyError) { + throw Error(`Hierarchy Constraints: ${hierarchyError}`); + } + }); + + /* + It is nice that the tool can move lists of contacts as one operation, but strange things happen when two contactIds are in the same lineage. + For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. + */ + const contactIds = sourceDocs.map(doc => doc._id); + sourceDocs + .forEach(doc => { + const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; + const violatingParentId = parentIdsOfDoc.find(parentId => contactIds.includes(parentId)); + if (violatingParentId) { + throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); + } + }); + } }; }; @@ -47,51 +46,62 @@ const lineageConstraints = async (repository, parentDoc, options) => { Enforce the list of allowed parents for each contact type Ensure we are not creating a circular hierarchy */ -const getMoveContactHierarchyViolations = (mapTypeToAllowedParents, contactDoc, parentDoc) => { - // TODO reuse this code - const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); - const contactType = getContactType(contactDoc); - const parentType = getContactType(parentDoc); - if (!contactType) return 'contact required attribute "type" is undefined'; - if (parentDoc && !parentType) return `parent contact "${parentDoc._id}" required attribute "type" is undefined`; - if (!mapTypeToAllowedParents) return 'hierarchy constraints are undefined'; - - const rulesForContact = mapTypeToAllowedParents[contactType]; - if (!rulesForContact) return `cannot move contact with unknown type '${contactType}'`; - - const isPermittedMoveToRoot = !parentDoc && rulesForContact.length === 0; - if (!isPermittedMoveToRoot && !rulesForContact.includes(parentType)) return `contacts of type '${contactType}' cannot have parent of type '${parentType}'`; - - if (parentDoc && contactDoc._id) { - const parentAncestry = [parentDoc._id, ...pluckIdsFromLineage(parentDoc.parent)]; - if (parentAncestry.includes(contactDoc._id)) { - return `Circular hierarchy: Cannot set parent of contact '${contactDoc._id}' as it would create a circular hierarchy.`; +const getMovingViolations = (mapTypeToAllowedParents, sourceDoc, destinationDoc) => { + const commonViolations = getCommonViolations(sourceDoc, destinationDoc); + if (commonViolations) { + return commonViolations; + } + + if (!mapTypeToAllowedParents) { + return 'hierarchy constraints are undefined'; + } + + const sourceContactType = getContactType(sourceDoc); + const destinationType = getContactType(destinationDoc); + const rulesForContact = mapTypeToAllowedParents[sourceContactType]; + if (!rulesForContact) { + return `cannot move contact with unknown type '${sourceContactType}'`; + } + + const isPermittedMoveToRoot = !destinationDoc && rulesForContact.length === 0; + if (!isPermittedMoveToRoot && !rulesForContact.includes(destinationType)) { + return `contacts of type '${sourceContactType}' cannot have parent of type '${destinationType}'`; + } + + if (destinationDoc && sourceDoc._id) { + const parentAncestry = [destinationDoc._id, ...lineageManipulation.pluckIdsFromLineage(destinationDoc.parent)]; + if (parentAncestry.includes(sourceDoc._id)) { + return `Circular hierarchy: Cannot set parent of contact '${sourceDoc._id}' as it would create a circular hierarchy.`; } } }; -/* -Enforce the list of allowed parents for each contact type -Ensure we are not creating a circular hierarchy -*/ -const getMergeContactHierarchyViolations = (removedDoc, keptDoc) => { - const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); - const removedContactType = getContactType(removedDoc); - const keptContactType = getContactType(keptDoc); - if (!removedContactType) { - return 'contact required attribute "type" is undefined'; +const getCommonViolations = (sourceDoc, destinationDoc) => { + const sourceContactType = getContactType(sourceDoc); + const destinationContactType = getContactType(destinationDoc); + if (!sourceContactType) { + return `source contact "${sourceDoc._id}" required attribute "type" is undefined`; } - if (keptDoc && !keptContactType) { - return `kept contact "${keptDoc._id}" required attribute "type" is undefined`; + if (destinationDoc && !destinationContactType) { + return `destination contact "${destinationDoc._id}" required attribute "type" is undefined`; + } +}; + +const getMergeViolations = (sourceDoc, destinationDoc) => { + const commonViolations = getCommonViolations(sourceDoc, destinationDoc); + if (commonViolations) { + return commonViolations; } - if (removedContactType !== keptContactType) { - return `contact "${removedDoc._id}" must have same contact type as "${keptContactType}". Former is "${removedContactType}" while later is "${keptContactType}".`; + const sourceContactType = getContactType(sourceDoc); + const destinationContactType = getContactType(destinationDoc); + if (sourceContactType !== destinationContactType) { + return `source and destinations must have the same type. Source is "${sourceContactType}" while destination is "${destinationContactType}".`; } - if (removedDoc._id === keptDoc._id) { - return `Cannot merge contact with self`; + if (sourceDoc._id === destinationDoc._id) { + return `cannot move contact to destination that is itself`; } }; @@ -101,8 +111,8 @@ 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 (repository, contactDoc, parentDoc, descendantDocs) => { - const safeGetLineageFromDoc = doc => doc ? pluckIdsFromLineage(doc.parent) : []; +const getPrimaryContactViolations = async (db, contactDoc, parentDoc, descendantDocs) => { + const safeGetLineageFromDoc = doc => doc ? lineageManipulation.pluckIdsFromLineage(doc.parent) : []; const contactsLineageIds = safeGetLineageFromDoc(contactDoc); const parentsLineageIds = safeGetLineageFromDoc(parentDoc); @@ -111,7 +121,7 @@ const getPrimaryContactViolations = async (repository, contactDoc, parentDoc, de } const docIdsRemovedFromContactLineage = contactsLineageIds.filter(value => !parentsLineageIds.includes(value)); - const docsRemovedFromContactLineage = await repository.allDocs({ + const docsRemovedFromContactLineage = await db.allDocs({ keys: docIdsRemovedFromContactLineage, include_docs: true, }); @@ -123,4 +133,31 @@ const getPrimaryContactViolations = async (repository, contactDoc, parentDoc, de return descendantDocs.find(descendant => primaryContactIds.some(primaryId => descendant._id === primaryId)); }; -module.exports = lineageConstraints; +const getContactType = doc => doc && (doc.type === 'contact' ? doc.contact_type : doc.type); + +async function fetchAllowedParents(db) { + try { + const { settings } = await db.get('settings'); + const { contact_types } = settings; + + if (Array.isArray(contact_types)) { + trace('Found app_settings.contact_types. Configurable hierarchy constraints will be enforced.'); + return contact_types + .filter(rule => rule) + .reduce((agg, curr) => Object.assign(agg, { [curr.id]: curr.parents }), {}); + } + } catch (err) { + if (err.name !== 'not_found') { + throw err; + } + } + + trace('Default hierarchy constraints will be enforced.'); + return { + district_hospital: [], + health_center: ['district_hospital'], + clinic: ['health_center'], + person: ['district_hospital', 'health_center', 'clinic'], + }; +} + diff --git a/src/lib/move-contacts/move-contacts-lib.js b/src/lib/move-contacts/move-contacts-lib.js index 5391d3459..00c0021a3 100644 --- a/src/lib/move-contacts/move-contacts-lib.js +++ b/src/lib/move-contacts/move-contacts-lib.js @@ -1,5 +1,5 @@ const lineageManipulation = require('./lineage-manipulation'); -const lineageConstraints = require('./lineage-constraints'); +const LineageConstraints = require('./lineage-constraints'); const { trace, info } = require('../log'); const Shared = require('./mm-shared'); @@ -8,11 +8,10 @@ module.exports = (options) => { const move = async (sourceIds, destinationId, db) => { Shared.prepareDocumentDirectory(options); trace(`Fetching contact details: ${destinationId}`); + const constraints = await LineageConstraints(db, options); const destinationDoc = await Shared.fetch.contact(db, destinationId); - - const constraints = await lineageConstraints(db, destinationDoc, options); const sourceDocs = await Shared.fetch.contactList(db, sourceIds); - await validateContacts(sourceDocs, constraints); + await constraints.assertHierarchyErrors(Object.values(sourceDocs), destinationDoc); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(destinationDoc); @@ -31,7 +30,7 @@ module.exports = (options) => { const prettyPrintDocument = doc => `'${doc.name}' (${doc._id})`; // Check that primary contact is not removed from areas where they are required - const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(sourceDoc, descendantsAndSelf); + const invalidPrimaryContactDoc = await constraints.getPrimaryContactViolations(sourceDoc, destinationDoc, descendantsAndSelf); if (invalidPrimaryContactDoc) { throw Error(`Cannot remove contact ${prettyPrintDocument(invalidPrimaryContactDoc)} from the hierarchy for which they are a primary contact.`); } @@ -57,32 +56,6 @@ module.exports = (options) => { info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); }; - /* - Checks for any errors which this will create in the hierarchy (hierarchy schema, circular hierarchies) - Confirms the list of contacts are possible to move - */ - const validateContacts = async (sourceDocs, constraints) => { - Object.values(sourceDocs).forEach(doc => { - const hierarchyError = constraints.validate(doc); - if (hierarchyError) { - throw Error(`Hierarchy Constraints: ${hierarchyError}`); - } - }); - - /* - It is nice that the tool can move lists of contacts as one operation, but strange things happen when two contactIds are in the same lineage. - For example, moving a district_hospital and moving a contact under that district_hospital to a new clinic causes multiple colliding writes to the same json file. - */ - const contactIds = Object.keys(sourceDocs); - Object.values(sourceDocs) - .forEach(doc => { - const parentIdsOfDoc = (doc.parent && lineageManipulation.pluckIdsFromLineage(doc.parent)) || []; - const violatingParentId = parentIdsOfDoc.find(parentId => contactIds.includes(parentId)); - if (violatingParentId) { - throw Error(`Unable to move two documents from the same lineage: '${doc._id}' and '${violatingParentId}'`); - } - }); - }; const moveReports = async (db, descendantsAndSelf, replacementLineage, sourceId, destinationId) => { const descendantIds = descendantsAndSelf.map(contact => contact._id); diff --git a/test/lib/move-contacts/lineage-constraints.spec.js b/test/lib/move-contacts/lineage-constraints.spec.js index 52e612c61..0e2d6d0ce 100644 --- a/test/lib/move-contacts/lineage-constraints.spec.js +++ b/test/lib/move-contacts/lineage-constraints.spec.js @@ -11,19 +11,13 @@ const log = require('../../../src/lib/log'); log.level = log.LEVEL_INFO; describe('lineage constriants', () => { - describe('getMoveContactHierarchyViolations', () => { - const scenario = async (contact_types, contactType, parentType) => { - const mockDb = { get: () => ({ settings: { contact_types } }) }; - const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: parentType }); - return getMoveContactHierarchyViolations({ type: contactType }); - }; + describe('getHierarchyErrors', () => { + it('empty rules yields error', async () => expect(await runScenario([], 'person', 'health_center')).to.include('unknown type')); - it('empty rules yields error', async () => expect(await scenario([], 'person', 'health_center')).to.include('unknown type')); - - it('no valid parent yields error', async () => expect(await scenario([undefined], 'person', 'health_center')).to.include('unknown type')); + it('no valid parent yields error', async () => expect(await runScenario([undefined], 'person', 'health_center')).to.include('unknown type')); it('valid parent yields no error', async () => { - const actual = await scenario([{ + const actual = await runScenario([{ id: 'person', parents: ['health_center'], }], 'person', 'health_center'); @@ -31,52 +25,56 @@ describe('lineage constriants', () => { expect(actual).to.be.undefined; }); - it('no contact type yields undefined error', async () => expect(await scenario([])).to.include('undefined')); + it('no contact type yields undefined error', async () => expect(await runScenario([])).to.include('undefined')); - it('no parent type yields undefined error', async () => expect(await scenario([], 'person')).to.include('undefined')); + it('no parent type yields undefined error', async () => expect(await runScenario([], 'person')).to.include('undefined')); - it('no valid parents yields not defined', async () => expect(await scenario([{ + it('no valid parents yields not defined', async () => expect(await runScenario([{ id: 'person', parents: ['district_hospital'], }], 'person', 'health_center')).to.include('cannot have parent of type')); it('no settings doc requires valid parent type', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'dne' }); - const actual = getMoveContactHierarchyViolations({ type: 'person' }); + const { getHierarchyErrors } = await lineageConstraints(mockDb); + const actual = getHierarchyErrors({ type: 'person' }, { type: 'dne' }); expect(actual).to.include('cannot have parent of type'); }); it('no settings doc requires valid contact type', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'clinic' }); - const actual = getMoveContactHierarchyViolations({ type: 'dne' }); + const { getHierarchyErrors } = await lineageConstraints(mockDb); + const actual = getHierarchyErrors({ type: 'dne' }, { type: 'clinic' }); expect(actual).to.include('unknown type'); }); it('no settings doc yields not defined', async () => { const mockDb = { get: () => { throw { name: 'not_found' }; } }; - const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, { type: 'clinic' }); - const actual = getMoveContactHierarchyViolations({ type: 'person' }); + const { getHierarchyErrors } = await lineageConstraints(mockDb); + const actual = getHierarchyErrors({ type: 'person' }, { type: 'clinic' }); expect(actual).to.be.undefined; }); + it('cannot merge with self', async () => { + expect(await runScenario([], 'a', 'a', true)).to.include('self'); + }); + describe('default schema', () => { - it('no defined rules enforces defaults schema', async () => expect(await scenario(undefined, 'district_hospital', 'health_center')).to.include('cannot have parent')); + it('no defined rules enforces defaults schema', async () => expect(await runScenario(undefined, 'district_hospital', 'health_center')).to.include('cannot have parent')); - it('nominal case', async () => expect(await scenario(undefined, 'person', 'health_center')).to.be.undefined); + it('nominal case', async () => expect(await runScenario(undefined, 'person', 'health_center')).to.be.undefined); it('can move district_hospital to root', async () => { const mockDb = { get: () => ({ settings: { } }) }; - const { getMoveContactHierarchyViolations } = await lineageConstraints(mockDb, undefined); - const actual = getMoveContactHierarchyViolations({ type: 'district_hospital' }); + const { getHierarchyErrors } = await lineageConstraints(mockDb); + const actual = getHierarchyErrors({ type: 'district_hospital' }, undefined); expect(actual).to.be.undefined; }); }); }); describe('getPrimaryContactViolations', () => { - const getMoveContactHierarchyViolations = lineageConstraints.__get__('getPrimaryContactViolations'); + const getHierarchyErrors = lineageConstraints.__get__('getPrimaryContactViolations'); describe('on memory pouchdb', async () => { let pouchDb, scenarioCount = 0; @@ -106,13 +104,13 @@ describe('lineage constriants', () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_2'); - const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); }); it('cannot move clinic_1_contact to root', async () => { const contactDoc = await pouchDb.get('clinic_1_contact'); - const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, undefined, [contactDoc]); + const doc = await getHierarchyErrors(pouchDb, contactDoc, undefined, [contactDoc]); expect(doc).to.deep.include({ _id: 'clinic_1_contact' }); }); @@ -120,7 +118,7 @@ describe('lineage constriants', () => { const contactDoc = await pouchDb.get('clinic_1_contact'); const parentDoc = await pouchDb.get('clinic_1'); - const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); @@ -129,7 +127,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 getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, descendants); + const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); expect(doc).to.be.undefined; }); @@ -142,7 +140,7 @@ 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 getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, descendants); + const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, descendants); expect(doc).to.deep.include({ _id: 'patient_1' }); }); @@ -153,9 +151,15 @@ describe('lineage constriants', () => { contactDoc.parent._id = 'dne'; - const doc = await getMoveContactHierarchyViolations(pouchDb, contactDoc, parentDoc, [contactDoc]); + const doc = await getHierarchyErrors(pouchDb, contactDoc, parentDoc, [contactDoc]); expect(doc).to.be.undefined; }); }); }); }); + +const runScenario = async (contact_types, sourceType, destinationType, merge = false) => { + const mockDb = { get: () => ({ settings: { contact_types } }) }; + const { getHierarchyErrors } = await lineageConstraints(mockDb, { merge }); + return getHierarchyErrors({ type: sourceType }, { type: destinationType }); +}; diff --git a/test/lib/move-contacts/merge-contacts.spec.js b/test/lib/move-contacts/merge-contacts.spec.js index 2d17eddcf..b4a7a74a5 100644 --- a/test/lib/move-contacts/merge-contacts.spec.js +++ b/test/lib/move-contacts/merge-contacts.spec.js @@ -223,6 +223,6 @@ describe('merge-contacts', () => { it('throw if removed is kept', async () => { const actual = move(['district_1', 'district_2'], 'district_2', pouchDb); - await expect(actual).to.eventually.rejectedWith('merge contact with self'); + await expect(actual).to.eventually.rejectedWith('that is itself'); }); }); From 5ad9d854d2ac38f49000beebf8862e06f029450d Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 17:20:32 -0700 Subject: [PATCH 10/21] Rename to Hierarchy Operations --- src/fn/merge-contacts.js | 4 ++-- src/fn/move-contacts.js | 4 ++-- .../index.js} | 13 ++++++++++++- .../lineage-constraints.js | 0 .../lineage-manipulation.js | 0 .../mm-shared.js | 12 ------------ .../lineage-constraints.spec.js | 2 +- .../lineage-manipulation.spec.js | 2 +- .../merge-contacts.spec.js | 8 ++++---- .../mm-shared.spec.js | 2 +- .../move-contacts.spec.js | 8 ++++---- 11 files changed, 27 insertions(+), 28 deletions(-) rename src/lib/{move-contacts/move-contacts-lib.js => hierarchy-operations/index.js} (91%) rename src/lib/{move-contacts => hierarchy-operations}/lineage-constraints.js (100%) rename src/lib/{move-contacts => hierarchy-operations}/lineage-manipulation.js (100%) rename src/lib/{move-contacts => hierarchy-operations}/mm-shared.js (89%) rename test/lib/{move-contacts => hierarchy-operations}/lineage-constraints.spec.js (98%) rename test/lib/{move-contacts => hierarchy-operations}/lineage-manipulation.spec.js (98%) rename test/lib/{move-contacts => hierarchy-operations}/merge-contacts.spec.js (96%) rename test/lib/{move-contacts => hierarchy-operations}/mm-shared.spec.js (95%) rename test/lib/{move-contacts => hierarchy-operations}/move-contacts.spec.js (98%) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 13c3e9a19..88575bf69 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -5,7 +5,7 @@ const environment = require('../lib/environment'); const pouch = require('../lib/db'); const { info } = require('../lib/log'); -const MoveContactsLib = require('../lib/move-contacts/move-contacts-lib'); +const HierarchyOperations = require('../lib/hierarchy-operations'); module.exports = { requiresInstance: true, @@ -17,7 +17,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return MoveContactsLib(options).move(args.removeIds, args.keepId, db); + return HierarchyOperations(options).move(args.removeIds, args.keepId, db); } }; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 7e1e68a56..2d97b1137 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -5,7 +5,7 @@ const environment = require('../lib/environment'); const pouch = require('../lib/db'); const { info } = require('../lib/log'); -const MoveContactsLib = require('../lib/move-contacts/move-contacts-lib'); +const HierarchyOperations = require('../lib/hierarchy-operations'); module.exports = { requiresInstance: true, @@ -17,7 +17,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return MoveContactsLib(options).move(args.contactIds, args.parentId, db); + return HierarchyOperations(options).move(args.contactIds, args.parentId, db); } }; diff --git a/src/lib/move-contacts/move-contacts-lib.js b/src/lib/hierarchy-operations/index.js similarity index 91% rename from src/lib/move-contacts/move-contacts-lib.js rename to src/lib/hierarchy-operations/index.js index 00c0021a3..c6c7dca15 100644 --- a/src/lib/move-contacts/move-contacts-lib.js +++ b/src/lib/hierarchy-operations/index.js @@ -40,7 +40,7 @@ module.exports = (options) => { const ancestors = await Shared.fetch.ancestorsOf(db, sourceDoc); trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(sourceDoc)}.`); - const updatedAncestors = Shared.replaceLineageInAncestors(descendantsAndSelf, ancestors); + const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors]); @@ -117,6 +117,17 @@ module.exports = (options) => { return agg; }, []); + const replaceLineageInAncestors = (descendantsAndSelf, ancestors) => ancestors.reduce((agg, ancestor) => { + let result = agg; + const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); + if (primaryContact) { + ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); + result = [ancestor, ...result]; + } + + return result; + }, []); + const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, destinationId) => descendantsAndSelf.reduce((agg, doc) => { const startingFromIdInLineage = options.merge ? destinationId : doc._id === destinationId ? undefined : destinationId; diff --git a/src/lib/move-contacts/lineage-constraints.js b/src/lib/hierarchy-operations/lineage-constraints.js similarity index 100% rename from src/lib/move-contacts/lineage-constraints.js rename to src/lib/hierarchy-operations/lineage-constraints.js diff --git a/src/lib/move-contacts/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js similarity index 100% rename from src/lib/move-contacts/lineage-manipulation.js rename to src/lib/hierarchy-operations/lineage-manipulation.js diff --git a/src/lib/move-contacts/mm-shared.js b/src/lib/hierarchy-operations/mm-shared.js similarity index 89% rename from src/lib/move-contacts/mm-shared.js rename to src/lib/hierarchy-operations/mm-shared.js index 37f6f7fc7..441d57369 100644 --- a/src/lib/move-contacts/mm-shared.js +++ b/src/lib/hierarchy-operations/mm-shared.js @@ -32,17 +32,6 @@ const writeDocumentToDisk = ({ docDirectoryPath }, doc) => { fs.writeJson(destinationPath, doc); }; -const replaceLineageInAncestors = (descendantsAndSelf, ancestors) => ancestors.reduce((agg, ancestor) => { - let result = agg; - const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); - if (primaryContact) { - ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); - result = [ancestor, ...result]; - } - - return result; -}, []); - const fetch = { /* @@ -135,7 +124,6 @@ module.exports = { HIERARCHY_ROOT, BATCH_SIZE, prepareDocumentDirectory, - replaceLineageInAncestors, writeDocumentToDisk, fetch, }; diff --git a/test/lib/move-contacts/lineage-constraints.spec.js b/test/lib/hierarchy-operations/lineage-constraints.spec.js similarity index 98% rename from test/lib/move-contacts/lineage-constraints.spec.js rename to test/lib/hierarchy-operations/lineage-constraints.spec.js index 0e2d6d0ce..d4812d115 100644 --- a/test/lib/move-contacts/lineage-constraints.spec.js +++ b/test/lib/hierarchy-operations/lineage-constraints.spec.js @@ -6,7 +6,7 @@ PouchDB.plugin(require('pouchdb-mapreduce')); const { mockHierarchy } = require('../../mock-hierarchies'); -const lineageConstraints = rewire('../../../src/lib/move-contacts/lineage-constraints'); +const lineageConstraints = rewire('../../../src/lib/hierarchy-operations/lineage-constraints'); const log = require('../../../src/lib/log'); log.level = log.LEVEL_INFO; diff --git a/test/lib/move-contacts/lineage-manipulation.spec.js b/test/lib/hierarchy-operations/lineage-manipulation.spec.js similarity index 98% rename from test/lib/move-contacts/lineage-manipulation.spec.js rename to test/lib/hierarchy-operations/lineage-manipulation.spec.js index 1a4fc467a..f73bf3e4e 100644 --- a/test/lib/move-contacts/lineage-manipulation.spec.js +++ b/test/lib/hierarchy-operations/lineage-manipulation.spec.js @@ -1,5 +1,5 @@ const { expect } = require('chai'); -const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../../src/lib/move-contacts/lineage-manipulation'); +const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../../src/lib/hierarchy-operations/lineage-manipulation'); const log = require('../../../src/lib/log'); log.level = log.LEVEL_TRACE; diff --git a/test/lib/move-contacts/merge-contacts.spec.js b/test/lib/hierarchy-operations/merge-contacts.spec.js similarity index 96% rename from test/lib/move-contacts/merge-contacts.spec.js rename to test/lib/hierarchy-operations/merge-contacts.spec.js index b4a7a74a5..120b6b602 100644 --- a/test/lib/move-contacts/merge-contacts.spec.js +++ b/test/lib/hierarchy-operations/merge-contacts.spec.js @@ -3,7 +3,7 @@ const chai = require('chai'); const chaiAsPromised = require('chai-as-promised'); const rewire = require('rewire'); -const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); +const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); chai.use(chaiAsPromised); const { expect } = chai; @@ -12,10 +12,10 @@ const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const MoveContactsLib = rewire('../../../src/lib/move-contacts/move-contacts-lib'); -MoveContactsLib.__set__('Shared', Shared); +const HierarchyOperations = rewire('../../../src/lib/hierarchy-operations/index.js'); +HierarchyOperations.__set__('Shared', Shared); -const move = MoveContactsLib({ merge: true }).move; +const move = HierarchyOperations({ merge: true }).move; const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); diff --git a/test/lib/move-contacts/mm-shared.spec.js b/test/lib/hierarchy-operations/mm-shared.spec.js similarity index 95% rename from test/lib/move-contacts/mm-shared.spec.js rename to test/lib/hierarchy-operations/mm-shared.spec.js index 30cb03b61..1d686ef17 100644 --- a/test/lib/move-contacts/mm-shared.spec.js +++ b/test/lib/hierarchy-operations/mm-shared.spec.js @@ -4,7 +4,7 @@ const sinon = require('sinon'); const environment = require('../../../src/lib/environment'); const fs = require('../../../src/lib/sync-fs'); -const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); +const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); const userPrompt = rewire('../../../src/lib/user-prompt'); diff --git a/test/lib/move-contacts/move-contacts.spec.js b/test/lib/hierarchy-operations/move-contacts.spec.js similarity index 98% rename from test/lib/move-contacts/move-contacts.spec.js rename to test/lib/hierarchy-operations/move-contacts.spec.js index a15a4170a..4788ce316 100644 --- a/test/lib/move-contacts/move-contacts.spec.js +++ b/test/lib/hierarchy-operations/move-contacts.spec.js @@ -3,16 +3,16 @@ const rewire = require('rewire'); const sinon = require('sinon'); const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); -const Shared = rewire('../../../src/lib/move-contacts/mm-shared'); +const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); const PouchDB = require('pouchdb-core'); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const MoveContactsLib = rewire('../../../src/lib/move-contacts/move-contacts-lib'); -MoveContactsLib.__set__('Shared', Shared); +const HierarchyOperations = rewire('../../../src/lib/hierarchy-operations/index.js'); +HierarchyOperations.__set__('Shared', Shared); -const move = MoveContactsLib({ merge: false }).move; +const move = HierarchyOperations({ merge: false }).move; const contacts_by_depth = { // eslint-disable-next-line quotes From 7ea3393f3e1890e6928a98f4e1a39001fae78077 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Thu, 21 Nov 2024 17:48:09 -0700 Subject: [PATCH 11/21] replaceRelevantLineage --- src/lib/hierarchy-operations/index.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index c6c7dca15..7873ca2bf 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -109,9 +109,16 @@ module.exports = (options) => { }); }; + const replaceRelevantLineage = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { + if (options?.merge) { + return lineageManipulation.replaceLineageAt(doc, lineageAttributeName, replaceWith, startingFromIdInLineage); + } + + return lineageManipulation.replaceLineageAfter(doc, lineageAttributeName, replaceWith, startingFromIdInLineage); + }; + const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - const operation = options.merge ? lineageManipulation.replaceLineageAt : lineageManipulation.replaceLineageAfter; - if (operation(doc, 'contact', replaceWith, startingFromIdInLineage)) { + if (replaceRelevantLineage(doc, 'contact', replaceWith, startingFromIdInLineage)) { agg.push(doc); } return agg; @@ -139,9 +146,8 @@ module.exports = (options) => { } } - const lineageOperation = options.merge ? lineageManipulation.replaceLineageAt : lineageManipulation.replaceLineageAfter; - const parentWasUpdated = lineageOperation(doc, 'parent', replacementLineage, startingFromIdInLineage); - const contactWasUpdated = lineageOperation(doc, 'contact', replacementLineage, destinationId); + const parentWasUpdated = replaceRelevantLineage(doc, 'parent', replacementLineage, startingFromIdInLineage); + const contactWasUpdated = replaceRelevantLineage(doc, 'contact', replacementLineage, destinationId); if (parentWasUpdated || contactWasUpdated) { agg.push(doc); } From 78f2c0178bd7afbb038d0818939f531863112303 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 18:30:44 -0700 Subject: [PATCH 12/21] Refacatoring for lineage-manipulation --- src/lib/hierarchy-operations/index.js | 14 +---- .../lineage-manipulation.js | 63 +++++++++---------- .../lineage-manipulation.spec.js | 39 +++++++++--- 3 files changed, 64 insertions(+), 52 deletions(-) diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index 7873ca2bf..4a382236e 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -109,16 +109,8 @@ module.exports = (options) => { }); }; - const replaceRelevantLineage = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { - if (options?.merge) { - return lineageManipulation.replaceLineageAt(doc, lineageAttributeName, replaceWith, startingFromIdInLineage); - } - - return lineageManipulation.replaceLineageAfter(doc, lineageAttributeName, replaceWith, startingFromIdInLineage); - }; - const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - if (replaceRelevantLineage(doc, 'contact', replaceWith, startingFromIdInLineage)) { + if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage, options)) { agg.push(doc); } return agg; @@ -146,8 +138,8 @@ module.exports = (options) => { } } - const parentWasUpdated = replaceRelevantLineage(doc, 'parent', replacementLineage, startingFromIdInLineage); - const contactWasUpdated = replaceRelevantLineage(doc, 'contact', replacementLineage, destinationId); + const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); + const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); if (parentWasUpdated || contactWasUpdated) { agg.push(doc); } diff --git a/src/lib/hierarchy-operations/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js index 001c637dc..ebd7df97f 100644 --- a/src/lib/hierarchy-operations/lineage-manipulation.js +++ b/src/lib/hierarchy-operations/lineage-manipulation.js @@ -1,49 +1,49 @@ -/* -Given a doc, replace the lineage information therein with "replaceWith" - -startingFromIdInLineage (optional) - Will result in a partial replacement of the lineage. Only the part of the lineage "after" the parent -with _id=startingFromIdInLineage will be replaced by "replaceWith" -*/ -const replaceLineageAfter = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { +/** + * Given a doc, replace the lineage information therein with "replaceWith" + * + * @param {Object} doc A CouchDB document containing a hierarchy that needs replacing + * @param {string} lineageAttributeName Name of the attribute which is a lineage in doc (contact or parent) + * @param {Object} replaceWith The new hierarchy { parent: { _id: 'parent', parent: { _id: 'grandparent' } } + * @param {string} [startingFromIdInLineage] Only the part of the lineage "after" this id will be replaced + * @param {Object} options + * @param {boolean} merge When true, startingFromIdInLineage is replaced and when false, startingFromIdInLineage's parent is replaced + */ +const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage, options={}) => { // Replace the full lineage if (!startingFromIdInLineage) { - return _doReplaceInLineage(doc, lineageAttributeName, replaceWith); + return replaceWithinLineage(doc, lineageAttributeName, replaceWith); } - // Replace part of a lineage - let currentParent = doc[lineageAttributeName]; - while (currentParent) { - if (currentParent._id === startingFromIdInLineage) { - return _doReplaceInLineage(currentParent, 'parent', replaceWith); + const initialState = () => { + if (options.merge) { + return { + element: doc, + attributeName: lineageAttributeName, + }; } - currentParent = currentParent.parent; - } - return false; -}; - -const replaceLineageAt = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage) => { - if (!replaceWith || !startingFromIdInLineage) { - throw Error('replaceWith and startingFromIdInLineage must be defined'); + return { + element: doc[lineageAttributeName], + attributeName: 'parent', + }; } - // Replace part of a lineage - let currentElement = doc; - let currentAttributeName = lineageAttributeName; - while (currentElement) { - if (currentElement[currentAttributeName]?._id === startingFromIdInLineage) { - return _doReplaceInLineage(currentElement, currentAttributeName, replaceWith); + const state = initialState(); + while (state.element) { + const compare = options.merge ? state.element[state.attributeName] : state.element; + if (compare?._id === startingFromIdInLineage) { + return replaceWithinLineage(state.element, state.attributeName, replaceWith); } - currentElement = currentElement[currentAttributeName]; - currentAttributeName = 'parent'; + state.element = state.element[state.attributeName]; + state.attributeName = 'parent'; } return false; }; -const _doReplaceInLineage = (replaceInDoc, lineageAttributeName, replaceWith) => { +const replaceWithinLineage = (replaceInDoc, lineageAttributeName, replaceWith) => { if (!replaceWith) { const lineageWasDeleted = !!replaceInDoc[lineageAttributeName]; replaceInDoc[lineageAttributeName] = undefined; @@ -123,6 +123,5 @@ module.exports = { createLineageFromDoc, minifyLineagesInDoc, pluckIdsFromLineage, - replaceLineageAfter, - replaceLineageAt, + replaceLineage, }; diff --git a/test/lib/hierarchy-operations/lineage-manipulation.spec.js b/test/lib/hierarchy-operations/lineage-manipulation.spec.js index f73bf3e4e..be324009e 100644 --- a/test/lib/hierarchy-operations/lineage-manipulation.spec.js +++ b/test/lib/hierarchy-operations/lineage-manipulation.spec.js @@ -1,18 +1,19 @@ const { expect } = require('chai'); -const { replaceLineageAfter, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../../src/lib/hierarchy-operations/lineage-manipulation'); +const { replaceLineage, pluckIdsFromLineage, minifyLineagesInDoc } = require('../../../src/lib/hierarchy-operations/lineage-manipulation'); const log = require('../../../src/lib/log'); log.level = log.LEVEL_TRACE; const { parentsToLineage } = require('../../mock-hierarchies'); +const mergeOption = { merge: true }; describe('lineage manipulation', () => { - describe('replaceLineageAfter', () => { + describe('kenn replaceLineage', () => { const mockReport = data => Object.assign({ _id: 'r', type: 'data_record', contact: parentsToLineage('parent', 'grandparent') }, data); const mockContact = data => Object.assign({ _id: 'c', type: 'person', parent: parentsToLineage('parent', 'grandparent') }, data); it('replace with empty lineage', () => { const mock = mockReport(); - expect(replaceLineageAfter(mock, 'contact', undefined)).to.be.true; + expect(replaceLineage(mock, 'contact', undefined)).to.be.true; expect(mock).to.deep.eq({ _id: 'r', type: 'data_record', @@ -22,7 +23,7 @@ describe('lineage manipulation', () => { it('replace full lineage', () => { const mock = mockContact(); - expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -34,7 +35,7 @@ describe('lineage manipulation', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -45,12 +46,12 @@ describe('lineage manipulation', () => { it('replace empty with empty', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineageAfter(mock, 'parent', undefined)).to.be.false; + expect(replaceLineage(mock, 'parent', undefined)).to.be.false; }); it('replace lineage starting at contact', () => { const mock = mockContact(); - expect(replaceLineageAfter(mock, 'parent', parentsToLineage('new_grandparent'), 'parent')).to.be.true; + expect(replaceLineage(mock, 'parent', parentsToLineage('new_grandparent'), 'parent')).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -58,9 +59,29 @@ describe('lineage manipulation', () => { }); }); + it('merge new parent', () => { + const mock = mockContact(); + expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent', 'new_grandparent'), 'parent', mergeOption)).to.be.true; + expect(mock).to.deep.eq({ + _id: 'c', + type: 'person', + parent: parentsToLineage('new_parent', 'new_grandparent'), + }); + }); + + it('merge grandparent of contact', () => { + const mock = mockReport(); + expect(replaceLineage(mock, 'contact', parentsToLineage('new_grandparent'), 'grandparent', mergeOption)).to.be.true; + expect(mock).to.deep.eq({ + _id: 'r', + type: 'data_record', + contact: parentsToLineage('parent', 'new_grandparent'), + }); + }); + it('replace empty starting at contact', () => { const mock = mockContact(); - expect(replaceLineageAfter(mock, 'parent', undefined, 'parent')).to.be.true; + expect(replaceLineage(mock, 'parent', undefined, 'parent')).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -70,7 +91,7 @@ describe('lineage manipulation', () => { it('replace starting at non-existant contact', () => { const mock = mockContact(); - expect(replaceLineageAfter(mock, 'parent', parentsToLineage('irrelevant'), 'dne')).to.be.false; + expect(replaceLineage(mock, 'parent', parentsToLineage('irrelevant'), 'dne')).to.be.false; }); }); From d677b487c96649607b11ff46ff7b658a8049622a Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 19:14:04 -0700 Subject: [PATCH 13/21] Tests for fn folder --- src/fn/merge-contacts.js | 11 +- src/fn/move-contacts.js | 13 +- .../{mm-shared.js => backend.js} | 31 -- src/lib/hierarchy-operations/index.js | 30 +- src/lib/hierarchy-operations/jsdocFolder.js | 32 +++ test/fn/merge-contacts.spec.js | 26 ++ test/fn/move-contacts.spec.js | 27 ++ ...s.spec.js => hierarchy-operations.spec.js} | 265 ++++++++++++------ .../{mm-shared.spec.js => jsdocs.spec.js} | 13 +- .../merge-contacts.spec.js | 228 --------------- 10 files changed, 298 insertions(+), 378 deletions(-) rename src/lib/hierarchy-operations/{mm-shared.js => backend.js} (71%) create mode 100644 src/lib/hierarchy-operations/jsdocFolder.js create mode 100644 test/fn/merge-contacts.spec.js create mode 100644 test/fn/move-contacts.spec.js rename test/lib/hierarchy-operations/{move-contacts.spec.js => hierarchy-operations.spec.js} (76%) rename test/lib/hierarchy-operations/{mm-shared.spec.js => jsdocs.spec.js} (82%) delete mode 100644 test/lib/hierarchy-operations/merge-contacts.spec.js diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 88575bf69..6ad0edb98 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -13,11 +13,10 @@ module.exports = { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); const options = { - merge: true, docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options).move(args.removeIds, args.keepId, db); + return HierarchyOperations(options).merge(args.sourceIds, args.destinationId, db); } }; @@ -25,7 +24,7 @@ module.exports = { const parseExtraArgs = (projectDir, extraArgs = []) => { const args = minimist(extraArgs, { boolean: true }); - const removeIds = (args.remove || '') + const sourceIds = (args.remove || '') .split(',') .filter(Boolean); @@ -34,14 +33,14 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { throw Error(`Action "merge-contacts" is missing required contact ID ${bold('--keep')}. Other contacts will be merged into this contact.`); } - if (removeIds.length === 0) { + if (sourceIds.length === 0) { usage(); throw Error(`Action "merge-contacts" is missing required contact ID(s) ${bold('--remove')}. These contacts will be merged into the contact specified by ${bold('--keep')}`); } return { - keepId: args.keep, - removeIds, + destinationId: args.keep, + sourceIds, docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'), force: !!args.force, }; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 2d97b1137..75de128dc 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -13,11 +13,10 @@ module.exports = { const args = parseExtraArgs(environment.pathToProject, environment.extraArgs); const db = pouch(); const options = { - merge: false, docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options).move(args.contactIds, args.parentId, db); + return HierarchyOperations(options).move(args.sourceIds, args.destinationId, db); } }; @@ -25,11 +24,11 @@ module.exports = { const parseExtraArgs = (projectDir, extraArgs = []) => { const args = minimist(extraArgs, { boolean: true }); - const contactIds = (args.contacts || args.contact || '') + const sourceIds = (args.contacts || args.contact || '') .split(',') .filter(id => id); - if (contactIds.length === 0) { + if (sourceIds.length === 0) { usage(); throw Error('Action "move-contacts" is missing required list of contacts to be moved'); } @@ -40,8 +39,8 @@ const parseExtraArgs = (projectDir, extraArgs = []) => { } return { - parentId: args.parent, - contactIds, + destinationId: args.parent, + sourceIds, docDirectoryPath: path.resolve(projectDir, args.docDirectoryPath || 'json_docs'), force: !!args.force, }; @@ -61,7 +60,7 @@ ${bold('OPTIONS')} A comma delimited list of ids of contacts to be moved. --parent= - Specifies the ID of the new parent. Use '${Shared.HIERARCHY_ROOT}' to identify the top of the hierarchy (no parent). + Specifies the ID of the new parent. Use '${HierarchyOperations.HIERARCHY_ROOT}' to identify the top of the hierarchy (no parent). --docDirectoryPath= Specifies the folder used to store the documents representing the changes in hierarchy. diff --git a/src/lib/hierarchy-operations/mm-shared.js b/src/lib/hierarchy-operations/backend.js similarity index 71% rename from src/lib/hierarchy-operations/mm-shared.js rename to src/lib/hierarchy-operations/backend.js index 441d57369..dd794c125 100644 --- a/src/lib/hierarchy-operations/mm-shared.js +++ b/src/lib/hierarchy-operations/backend.js @@ -1,38 +1,9 @@ const _ = require('lodash'); -const path = require('path'); - -const userPrompt = require('../user-prompt'); -const fs = require('../sync-fs'); -const { warn, trace } = require('../log'); const lineageManipulation = require('./lineage-manipulation'); const HIERARCHY_ROOT = 'root'; const BATCH_SIZE = 10000; -const prepareDocumentDirectory = ({ docDirectoryPath, force }) => { - if (!fs.exists(docDirectoryPath)) { - fs.mkdir(docDirectoryPath); - } else if (!force && fs.recurseFiles(docDirectoryPath).length > 0) { - warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); - if(userPrompt.keyInYN()) { - fs.deleteFilesInFolder(docDirectoryPath); - } else { - throw new Error('User aborted execution.'); - } - } -}; - -const writeDocumentToDisk = ({ docDirectoryPath }, doc) => { - const destinationPath = path.join(docDirectoryPath, `${doc._id}.doc.json`); - if (fs.exists(destinationPath)) { - warn(`File at ${destinationPath} already exists and is being overwritten.`); - } - - trace(`Writing updated document to ${destinationPath}`); - fs.writeJson(destinationPath, doc); -}; - - const fetch = { /* Fetches all of the documents associated with the "contactIds" and confirms they exist. @@ -123,7 +94,5 @@ const fetch = { module.exports = { HIERARCHY_ROOT, BATCH_SIZE, - prepareDocumentDirectory, - writeDocumentToDisk, fetch, }; diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index 4a382236e..5b72875ba 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -2,26 +2,27 @@ const lineageManipulation = require('./lineage-manipulation'); const LineageConstraints = require('./lineage-constraints'); const { trace, info } = require('../log'); -const Shared = require('./mm-shared'); +const JsDocs = require('./jsdocFolder'); +const Backend = require('./backend'); -module.exports = (options) => { +const HierarchyOperations = (options) => { const move = async (sourceIds, destinationId, db) => { - Shared.prepareDocumentDirectory(options); + JsDocs.prepareFolder(options); trace(`Fetching contact details: ${destinationId}`); const constraints = await LineageConstraints(db, options); - const destinationDoc = await Shared.fetch.contact(db, destinationId); - const sourceDocs = await Shared.fetch.contactList(db, sourceIds); + const destinationDoc = await Backend.fetch.contact(db, destinationId); + const sourceDocs = await Backend.fetch.contactList(db, sourceIds); await constraints.assertHierarchyErrors(Object.values(sourceDocs), destinationDoc); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(destinationDoc); for (let sourceId of sourceIds) { const sourceDoc = sourceDocs[sourceId]; - const descendantsAndSelf = await Shared.fetch.descendantsOf(db, sourceId); + const descendantsAndSelf = await Backend.fetch.descendantsOf(db, sourceId); if (options.merge) { const self = descendantsAndSelf.find(d => d._id === sourceId); - Shared.writeDocumentToDisk(options, { + JsDocs.writeDoc(options, { _id: self._id, _rev: self._rev, _deleted: true, @@ -38,7 +39,7 @@ module.exports = (options) => { trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(sourceDoc)}.`); const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, sourceId); - const ancestors = await Shared.fetch.ancestorsOf(db, sourceDoc); + const ancestors = await Backend.fetch.ancestorsOf(db, sourceDoc); trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(sourceDoc)}.`); const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); @@ -63,9 +64,9 @@ module.exports = (options) => { let skip = 0; let reportDocsBatch; do { - info(`Processing ${skip} to ${skip + Shared.BATCH_SIZE} report docs`); + info(`Processing ${skip} to ${skip + Backend.BATCH_SIZE} report docs`); const createdAtId = options.merge && sourceId; - reportDocsBatch = await Shared.fetch.reportsCreatedByOrAt(db, descendantIds, createdAtId, skip); + reportDocsBatch = await Backend.fetch.reportsCreatedByOrAt(db, descendantIds, createdAtId, skip); const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, sourceId); @@ -97,7 +98,7 @@ module.exports = (options) => { minifyLineageAndWriteToDisk(updatedReports); skip += reportDocsBatch.length; - } while (reportDocsBatch.length >= Shared.BATCH_SIZE); + } while (reportDocsBatch.length >= Backend.BATCH_SIZE); return skip; }; @@ -105,7 +106,7 @@ module.exports = (options) => { const minifyLineageAndWriteToDisk = (docs) => { docs.forEach(doc => { lineageManipulation.minifyLineagesInDoc(doc); - Shared.writeDocumentToDisk(options, doc); + JsDocs.writeDoc(options, doc); }); }; @@ -149,3 +150,8 @@ module.exports = (options) => { return { move }; }; +module.exports = options => ({ + HIERARCHY_ROOT: Backend.HIERARCHY_ROOT, + move: HierarchyOperations({ ...options, merge: false }).move, + merge: HierarchyOperations({ ...options, merge: true }).move, +}); diff --git a/src/lib/hierarchy-operations/jsdocFolder.js b/src/lib/hierarchy-operations/jsdocFolder.js new file mode 100644 index 000000000..fd46ca5cf --- /dev/null +++ b/src/lib/hierarchy-operations/jsdocFolder.js @@ -0,0 +1,32 @@ +const path = require('path'); +const userPrompt = require('../user-prompt'); +const fs = require('../sync-fs'); +const { warn, trace } = require('../log'); + +const prepareFolder = ({ docDirectoryPath, force }) => { + if (!fs.exists(docDirectoryPath)) { + fs.mkdir(docDirectoryPath); + } else if (!force && fs.recurseFiles(docDirectoryPath).length > 0) { + warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); + if(userPrompt.keyInYN()) { + fs.deleteFilesInFolder(docDirectoryPath); + } else { + throw new Error('User aborted execution.'); + } + } +}; + +const writeDoc = ({ docDirectoryPath }, doc) => { + const destinationPath = path.join(docDirectoryPath, `${doc._id}.doc.json`); + if (fs.exists(destinationPath)) { + warn(`File at ${destinationPath} already exists and is being overwritten.`); + } + + trace(`Writing updated document to ${destinationPath}`); + fs.writeJson(destinationPath, doc); +}; + +module.exports = { + prepareFolder, + writeDoc, +}; diff --git a/test/fn/merge-contacts.spec.js b/test/fn/merge-contacts.spec.js new file mode 100644 index 000000000..c4f519ad5 --- /dev/null +++ b/test/fn/merge-contacts.spec.js @@ -0,0 +1,26 @@ +const { expect } = require('chai'); +const rewire = require('rewire'); +const Mergeremove = rewire('../../src/fn/merge-contacts'); +const parseExtraArgs = Mergeremove.__get__('parseExtraArgs'); + +describe('merge-contacts', () => { + describe('parseExtraArgs', () => { + it('undefined arguments', () => { + expect(() => parseExtraArgs(__dirname, undefined)).to.throw('required contact'); + }); + + it('empty arguments', () => expect(() => parseExtraArgs(__dirname, [])).to.throw('required contact')); + + it('remove only', () => expect(() => parseExtraArgs(__dirname, ['--remove=a'])).to.throw('required contact')); + + it('remove and keeps', () => { + const args = ['--remove=food,is,tasty', '--keep=bar', '--docDirectoryPath=/', '--force=hi']; + expect(parseExtraArgs(__dirname, args)).to.deep.eq({ + sourceIds: ['food', 'is', 'tasty'], + destinationId: 'bar', + force: true, + docDirectoryPath: '/', + }); + }); + }); +}); diff --git a/test/fn/move-contacts.spec.js b/test/fn/move-contacts.spec.js new file mode 100644 index 000000000..60068c13b --- /dev/null +++ b/test/fn/move-contacts.spec.js @@ -0,0 +1,27 @@ +const { expect } = require('chai'); +const rewire = require('rewire'); +const MoveContacts = rewire('../../src/fn/move-contacts'); +const parseExtraArgs = MoveContacts.__get__('parseExtraArgs'); + +describe('move-contacts', () => { + describe('parseExtraArgs', () => { + // const parseExtraArgs = MoveContactsLib.__get__('parseExtraArgs'); + it('undefined arguments', () => { + expect(() => parseExtraArgs(__dirname, undefined)).to.throw('required list of contacts'); + }); + + it('empty arguments', () => expect(() => parseExtraArgs(__dirname, [])).to.throw('required list of contacts')); + + it('contacts only', () => expect(() => parseExtraArgs(__dirname, ['--contacts=a'])).to.throw('required parameter parent')); + + it('contacts and parents', () => { + const args = ['--contacts=food,is,tasty', '--parent=bar', '--docDirectoryPath=/', '--force=hi']; + expect(parseExtraArgs(__dirname, args)).to.deep.eq({ + sourceIds: ['food', 'is', 'tasty'], + destinationId: 'bar', + force: true, + docDirectoryPath: '/', + }); + }); + }); +}); diff --git a/test/lib/hierarchy-operations/move-contacts.spec.js b/test/lib/hierarchy-operations/hierarchy-operations.spec.js similarity index 76% rename from test/lib/hierarchy-operations/move-contacts.spec.js rename to test/lib/hierarchy-operations/hierarchy-operations.spec.js index 4788ce316..525cb00cc 100644 --- a/test/lib/hierarchy-operations/move-contacts.spec.js +++ b/test/lib/hierarchy-operations/hierarchy-operations.spec.js @@ -1,18 +1,23 @@ -const { assert, expect } = require('chai'); +const chai = require('chai'); +const chaiAsPromised = require('chai-as-promised'); const rewire = require('rewire'); const sinon = require('sinon'); const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); -const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); +const JsDocs = rewire('../../../src/lib/hierarchy-operations/jsdocFolder.js'); +const Backend = rewire('../../../src/lib/hierarchy-operations/backend.js'); const PouchDB = require('pouchdb-core'); + +chai.use(chaiAsPromised); PouchDB.plugin(require('pouchdb-adapter-memory')); PouchDB.plugin(require('pouchdb-mapreduce')); -const HierarchyOperations = rewire('../../../src/lib/hierarchy-operations/index.js'); -HierarchyOperations.__set__('Shared', Shared); +const { assert, expect } = chai; -const move = HierarchyOperations({ merge: false }).move; +const HierarchyOperations = rewire('../../../src/lib/hierarchy-operations/index.js'); +HierarchyOperations.__set__('JsDocs', JsDocs); +HierarchyOperations.__set__('Backend', Backend); const contacts_by_depth = { // eslint-disable-next-line quotes @@ -25,7 +30,6 @@ const reports_by_freetext = { }; describe('move-contacts', () => { - let pouchDb, scenarioCount = 0; const writtenDocs = []; const getWrittenDoc = docId => { @@ -39,7 +43,7 @@ describe('move-contacts', () => { delete result._rev; return result; }; - const expectWrittenDocs = expected => expect(writtenDocs.map(doc => doc._id)).to.deep.eq(expected); + const expectWrittenDocs = expected => expect(writtenDocs.map(doc => doc._id)).to.have.members(expected); const upsert = async (id, content) => { const { _rev } = await pouchDb.get(id); @@ -61,7 +65,13 @@ describe('move-contacts', () => { }, }, }, - district_2: {}, + district_2: { + health_center_2: { + clinic_2: { + patient_2: {}, + } + } + }, }); await pouchDb.put({ _id: 'settings', settings: {} }); @@ -81,15 +91,15 @@ describe('move-contacts', () => { views: { contacts_by_depth }, }); - Shared.writeDocumentToDisk = (docDirectoryPath, doc) => writtenDocs.push(doc); - Shared.prepareDocumentDirectory = () => {}; + JsDocs.writeDoc = (docDirectoryPath, doc) => writtenDocs.push(doc); + JsDocs.prepareFolder = () => {}; writtenDocs.length = 0; }); afterEach(async () => pouchDb.destroy()); - + it('move health_center_1 to district_2', async () => { - await move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -131,7 +141,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: [] }]); - await move(['health_center_1'], 'root', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'root', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -184,7 +194,7 @@ describe('move-contacts', () => { it('move district_1 from root', async () => { await updateHierarchyRules([{ id: 'district_hospital', parents: ['district_hospital'] }]); - await move(['district_1'], 'district_2', pouchDb); + await HierarchyOperations().move(['district_1'], 'district_2', pouchDb); expect(getWrittenDoc('district_1')).to.deep.eq({ _id: 'district_1', @@ -240,7 +250,7 @@ describe('move-contacts', () => { { id: 'district_hospital', parents: ['county'] }, ]); - await move(['district_1'], 'county_1', pouchDb); + await HierarchyOperations().move(['district_1'], 'county_1', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -295,7 +305,7 @@ describe('move-contacts', () => { creatorId: 'focal', }); - await move(['focal'], 'subcounty', pouchDb); + await HierarchyOperations().move(['focal'], 'subcounty', pouchDb); expect(getWrittenDoc('focal')).to.deep.eq({ _id: 'focal', @@ -340,7 +350,7 @@ describe('move-contacts', () => { parent: parentsToLineage(), }); - await move(['t_patient_1'], 't_clinic_2', pouchDb); + await HierarchyOperations().move(['t_patient_1'], 't_clinic_2', pouchDb); expect(getWrittenDoc('t_health_center_1')).to.deep.eq({ _id: 't_health_center_1', @@ -361,7 +371,7 @@ describe('move-contacts', () => { // We don't want lineage { id, parent: '' } to result from district_hospitals which have parent: '' it('district_hospital with empty string parent is not preserved', async () => { await upsert('district_2', { parent: '', type: 'district_hospital' }); - await move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); expect(getWrittenDoc('health_center_1')).to.deep.eq({ _id: 'health_center_1', @@ -371,6 +381,133 @@ describe('move-contacts', () => { }); }); + describe('merging', () => { + it('merge district_2 into district_1', async () => { + // setup + await mockReport(pouchDb, { + id: 'changing_subject_and_contact', + creatorId: 'health_center_2_contact', + patientId: 'district_2' + }); + + await mockReport(pouchDb, { + id: 'changing_contact', + creatorId: 'health_center_2_contact', + patientId: 'patient_2' + }); + + await mockReport(pouchDb, { + id: 'changing_subject', + patientId: 'district_2' + }); + + // action + await HierarchyOperations().merge(['district_2'], 'district_1', pouchDb); + + // assert + expectWrittenDocs([ + 'district_2', 'district_2_contact', + 'health_center_2', 'health_center_2_contact', + 'clinic_2', 'clinic_2_contact', + 'patient_2', + 'changing_subject_and_contact', 'changing_contact', 'changing_subject' + ]); + + expect(getWrittenDoc('district_2')).to.deep.eq({ + _id: 'district_2', + _deleted: true, + }); + + expect(getWrittenDoc('health_center_2')).to.deep.eq({ + _id: 'health_center_2', + type: 'health_center', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + parent: parentsToLineage('district_1'), + }); + + expect(getWrittenDoc('clinic_2')).to.deep.eq({ + _id: 'clinic_2', + type: 'clinic', + contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_1'), + parent: parentsToLineage('health_center_2', 'district_1'), + }); + + expect(getWrittenDoc('patient_2')).to.deep.eq({ + _id: 'patient_2', + type: 'person', + parent: parentsToLineage('clinic_2', 'health_center_2', 'district_1'), + }); + + expect(getWrittenDoc('changing_subject_and_contact')).to.deep.eq({ + _id: 'changing_subject_and_contact', + form: 'foo', + type: 'data_record', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + fields: { + patient_uuid: 'district_1' + } + }); + + expect(getWrittenDoc('changing_contact')).to.deep.eq({ + _id: 'changing_contact', + form: 'foo', + type: 'data_record', + contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), + fields: { + patient_uuid: 'patient_2' + } + }); + + expect(getWrittenDoc('changing_subject')).to.deep.eq({ + _id: 'changing_subject', + form: 'foo', + type: 'data_record', + contact: { + _id: 'dne', + }, + fields: { + patient_uuid: 'district_1' + } + }); + }); + + it('merge two patients', async () => { + // setup + await mockReport(pouchDb, { + id: 'pat1', + creatorId: 'clinic_1_contact', + patientId: 'patient_1' + }); + + await mockReport(pouchDb, { + id: 'pat2', + creatorId: 'clinic_2_contact', + patientId: 'patient_2' + }); + + // action + await HierarchyOperations().merge(['patient_2'], 'patient_1', pouchDb); + + await expectWrittenDocs(['patient_2', 'pat2']); + + expect(getWrittenDoc('patient_2')).to.deep.eq({ + _id: 'patient_2', + _deleted: true, + }); + + expect(getWrittenDoc('pat2')).to.deep.eq({ + _id: 'pat2', + form: 'foo', + type: 'data_record', + // still created by the user in district-2 + contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_2'), + fields: { + patient_uuid: 'patient_1' + } + }); + }); + }); + it('documents should be minified', async () => { await updateHierarchyRules([{ id: 'clinic', parents: ['district_hospital'] }]); const patient = { @@ -389,7 +526,7 @@ describe('move-contacts', () => { await upsert('clinic_1', clinic); await upsert('patient_1', patient); - await move(['clinic_1'], 'district_2', pouchDb); + await HierarchyOperations().move(['clinic_1'], 'district_2', pouchDb); expect(getWrittenDoc('clinic_1')).to.deep.eq({ _id: 'clinic_1', @@ -410,7 +547,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: ['clinic'] }]); try { - await move(['health_center_1'], 'clinic_1', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'clinic_1', pouchDb); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('circular'); @@ -418,85 +555,39 @@ describe('move-contacts', () => { }); it('throw if parent does not exist', async () => { - try { - await move(['clinic_1'], 'dne_parent_id', pouchDb); - assert.fail('should throw when parent is not defined'); - } catch (err) { - expect(err.message).to.include('could not be found'); - } + const actual = HierarchyOperations().move(['clinic_1'], 'dne_parent_id', pouchDb); + await expect(actual).to.eventually.rejectedWith('could not be found'); }); it('throw when altering same lineage', async () => { - try { - await move(['patient_1', 'health_center_1'], 'district_2', pouchDb); - assert.fail('should throw'); - } catch (err) { - expect(err.message).to.include('same lineage'); - } + const actual = HierarchyOperations().move(['patient_1', 'health_center_1'], 'district_2', pouchDb); + await expect(actual).to.eventually.rejectedWith('same lineage'); }); it('throw if contact_id is not a contact', async () => { - try { - await move(['report_1'], 'clinic_1', pouchDb); - assert.fail('should throw'); - } catch (err) { - expect(err.message).to.include('unknown type'); - } + const actual = HierarchyOperations().move(['report_1'], 'clinic_1', pouchDb); + await expect(actual).to.eventually.rejectedWith('unknown type'); }); it('throw if moving primary contact of parent', async () => { - try { - await move(['clinic_1_contact'], 'district_1', pouchDb); - assert.fail('should throw'); - } catch (err) { - expect(err.message).to.include('primary contact'); - } + const actual = HierarchyOperations().move(['clinic_1_contact'], 'district_1', pouchDb); + await expect(actual).to.eventually.rejectedWith('primary contact'); }); it('throw if setting parent to self', async () => { await updateHierarchyRules([{ id: 'clinic', parents: ['clinic'] }]); - try { - await move(['clinic_1'], 'clinic_1', pouchDb); - assert.fail('should throw'); - } catch (err) { - expect(err.message).to.include('circular'); - } + const actual = HierarchyOperations().move(['clinic_1'], 'clinic_1', pouchDb); + await expect(actual).to.eventually.rejectedWith('circular'); }); it('throw when moving place to unconfigured parent', async () => { await updateHierarchyRules([{ id: 'district_hospital', parents: [] }]); - - try { - await move(['district_1'], 'district_2', pouchDb); - assert.fail('Expected error'); - } catch (err) { - expect(err.message).to.include('parent of type'); - } - }); - - xdescribe('parseExtraArgs', () => { - // const parseExtraArgs = MoveContactsLib.__get__('parseExtraArgs'); - it('undefined arguments', () => { - expect(() => parseExtraArgs(__dirname, undefined)).to.throw('required list of contacts'); - }); - - it('empty arguments', () => expect(() => parseExtraArgs(__dirname, [])).to.throw('required list of contacts')); - - it('contacts only', () => expect(() => parseExtraArgs(__dirname, ['--contacts=a'])).to.throw('required parameter parent')); - - it('contacts and parents', () => { - const args = ['--contacts=food,is,tasty', '--parent=bar', '--docDirectoryPath=/', '--force=hi']; - expect(parseExtraArgs(__dirname, args)).to.deep.eq({ - sourceIds: ['food', 'is', 'tasty'], - destinationId: 'bar', - force: true, - docDirectoryPath: '/', - }); - }); + const actual = HierarchyOperations().move(['district_1'], 'district_2', pouchDb); + await expect(actual).to.eventually.rejectedWith('parent of type'); }); describe('batching works as expected', () => { - const initialBatchSize = Shared.BATCH_SIZE; + const initialBatchSize = Backend.BATCH_SIZE; beforeEach(async () => { await mockReport(pouchDb, { id: 'report_2', @@ -515,16 +606,16 @@ describe('move-contacts', () => { }); afterEach(() => { - Shared.BATCH_SIZE = initialBatchSize; - Shared.__set__('BATCH_SIZE', initialBatchSize); + Backend.BATCH_SIZE = initialBatchSize; + Backend.__set__('BATCH_SIZE', initialBatchSize); }); it('move health_center_1 to district_2 in batches of 1', async () => { - Shared.__set__('BATCH_SIZE', 1); - Shared.BATCH_SIZE = 1; + Backend.__set__('BATCH_SIZE', 1); + Backend.BATCH_SIZE = 1; sinon.spy(pouchDb, 'query'); - await move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -596,11 +687,11 @@ describe('move-contacts', () => { }); it('should health_center_1 to district_1 in batches of 2', async () => { - Shared.__set__('BATCH_SIZE', 2); - Shared.BATCH_SIZE = 2; + Backend.__set__('BATCH_SIZE', 2); + Backend.BATCH_SIZE = 2; sinon.spy(pouchDb, 'query'); - await move(['health_center_1'], 'district_1', pouchDb); + await HierarchyOperations().move(['health_center_1'], 'district_1', pouchDb); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', diff --git a/test/lib/hierarchy-operations/mm-shared.spec.js b/test/lib/hierarchy-operations/jsdocs.spec.js similarity index 82% rename from test/lib/hierarchy-operations/mm-shared.spec.js rename to test/lib/hierarchy-operations/jsdocs.spec.js index 1d686ef17..d0aec6e11 100644 --- a/test/lib/hierarchy-operations/mm-shared.spec.js +++ b/test/lib/hierarchy-operations/jsdocs.spec.js @@ -4,18 +4,17 @@ const sinon = require('sinon'); const environment = require('../../../src/lib/environment'); const fs = require('../../../src/lib/sync-fs'); -const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); +const JsDocs = rewire('../../../src/lib/hierarchy-operations/jsdocFolder'); const userPrompt = rewire('../../../src/lib/user-prompt'); - -describe('mm-shared', () => { +describe('JsDocs', () => { let readline; let docOnj = { docDirectoryPath: '/test/path/for/testing ', force: false }; beforeEach(() => { readline = { keyInYN: sinon.stub() }; userPrompt.__set__('readline', readline); - Shared.__set__('userPrompt', userPrompt); + JsDocs.__set__('userPrompt', userPrompt); sinon.stub(fs, 'exists').returns(true); sinon.stub(fs, 'recurseFiles').returns(Array(20)); sinon.stub(fs, 'deleteFilesInFolder').returns(true); @@ -28,7 +27,7 @@ describe('mm-shared', () => { readline.keyInYN.returns(false); sinon.stub(environment, 'force').get(() => false); try { - Shared.prepareDocumentDirectory(docOnj); + JsDocs.prepareFolder(docOnj); assert.fail('Expected error to be thrown'); } catch(e) { assert.equal(fs.deleteFilesInFolder.callCount, 0); @@ -38,13 +37,13 @@ describe('mm-shared', () => { it('deletes files in directory when user presses y', () => { readline.keyInYN.returns(true); sinon.stub(environment, 'force').get(() => false); - Shared.prepareDocumentDirectory(docOnj); + JsDocs.prepareFolder(docOnj); assert.equal(fs.deleteFilesInFolder.callCount, 1); }); it('deletes files in directory when force is set', () => { sinon.stub(environment, 'force').get(() => true); - Shared.prepareDocumentDirectory(docOnj); + JsDocs.prepareFolder(docOnj); assert.equal(fs.deleteFilesInFolder.callCount, 1); }); }); diff --git a/test/lib/hierarchy-operations/merge-contacts.spec.js b/test/lib/hierarchy-operations/merge-contacts.spec.js deleted file mode 100644 index 120b6b602..000000000 --- a/test/lib/hierarchy-operations/merge-contacts.spec.js +++ /dev/null @@ -1,228 +0,0 @@ - -const chai = require('chai'); -const chaiAsPromised = require('chai-as-promised'); -const rewire = require('rewire'); - -const Shared = rewire('../../../src/lib/hierarchy-operations/mm-shared'); - -chai.use(chaiAsPromised); -const { expect } = chai; - -const PouchDB = require('pouchdb-core'); -PouchDB.plugin(require('pouchdb-adapter-memory')); -PouchDB.plugin(require('pouchdb-mapreduce')); - -const HierarchyOperations = rewire('../../../src/lib/hierarchy-operations/index.js'); -HierarchyOperations.__set__('Shared', Shared); - -const move = HierarchyOperations({ merge: true }).move; - -const { mockReport, mockHierarchy, parentsToLineage } = require('../../mock-hierarchies'); - -const contacts_by_depth = { - // eslint-disable-next-line quotes - map: "function(doc) {\n if (doc.type === 'tombstone' && doc.tombstone) {\n doc = doc.tombstone;\n }\n if (['contact', 'person', 'clinic', 'health_center', 'district_hospital'].indexOf(doc.type) !== -1) {\n var value = doc.patient_id || doc.place_id;\n var parent = doc;\n var depth = 0;\n while (parent) {\n if (parent._id) {\n emit([parent._id], value);\n emit([parent._id, depth], value);\n }\n depth++;\n parent = parent.parent;\n }\n }\n}", -}; - -const reports_by_freetext = { - // eslint-disable-next-line quotes - map: "function(doc) {\n var skip = [ '_id', '_rev', 'type', 'refid', 'content' ];\n\n var usedKeys = [];\n var emitMaybe = function(key, value) {\n if (usedKeys.indexOf(key) === -1 && // Not already used\n key.length > 2 // Not too short\n ) {\n usedKeys.push(key);\n emit([key], value);\n }\n };\n\n var emitField = function(key, value, reportedDate) {\n if (!key || !value) {\n return;\n }\n key = key.toLowerCase();\n if (skip.indexOf(key) !== -1 || /_date$/.test(key)) {\n return;\n }\n if (typeof value === 'string') {\n value = value.toLowerCase();\n value.split(/\\s+/).forEach(function(word) {\n emitMaybe(word, reportedDate);\n });\n }\n if (typeof value === 'number' || typeof value === 'string') {\n emitMaybe(key + ':' + value, reportedDate);\n }\n };\n\n if (doc.type === 'data_record' && doc.form) {\n Object.keys(doc).forEach(function(key) {\n emitField(key, doc[key], doc.reported_date);\n });\n if (doc.fields) {\n Object.keys(doc.fields).forEach(function(key) {\n emitField(key, doc.fields[key], doc.reported_date);\n });\n }\n if (doc.contact && doc.contact._id) {\n emitMaybe('contact:' + doc.contact._id.toLowerCase(), doc.reported_date);\n }\n }\n}" -}; - -describe('merge-contacts', () => { - let pouchDb, scenarioCount = 0; - const writtenDocs = []; - const getWrittenDoc = docId => { - const matches = writtenDocs.filter(doc => doc && doc._id === docId); - if (matches.length === 0) { - return undefined; - } - - // Remove _rev because it makes expectations harder to write - const result = matches[matches.length - 1]; - delete result._rev; - return result; - }; - const expectWrittenDocs = expected => expect(writtenDocs.map(doc => doc._id)).to.have.members(expected); - - beforeEach(async () => { - pouchDb = new PouchDB(`merge-contacts-${scenarioCount++}`); - - await mockHierarchy(pouchDb, { - district_1: { - health_center_1: { - clinic_1: { - patient_1: {}, - }, - } - }, - district_2: { - health_center_2: { - clinic_2: { - patient_2: {}, - }, - } - }, - }); - - await pouchDb.put({ _id: 'settings', settings: {} }); - - await pouchDb.put({ - _id: '_design/medic-client', - views: { reports_by_freetext }, - }); - - await pouchDb.put({ - _id: '_design/medic', - views: { contacts_by_depth }, - }); - - Shared.writeDocumentToDisk = (docDirectoryPath, doc) => writtenDocs.push(doc); - Shared.prepareDocumentDirectory = () => {}; - writtenDocs.length = 0; - }); - - afterEach(async () => pouchDb.destroy()); - - it('merge district_2 into district_1', async () => { - // setup - await mockReport(pouchDb, { - id: 'changing_subject_and_contact', - creatorId: 'health_center_2_contact', - patientId: 'district_2' - }); - - await mockReport(pouchDb, { - id: 'changing_contact', - creatorId: 'health_center_2_contact', - patientId: 'patient_2' - }); - - await mockReport(pouchDb, { - id: 'changing_subject', - patientId: 'district_2' - }); - - // action - await move(['district_2'], 'district_1', pouchDb); - - // assert - expectWrittenDocs([ - 'district_2', 'district_2_contact', - 'health_center_2', 'health_center_2_contact', - 'clinic_2', 'clinic_2_contact', - 'patient_2', - 'changing_subject_and_contact', 'changing_contact', 'changing_subject' - ]); - - expect(getWrittenDoc('district_2')).to.deep.eq({ - _id: 'district_2', - _deleted: true, - }); - - expect(getWrittenDoc('health_center_2')).to.deep.eq({ - _id: 'health_center_2', - type: 'health_center', - contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), - parent: parentsToLineage('district_1'), - }); - - expect(getWrittenDoc('clinic_2')).to.deep.eq({ - _id: 'clinic_2', - type: 'clinic', - contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_1'), - parent: parentsToLineage('health_center_2', 'district_1'), - }); - - expect(getWrittenDoc('patient_2')).to.deep.eq({ - _id: 'patient_2', - type: 'person', - parent: parentsToLineage('clinic_2', 'health_center_2', 'district_1'), - }); - - expect(getWrittenDoc('changing_subject_and_contact')).to.deep.eq({ - _id: 'changing_subject_and_contact', - form: 'foo', - type: 'data_record', - contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), - fields: { - patient_uuid: 'district_1' - } - }); - - expect(getWrittenDoc('changing_contact')).to.deep.eq({ - _id: 'changing_contact', - form: 'foo', - type: 'data_record', - contact: parentsToLineage('health_center_2_contact', 'health_center_2', 'district_1'), - fields: { - patient_uuid: 'patient_2' - } - }); - - expect(getWrittenDoc('changing_subject')).to.deep.eq({ - _id: 'changing_subject', - form: 'foo', - type: 'data_record', - contact: { - _id: 'dne', - }, - fields: { - patient_uuid: 'district_1' - } - }); - }); - - it('merge two patients', async () => { - // setup - await mockReport(pouchDb, { - id: 'pat1', - creatorId: 'clinic_1_contact', - patientId: 'patient_1' - }); - - await mockReport(pouchDb, { - id: 'pat2', - creatorId: 'clinic_2_contact', - patientId: 'patient_2' - }); - - // action - await move(['patient_2'], 'patient_1', pouchDb); - - await expectWrittenDocs(['patient_2', 'pat2']); - - expect(getWrittenDoc('patient_2')).to.deep.eq({ - _id: 'patient_2', - _deleted: true, - }); - - expect(getWrittenDoc('pat2')).to.deep.eq({ - _id: 'pat2', - form: 'foo', - type: 'data_record', - // still created by the user in district-2 - contact: parentsToLineage('clinic_2_contact', 'clinic_2', 'health_center_2', 'district_2'), - fields: { - patient_uuid: 'patient_1' - } - }); - }); - - xit('write to ancestors', () => {}); - - it('throw if removed does not exist', async () => { - const actual = move(['dne'], 'district_1', pouchDb); - await expect(actual).to.eventually.rejectedWith('could not be found'); - }); - - it('throw if kept does not exist', async () => { - const actual = move(['district_1'], 'dne', pouchDb); - await expect(actual).to.eventually.rejectedWith('could not be found'); - }); - - it('throw if removed is kept', async () => { - const actual = move(['district_1', 'district_2'], 'district_2', pouchDb); - await expect(actual).to.eventually.rejectedWith('that is itself'); - }); -}); From 2442fcccf235a27bfb14ebded3fb6bae38695e98 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 19:15:22 -0700 Subject: [PATCH 14/21] Pass eslint --- src/lib/hierarchy-operations/backend.js | 2 +- src/lib/hierarchy-operations/lineage-manipulation.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/hierarchy-operations/backend.js b/src/lib/hierarchy-operations/backend.js index dd794c125..d1755fc67 100644 --- a/src/lib/hierarchy-operations/backend.js +++ b/src/lib/hierarchy-operations/backend.js @@ -84,7 +84,7 @@ const fetch = { const ancestorIdsNotFound = ancestors.rows.filter(ancestor => !ancestor.doc).map(ancestor => ancestor.key); if (ancestorIdsNotFound.length > 0) { - throw Error(`Contact '${prettyPrintDocument(contactDoc)} has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); + throw Error(`Contact '${contactDoc?.name}' (${contactDoc?._id}) has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); } return ancestors.rows.map(ancestor => ancestor.doc); diff --git a/src/lib/hierarchy-operations/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js index ebd7df97f..8df1aa772 100644 --- a/src/lib/hierarchy-operations/lineage-manipulation.js +++ b/src/lib/hierarchy-operations/lineage-manipulation.js @@ -27,7 +27,7 @@ const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdIn element: doc[lineageAttributeName], attributeName: 'parent', }; - } + }; const state = initialState(); while (state.element) { From a0a0c845e6c50a249282912dc3adee34202f87e6 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 19:19:59 -0700 Subject: [PATCH 15/21] Backend interface change --- src/lib/hierarchy-operations/backend.js | 168 ++++++++++++------------ src/lib/hierarchy-operations/index.js | 10 +- 2 files changed, 90 insertions(+), 88 deletions(-) diff --git a/src/lib/hierarchy-operations/backend.js b/src/lib/hierarchy-operations/backend.js index d1755fc67..5a27882c4 100644 --- a/src/lib/hierarchy-operations/backend.js +++ b/src/lib/hierarchy-operations/backend.js @@ -4,95 +4,97 @@ const lineageManipulation = require('./lineage-manipulation'); const HIERARCHY_ROOT = 'root'; const BATCH_SIZE = 10000; -const fetch = { - /* - Fetches all of the documents associated with the "contactIds" and confirms they exist. - */ - contactList: async (db, ids) => { - const contactDocs = await db.allDocs({ - keys: ids, - include_docs: true, - }); - - const missingContactErrors = contactDocs.rows.filter(row => !row.doc).map(row => `Contact with id '${row.key}' could not be found.`); - if (missingContactErrors.length > 0) { - throw Error(missingContactErrors); +/* +Fetches all of the documents associated with the "contactIds" and confirms they exist. +*/ +async function contactList(db, ids) { + const contactDocs = await db.allDocs({ + keys: ids, + include_docs: true, + }); + + const missingContactErrors = contactDocs.rows.filter(row => !row.doc).map(row => `Contact with id '${row.key}' could not be found.`); + if (missingContactErrors.length > 0) { + throw Error(missingContactErrors); + } + + return contactDocs.rows.reduce((agg, curr) => Object.assign(agg, { [curr.doc._id]: curr.doc }), {}); +} + +async function contact(db, id) { + try { + if (id === HIERARCHY_ROOT) { + return undefined; } - return contactDocs.rows.reduce((agg, curr) => Object.assign(agg, { [curr.doc._id]: curr.doc }), {}); - }, - - contact: async (db, id) => { - try { - if (id === HIERARCHY_ROOT) { - return undefined; - } - - return await db.get(id); - } catch (err) { - if (err.name !== 'not_found') { - throw err; - } - - throw Error(`Contact with id '${id}' could not be found`); - } - }, - - /* - Given a contact's id, obtain the documents of all descendant contacts - */ - descendantsOf: async (db, contactId) => { - const descendantDocs = await db.query('medic/contacts_by_depth', { - key: [contactId], - include_docs: true, - }); - - return descendantDocs.rows - .map(row => row.doc) - /* We should not move or update tombstone documents */ - .filter(doc => doc && doc.type !== 'tombstone'); - }, - - reportsCreatedByOrAt: async (db, createdByIds, createdAtId, skip) => { - const createdByKeys = createdByIds.map(descendantId => [`contact:${descendantId}`]); - const createdAtKeys = createdAtId ? [ - [`patient_id:${createdAtId}`], - [`patient_uuid:${createdAtId}`], - [`place_id:${createdAtId}`], - [`place_uuid:${createdAtId}`] - ] : []; - - const reports = await db.query('medic-client/reports_by_freetext', { - keys: [ - ...createdByKeys, - ...createdAtKeys, - ], - include_docs: true, - limit: BATCH_SIZE, - skip, - }); - - return _.uniqBy(reports.rows.map(row => row.doc), '_id'); - }, - - ancestorsOf: async (db, contactDoc) => { - const ancestorIds = lineageManipulation.pluckIdsFromLineage(contactDoc.parent); - const ancestors = await db.allDocs({ - keys: ancestorIds, - include_docs: true, - }); - - const ancestorIdsNotFound = ancestors.rows.filter(ancestor => !ancestor.doc).map(ancestor => ancestor.key); - if (ancestorIdsNotFound.length > 0) { - throw Error(`Contact '${contactDoc?.name}' (${contactDoc?._id}) has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); + return await db.get(id); + } catch (err) { + if (err.name !== 'not_found') { + throw err; } - return ancestors.rows.map(ancestor => ancestor.doc); - }, -}; + throw Error(`Contact with id '${id}' could not be found`); + } +} + +/* +Given a contact's id, obtain the documents of all descendant contacts +*/ +async function descendantsOf(db, contactId) { + const descendantDocs = await db.query('medic/contacts_by_depth', { + key: [contactId], + include_docs: true, + }); + + return descendantDocs.rows + .map(row => row.doc) + /* We should not move or update tombstone documents */ + .filter(doc => doc && doc.type !== 'tombstone'); +} + +async function reportsCreatedByOrAt(db, createdByIds, createdAtId, skip) { + const createdByKeys = createdByIds.map(descendantId => [`contact:${descendantId}`]); + const createdAtKeys = createdAtId ? [ + [`patient_id:${createdAtId}`], + [`patient_uuid:${createdAtId}`], + [`place_id:${createdAtId}`], + [`place_uuid:${createdAtId}`] + ] : []; + + const reports = await db.query('medic-client/reports_by_freetext', { + keys: [ + ...createdByKeys, + ...createdAtKeys, + ], + include_docs: true, + limit: BATCH_SIZE, + skip, + }); + + return _.uniqBy(reports.rows.map(row => row.doc), '_id'); +} + +async function ancestorsOf(db, contactDoc) { + const ancestorIds = lineageManipulation.pluckIdsFromLineage(contactDoc.parent); + const ancestors = await db.allDocs({ + keys: ancestorIds, + include_docs: true, + }); + + const ancestorIdsNotFound = ancestors.rows.filter(ancestor => !ancestor.doc).map(ancestor => ancestor.key); + if (ancestorIdsNotFound.length > 0) { + throw Error(`Contact '${contactDoc?.name}' (${contactDoc?._id}) has parent id(s) '${ancestorIdsNotFound.join(',')}' which could not be found.`); + } + + return ancestors.rows.map(ancestor => ancestor.doc); +} module.exports = { HIERARCHY_ROOT, BATCH_SIZE, - fetch, + ancestorsOf, + descendantsOf, + contact, + contactList, + reportsCreatedByOrAt, }; diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index 5b72875ba..62014d154 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -10,15 +10,15 @@ const HierarchyOperations = (options) => { JsDocs.prepareFolder(options); trace(`Fetching contact details: ${destinationId}`); const constraints = await LineageConstraints(db, options); - const destinationDoc = await Backend.fetch.contact(db, destinationId); - const sourceDocs = await Backend.fetch.contactList(db, sourceIds); + const destinationDoc = await Backend.contact(db, destinationId); + const sourceDocs = await Backend.contactList(db, sourceIds); await constraints.assertHierarchyErrors(Object.values(sourceDocs), destinationDoc); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(destinationDoc); for (let sourceId of sourceIds) { const sourceDoc = sourceDocs[sourceId]; - const descendantsAndSelf = await Backend.fetch.descendantsOf(db, sourceId); + const descendantsAndSelf = await Backend.descendantsOf(db, sourceId); if (options.merge) { const self = descendantsAndSelf.find(d => d._id === sourceId); @@ -39,7 +39,7 @@ const HierarchyOperations = (options) => { trace(`Considering lineage updates to ${descendantsAndSelf.length} descendant(s) of contact ${prettyPrintDocument(sourceDoc)}.`); const updatedDescendants = replaceLineageInContacts(descendantsAndSelf, replacementLineage, sourceId); - const ancestors = await Backend.fetch.ancestorsOf(db, sourceDoc); + const ancestors = await Backend.ancestorsOf(db, sourceDoc); trace(`Considering primary contact updates to ${ancestors.length} ancestor(s) of contact ${prettyPrintDocument(sourceDoc)}.`); const updatedAncestors = replaceLineageInAncestors(descendantsAndSelf, ancestors); @@ -66,7 +66,7 @@ const HierarchyOperations = (options) => { do { info(`Processing ${skip} to ${skip + Backend.BATCH_SIZE} report docs`); const createdAtId = options.merge && sourceId; - reportDocsBatch = await Backend.fetch.reportsCreatedByOrAt(db, descendantIds, createdAtId, skip); + reportDocsBatch = await Backend.reportsCreatedByOrAt(db, descendantIds, createdAtId, skip); const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, sourceId); From f73f9c6b48c59063f9d66de81858bea03fe96004 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 19:34:32 -0700 Subject: [PATCH 16/21] Fix failing test in mock-hierarchies --- src/lib/hierarchy-operations/backend.js | 2 +- src/lib/hierarchy-operations/index.js | 134 +++++++++++++----------- test/mock-hierarchies.spec.js | 1 + 3 files changed, 74 insertions(+), 63 deletions(-) diff --git a/src/lib/hierarchy-operations/backend.js b/src/lib/hierarchy-operations/backend.js index 5a27882c4..30990d8b3 100644 --- a/src/lib/hierarchy-operations/backend.js +++ b/src/lib/hierarchy-operations/backend.js @@ -53,7 +53,7 @@ async function descendantsOf(db, contactId) { } async function reportsCreatedByOrAt(db, createdByIds, createdAtId, skip) { - const createdByKeys = createdByIds.map(descendantId => [`contact:${descendantId}`]); + const createdByKeys = createdByIds.map(id => [`contact:${id}`]); const createdAtKeys = createdAtId ? [ [`patient_id:${createdAtId}`], [`patient_uuid:${createdAtId}`], diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index 62014d154..e4d4fc4d3 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -6,7 +6,7 @@ const JsDocs = require('./jsdocFolder'); const Backend = require('./backend'); const HierarchyOperations = (options) => { - const move = async (sourceIds, destinationId, db) => { + async function move(sourceIds, destinationId, db) { JsDocs.prepareFolder(options); trace(`Fetching contact details: ${destinationId}`); const constraints = await LineageConstraints(db, options); @@ -55,10 +55,9 @@ const HierarchyOperations = (options) => { } info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); - }; + } - - const moveReports = async (db, descendantsAndSelf, replacementLineage, sourceId, destinationId) => { + async function moveReports(db, descendantsAndSelf, replacementLineage, sourceId, destinationId) { const descendantIds = descendantsAndSelf.map(contact => contact._id); let skip = 0; @@ -71,28 +70,7 @@ const HierarchyOperations = (options) => { const updatedReports = replaceLineageInReports(reportDocsBatch, replacementLineage, sourceId); if (options.merge) { - reportDocsBatch.forEach(report => { - let updated = false; - const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; - for (const subjectId of subjectIds) { - if (report[subjectId] === sourceId) { - report[subjectId] = destinationId; - updated = true; - } - - if (report.fields[subjectId] === sourceId) { - report.fields[subjectId] = destinationId; - updated = true; - } - - if (updated) { - const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); - if (!isAlreadyUpdated) { - updatedReports.push(report); - } - } - } - }); + reassignReports(reportDocsBatch, sourceId, destinationId, updatedReports); } minifyLineageAndWriteToDisk(updatedReports); @@ -101,51 +79,82 @@ const HierarchyOperations = (options) => { } while (reportDocsBatch.length >= Backend.BATCH_SIZE); return skip; - }; + } + + function reassignReports(reports, sourceId, destinationId, updatedReports) { + reports.forEach(report => { + let updated = false; + const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; + for (const subjectId of subjectIds) { + if (report[subjectId] === sourceId) { + report[subjectId] = destinationId; + updated = true; + } + + if (report.fields[subjectId] === sourceId) { + report.fields[subjectId] = destinationId; + updated = true; + } + + if (updated) { + const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); + if (!isAlreadyUpdated) { + updatedReports.push(report); + } + } + } + }); + } - const minifyLineageAndWriteToDisk = (docs) => { + function minifyLineageAndWriteToDisk(docs) { docs.forEach(doc => { lineageManipulation.minifyLineagesInDoc(doc); JsDocs.writeDoc(options, doc); }); - }; - - const replaceLineageInReports = (reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) => reportsCreatedByDescendants.reduce((agg, doc) => { - if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage, options)) { - agg.push(doc); - } - return agg; - }, []); - - const replaceLineageInAncestors = (descendantsAndSelf, ancestors) => ancestors.reduce((agg, ancestor) => { - let result = agg; - const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); - if (primaryContact) { - ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); - result = [ancestor, ...result]; - } - - return result; - }, []); + } - const replaceLineageInContacts = (descendantsAndSelf, replacementLineage, destinationId) => descendantsAndSelf.reduce((agg, doc) => { - const startingFromIdInLineage = options.merge ? destinationId : - doc._id === destinationId ? undefined : destinationId; + function replaceLineageInReports(reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) { + return reportsCreatedByDescendants.reduce((agg, doc) => { + if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage, options)) { + agg.push(doc); + } + return agg; + }, []); + } + + function replaceLineageInAncestors(descendantsAndSelf, ancestors) { + return ancestors.reduce((agg, ancestor) => { + let result = agg; + const primaryContact = descendantsAndSelf.find(descendant => ancestor.contact && descendant._id === ancestor.contact._id); + if (primaryContact) { + ancestor.contact = lineageManipulation.createLineageFromDoc(primaryContact); + result = [ancestor, ...result]; + } - // skip top-level because it will be deleted - if (options.merge) { - if (doc._id === destinationId) { - return agg; + return result; + }, []); + } + + function replaceLineageInContacts(descendantsAndSelf, replacementLineage, destinationId) { + return descendantsAndSelf.reduce((agg, doc) => { + const startingFromIdInLineage = options.merge ? destinationId : + doc._id === destinationId ? undefined : destinationId; + + // skip top-level because it will be deleted + if (options.merge) { + if (doc._id === destinationId) { + return agg; + } } - } - const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); - const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); - if (parentWasUpdated || contactWasUpdated) { - agg.push(doc); - } - return agg; - }, []); + const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); + const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); + if (parentWasUpdated || contactWasUpdated) { + agg.push(doc); + } + return agg; + }, []); + } return { move }; }; @@ -155,3 +164,4 @@ module.exports = options => ({ move: HierarchyOperations({ ...options, merge: false }).move, merge: HierarchyOperations({ ...options, merge: true }).move, }); + diff --git a/test/mock-hierarchies.spec.js b/test/mock-hierarchies.spec.js index c8a21933a..3177a7172 100644 --- a/test/mock-hierarchies.spec.js +++ b/test/mock-hierarchies.spec.js @@ -84,6 +84,7 @@ describe('mocks', () => { _id: 'report_1', type: 'data_record', form: 'foo', + fields: {}, contact: { _id: 'health_center_1_contact', parent: { From 8e35f2d449cc25e12db066f8e356cdeee59290ef Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 20:01:55 -0700 Subject: [PATCH 17/21] SonarCube --- src/fn/merge-contacts.js | 2 +- src/fn/move-contacts.js | 2 +- src/lib/hierarchy-operations/index.js | 29 +++++++------- src/lib/hierarchy-operations/jsdocFolder.js | 25 +++++++----- .../lineage-constraints.js | 4 +- .../lineage-manipulation.js | 11 +++--- .../hierarchy-operations.spec.js | 39 ++++++++++--------- test/lib/hierarchy-operations/jsdocs.spec.js | 11 ++---- 8 files changed, 62 insertions(+), 61 deletions(-) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 6ad0edb98..591f9465c 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -16,7 +16,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options).merge(args.sourceIds, args.destinationId, db); + return HierarchyOperations(options, db).merge(args.sourceIds, args.destinationId); } }; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index 75de128dc..d3af4334b 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -16,7 +16,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options).move(args.sourceIds, args.destinationId, db); + return HierarchyOperations(options, db).move(args.sourceIds, args.destinationId); } }; diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index e4d4fc4d3..655f424c4 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -5,14 +5,14 @@ const { trace, info } = require('../log'); const JsDocs = require('./jsdocFolder'); const Backend = require('./backend'); -const HierarchyOperations = (options) => { - async function move(sourceIds, destinationId, db) { +const HierarchyOperations = (db, options) => { + async function move(sourceIds, destinationId) { JsDocs.prepareFolder(options); trace(`Fetching contact details: ${destinationId}`); const constraints = await LineageConstraints(db, options); const destinationDoc = await Backend.contact(db, destinationId); const sourceDocs = await Backend.contactList(db, sourceIds); - await constraints.assertHierarchyErrors(Object.values(sourceDocs), destinationDoc); + constraints.assertHierarchyErrors(Object.values(sourceDocs), destinationDoc); let affectedContactCount = 0, affectedReportCount = 0; const replacementLineage = lineageManipulation.createLineageFromDoc(destinationDoc); @@ -45,7 +45,7 @@ const HierarchyOperations = (options) => { minifyLineageAndWriteToDisk([...updatedDescendants, ...updatedAncestors]); - const movedReportsCount = await moveReports(db, descendantsAndSelf, replacementLineage, sourceId, destinationId); + const movedReportsCount = await moveReports(descendantsAndSelf, replacementLineage, sourceId, destinationId); trace(`${movedReportsCount} report(s) created by these affected contact(s) will be updated`); affectedContactCount += updatedDescendants.length + updatedAncestors.length; @@ -57,7 +57,7 @@ const HierarchyOperations = (options) => { info(`Staged changes to lineage information for ${affectedContactCount} contact(s) and ${affectedReportCount} report(s).`); } - async function moveReports(db, descendantsAndSelf, replacementLineage, sourceId, destinationId) { + async function moveReports(descendantsAndSelf, replacementLineage, sourceId, destinationId) { const descendantIds = descendantsAndSelf.map(contact => contact._id); let skip = 0; @@ -137,19 +137,18 @@ const HierarchyOperations = (options) => { function replaceLineageInContacts(descendantsAndSelf, replacementLineage, destinationId) { return descendantsAndSelf.reduce((agg, doc) => { - const startingFromIdInLineage = options.merge ? destinationId : - doc._id === destinationId ? undefined : destinationId; + const docIsDestination = doc._id === destinationId; + const startingFromIdInLineage = options.merge || !docIsDestination ? destinationId : undefined; // skip top-level because it will be deleted - if (options.merge) { - if (doc._id === destinationId) { - return agg; - } + if (options.merge && docIsDestination) { + return agg; } const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); - if (parentWasUpdated || contactWasUpdated) { + const isUpdated = parentWasUpdated || contactWasUpdated; + if (isUpdated) { agg.push(doc); } return agg; @@ -159,9 +158,9 @@ const HierarchyOperations = (options) => { return { move }; }; -module.exports = options => ({ +module.exports = (db, options) => ({ HIERARCHY_ROOT: Backend.HIERARCHY_ROOT, - move: HierarchyOperations({ ...options, merge: false }).move, - merge: HierarchyOperations({ ...options, merge: true }).move, + move: HierarchyOperations(db, { ...options, merge: false }).move, + merge: HierarchyOperations(db, { ...options, merge: true }).move, }); diff --git a/src/lib/hierarchy-operations/jsdocFolder.js b/src/lib/hierarchy-operations/jsdocFolder.js index fd46ca5cf..a45836c4d 100644 --- a/src/lib/hierarchy-operations/jsdocFolder.js +++ b/src/lib/hierarchy-operations/jsdocFolder.js @@ -3,20 +3,15 @@ const userPrompt = require('../user-prompt'); const fs = require('../sync-fs'); const { warn, trace } = require('../log'); -const prepareFolder = ({ docDirectoryPath, force }) => { +function prepareFolder({ docDirectoryPath, force }) { if (!fs.exists(docDirectoryPath)) { fs.mkdir(docDirectoryPath); } else if (!force && fs.recurseFiles(docDirectoryPath).length > 0) { - warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); - if(userPrompt.keyInYN()) { - fs.deleteFilesInFolder(docDirectoryPath); - } else { - throw new Error('User aborted execution.'); - } + deleteAfterConfirmation(docDirectoryPath); } -}; +} -const writeDoc = ({ docDirectoryPath }, doc) => { +function writeDoc({ docDirectoryPath }, doc) { const destinationPath = path.join(docDirectoryPath, `${doc._id}.doc.json`); if (fs.exists(destinationPath)) { warn(`File at ${destinationPath} already exists and is being overwritten.`); @@ -24,9 +19,19 @@ const writeDoc = ({ docDirectoryPath }, doc) => { trace(`Writing updated document to ${destinationPath}`); fs.writeJson(destinationPath, doc); -}; +} + +function deleteAfterConfirmation(docDirectoryPath) { + warn(`The document folder '${docDirectoryPath}' already contains files. It is recommended you start with a clean folder. Do you want to delete the contents of this folder and continue?`); + if (userPrompt.keyInYN()) { + fs.deleteFilesInFolder(docDirectoryPath); + } else { + throw new Error('User aborted execution.'); + } +} module.exports = { prepareFolder, writeDoc, }; + diff --git a/src/lib/hierarchy-operations/lineage-constraints.js b/src/lib/hierarchy-operations/lineage-constraints.js index 91d2845d1..84f0d8591 100644 --- a/src/lib/hierarchy-operations/lineage-constraints.js +++ b/src/lib/hierarchy-operations/lineage-constraints.js @@ -127,8 +127,8 @@ const getPrimaryContactViolations = async (db, contactDoc, parentDoc, descendant }); const primaryContactIds = docsRemovedFromContactLineage.rows - .map(row => row.doc && row.doc.contact && row.doc.contact._id) - .filter(id => id); + .map(row => row?.doc?.contact?._id) + .filter(Boolean); return descendantDocs.find(descendant => primaryContactIds.some(primaryId => descendant._id === primaryId)); }; diff --git a/src/lib/hierarchy-operations/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js index 8df1aa772..0add21ac3 100644 --- a/src/lib/hierarchy-operations/lineage-manipulation.js +++ b/src/lib/hierarchy-operations/lineage-manipulation.js @@ -9,13 +9,13 @@ * @param {Object} options * @param {boolean} merge When true, startingFromIdInLineage is replaced and when false, startingFromIdInLineage's parent is replaced */ -const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdInLineage, options={}) => { +function replaceLineage(doc, lineageAttributeName, replaceWith, startingFromIdInLineage, options={}) { // Replace the full lineage if (!startingFromIdInLineage) { return replaceWithinLineage(doc, lineageAttributeName, replaceWith); } - const initialState = () => { + const getInitialState = () => { if (options.merge) { return { element: doc, @@ -29,7 +29,7 @@ const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdIn }; }; - const state = initialState(); + const state = getInitialState(); while (state.element) { const compare = options.merge ? state.element[state.attributeName] : state.element; if (compare?._id === startingFromIdInLineage) { @@ -41,7 +41,7 @@ const replaceLineage = (doc, lineageAttributeName, replaceWith, startingFromIdIn } return false; -}; +} const replaceWithinLineage = (replaceInDoc, lineageAttributeName, replaceWith) => { if (!replaceWith) { @@ -63,7 +63,7 @@ Function borrowed from shared-lib/lineage */ const minifyLineagesInDoc = doc => { const minifyLineage = lineage => { - if (!lineage || !lineage._id) { + if (!lineage?._id) { return undefined; } @@ -85,7 +85,6 @@ const minifyLineagesInDoc = doc => { if ('contact' in doc) { doc.contact = minifyLineage(doc.contact); - if (doc.contact && !doc.contact.parent) delete doc.contact.parent; // for unit test clarity } if (doc.type === 'data_record') { diff --git a/test/lib/hierarchy-operations/hierarchy-operations.spec.js b/test/lib/hierarchy-operations/hierarchy-operations.spec.js index 525cb00cc..3d6eee0ab 100644 --- a/test/lib/hierarchy-operations/hierarchy-operations.spec.js +++ b/test/lib/hierarchy-operations/hierarchy-operations.spec.js @@ -99,7 +99,7 @@ describe('move-contacts', () => { afterEach(async () => pouchDb.destroy()); it('move health_center_1 to district_2', async () => { - await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'district_2'); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -141,7 +141,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: [] }]); - await HierarchyOperations().move(['health_center_1'], 'root', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'root'); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -194,7 +194,7 @@ describe('move-contacts', () => { it('move district_1 from root', async () => { await updateHierarchyRules([{ id: 'district_hospital', parents: ['district_hospital'] }]); - await HierarchyOperations().move(['district_1'], 'district_2', pouchDb); + await HierarchyOperations(pouchDb).move(['district_1'], 'district_2'); expect(getWrittenDoc('district_1')).to.deep.eq({ _id: 'district_1', @@ -250,7 +250,7 @@ describe('move-contacts', () => { { id: 'district_hospital', parents: ['county'] }, ]); - await HierarchyOperations().move(['district_1'], 'county_1', pouchDb); + await HierarchyOperations(pouchDb).move(['district_1'], 'county_1'); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -305,7 +305,7 @@ describe('move-contacts', () => { creatorId: 'focal', }); - await HierarchyOperations().move(['focal'], 'subcounty', pouchDb); + await HierarchyOperations(pouchDb).move(['focal'], 'subcounty'); expect(getWrittenDoc('focal')).to.deep.eq({ _id: 'focal', @@ -350,7 +350,7 @@ describe('move-contacts', () => { parent: parentsToLineage(), }); - await HierarchyOperations().move(['t_patient_1'], 't_clinic_2', pouchDb); + await HierarchyOperations(pouchDb).move(['t_patient_1'], 't_clinic_2'); expect(getWrittenDoc('t_health_center_1')).to.deep.eq({ _id: 't_health_center_1', @@ -371,7 +371,7 @@ describe('move-contacts', () => { // We don't want lineage { id, parent: '' } to result from district_hospitals which have parent: '' it('district_hospital with empty string parent is not preserved', async () => { await upsert('district_2', { parent: '', type: 'district_hospital' }); - await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'district_2'); expect(getWrittenDoc('health_center_1')).to.deep.eq({ _id: 'health_center_1', @@ -402,7 +402,7 @@ describe('move-contacts', () => { }); // action - await HierarchyOperations().merge(['district_2'], 'district_1', pouchDb); + await HierarchyOperations(pouchDb).merge(['district_2'], 'district_1'); // assert expectWrittenDocs([ @@ -464,6 +464,7 @@ describe('move-contacts', () => { type: 'data_record', contact: { _id: 'dne', + parent: undefined, }, fields: { patient_uuid: 'district_1' @@ -486,7 +487,7 @@ describe('move-contacts', () => { }); // action - await HierarchyOperations().merge(['patient_2'], 'patient_1', pouchDb); + await HierarchyOperations(pouchDb).merge(['patient_2'], 'patient_1'); await expectWrittenDocs(['patient_2', 'pat2']); @@ -526,7 +527,7 @@ describe('move-contacts', () => { await upsert('clinic_1', clinic); await upsert('patient_1', patient); - await HierarchyOperations().move(['clinic_1'], 'district_2', pouchDb); + await HierarchyOperations(pouchDb).move(['clinic_1'], 'district_2'); expect(getWrittenDoc('clinic_1')).to.deep.eq({ _id: 'clinic_1', @@ -547,7 +548,7 @@ describe('move-contacts', () => { await updateHierarchyRules([{ id: 'health_center', parents: ['clinic'] }]); try { - await HierarchyOperations().move(['health_center_1'], 'clinic_1', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'clinic_1'); assert.fail('should throw'); } catch (err) { expect(err.message).to.include('circular'); @@ -555,34 +556,34 @@ describe('move-contacts', () => { }); it('throw if parent does not exist', async () => { - const actual = HierarchyOperations().move(['clinic_1'], 'dne_parent_id', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['clinic_1'], 'dne_parent_id'); await expect(actual).to.eventually.rejectedWith('could not be found'); }); it('throw when altering same lineage', async () => { - const actual = HierarchyOperations().move(['patient_1', 'health_center_1'], 'district_2', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['patient_1', 'health_center_1'], 'district_2'); await expect(actual).to.eventually.rejectedWith('same lineage'); }); it('throw if contact_id is not a contact', async () => { - const actual = HierarchyOperations().move(['report_1'], 'clinic_1', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['report_1'], 'clinic_1'); await expect(actual).to.eventually.rejectedWith('unknown type'); }); it('throw if moving primary contact of parent', async () => { - const actual = HierarchyOperations().move(['clinic_1_contact'], 'district_1', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['clinic_1_contact'], 'district_1'); await expect(actual).to.eventually.rejectedWith('primary contact'); }); it('throw if setting parent to self', async () => { await updateHierarchyRules([{ id: 'clinic', parents: ['clinic'] }]); - const actual = HierarchyOperations().move(['clinic_1'], 'clinic_1', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['clinic_1'], 'clinic_1'); await expect(actual).to.eventually.rejectedWith('circular'); }); it('throw when moving place to unconfigured parent', async () => { await updateHierarchyRules([{ id: 'district_hospital', parents: [] }]); - const actual = HierarchyOperations().move(['district_1'], 'district_2', pouchDb); + const actual = HierarchyOperations(pouchDb).move(['district_1'], 'district_2'); await expect(actual).to.eventually.rejectedWith('parent of type'); }); @@ -615,7 +616,7 @@ describe('move-contacts', () => { Backend.BATCH_SIZE = 1; sinon.spy(pouchDb, 'query'); - await HierarchyOperations().move(['health_center_1'], 'district_2', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'district_2'); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', @@ -691,7 +692,7 @@ describe('move-contacts', () => { Backend.BATCH_SIZE = 2; sinon.spy(pouchDb, 'query'); - await HierarchyOperations().move(['health_center_1'], 'district_1', pouchDb); + await HierarchyOperations(pouchDb).move(['health_center_1'], 'district_1'); expect(getWrittenDoc('health_center_1_contact')).to.deep.eq({ _id: 'health_center_1_contact', diff --git a/test/lib/hierarchy-operations/jsdocs.spec.js b/test/lib/hierarchy-operations/jsdocs.spec.js index d0aec6e11..23353673a 100644 --- a/test/lib/hierarchy-operations/jsdocs.spec.js +++ b/test/lib/hierarchy-operations/jsdocs.spec.js @@ -1,4 +1,4 @@ -const { assert } = require('chai'); +const { assert, expect } = require('chai'); const rewire = require('rewire'); const sinon = require('sinon'); @@ -26,12 +26,9 @@ describe('JsDocs', () => { it('does not delete files in directory when user presses n', () => { readline.keyInYN.returns(false); sinon.stub(environment, 'force').get(() => false); - try { - JsDocs.prepareFolder(docOnj); - assert.fail('Expected error to be thrown'); - } catch(e) { - assert.equal(fs.deleteFilesInFolder.callCount, 0); - } + const actual = () => JsDocs.prepareFolder(docOnj); + expect(actual).to.throw('aborted execution'); + assert.equal(fs.deleteFilesInFolder.callCount, 0); }); it('deletes files in directory when user presses y', () => { From 17c4c047a00e92bcdc63c1492fce9db110f7fc30 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 20:39:09 -0700 Subject: [PATCH 18/21] SonarQube - Is his really better code? --- src/lib/hierarchy-operations/index.js | 52 +++++++++++-------- .../lineage-constraints.js | 46 ++++++++-------- .../lineage-manipulation.js | 15 ++++-- 3 files changed, 65 insertions(+), 48 deletions(-) diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index 655f424c4..fd6b5f6dc 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -82,28 +82,32 @@ const HierarchyOperations = (db, options) => { } function reassignReports(reports, sourceId, destinationId, updatedReports) { - reports.forEach(report => { + function reassignReportWithSubject(report, subjectId) { let updated = false; + if (report[subjectId] === sourceId) { + report[subjectId] = destinationId; + updated = true; + } + + if (report.fields[subjectId] === sourceId) { + report.fields[subjectId] = destinationId; + updated = true; + } + + if (updated) { + const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); + if (!isAlreadyUpdated) { + updatedReports.push(report); + } + } + } + + for (const report of reports) { const subjectIds = ['patient_id', 'patient_uuid', 'place_id', 'place_uuid']; for (const subjectId of subjectIds) { - if (report[subjectId] === sourceId) { - report[subjectId] = destinationId; - updated = true; - } - - if (report.fields[subjectId] === sourceId) { - report.fields[subjectId] = destinationId; - updated = true; - } - - if (updated) { - const isAlreadyUpdated = !!updatedReports.find(updated => updated._id === report._id); - if (!isAlreadyUpdated) { - updatedReports.push(report); - } - } + reassignReportWithSubject(report, subjectId); } - }); + } } function minifyLineageAndWriteToDisk(docs) { @@ -136,23 +140,25 @@ const HierarchyOperations = (db, options) => { } function replaceLineageInContacts(descendantsAndSelf, replacementLineage, destinationId) { - return descendantsAndSelf.reduce((agg, doc) => { + const result = []; + for (const doc of descendantsAndSelf) { const docIsDestination = doc._id === destinationId; const startingFromIdInLineage = options.merge || !docIsDestination ? destinationId : undefined; // skip top-level because it will be deleted if (options.merge && docIsDestination) { - return agg; + continue; } const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); const isUpdated = parentWasUpdated || contactWasUpdated; if (isUpdated) { - agg.push(doc); + result.push(doc); } - return agg; - }, []); + } + + return result; } return { move }; diff --git a/src/lib/hierarchy-operations/lineage-constraints.js b/src/lib/hierarchy-operations/lineage-constraints.js index 84f0d8591..5bfbae19f 100644 --- a/src/lib/hierarchy-operations/lineage-constraints.js +++ b/src/lib/hierarchy-operations/lineage-constraints.js @@ -47,33 +47,37 @@ Enforce the list of allowed parents for each contact type Ensure we are not creating a circular hierarchy */ const getMovingViolations = (mapTypeToAllowedParents, sourceDoc, destinationDoc) => { - const commonViolations = getCommonViolations(sourceDoc, destinationDoc); - if (commonViolations) { - return commonViolations; - } + function getContactTypeError() { + const sourceContactType = getContactType(sourceDoc); + const destinationType = getContactType(destinationDoc); + const rulesForContact = mapTypeToAllowedParents[sourceContactType]; + if (!rulesForContact) { + return `cannot move contact with unknown type '${sourceContactType}'`; + } - if (!mapTypeToAllowedParents) { - return 'hierarchy constraints are undefined'; - } - - const sourceContactType = getContactType(sourceDoc); - const destinationType = getContactType(destinationDoc); - const rulesForContact = mapTypeToAllowedParents[sourceContactType]; - if (!rulesForContact) { - return `cannot move contact with unknown type '${sourceContactType}'`; + const isPermittedMoveToRoot = !destinationDoc && rulesForContact.length === 0; + if (!isPermittedMoveToRoot && !rulesForContact.includes(destinationType)) { + return `contacts of type '${sourceContactType}' cannot have parent of type '${destinationType}'`; + } } - const isPermittedMoveToRoot = !destinationDoc && rulesForContact.length === 0; - if (!isPermittedMoveToRoot && !rulesForContact.includes(destinationType)) { - return `contacts of type '${sourceContactType}' cannot have parent of type '${destinationType}'`; + function findCircularHierarchyErrors() { + if (destinationDoc && sourceDoc._id) { + const parentAncestry = [destinationDoc._id, ...lineageManipulation.pluckIdsFromLineage(destinationDoc.parent)]; + if (parentAncestry.includes(sourceDoc._id)) { + return `Circular hierarchy: Cannot set parent of contact '${sourceDoc._id}' as it would create a circular hierarchy.`; + } + } } - if (destinationDoc && sourceDoc._id) { - const parentAncestry = [destinationDoc._id, ...lineageManipulation.pluckIdsFromLineage(destinationDoc.parent)]; - if (parentAncestry.includes(sourceDoc._id)) { - return `Circular hierarchy: Cannot set parent of contact '${sourceDoc._id}' as it would create a circular hierarchy.`; - } + if (!mapTypeToAllowedParents) { + return 'hierarchy constraints are undefined'; } + + const commonViolations = getCommonViolations(sourceDoc, destinationDoc); + const contactTypeError = getContactTypeError(); + const circularHierarchyError = findCircularHierarchyErrors(); + return commonViolations || contactTypeError || circularHierarchyError; }; const getCommonViolations = (sourceDoc, destinationDoc) => { diff --git a/src/lib/hierarchy-operations/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js index 0add21ac3..e4967a7e6 100644 --- a/src/lib/hierarchy-operations/lineage-manipulation.js +++ b/src/lib/hierarchy-operations/lineage-manipulation.js @@ -15,7 +15,7 @@ function replaceLineage(doc, lineageAttributeName, replaceWith, startingFromIdIn return replaceWithinLineage(doc, lineageAttributeName, replaceWith); } - const getInitialState = () => { + function getInitialState() { if (options.merge) { return { element: doc, @@ -27,10 +27,9 @@ function replaceLineage(doc, lineageAttributeName, replaceWith, startingFromIdIn element: doc[lineageAttributeName], attributeName: 'parent', }; - }; + } - const state = getInitialState(); - while (state.element) { + function traverseOne() { const compare = options.merge ? state.element[state.attributeName] : state.element; if (compare?._id === startingFromIdInLineage) { return replaceWithinLineage(state.element, state.attributeName, replaceWith); @@ -40,6 +39,14 @@ function replaceLineage(doc, lineageAttributeName, replaceWith, startingFromIdIn state.attributeName = 'parent'; } + const state = getInitialState(); + while (state.element) { + const result = traverseOne(); + if (result) { + return result; + } + } + return false; } From 7af035cd5df3fa10a993501ccc40b35200582a26 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 20:45:24 -0700 Subject: [PATCH 19/21] SonarQube - Fix? --- src/lib/hierarchy-operations/index.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index fd6b5f6dc..f19a8e21f 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -140,22 +140,27 @@ const HierarchyOperations = (db, options) => { } function replaceLineageInContacts(descendantsAndSelf, replacementLineage, destinationId) { + function replaceForSingleContact(doc) { + const docIsDestination = doc._id === destinationId; + const startingFromIdInLineage = options.merge || !docIsDestination ? destinationId : undefined; + const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); + const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); + const isUpdated = parentWasUpdated || contactWasUpdated; + if (isUpdated) { + result.push(doc); + } + } + const result = []; for (const doc of descendantsAndSelf) { const docIsDestination = doc._id === destinationId; - const startingFromIdInLineage = options.merge || !docIsDestination ? destinationId : undefined; // skip top-level because it will be deleted if (options.merge && docIsDestination) { continue; } - const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); - const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); - const isUpdated = parentWasUpdated || contactWasUpdated; - if (isUpdated) { - result.push(doc); - } + replaceForSingleContact(doc); } return result; From 687a6a231f088d53980754e0c03e48478d02fb2d Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 20:59:24 -0700 Subject: [PATCH 20/21] SonarQube --- src/lib/hierarchy-operations/index.js | 30 +++++++-- .../lineage-manipulation.js | 28 ++++---- .../lineage-manipulation.spec.js | 66 +++++++++++++++---- 3 files changed, 93 insertions(+), 31 deletions(-) diff --git a/src/lib/hierarchy-operations/index.js b/src/lib/hierarchy-operations/index.js index f19a8e21f..19b6b861b 100644 --- a/src/lib/hierarchy-operations/index.js +++ b/src/lib/hierarchy-operations/index.js @@ -117,9 +117,16 @@ const HierarchyOperations = (db, options) => { }); } - function replaceLineageInReports(reportsCreatedByDescendants, replaceWith, startingFromIdInLineage) { + function replaceLineageInReports(reportsCreatedByDescendants, replaceWith, startingFromId) { return reportsCreatedByDescendants.reduce((agg, doc) => { - if (lineageManipulation.replaceLineage(doc, 'contact', replaceWith, startingFromIdInLineage, options)) { + const replaceLineageOptions = { + lineageAttribute: 'contact', + replaceWith, + startingFromId, + merge: options.merge, + }; + + if (lineageManipulation.replaceLineage(doc, replaceLineageOptions)) { agg.push(doc); } return agg; @@ -139,18 +146,27 @@ const HierarchyOperations = (db, options) => { }, []); } - function replaceLineageInContacts(descendantsAndSelf, replacementLineage, destinationId) { + function replaceLineageInContacts(descendantsAndSelf, replaceWith, destinationId) { function replaceForSingleContact(doc) { const docIsDestination = doc._id === destinationId; - const startingFromIdInLineage = options.merge || !docIsDestination ? destinationId : undefined; - const parentWasUpdated = lineageManipulation.replaceLineage(doc, 'parent', replacementLineage, startingFromIdInLineage, options); - const contactWasUpdated = lineageManipulation.replaceLineage(doc, 'contact', replacementLineage, destinationId, options); + const startingFromId = options.merge || !docIsDestination ? destinationId : undefined; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith, + startingFromId, + merge: options.merge, + }; + const parentWasUpdated = lineageManipulation.replaceLineage(doc, replaceLineageOptions); + + replaceLineageOptions.lineageAttribute = 'contact'; + replaceLineageOptions.startingFromId = destinationId; + const contactWasUpdated = lineageManipulation.replaceLineage(doc, replaceLineageOptions); const isUpdated = parentWasUpdated || contactWasUpdated; if (isUpdated) { result.push(doc); } } - + const result = []; for (const doc of descendantsAndSelf) { const docIsDestination = doc._id === destinationId; diff --git a/src/lib/hierarchy-operations/lineage-manipulation.js b/src/lib/hierarchy-operations/lineage-manipulation.js index e4967a7e6..f966e330c 100644 --- a/src/lib/hierarchy-operations/lineage-manipulation.js +++ b/src/lib/hierarchy-operations/lineage-manipulation.js @@ -3,35 +3,37 @@ * Given a doc, replace the lineage information therein with "replaceWith" * * @param {Object} doc A CouchDB document containing a hierarchy that needs replacing - * @param {string} lineageAttributeName Name of the attribute which is a lineage in doc (contact or parent) - * @param {Object} replaceWith The new hierarchy { parent: { _id: 'parent', parent: { _id: 'grandparent' } } - * @param {string} [startingFromIdInLineage] Only the part of the lineage "after" this id will be replaced - * @param {Object} options - * @param {boolean} merge When true, startingFromIdInLineage is replaced and when false, startingFromIdInLineage's parent is replaced + * @param {Object} params SonarQube + * @param {string} params.lineageAttribute Name of the attribute which is a lineage in doc (contact or parent) + * @param {Object} params.replaceWith The new hierarchy { parent: { _id: 'parent', parent: { _id: 'grandparent' } } + * @param {string} params.startingFromId Only the part of the lineage "after" this id will be replaced + * @param {boolean} params.merge When true, startingFromId is replaced and when false, startingFromId's parent is replaced */ -function replaceLineage(doc, lineageAttributeName, replaceWith, startingFromIdInLineage, options={}) { +function replaceLineage(doc, params) { + const { lineageAttribute, replaceWith, startingFromId, merge } = params; + // Replace the full lineage - if (!startingFromIdInLineage) { - return replaceWithinLineage(doc, lineageAttributeName, replaceWith); + if (!startingFromId) { + return replaceWithinLineage(doc, lineageAttribute, replaceWith); } function getInitialState() { - if (options.merge) { + if (merge) { return { element: doc, - attributeName: lineageAttributeName, + attributeName: lineageAttribute, }; } return { - element: doc[lineageAttributeName], + element: doc[lineageAttribute], attributeName: 'parent', }; } function traverseOne() { - const compare = options.merge ? state.element[state.attributeName] : state.element; - if (compare?._id === startingFromIdInLineage) { + const compare = merge ? state.element[state.attributeName] : state.element; + if (compare?._id === startingFromId) { return replaceWithinLineage(state.element, state.attributeName, replaceWith); } diff --git a/test/lib/hierarchy-operations/lineage-manipulation.spec.js b/test/lib/hierarchy-operations/lineage-manipulation.spec.js index be324009e..80077aa9f 100644 --- a/test/lib/hierarchy-operations/lineage-manipulation.spec.js +++ b/test/lib/hierarchy-operations/lineage-manipulation.spec.js @@ -4,16 +4,19 @@ const log = require('../../../src/lib/log'); log.level = log.LEVEL_TRACE; const { parentsToLineage } = require('../../mock-hierarchies'); -const mergeOption = { merge: true }; describe('lineage manipulation', () => { - describe('kenn replaceLineage', () => { + describe('replaceLineage', () => { const mockReport = data => Object.assign({ _id: 'r', type: 'data_record', contact: parentsToLineage('parent', 'grandparent') }, data); const mockContact = data => Object.assign({ _id: 'c', type: 'person', parent: parentsToLineage('parent', 'grandparent') }, data); it('replace with empty lineage', () => { const mock = mockReport(); - expect(replaceLineage(mock, 'contact', undefined)).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'contact', + replaceWith: undefined, + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'r', type: 'data_record', @@ -23,7 +26,11 @@ describe('lineage manipulation', () => { it('replace full lineage', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: parentsToLineage('new_parent'), + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -35,7 +42,11 @@ describe('lineage manipulation', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent'))).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: parentsToLineage('new_parent'), + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -46,12 +57,23 @@ describe('lineage manipulation', () => { it('replace empty with empty', () => { const mock = mockContact(); delete mock.parent; - expect(replaceLineage(mock, 'parent', undefined)).to.be.false; + + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: undefined, + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.false; }); it('replace lineage starting at contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('new_grandparent'), 'parent')).to.be.true; + + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: parentsToLineage('new_grandparent'), + startingFromId: 'parent', + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -61,7 +83,13 @@ describe('lineage manipulation', () => { it('merge new parent', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('new_parent', 'new_grandparent'), 'parent', mergeOption)).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: parentsToLineage('new_parent', 'new_grandparent'), + startingFromId: 'parent', + merge: true, + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -71,7 +99,13 @@ describe('lineage manipulation', () => { it('merge grandparent of contact', () => { const mock = mockReport(); - expect(replaceLineage(mock, 'contact', parentsToLineage('new_grandparent'), 'grandparent', mergeOption)).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'contact', + replaceWith: parentsToLineage('new_grandparent'), + startingFromId: 'grandparent', + merge: true, + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'r', type: 'data_record', @@ -81,7 +115,12 @@ describe('lineage manipulation', () => { it('replace empty starting at contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', undefined, 'parent')).to.be.true; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: undefined, + startingFromId: 'parent', + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.true; expect(mock).to.deep.eq({ _id: 'c', type: 'person', @@ -91,7 +130,12 @@ describe('lineage manipulation', () => { it('replace starting at non-existant contact', () => { const mock = mockContact(); - expect(replaceLineage(mock, 'parent', parentsToLineage('irrelevant'), 'dne')).to.be.false; + const replaceLineageOptions = { + lineageAttribute: 'parent', + replaceWith: parentsToLineage('irrelevant'), + startingFromId: 'dne', + }; + expect(replaceLineage(mock, replaceLineageOptions)).to.be.false; }); }); From 49c6d5149d99219ef7e04dc4092a94cca0796dd2 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Fri, 22 Nov 2024 21:12:03 -0700 Subject: [PATCH 21/21] Oops --- src/fn/merge-contacts.js | 2 +- src/fn/move-contacts.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fn/merge-contacts.js b/src/fn/merge-contacts.js index 591f9465c..ef3f20211 100644 --- a/src/fn/merge-contacts.js +++ b/src/fn/merge-contacts.js @@ -16,7 +16,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options, db).merge(args.sourceIds, args.destinationId); + return HierarchyOperations(db, options).merge(args.sourceIds, args.destinationId); } }; diff --git a/src/fn/move-contacts.js b/src/fn/move-contacts.js index d3af4334b..97dc8e142 100644 --- a/src/fn/move-contacts.js +++ b/src/fn/move-contacts.js @@ -16,7 +16,7 @@ module.exports = { docDirectoryPath: args.docDirectoryPath, force: args.force, }; - return HierarchyOperations(options, db).move(args.sourceIds, args.destinationId); + return HierarchyOperations(db, options).move(args.sourceIds, args.destinationId); } };