From 4ac8e10b5cb24b7719261a57353acced37668c49 Mon Sep 17 00:00:00 2001
From: LaurenD <laurend@mitre.org>
Date: Fri, 26 Jul 2024 15:54:29 -0400
Subject: [PATCH 1/6] by patient export

---
 README.md                            |  1 +
 src/server/exportWorker.js           | 11 ++--
 src/services/export.service.js       | 48 ++++++++++++---
 src/util/exportToNDJson.js           | 89 ++++++++++++++++++++++------
 src/util/mongo.controller.js         |  5 +-
 test/fixtures/testBulkStatus.json    |  7 +++
 test/services/export.service.test.js | 59 ++++++++++++++++++
 7 files changed, 187 insertions(+), 33 deletions(-)

diff --git a/README.md b/README.md
index b316cbcb..5933b1bf 100644
--- a/README.md
+++ b/README.md
@@ -164,6 +164,7 @@ The server supports the following query parameters:
 - `_outputFormat`: The server supports the following formats: `application/fhir+ndjson`, `application/ndjson+fhir`, `application/ndjson`, `ndjson`
 - `_typeFilter`: Filters the response to only include resources that meet the criteria of the specified comma-delimited FHIR REST queries. Returns an error for queries specified by the client that are unsupported by the server. Supports queries on the ValueSets (`type:in`, `code:in`, etc.) of a given resource type.
 - `patient`: Only applicable to POST requests for group-level and patient-level requests. When provided, the server SHALL NOT return resources in the patient compartment definition belonging to patients outside the list. Can support multiple patient references in a single request.
+- `_bySubject`: Only applicable for group-level and patient-level requests. Creates export results, separating resources into files based on what subject they are associated with (rather than based on type). The only `_bySubject` value supported is `Patient`. This will result in an ndjson file for each patient in the returned data. If the `_type` parameter is used in conjunction with this parameter, `Patient` must be one of the types included in the passed value list.
 - `_elements`: Filters the content of the responses to omit unlisted, non-mandatory elements from the resources returned. These elements should be provided in the form `[resource type].[element name]` (e.g., `Patient.id`) which only filters the contents of those specified resources or in the form `[element name]` (e.g., `id`) which filters the contents of all of the returned resources.
 
 #### `_elements` Query Parameter
diff --git a/src/server/exportWorker.js b/src/server/exportWorker.js
index 7365a581..96c700e7 100644
--- a/src/server/exportWorker.js
+++ b/src/server/exportWorker.js
@@ -14,17 +14,16 @@ const exportQueue = new Queue('export', {
 
 // This handler pulls down the jobs on Redis to handle
 exportQueue.process(async job => {
-  // Payload of createJob exists on job.data
-  const { clientEntry, types, typeFilter, patient, systemLevelExport, patientIds, elements } = job.data;
-  console.log(`export-worker-${process.pid}: Processing Request: ${clientEntry}`);
+  console.log(`export-worker-${process.pid}: Processing Request: ${job.data.clientEntry}`);
   await client.connect();
   // Call the existing export ndjson function that writes the files
 
-  const result = await exportToNDJson(clientEntry, types, typeFilter, patient, systemLevelExport, patientIds, elements);
+  // Payload of createJob exists on job.data
+  const result = await exportToNDJson(job.data);
   if (result) {
-    console.log(`export-worker-${process.pid}: Completed Export Request: ${clientEntry}`);
+    console.log(`export-worker-${process.pid}: Completed Export Request: ${job.data.clientEntry}`);
   } else {
-    console.log(`export-worker-${process.pid}: Failed Export Request: ${clientEntry}`);
+    console.log(`export-worker-${process.pid}: Failed Export Request: ${job.data.clientEntry}`);
   }
   await client.close();
 });
diff --git a/src/services/export.service.js b/src/services/export.service.js
index 9490ff6c..59959517 100644
--- a/src/services/export.service.js
+++ b/src/services/export.service.js
@@ -21,11 +21,19 @@ const bulkExport = async (request, reply) => {
       })
     );
   }
