SonarCube

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);
   }
 };
 

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);
   }
 };
 

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,
 });
 

src/lib/hierarchy-operations/jsdocFolder.js

@@ -3,30 +3,35 @@ 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.`);
   }
 
   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,
 };
+

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));
 };

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') {

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,42 +548,42 @@ 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');
     }
   });
 
   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',

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', () => {