fixed feedback

Author: avigoldman

lib/transmissions.js

@@ -60,9 +60,6 @@ module.exports = function(client) {
      * @returns {Promise}
      */
     send: function(transmission, options, callback) {
-
-      const modifiedTransmission = _.cloneDeep(transmission);
-
       // Handle optional options argument
       if (typeof options === 'function') {
         callback = options;
@@ -73,16 +70,11 @@ module.exports = function(client) {
         return Promise.reject(new Error('transmission object is required')).asCallback(callback);
       }
 
-      // format the payload if we are given an array of recipients
-      if (_.has(modifiedTransmission, 'recipients') &&
-          _.isArray(modifiedTransmission.recipients) &&
-          !_.has(modifiedTransmission.recipients[0], 'list_id')) {
-        formatPayload(modifiedTransmission);
-      }
+      transmission = formatPayload(transmission);
 
       const reqOpts = {
         uri: api,
-        json: modifiedTransmission,
+        json: transmission,
         qs: options
       };
 
@@ -92,7 +84,14 @@ module.exports = function(client) {
 
 };
 
-function formatPayload(transmission) {
+function formatPayload(originalTransmission) {
+  const transmission = _.cloneDeep(originalTransmission);
+
+  // don't format the payload if we are not given an array of recipients
+  if (!_.isArray(transmission.recipients)) {
+    return transmission;
+  }
+
   // format all the original recipients to be in the object format
   transmission.recipients = _.map(transmission.recipients, (recipient) => {
     recipient.address = addressToObject(recipient.address);
@@ -101,53 +100,53 @@ function formatPayload(transmission) {
   });
 
   // add the CC headers
-  if (_.has(transmission, 'cc') && _.isArray(transmission.cc)) {
-    addCCHeader(transmission);
+  if (_.isArray(transmission.cc)) {
+    _.set(transmission, 'content.headers.CC', generateCCHeader(transmission));
   }
 
-  addListToRecipients(transmission, 'cc');
-  addListToRecipients(transmission, 'bcc');
+  const headerTo = generateHeaderTo(transmission.recipients);
+
+  transmission.recipients = addListToRecipients(transmission, 'cc', headerTo);
+  transmission.recipients = addListToRecipients(transmission, 'bcc', headerTo);
+
+  delete transmission.cc;
+  delete transmission.bcc;
 
   return transmission;
 }
 
-function addListToRecipients(transmission, listName) {
-  if (_.has(transmission, listName) && _.isArray(transmission[listName])) {
-    const headerTo = generateHeaderTo(transmission.recipients);
-    const list = transmission[listName];
-
-    transmission.recipients = transmission.recipients.concat(_.map(list, (recipient) => {
-      recipient.address = addressToObject(recipient.address);
+function addListToRecipients(transmission, listName, headerTo) {
+  if (!_.isArray(transmission[listName])) {
+    return transmission.recipients;
+  }
 
-      recipient.address.header_to = headerTo;
+  return transmission.recipients.concat(_.map(transmission[listName], (recipient) => {
+    recipient.address = addressToObject(recipient.address);
 
-      // remove name from address - name is only put in the header for cc and not at all for bcc
-      if (_.has(recipient.address, 'name')) {
-        delete recipient.address.name;
-      }
+    recipient.address.header_to = headerTo;
 
-      return recipient;
-    }));
+    // remove name from address - name is only put in the header for cc and not at all for bcc
+    if (_.has(recipient.address, 'name')) {
+      delete recipient.address.name;
+    }
 
-    delete transmission[listName];
-  }
+    return recipient;
+  }));
 }
 
-function addCCHeader(transmission) {
-  // default the headers to a blank object
-  transmission.content.headers = transmission.content.headers || {};
-
-  transmission.content.headers.CC = _.map(transmission.cc, (ccRecipient) => addressToString(ccRecipient.address)).join(', ');
+function generateCCHeader(transmission) {
+  return _.map(transmission.cc, (ccRecipient) => addressToString(ccRecipient.address)).join(', ');
 }
 
 function generateHeaderTo(recipients) {
+  // if a recipient has a header_to then it is cc'd or bcc'd and we don't want it in the header_to value
   const originalRecipients = _.filter(recipients, (recipient) => !_.has(recipient.address, 'header_to'));
 
   return _.map(originalRecipients, (recipient) => addressToString(recipient.address)).join(', ');
 }
 
 function addressToString(address) {
-  if (!_.isString(address)) {
+  if (_.isPlainObject(address)) {
     if (_.has(address, 'name')) {
       address = `"${address.name}" <${address.email}>`;
     } else {
@@ -164,24 +163,15 @@ function addressToObject(address) {
   if (_.isString(address)) {
     addressObject = {};
 
-    if (isEmail(address)) {
-      addressObject.email = address;
-    } else {
-      const matches = /"?(.[^"]*)?"?\s*<(.+)>/gi.exec(address);
+    const matches = /"?(.[^"]*)?"?\s*<(.+)>/gi.exec(address);
 
-      if (matches) {
-        addressObject.name = matches[1];
-        addressObject.email = matches[2];
-      } else {
-        throw new Error(`Invalid address format: ${address}`);
-      }
+    if (matches) {
+      addressObject.name = matches[1];
+      addressObject.email = matches[2];
+    } else {
+      addressObject.email = address;
     }
   }
 
   return addressObject;
 }
-
-function isEmail(email) {
-  const emailRegex = /^[-a-z0-9~!$%^&*_=+}{\'?]+(\.[-a-z0-9~!$%^&*_=+}{\'?]+)*@([a-z0-9_][-a-z0-9_]*(\.[-a-z0-9_]+[a-z][a-z])|([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}))(:[0-9]{1,5})?$/i;
-  return emailRegex.test(email);
-}

test/spec/transmissions.spec.js

@@ -201,11 +201,9 @@ describe('Transmissions Library', function() {
 
     it('should allow a list_id and template through', function() {
       var transmission = {
-        recipients: [
-          {
-            list_id: 'my-list-id'
-          }
-        ],
+        recipients: {
+          list_id: 'my-list-id'
+        },
         content: {
           template_id: 'my-template-id'
         }
@@ -217,19 +215,6 @@ describe('Transmissions Library', function() {
         });
     });
 
-    it('should throw an error due to an invalid string format', function() {
-      var transmission = {
-        recipients: [
-          {
-            address: '"Bob" [email protected]>'
-          }
-        ]
-      };
-
-      expect(function() {
-        transmissions.send(transmission);
-      }).to.throw(Error);
-    });
 
     it('should convert cc to the correct recipients and headers', function() {
       return transmissions.send(ccTransmission)