+  if (parameters._bySubject) {
+    reply.code(400).send(
+      createOperationOutcome('The "_bySubject" parameter cannot be used in a system-level export request.', {
+        issueCode: 400,
+        severity: 'error'
+      })
+    );
+  }
   if (validateExportParams(parameters, reply)) {
     request.log.info('Base >>> $export');
-    const clientEntry = await addPendingBulkExportRequest();
+    const clientEntry = await addPendingBulkExportRequest(parameters._bySubject === 'Patient');
 
-    const types = request.query._type?.split(',') || parameters._type?.split(',');
+    const types = request.query._type?.split(',') || parameters._type?.split(','); //TODO, does gatherParams not pull from the query as well? Why is this OR required?
 
     const elements = request.query._elements?.split(',') || parameters._elements?.split(',');
 
@@ -35,7 +43,8 @@ const bulkExport = async (request, reply) => {
       types: types,
       typeFilter: request.query._typeFilter,
       systemLevelExport: true,
-      elements: elements
+      elements: elements,
+      byPatient: parameters._bySubject === 'Patient'
     };
     await exportQueue.createJob(job).save();
     reply.code(202).header('Content-location', `${process.env.BULK_BASE_URL}/bulkstatus/${clientEntry}`).send();
@@ -65,7 +74,7 @@ const patientBulkExport = async (request, reply) => {
       await validatePatientReferences(parameters.patient, reply);
     }
     request.log.info('Patient >>> $export');
-    const clientEntry = await addPendingBulkExportRequest();
+    const clientEntry = await addPendingBulkExportRequest(parameters._bySubject === 'Patient');
 
     let types = request.query._type?.split(',') || parameters._type?.split(',');
     if (types) {
@@ -83,7 +92,8 @@ const patientBulkExport = async (request, reply) => {
       typeFilter: parameters._typeFilter,
       patient: parameters.patient,
       systemLevelExport: false,
-      elements: elements
+      elements: elements,
+      byPatient: parameters._bySubject === 'Patient'
     };
     await exportQueue.createJob(job).save();
     reply.code(202).header('Content-location', `${process.env.BULK_BASE_URL}/bulkstatus/${clientEntry}`).send();
@@ -122,7 +132,7 @@ const groupBulkExport = async (request, reply) => {
       return splitRef[splitRef.length - 1];
     });
 
-    const clientEntry = await addPendingBulkExportRequest();
+    const clientEntry = await addPendingBulkExportRequest(parameters._bySubject === 'Patient');
     let types = request.query._type?.split(',') || parameters._type?.split(',');
     if (types) {
       types = filterPatientResourceTypes(request, reply, types);
@@ -140,7 +150,8 @@ const groupBulkExport = async (request, reply) => {
       patient: parameters.patient,
       systemLevelExport: false,
       patientIds: patientIds,
-      elements: elements
+      elements: elements,
+      byPatient: parameters._bySubject === 'Patient'
     };
     await exportQueue.createJob(job).save();
     reply.code(202).header('Content-location', `${process.env.BULK_BASE_URL}/bulkstatus/${clientEntry}`).send();
@@ -172,6 +183,16 @@ function validateExportParams(parameters, reply) {
     }
   }
 
+  if (parameters._bySubject && parameters._bySubject !== 'Patient') {
+    reply.code(400).send(
+      createOperationOutcome(`Server does not support the _bySubject parameter for values other than Patient.`, {
+        issueCode: 400,
+        severity: 'error'
+      })
+    );
+    return false;
+  }
+
   if (parameters._type) {
     // type filter is comma-delimited
     const requestTypes = parameters._type.split(',');
@@ -194,6 +215,17 @@ function validateExportParams(parameters, reply) {
         );
       return false;
     }
+    if (parameters._bySubject === 'Patient' && !requestTypes.includes('Patient')) {
+      reply
+        .code(400)
+        .send(
+          createOperationOutcome(
+            `When _type is specified with _bySubject Patient, the Patient type must be included in the _type parameter.`,
+            { issueCode: 400, severity: 'error' }
+          )
+        );
+      return false;
+    }
   }
 
   if (parameters._typeFilter) {
@@ -280,7 +312,7 @@ function validateExportParams(parameters, reply) {
 
   let unrecognizedParams = [];
   Object.keys(parameters).forEach(param => {
-    if (!['_outputFormat', '_type', '_typeFilter', 'patient', '_elements'].includes(param)) {
+    if (!['_outputFormat', '_type', '_typeFilter', 'patient', '_elements', '_bySubject'].includes(param)) {
       unrecognizedParams.push(param);
     }
   });
diff --git a/src/util/exportToNDJson.js b/src/util/exportToNDJson.js
index f50398cf..e667c63c 100644
--- a/src/util/exportToNDJson.js
+++ b/src/util/exportToNDJson.js
@@ -56,7 +56,8 @@ const buildSearchParamList = resourceType => {
  * @param {boolean} systemLevelExport boolean flag from job that signals whether request is for system-level export (determines filtering)
  * @param {Array} patientIds Array of patient ids for patients relevant to this export (undefined if all patients)
  */
-const exportToNDJson = async (clientId, types, typeFilter, patient, systemLevelExport, patientIds, elements) => {
+const exportToNDJson = async jobOptions => {
+  const { clientEntry, types, typeFilter, patient, systemLevelExport, patientIds, elements, byPatient } = jobOptions;
   try {
     const dirpath = './tmp/';
     fs.mkdirSync(dirpath, { recursive: true });
@@ -185,19 +186,73 @@ const exportToNDJson = async (clientId, types, typeFilter, patient, systemLevelE
       return splitRef[splitRef.length - 1];
     });
 
-    let docs = exportTypes.map(async collectionName => {
-      return getDocuments(
-        collectionName,
-        searchParameterQueries[collectionName],
-        valueSetQueries[collectionName],
-        patientParamIds || patientIds,
-        elementsQueries[collectionName]
-      );
-    });
+    let docs;
+    if (byPatient) {
+      // use parameter patient or group patient ids or pull all patient ids from database
+      const ids =
+        patientParamIds ||
+        patientIds ||
+        (
+          await getDocuments(
+            'Patient',
+            searchParameterQueries['Patient'],
+            valueSetQueries['Patient'],
+            patientParamIds || patientIds,
+            elementsQueries['Patient']
+          )
+        ).document.map(p => p.id);
+
+      docs = ids.map(async patientId => {
+        // create patient-based subject header
+        const subjectHeader = {
+          resourceType: 'Parameters',
+          parameter: [
+            {
+              name: 'subject',
+              valueReference: {
+                reference: `Patient/${patientId}`
+              }
+            }
+          ]
+        };
+        // for each patient, collect resource documents from each export type collection
+        const typeDocs = exportTypes.map(async collectionName => {
+          return (
+            await getDocuments(
+              collectionName,
+              searchParameterQueries[collectionName],
+              valueSetQueries[collectionName],
+              [patientId],
+              elementsQueries[collectionName]
+            )
+          ).document;
+        });
+
+        //flatten all exportType arrays into a single array
+        const patientDocuments = (await Promise.all(typeDocs)).flat();
+        // append subject header as the first resource
+        patientDocuments.unshift(subjectHeader);
+        return {
+          // use the patient id as the document name (will be "{patientId}.ndjson")
+          name: patientId,
+          document: patientDocuments
+        };
+      });
+    } else {
+      docs = exportTypes.map(async collectionName => {
+        return getDocuments(
+          collectionName,
+          searchParameterQueries[collectionName],
+          valueSetQueries[collectionName],
+          patientParamIds || patientIds,
+          elementsQueries[collectionName]
+        );
+      });
+    }
     docs = await Promise.all(docs);
     docs.forEach(doc => {
       if (doc.document) {
-        writeToFile(doc.document, doc.collectionName, clientId);
+        writeToFile(doc.document, doc.name, clientEntry);
       }
     });
 
@@ -217,10 +272,10 @@ const exportToNDJson = async (clientId, types, typeFilter, patient, systemLevelE
     */
 
     // mark bulk status job as complete after all files have been written
-    await updateBulkExportStatus(clientId, BULKSTATUS_COMPLETED);
+    await updateBulkExportStatus(clientEntry, BULKSTATUS_COMPLETED);
     return true;
   } catch (e) {
-    await updateBulkExportStatus(clientId, BULKSTATUS_FAILED, { message: e.message, code: 500 });
+    await updateBulkExportStatus(clientEntry, BULKSTATUS_FAILED, { message: e.message, code: 500 });
     return false;
   }
 };
@@ -317,21 +372,21 @@ const getDocuments = async (collectionName, searchParameterQueries, valueSetQuer
     });
   }
 
-  return { document: docs, collectionName: collectionName };
+  return { document: docs, name: collectionName };
 };
 
 /**
  * Writes the contents of a mongo document to an ndjson file with the appropriate resource
  * name, stored in a directory under the client's id
  * @param {Object} doc A mongodb document containing fhir resources
- * @param {string} type The fhir resourceType contained in the mongo document
+ * @param {string} filebase The base filename: either the fhir resourceType or patient id
  * @param {string} clientId The id of the client making the export request
  * @returns
  */
-const writeToFile = function (doc, type, clientId) {
+const writeToFile = function (doc, filebase, clientId) {
   let dirpath = './tmp/' + clientId;
   fs.mkdirSync(dirpath, { recursive: true });
-  const filename = path.join(dirpath, `${type}.ndjson`);
+  const filename = path.join(dirpath, `${filebase}.ndjson`);
 
   let lineCount = 0;
   if (Object.keys(doc).length > 0) {
diff --git a/src/util/mongo.controller.js b/src/util/mongo.controller.js
index 63154da1..1e1efe5f 100644
--- a/src/util/mongo.controller.js
+++ b/src/util/mongo.controller.js
@@ -111,7 +111,7 @@ const findResourcesWithAggregation = async (query, resourceType, options = {}) =
  * which can be queried to get updates on the status of the bulk export
  * @returns the id of the inserted client
  */
-const addPendingBulkExportRequest = async () => {
+const addPendingBulkExportRequest = async (byPatient = false) => {
   const collection = db.collection('bulkExportStatuses');
   const clientId = uuidv4();
   const bulkExportClient = {
@@ -120,7 +120,8 @@ const addPendingBulkExportRequest = async () => {
     numberOfRequestsInWindow: 0,
     timeOfFirstValidRequest: null,
     error: {},
-    warnings: []
+    warnings: [],
+    byPatient: byPatient
   };
   await collection.insertOne(bulkExportClient);
   return clientId;
diff --git a/test/fixtures/testBulkStatus.json b/test/fixtures/testBulkStatus.json
index a8cdcf7c..0dba4567 100644
--- a/test/fixtures/testBulkStatus.json
+++ b/test/fixtures/testBulkStatus.json
@@ -34,5 +34,12 @@
     "status": "Completed",
     "error": {},
     "warnings": []
+  },
+  {
+    "id": "BY_PATIENT",
+    "status": "Completed",
+    "error": {},
+    "warnings": [],
+    "byPatient": true
   }
 ]
diff --git a/test/services/export.service.test.js b/test/services/export.service.test.js
index f1c4c560..061208b5 100644
--- a/test/services/export.service.test.js
+++ b/test/services/export.service.test.js
@@ -293,6 +293,28 @@ describe('Check barebones bulk export logic (failure)', () => {
       });
   });
 
+  test('throws 400 error when "_bySubject" parameter used in system-level export', async () => {
+    await supertest(app.server)
+      .post('/$export')
+      .send({
+        resourceType: 'Parameters',
+        parameter: [
+          {
+            name: '_bySubject',
+            valueString: 'test'
+          }
+        ]
+      })
+      .expect(400)
+      .then(response => {
+        expect(response.body.resourceType).toEqual('OperationOutcome');
+        expect(response.body.issue[0].code).toEqual(400);
+        expect(response.body.issue[0].details.text).toEqual(
+          'The "_bySubject" parameter cannot be used in a system-level export request.'
+        );
+      });
+  });
+
   test('throws 400 error when POST request body is not of resourceType "Parameters"', async () => {
     await supertest(app.server)
       .post('/$export')
@@ -662,6 +684,43 @@ describe('Check group-level export logic (failure)', () => {
   });
 });
 
+describe('Check by subject export logic', () => {
+  beforeEach(async () => {
+    await bulkStatusSetup();
+    await app.ready();
+  });
+
+  test('returns 400 for a _bySubject call that specifies a subject other than Patient', async () => {
+    await supertest(app.server)
+      .get('/Patient/$export?_bySubject=Other')
+      .expect(400)
+      .then(response => {
+        expect(response.body.resourceType).toEqual('OperationOutcome');
+        expect(response.body.issue[0].code).toEqual(400);
+        expect(response.body.issue[0].details.text).toEqual(
+          'Server does not support the _bySubject parameter for values other than Patient.'
+        );
+      });
+  });
+
+  test('returns 400 for a _bySubject call where _type does not include Patient', async () => {
+    await supertest(app.server)
+      .get('/Patient/$export?_bySubject=Patient&_type=Condition')
+      .expect(400)
+      .then(response => {
+        expect(response.body.resourceType).toEqual('OperationOutcome');
+        expect(response.body.issue[0].code).toEqual(400);
+        expect(response.body.issue[0].details.text).toEqual(
+          'When _type is specified with _bySubject Patient, the Patient type must be included in the _type parameter.'
+        );
+      });
+  });
+
+  afterEach(async () => {
+    await cleanUpDb();
+  });
+});
+
 afterAll(async () => {
   await queue.close();
 });

From fa6c8b12b5c62a7ed3c438c4e576668b73f8f1b2 Mon Sep 17 00:00:00 2001
From: LaurenD <laurend@mitre.org>
Date: Mon, 29 Jul 2024 16:09:59 -0400
Subject: [PATCH 2/6] Add/fix tests and add comments

---
 src/util/exportToNDJson.js            |  5 +++-
 src/util/mongo.controller.js          |  1 +
 test/fixtures/testEncounter.json      |  8 ++++++
 test/fixtures/testPatient.json        | 10 ++++++-
 test/fixtures/testServiceRequest.json | 10 ++++++-
 test/services/export.service.test.js  | 41 +++++++++++++++++++++++++++
 test/services/ndjson.service.test.js  |  2 +-
 test/util/exportToNDJson.test.js      | 35 +++++++++++++++--------
 8 files changed, 96 insertions(+), 16 deletions(-)

diff --git a/src/util/exportToNDJson.js b/src/util/exportToNDJson.js
index e667c63c..f3215878 100644
--- a/src/util/exportToNDJson.js
+++ b/src/util/exportToNDJson.js
@@ -49,12 +49,15 @@ const buildSearchParamList = resourceType => {
  * If the _type parameter doesn't exist, the function will simply export all resource types included in the supportedResources list.
  * If the _typeFilter parameter is defined but the _type parameter is *not* defined, the function will export all resource types
  * included in the supportedResources list, but the resource types specified in the _typeFilter query will be filtered.
- * @param {string} clientId  an id to add to the file name so the client making the request can be tracked
+ * @param {Object} jobOptions options object allowing for the following members:
+ * @param {string} clientEntry an id to add to the file name so the client making the request can be tracked
  * @param {Array} types Array of types to be queried for, retrieved from request params
  * @param {string} typeFilter String of comma separated FHIR REST search queries
  * @param {string | Array} patient Patient references from the "patient" param, used to filter results
  * @param {boolean} systemLevelExport boolean flag from job that signals whether request is for system-level export (determines filtering)
  * @param {Array} patientIds Array of patient ids for patients relevant to this export (undefined if all patients)
+ * @param {Array} elements Array of elements parameters that indicate how to omit any unlisted, non-mandatory elements
+ * @param {boolean} byPatient boolean flag from job that signals whether resulting files should be grouped by patient (versus by type)
  */
 const exportToNDJson = async jobOptions => {
   const { clientEntry, types, typeFilter, patient, systemLevelExport, patientIds, elements, byPatient } = jobOptions;
diff --git a/src/util/mongo.controller.js b/src/util/mongo.controller.js
index 1e1efe5f..103ec4a2 100644
--- a/src/util/mongo.controller.js
+++ b/src/util/mongo.controller.js
@@ -109,6 +109,7 @@ const findResourcesWithAggregation = async (query, resourceType, options = {}) =
 /**
  * Called as a result of export request. Adds a new clientId to db
  * which can be queried to get updates on the status of the bulk export
+ * @param {boolean} byPatient indicated whether this export request groups data by patient (versus by type)
  * @returns the id of the inserted client
  */
 const addPendingBulkExportRequest = async (byPatient = false) => {
diff --git a/test/fixtures/testEncounter.json b/test/fixtures/testEncounter.json
index eecf8161..bb56b4ff 100644
--- a/test/fixtures/testEncounter.json
+++ b/test/fixtures/testEncounter.json
@@ -3,5 +3,13 @@
     "id": "testEncounter",
     "subject": {
       "reference": "Patient/testPatient"
+    },
+    "type": {
+      "coding": [
+        {
+          "system": "http://example.com",
+          "code": "example-code-1"
+        }
+      ]
     }
   }
\ No newline at end of file
diff --git a/test/fixtures/testPatient.json b/test/fixtures/testPatient.json
index 96aa3e45..466a918f 100644
--- a/test/fixtures/testPatient.json
+++ b/test/fixtures/testPatient.json
@@ -1,4 +1,12 @@
 {
     "resourceType": "Patient",
-    "id": "testPatient"
+    "id": "testPatient",
+    "maritalStatus": {
+      "coding": [
+        {
+          "system": "http://example.com",
+          "code": "example-code-1"
+        }
+      ]
+    }
   }
\ No newline at end of file
diff --git a/test/fixtures/testServiceRequest.json b/test/fixtures/testServiceRequest.json
index c68c15df..1c087890 100644
--- a/test/fixtures/testServiceRequest.json
+++ b/test/fixtures/testServiceRequest.json
@@ -1,4 +1,12 @@
 {
     "resourceType": "ServiceRequest",
-    "id": "testServiceRequest"
+    "id": "testServiceRequest",
+    "code": {
+      "coding": [
+        {
+          "system": "http://example.com",
+          "code": "example-code-1"
+        }
+      ]
+    }
   }
\ No newline at end of file
diff --git a/test/services/export.service.test.js b/test/services/export.service.test.js
index 061208b5..2e2a567a 100644
--- a/test/services/export.service.test.js
+++ b/test/services/export.service.test.js
@@ -690,6 +690,47 @@ describe('Check by subject export logic', () => {
     await app.ready();
   });
 
+  test('check 202 for Patient bySubject', async () => {
+    const createJobSpy = jest.spyOn(queue, 'createJob');
+    await supertest(app.server)
+      .post('/Patient/$export')
+      .send({
+        resourceType: 'Parameters',
+        parameter: [
+          {
+            name: '_bySubject',
+            valueString: 'Patient'
+          }
+        ]
+      })
+      .expect(202)
+      .then(response => {
+        expect(response.headers['content-location']).toBeDefined();
+        expect(createJobSpy).toHaveBeenCalled();
+      });
+  });
+
+  test('check 202 for Group bySubject', async () => {
+    await createTestResource(testGroup, 'Group');
+    const createJobSpy = jest.spyOn(queue, 'createJob');
+    await supertest(app.server)
+      .post('/Group/testGroup/$export')
+      .send({
+        resourceType: 'Parameters',
+        parameter: [
+          {
+            name: '_bySubject',
+            valueString: 'Patient'
+          }
+        ]
+      })
+      .expect(202)
+      .then(response => {
+        expect(response.headers['content-location']).toBeDefined();
+        expect(createJobSpy).toHaveBeenCalled();
+      });
+  });
+
   test('returns 400 for a _bySubject call that specifies a subject other than Patient', async () => {
     await supertest(app.server)
       .get('/Patient/$export?_bySubject=Other')
diff --git a/test/services/ndjson.service.test.js b/test/services/ndjson.service.test.js
index 1e6d7a91..e863debd 100644
--- a/test/services/ndjson.service.test.js
+++ b/test/services/ndjson.service.test.js
@@ -32,7 +32,7 @@ describe('Test ndjson retrieval from specified url', () => {
 
   test('Retrieve ndjson content for valid url', async () => {
     await createTestResourceWithConnect(testPatient, 'Patient');
-    await exportToNDJson(clientId, mockType);
+    await exportToNDJson({ clientEntry: clientId, types: [mockType] });
     await supertest(app.server)
       .get(`/${clientId}/Patient.ndjson`)
       .expect(200)
diff --git a/test/util/exportToNDJson.test.js b/test/util/exportToNDJson.test.js
index e3293165..d2589ace 100644
--- a/test/util/exportToNDJson.test.js
+++ b/test/util/exportToNDJson.test.js
@@ -10,6 +10,7 @@ const { cleanUpDb, createTestResourceWithConnect } = require('../populateTestDat
 const testPatient = require('../fixtures/testPatient.json');
 const testEncounter = require('../fixtures/testEncounter.json');
 const testCondition = require('../fixtures/testCondition.json');
+const testValueSet = require('../fixtures/valuesets/example-vs-1.json');
 
 const testServiceRequest = require('../fixtures/testServiceRequest.json');
 const app = build();
@@ -22,14 +23,14 @@ const mockType = ['Patient'];
 
 const expectedFileName = './tmp/123456/Patient.ndjson';
 const clientId = '123456';
-const mockTypeFilter = 'Patient?type:in=http://example.com/fhir/ValueSet/test';
+const mockTypeFilter = 'Patient?maritalStatus:in=http://example.com/example-valueset-1';
 
 const complexMockTypeFilter =
-  'Patient?type:in=http://example.com/fhir/ValueSet/test,Encounter?type:in=http://example.com/fhir/ValueSet/test2,ServiceRequest?code:in=http://example.com/fhir/ValueSet/test';
+  'Patient?maritalStatus:in=http://example.com/example-valueset-1,Encounter?type:in=http://example.com/example-valueset-1,ServiceRequest?code:in=http://example.com/example-valueset-1';
 const expectedFileNameEncounter = './tmp/123456/Encounter.ndjson';
 const expectedFileNameServiceRequest = './tmp/123456/ServiceRequest.ndjson';
 const typeFilterWOValueSet = 'Procedure?type:in=http';
-const typeFilterWithInvalidType = 'Dog?type:in=http://example.com/fhir/ValueSet/test';
+const typeFilterWithInvalidType = 'Dog?type:in=http://example.com/example-valueset-1';
 const expectedFileNameInvalidType = './tmp/123456/Dog.ndjson';
 const expectedFileNameWOValueSet = './tmp/123456/Procedure.ndjson';
 describe('check export logic', () => {
@@ -38,6 +39,7 @@ describe('check export logic', () => {
     await createTestResourceWithConnect(testEncounter, 'Encounter');
     await createTestResourceWithConnect(testCondition, 'Condition');
     await createTestResourceWithConnect(testServiceRequest, 'ServiceRequest');
+    await createTestResourceWithConnect(testValueSet, 'ValueSet');
     console.log('created test resource');
   });
 
@@ -59,33 +61,42 @@ describe('check export logic', () => {
   });
 
   describe('exportToNDJson', () => {
+    beforeEach(async () => {
+      fs.rmSync('./tmp/123456', { recursive: true, force: true });
+    });
     test('Expect folder created and export successful when _type parameter is retrieved from request', async () => {
-      await exportToNDJson(clientId, mockType);
+      await exportToNDJson({ clientEntry: clientId, types: mockType });
       expect(fs.existsSync(expectedFileName)).toBe(true);
     });
     test('Expect folder created and export successful when _type parameter is not present in request', async () => {
-      await exportToNDJson(clientId);
+      await exportToNDJson({ clientEntry: clientId });
       expect(fs.existsSync(expectedFileName)).toBe(true);
     });
 
-    test('Expect folder created and export successful when _typeFilter  parameter is retrieved from request', async () => {
-      await exportToNDJson(clientId, mockType, mockTypeFilter);
+    test('Expect folder created and export successful when _typeFilter parameter is retrieved from request', async () => {
+      await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: mockTypeFilter });
       expect(fs.existsSync(expectedFileName)).toBe(true);
     });
-    test('Expect folder created and export successful when _typeFilter  parameter is retrieved from request', async () => {
-      await exportToNDJson(clientId, mockType, complexMockTypeFilter);
+    test('Expect folder created and export successful when complex _typeFilter parameter is retrieved from request', async () => {
+      await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: complexMockTypeFilter });
       expect(fs.existsSync(expectedFileName)).toBe(true);
       expect(fs.existsSync(expectedFileNameEncounter)).toBe(true);
       expect(fs.existsSync(expectedFileNameServiceRequest)).toBe(true);
     });
     test('Expect folder created and export to fail when _typeFilter parameter is retrieved from request and contains an invalid param', async () => {
-      await exportToNDJson(clientId, mockType, typeFilterWithInvalidType);
+      await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: typeFilterWithInvalidType });
+      expect(fs.existsSync('./tmp/123456')).toBe(true);
       expect(fs.existsSync(expectedFileNameInvalidType)).toBe(false);
     });
-    test('Expect folder created and export to fail when _typeFilter  parameter is retrieved from request but the value set is invalid', async () => {
-      await exportToNDJson(clientId, mockType, typeFilterWOValueSet);
+    test('Expect export to fail when _typeFilter parameter is retrieved from request but the value set is invalid', async () => {
+      await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: typeFilterWOValueSet });
+      // TODO: this currently fails and does not create a folder (throws error instead). Do we want a more graceful response?
       expect(fs.existsSync(expectedFileNameWOValueSet)).toBe(false);
     });
+    test('Expect folder created and export successful when _bySubject parameter is retrieved from request', async () => {
+      await exportToNDJson({ clientEntry: clientId, types: mockType, typeFilter: mockTypeFilter, byPatient: true });
+      expect(fs.existsSync('./tmp/123456/testPatient.ndjson')).toBe(true);
+    });
   });
 
   describe('patientsQueryForType', () => {

From 5bba3572c254791c1f68c831028f3b56fc3019f2 Mon Sep 17 00:00:00 2001
From: lmd59 <laurend@mitre.org>
Date: Wed, 31 Jul 2024 17:28:39 -0400
Subject: [PATCH 3/6] Apply suggestions from code review

Typo and remove unnecessary parameter

Co-authored-by: Elsa Perelli <eperelli@mitre.org>
---
 src/services/export.service.js | 2 +-
 src/util/mongo.controller.js   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/services/export.service.js b/src/services/export.service.js
index 59959517..ae606db6 100644
--- a/src/services/export.service.js
+++ b/src/services/export.service.js
@@ -31,7 +31,7 @@ const bulkExport = async (request, reply) => {
   }
   if (validateExportParams(parameters, reply)) {
     request.log.info('Base >>> $export');
-    const clientEntry = await addPendingBulkExportRequest(parameters._bySubject === 'Patient');
+    const clientEntry = await addPendingBulkExportRequest();
 
     const types = request.query._type?.split(',') || parameters._type?.split(','); //TODO, does gatherParams not pull from the query as well? Why is this OR required?
 
diff --git a/src/util/mongo.controller.js b/src/util/mongo.controller.js
index 103ec4a2..c6459a40 100644
--- a/src/util/mongo.controller.js
+++ b/src/util/mongo.controller.js
@@ -109,7 +109,7 @@ const findResourcesWithAggregation = async (query, resourceType, options = {}) =
 /**
  * Called as a result of export request. Adds a new clientId to db
  * which can be queried to get updates on the status of the bulk export
- * @param {boolean} byPatient indicated whether this export request groups data by patient (versus by type)
+ * @param {boolean} byPatient indicates whether this export request groups data by patient (versus by type)
  * @returns the id of the inserted client
  */
 const addPendingBulkExportRequest = async (byPatient = false) => {

From 68b892ac804dd9f26b89febc5ffe48e4c10c610b Mon Sep 17 00:00:00 2001
From: LaurenD <laurend@mitre.org>
Date: Fri, 2 Aug 2024 09:08:06 -0400
Subject: [PATCH 4/6] Update import manifest for kickoff-import

---
 src/services/bulkstatus.service.js | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/services/bulkstatus.service.js b/src/services/bulkstatus.service.js
index 066b2ca8..38d4e69e 100644
--- a/src/services/bulkstatus.service.js
+++ b/src/services/bulkstatus.service.js
@@ -44,6 +44,18 @@ async function kickoffImport(request, reply) {
           ]
         };
       });
+      // check if bulk status is byPatient, if so, add inputdetails
+      if (bulkStatus.byPatient) {
+        importManifest.parameter.push({
+          name: 'inputDetails',
+          part: [
+            {
+              name: 'subjectType',
+              valueCode: 'Patient'
+            }
+          ]
+        });
+      }
 
       // TODO: add provenance?
       const headers = {

From dd61375d428efa960d733e8a1d2d867e9ffd3281 Mon Sep 17 00:00:00 2001
From: LaurenD <laurend@mitre.org>
Date: Mon, 5 Aug 2024 10:21:33 -0400
Subject: [PATCH 5/6] Simplify parameter gathering

---
 src/services/export.service.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/services/export.service.js b/src/services/export.service.js
index ae606db6..2072fcb0 100644
--- a/src/services/export.service.js
+++ b/src/services/export.service.js
@@ -33,9 +33,9 @@ const bulkExport = async (request, reply) => {
     request.log.info('Base >>> $export');
     const clientEntry = await addPendingBulkExportRequest();
 
-    const types = request.query._type?.split(',') || parameters._type?.split(','); //TODO, does gatherParams not pull from the query as well? Why is this OR required?
+    const types = parameters._type?.split(',');
 
-    const elements = request.query._elements?.split(',') || parameters._elements?.split(',');
+    const elements = parameters._elements?.split(',');
 
     // Enqueue a new job into Redis for handling
     const job = {

From 28e5af2ab9a688623443afa9f3d784574732ff9f Mon Sep 17 00:00:00 2001
From: LaurenD <laurend@mitre.org>
Date: Tue, 6 Aug 2024 09:26:53 -0400
Subject: [PATCH 6/6] Remove TODO and add note comment

---
 test/util/exportToNDJson.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/util/exportToNDJson.test.js b/test/util/exportToNDJson.test.js
index d2589ace..9cb238be 100644
--- a/test/util/exportToNDJson.test.js
+++ b/test/util/exportToNDJson.test.js
@@ -84,13 +84,13 @@ describe('check export logic', () => {
       expect(fs.existsSync(expectedFileNameServiceRequest)).toBe(true);
     });
     test('Expect folder created and export to fail when _typeFilter parameter is retrieved from request and contains an invalid param', async () => {
+      // Note: invalid types are checked in the export service
       await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: typeFilterWithInvalidType });
       expect(fs.existsSync('./tmp/123456')).toBe(true);
       expect(fs.existsSync(expectedFileNameInvalidType)).toBe(false);
     });
     test('Expect export to fail when _typeFilter parameter is retrieved from request but the value set is invalid', async () => {
       await exportToNDJson({ clientEntry: clientId, type: mockType, typeFilter: typeFilterWOValueSet });
-      // TODO: this currently fails and does not create a folder (throws error instead). Do we want a more graceful response?
       expect(fs.existsSync(expectedFileNameWOValueSet)).toBe(false);
     });
     test('Expect folder created and export successful when _bySubject parameter is retrieved from request', async () => {