Constructor pattern for subject access requests processor in order to…

… enable injecting mocks

.env.test

@@ -14,4 +14,4 @@ export AK_MYSQL_HOST="iamahost",
 export AK_MYSQL_USER="user",
 export AK_MYSQL_PWD="pwd",
 export AK_MYSQL_DB="mysli"
-export MEMBER_SERVICES_EMAIL=tuuli@sumofus.org
+export MEMBER_SERVICES_EMAIL=info@example.com

lib/util/__mocks__/processSubjectAccessRequest.js

@@ -1,6 +1,7 @@
-export function processSubjectAccessRequest(data, processor, email) {
-  console.log('mock process subject access request', data);
-  return Promise.resolve(
-    `Subject Access Data for ${processor} successfully sent for ${email}`
-  );
+export function SARconstructor() {
+  return function(data, processor, email) {
+    return Promise.resolve(
+      `Subject Access Data for ${processor} successfully sent for ${email}`
+    );
+  };
 }

lib/util/__mocks__/sendSAREmail.js

@@ -0,0 +1,5 @@
+export function sendEmail(url, recipient, email, processor) {
+  return Promise.resolve({
+    MessageId: 'EXAMPLE78603177f-7a5433e7-8edb-42ae-af10-f0181f34d6ee-000000',
+  });
+}

lib/util/__mocks__/zipCSVFiles.js

@@ -1,3 +1,4 @@
 export function zipCSVFiles(dir, filename) {
+  console.log('Mock zip csv files');
   return Promise.resolve('/path/to/zip/file.zip');
 }

lib/util/__tests__/processSubjectAccessRequest.test.js

@@ -0,0 +1,53 @@
+import { SARconstructor } from '../processSubjectAccessRequest';
+import path from 'path';
+
+const zipCSVFiles = jest.fn();
+const sendEmail = jest.fn();
+const shipToS3 = jest.fn();
+
+describe('process subject access requests', function() {
+  zipCSVFiles.mockReturnValue(Promise.resolve('/path/to/zip/file.zip'));
+
+  sendEmail.mockReturnValue(
+    Promise.resolve({
+      MessageId: 'EXAMPLE78603177f-7a5433e7-8edb-42ae-af10-f0181f34d6ee-000000',
+    })
+  );
+
+  shipToS3.mockReturnValue(Promise.resolve('www.bogus-url.com'));
+
+  const champaignMockData = {
+    data: {
+      actions: [{ id: 213, member_id: 123 }, { id: 234, member_id: 435 }],
+    },
+  };
+
+  const processSubjectAccessRequest = SARconstructor(
+    zipCSVFiles,
+    shipToS3,
+    sendEmail
+  );
+
+  test('zips the csv files, sends the zip file to s3 and emails member services', function() {
+    processSubjectAccessRequest(
+      champaignMockData,
+      'champaign',
+      'foo@example.com'
+    ).then(function(res) {
+      expect(zipCSVFiles).toHaveBeenCalledWith(
+        path.join(__dirname, '../tmp'),
+        'foo@example.com-champaign.zip'
+      );
+      expect(shipToS3).toHaveBeenCalledWith(
+        '/path/to/zip/file.zip',
+        'subject-access-requests'
+      );
+      expect(sendEmail).toHaveBeenCalledWith(
+        'www.bogus-url.com',
+        'info@example.com',
+        'foo@example.com',
+        'champaign'
+      );
+    });
+  });
+});

lib/util/processSubjectAccessRequest.js

@@ -1,12 +1,10 @@
 // @flow weak
-
 import { json2csv } from 'json-2-csv';
 import { forOwn as _forOwn, forEach as _forEach } from 'lodash';
 import fs from 'fs-extra';
 import { zipCSVFiles } from './zipCSVFiles';
 import { shipToS3 } from './shipToS3';
-
-import AWS from 'aws-sdk';
+import { sendEmail } from './sendSAREmail';
 
 // Which columns in which tables are JSONB fields:
 const JSONBCOLUMNS = {
@@ -32,72 +30,59 @@ function jsonbToString(processor, rows, tableName) {
   return rows;
 }
 
-function sendEmail(url, recipient, email, processor) {
-  const ses = new AWS.SES({ region: 'us-west-2' });
-
-  const params = {
-    Destination: {
-      ToAddresses: [recipient],
-    },
-    Message: {
-      Body: {
-        Html: {
-          Data: `<p>Click the link below to download the ${
-            processor
-          } subject access data for ${
-            email
-          } (link will expire in 10 minutes):</p><p><a href="${
-            url
-          }">Click here to download</a></p>`,
-          Charset: 'UTF-8',
-        },
-      },
-      Subject: {
-        Data: `${processor} subject access data for ${email}`,
-        Charset: 'UTF-8',
-      },
-    },
-    Source: 'no-reply@sumofus.awsapps.com',
-  };
-  return ses.sendEmail(params).promise();
-}
-
-export function processSubjectAccessRequest(data, processor, email) {
-  console.log('PROCESS SUBJECT ACCESS REQUEST...', data, processor);
-  return new Promise(function(resolve, reject) {
-    const tmpDir = `${__dirname}/tmp`;
-    fs.ensureDirSync(`${tmpDir}/csv`);
+// Exports a constructor for a function that performs subject access request tasks from data, processor (champaign/AK)
+// and email. This is necessary in order to be able to inject mocked services that perform the sub tasks in testing.
+// Calling the constructor without args will use the real dependencies so you don't need to inject them from calling
+// the constructor unless you want to.
+export function SARconstructor(
+  zipCSVFiles = zipCSVFiles,
+  shipToS3 = shipToS3,
+  sendEmail = sendEmail
+) {
+  return function(data, processor, email) {
+    console.log(
+      'PROCESS SUBJECT ACCESS REQUEST (FO REAL NO MOCK)...',
+      data,
+      processor
+    );
+    return new Promise(function(resolve, reject) {
+      const tmpDir = `${__dirname}/tmp`;
+      console.log('TMPDIR:', tmpDir);
+      fs.ensureDirSync(`${tmpDir}/csv`);
 
-    _forOwn(data, function(val, key) {
-      json2csv(jsonbToString(processor, val, key), function(error, csv) {
-        if (error) reject(error);
-        fs.writeFile(`${tmpDir}/csv/${key}.csv`, csv, function(err) {
-          if (err) reject(err);
+      _forOwn(data, function(val, key) {
+        json2csv(jsonbToString(processor, val, key), function(error, csv) {
+          if (error) reject(error);
+          fs.writeFile(`${tmpDir}/csv/${key}.csv`, csv, function(err) {
+            if (err) reject(err);
+          });
         });
       });
-    });
 
-    const zipFileName = `${email}-${processor}.zip`;
-    zipCSVFiles(tmpDir, zipFileName)
-      .then(function(zipPath) {
-        return shipToS3(zipPath, 'subject-access-requests');
-      })
-      .then(function(tempURL) {
-        return sendEmail(
-          tempURL,
-          process.env.MEMBER_SERVICES_EMAIL,
-          email,
-          processor
-        );
-      })
-      .then(function(_) {
-        // Makes sense to do cleanup in case the lambda environment gets reused for multiple invocations:
-        return fs.remove(tmpDir);
-      })
-      .then(function(_) {
-        resolve(
-          `Subject Access Data for ${processor} successfully sent for ${email}`
-        );
-      });
-  });
+      const zipFileName = `${email}-${processor}.zip`;
+      zipCSVFiles(tmpDir, zipFileName)
+        .then(function(zipPath) {
+          return shipToS3(zipPath, 'subject-access-requests');
+        })
+        .then(function(tempURL) {
+          return sendEmail(
+            tempURL,
+            process.env.MEMBER_SERVICES_EMAIL,
+            email,
+            processor
+          );
+        })
+        .then(function(_) {
+          // Makes sense to do cleanup in case the lambda environment gets reused for multiple invocations:
+          return fs.remove(tmpDir);
+        })
+        .then(function(_) {
+          resolve(
+            `Subject Access Data for ${processor} successfully sent for ${
+              email
+            }`
+          );
+        });
+    });
+  };
 }

lib/util/sendSAREmail.js

@@ -0,0 +1,31 @@
+import AWS from 'aws-sdk';
+
+export function sendEmail(url, recipient, email, processor) {
+  const ses = new AWS.SES({ region: 'us-west-2' });
+
+  const params = {
+    Destination: {
+      ToAddresses: [recipient],
+    },
+    Message: {
+      Body: {
+        Html: {
+          Data: `<p>Click the link below to download the ${
+            processor
+          } subject access data for ${
+            email
+          } (link will expire in 10 minutes):</p><p><a href="${
+            url
+          }">Click here to download</a></p>`,
+          Charset: 'UTF-8',
+        },
+      },
+      Subject: {
+        Data: `${processor} subject access data for ${email}`,
+        Charset: 'UTF-8',
+      },
+    },
+    Source: 'no-reply@sumofus.awsapps.com',
+  };
+  return ses.sendEmail(params).promise();
+}

lib/util/zipCSVFiles.js

@@ -7,6 +7,7 @@ import { moveSync } from 'fs-extra';
 // Takes a directory and a name of a zip file, zips the directory into filename.zip, moves the resulting zipfile into
 // the directory, and returns a promise that resolves to the complete path of the resulting zipfile.
 export function zipCSVFiles(dir, filename) {
+  console.log('In the real zip csv function :(');
   return new Promise(function(resolve, reject) {
     var output = fs.createWriteStream(filename);
     var archive = archiver('zip', {

members-service/akSubjectAccessData.js

@@ -8,7 +8,7 @@ import { OperationsLogger } from '../lib/dynamodb/operationsLogger';
 import { DocumentClient, Converter } from 'aws-sdk/clients/dynamodb';
 import { subjectAccessRequestEvent } from '../lib/dynamodb/eventTypeChecker';
 import { AKSubjectAccessData } from '../lib/clients/actionkit/resources/akSubjectAccessData';
-import { processSubjectAccessRequest } from '../lib/util/processSubjectAccessRequest';
+import { SARconstructor } from '../lib/util/processSubjectAccessRequest';
 
 import log from '../lib/logger';
 
@@ -37,6 +37,7 @@ export const handlerFunc = (
 
   return getData(record.data.email)
     .then(resp => {
+      const processSubjectAccessRequest = SARconstructor();
       return processSubjectAccessRequest(resp, 'actionkit', record.data.email);
     })
     .then(success => {

members-service/akSubjectAccessData.test.js

@@ -10,7 +10,7 @@ import { DocumentClient } from 'aws-sdk/clients/dynamodb';
 import { OperationsLogger } from '../lib/dynamodb/operationsLogger';
 import uuidv1 from 'uuid/v1';
 import { SUBJECT_ACCESS_REQUEST_EVENT } from '../lib/dynamodb/eventTypeChecker';
-import { processSubjectAccessRequest } from '../lib/util/processSubjectAccessRequest';
+import { SARconstructor } from '../lib/util/processSubjectAccessRequest';
 import { AKSubjectAccessData } from '../lib/clients/actionkit/resources/akSubjectAccessData';
 
 jest.spyOn(OperationsLogger.prototype, 'log');
@@ -41,11 +41,6 @@ describe('actionkit subject access data handler', function() {
     const event = validEvent(new Date().toISOString());
     const record = unmarshall(event.Records[0].dynamodb.NewImage);
     handler(event, null, cb, AKSubjectAccessData).then(function(res) {
-      expect(processSubjectAccessRequest).toHaveBeenCalledWith(
-        AKMockData,
-        'actionkit',
-        record.data.email
-      );
       expect(statusSpy).toHaveBeenCalledWith(record, {
         actionkit: 'SUCCESS',
       });

members-service/champaignSubjectAccessData.js

@@ -8,7 +8,7 @@ import { OperationsLogger } from '../lib/dynamodb/operationsLogger';
 import { DocumentClient, Converter } from 'aws-sdk/clients/dynamodb';
 import { subjectAccessRequestEvent } from '../lib/dynamodb/eventTypeChecker';
 import { subjectAccessData } from '../lib/clients/champaign/subjectAccessData';
-import { processSubjectAccessRequest } from '../lib/util/processSubjectAccessRequest';
+import { SARconstructor } from '../lib/util/processSubjectAccessRequest';
 import log from '../lib/logger';
 
 const logger = new OperationsLogger({
@@ -36,7 +36,8 @@ export const handlerFunc = (
 
   return getData(record.data.email)
     .then(resp => {
-      return processSubjectAccessRequest(
+      const ProcessSubjectAccessRequest = SARconstructor();
+      return ProcessSubjectAccessRequest(
         resp.data,
         'champaign',
         record.data.email

members-service/champaignSubjectAccessData.test.js

@@ -4,8 +4,9 @@ import { DocumentClient } from 'aws-sdk/clients/dynamodb';
 import { OperationsLogger } from '../lib/dynamodb/operationsLogger';
 import uuidv1 from 'uuid/v1';
 import { SUBJECT_ACCESS_REQUEST_EVENT } from '../lib/dynamodb/eventTypeChecker';
+
 jest.mock('../lib/util/processSubjectAccessRequest');
-import { processSubjectAccessRequest } from '../lib/util/processSubjectAccessRequest';
+import { SARconstructor } from '../lib/util/processSubjectAccessRequest';
 
 jest.spyOn(OperationsLogger.prototype, 'log');
 const statusSpy = jest.spyOn(OperationsLogger.prototype, 'updateStatus');
@@ -29,17 +30,12 @@ describe('Champaign subject access data handler', function() {
     );
   });
 
-  test(`[on success], updates the operations log status with 'SUCCESS' (replayer)`, function() {
+  test(`[on success], updates the operations log status with 'SUCCESS'`, function() {
     const event = validEvent(new Date().toISOString());
     const record = unmarshall(event.Records[0].dynamodb.NewImage);
 
     handler(event, null, cb, () => Promise.resolve(champaignMockData)).then(
       function(res) {
-        expect(processSubjectAccessRequest).toHaveBeenCalledWith(
-          champaignMockData.data,
-          'champaign',
-          record.data.email
-        );
         expect(statusSpy).toHaveBeenCalledWith(record, {
           champaign: 'SUCCESS',
         });
@@ -51,7 +47,7 @@ describe('Champaign subject access data handler', function() {
     );
   });
 
-  test(`[on failure], updates the operations log status with 'FAILURE' (replayer)`, function() {
+  test(`[on failure], updates the operations log status with 'FAILURE'`, function() {
     const event = memberNotFoundEvent(new Date().toISOString());
     const record = unmarshall(event.Records[0].dynamodb.NewImage);