Merge branch 'master' into production

config/lib/helpers/contentfulEntry.js

@@ -1,5 +1,10 @@
 'use strict';
 
+const templateFieldTypes = {
+  text: 'text',
+  transition: 'transition',
+};
+
 /**
  * This maps the fields in our Contentful types into broadcast, topic, and defaultTopicTriggers.
  *
@@ -26,26 +31,57 @@ module.exports = {
     askVotingPlanStatus: {
       type: 'askVotingPlanStatus',
       broadcastable: true,
-      templates: [
-        'votingPlanStatusCantVote',
-        'votingPlanStatusNotVoting',
-        'votingPlanStatusVoted',
-      ],
+      templates: {
+        // These template names correspond to the macros that get executed if user matches a trigger
+        // within the ask_voting_plan_status topic in Gambit Conversations.
+        // @see https://github.com/DoSomething/gambit-conversations/blob/master/brain/topics/askVotingPlanStatus.rive
+        votingPlanStatusCantVote: {
+          fieldName: 'cantVoteTransition',
+          fieldType: templateFieldTypes.transition,
+          name: 'votingPlanStatusCantVote',
+        },
+        votingPlanStatusNotVoting: {
+          fieldName: 'notVotingTransition',
+          fieldType: templateFieldTypes.transition,
+          name: 'votingPlanStatusNotVoting',
+        },
+        votingPlanStatusVoted: {
+          fieldName: 'votedTransition',
+          fieldType: templateFieldTypes.transition,
+          name: 'votingPlanStatusVoted',
+        },
+      },
     },
     askYesNo: {
       type: 'askYesNo',
       broadcastable: true,
-      templates: [
-        'saidYes',
-        'saidNo',
-        'invalidAskYesNoResponse',
-      ],
+      templates: {
+        invalidAskYesNoResponse: {
+          fieldName: 'invalidAskYesNoResponse',
+          fieldType: templateFieldTypes.text,
+          name: 'invalidAskYesNoResponse',
+        },
+        saidYes: {
+          fieldName: 'yesTransition',
+          fieldType: templateFieldTypes.transition,
+          name: 'saidYes',
+        },
+        saidNo: {
+          fieldName: 'noTransition',
+          fieldType: templateFieldTypes.transition,
+          name: 'saidNo',
+        },
+      },
     },
     autoReply: {
       type: 'autoReply',
-      templates: [
-        'autoReply',
-      ],
+      templates: {
+        autoReply: {
+          fieldName: 'autoReply',
+          fieldType: templateFieldTypes.text,
+          name: 'autoReply',
+        },
+      },
     },
     autoReplyBroadcast: {
       type: 'autoReplyBroadcast',
@@ -63,6 +99,7 @@ module.exports = {
     },
     photoPostConfig: {
       type: 'photoPostConfig',
+      // TODO: Refactor photoPostConfig in config/lib/helpers/topic here to DRY.
       postType: 'photo',
     },
     textPostBroadcast: {
@@ -71,6 +108,7 @@ module.exports = {
     },
     textPostConfig: {
       type: 'textPostConfig',
+      // TODO: Move textPostConfig in config/lib/helpers/topic here to DRY.
       postType: 'text',
     },
     // Legacy types:
@@ -87,4 +125,5 @@ module.exports = {
       broadcastable: true,
     },
   },
+  templateFieldTypes,
 };

lib/helpers/broadcast.js

@@ -75,7 +75,7 @@ async function parseBroadcastFromContentfulEntry(contentfulEntry) {
   const [message, templates] = await Promise.all([
     // Parse the outbound broadcast message to send.
     helpers.contentfulEntry
-      .getMessageTemplateFromContentfulEntryAndTemplateName(contentfulEntry, data.type),
+      .getMessageTemplate(contentfulEntry, data.type),
     // Parse topic templates if they exist.
     helpers.contentfulEntry.getTopicTemplates(contentfulEntry),
   ]);

lib/helpers/campaign.js

@@ -96,7 +96,7 @@ async function parseCampaignConfig(contentfulEntry) {
   }
 
   data.templates.webSignup = await helpers.contentfulEntry
-    .getMessageTemplateFromContentfulEntryAndTemplateName(webSignupEntry, 'webSignup');
+    .getMessageTemplate(webSignupEntry, 'webSignup');
 
   return data;
 }

lib/helpers/contentfulEntry.js

@@ -104,10 +104,10 @@ function getSummaryFromContentfulEntry(contentfulEntry) {
 
 /**
  * @param {Object} contentfulEntry
- * @param {String} templateName
+ * @param {String} templateName - optional
  * @return {Promise}
  */
-async function getMessageTemplateFromContentfulEntryAndTemplateName(contentfulEntry, templateName) {
+async function getMessageTemplate(contentfulEntry, templateName) {
   if (!contentfulEntry || !contentfulEntry.fields) {
     return {};
   }
@@ -123,35 +123,66 @@ async function getMessageTemplateFromContentfulEntryAndTemplateName(contentfulEn
 }
 
 /**
+ * TODO: We need pass a parameter to force clearing cache for all topics within the templates.
  * @param {Object} contentfulEntry
  * @return {Promise}
  */
 async function getTopicTemplates(contentfulEntry) {
   const contentType = contentful.getContentTypeFromContentfulEntry(contentfulEntry);
-  const templateFieldNames = config.contentTypes[contentType].templates;
+  const templatesConfig = config.contentTypes[contentType].templates;
   const templates = {};
 
-  if (!contentfulEntry || !templateFieldNames) {
+  if (!contentfulEntry || !templatesConfig) {
     return templates;
   }
 
-  const fields = contentfulEntry.fields;
-  /* eslint-disable */
-  // TODO: Disabling our linter probably isn't best way to handle this, find topics in parallel
-  // instead of serially per https://github.com/DoSomething/gambit-campaigns/pull/1088/files#r220969795
-  for (const fieldName of templateFieldNames) {
-    const templateTopicEntry = fields[`${fieldName}Topic`];
-    templates[fieldName] = {
-      text: fields[fieldName],
-      topic: templateTopicEntry ? await helpers.topic
-        .getById(contentful.getContentfulIdFromContentfulEntry(templateTopicEntry)) : {},
-    };
+  const templateNames = Object.keys(templatesConfig);
+  const promises = templateNames.map((templateName) => {
+    const fieldValue = contentfulEntry.fields[templatesConfig[templateName].fieldName];
+    const isTransition = module.exports.isTransitionTemplate(contentType, templateName);
+    return isTransition ? module.exports.getMessageTemplate(fieldValue) : Promise.resolve({
+      text: fieldValue,
+      topic: {},
+    });
+  });
+  const messageTemplates = await Promise.all(promises);
+  templateNames.forEach((templateName, index) => {
+    templates[templateName] = messageTemplates[index];
+  });
+
+  // The first iteration of askYesNo did not use transition fields for saidYes and saidNo templates;
+  // it had separate text and topic fields per template. This code block supports entries created
+  // during that first iteration -- ideally we could run a migration to backfill these entries with
+  // transition references created from the respective text and topic fields.
+  if (module.exports.isAskYesNo(contentfulEntry)) {
+    const fields = contentfulEntry.fields;
+    if (!templates.saidYes || !templates.saidYes.text) {
+      const saidYesTopicId = contentful.getContentfulIdFromContentfulEntry(fields.saidYesTopic);
+      templates.saidYes = {
+        text: fields.saidYes,
+        topic: saidYesTopicId ? await helpers.topic.getById(saidYesTopicId) : {},
+      };
+    }
+    if (!templates.saidNo || !templates.saidNo.text) {
+      const saidNoTopicId = contentful.getContentfulIdFromContentfulEntry(fields.saidNoTopic);
+      templates.saidNo = {
+        text: fields.saidNo,
+        topic: saidNoTopicId ? await helpers.topic.getById(saidNoTopicId) : {},
+      };
+    }
   }
-  /* eslint-enable */
 
   return templates;
 }
 
+/**
+ * @param {Object} contentfulEntry
+ * @return {Boolean}
+ */
+function isAskYesNo(contentfulEntry) {
+  return contentful.isContentType(contentfulEntry, config.contentTypes.askYesNo.type);
+}
+
 /**
  * @param {Object} contentfulEntry
  * @return {Boolean}
@@ -193,19 +224,31 @@ function isMessage(contentfulEntry) {
   return contentful.isContentType(contentfulEntry, config.contentTypes.message.type);
 }
 
+/**
+ * @param {String} contentType
+ * @param {String} templateName
+ * @return {Boolean}
+ */
+function isTransitionTemplate(contentType, templateName) {
+  const templatesConfig = config.contentTypes[contentType].templates;
+  return templatesConfig[templateName].fieldType === config.templateFieldTypes.transition;
+}
+
 module.exports = {
   fetch,
   fetchById,
   formatForApi,
   formatSys,
   getContentTypeConfigs,
-  getMessageTemplateFromContentfulEntryAndTemplateName,
+  getMessageTemplate,
   getSummaryFromContentfulEntry,
   getTopicTemplates,
+  isAskYesNo,
   isAutoReply,
   isBroadcastable,
   isDefaultTopicTrigger,
   isLegacyBroadcast,
   isMessage,
+  isTransitionTemplate,
   parseContentfulEntry,
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gambit-campaigns",
-  "version": "5.14.3",
+  "version": "5.15.0",
   "description": "The DoSomething.org chatbot service for campaigns and their activity.",
   "license": "MIT",
   "repository": {

test/lib/lib-helpers/broadcast.test.js

@@ -123,21 +123,21 @@ test('getById returns fetchById if resetCache arg is true', async () => {
 });
 
 // parseBroadcastFromContentfulEntry
-test('parseBroadcastFromContentfulEntry returns object with message from getMessageTemplateFromContentfulEntryAndTemplateName', async () => {
+test('parseBroadcastFromContentfulEntry returns object with message from getMessageTemplate', async () => {
   const contentfulEntry = autoReplyBroadcastFactory.getValidAutoReplyBroadcast();
   const stubContentType = stubs.getRandomWord();
   sandbox.stub(helpers.contentfulEntry, 'getSummaryFromContentfulEntry')
     .returns({ type: stubContentType });
   const stubTemplate = { text: stubs.getRandomMessageText(), template: stubContentType };
-  sandbox.stub(helpers.contentfulEntry, 'getMessageTemplateFromContentfulEntryAndTemplateName')
+  sandbox.stub(helpers.contentfulEntry, 'getMessageTemplate')
     .returns(stubTemplate);
 
   const result = await broadcastHelper.parseBroadcastFromContentfulEntry(contentfulEntry);
   helpers.contentfulEntry.getSummaryFromContentfulEntry
     .should.have.been.calledWith(contentfulEntry);
   result.message.text.should.equal(stubTemplate.text);
   result.message.template.should.equal(stubContentType);
-  helpers.contentfulEntry.getMessageTemplateFromContentfulEntryAndTemplateName
+  helpers.contentfulEntry.getMessageTemplate
     .should.have.been.calledWith(contentfulEntry, stubContentType);
 });
 

test/lib/lib-helpers/campaign.test.js

@@ -185,7 +185,7 @@ test('parseCampaignConfig should return empty object if contentfulEntry undefine
 
 test('parseCampaignConfig should return object with id and templates properties', async () => {
   const stubTemplate = { text: stubs.getRandomMessageText() };
-  sandbox.stub(helpers.contentfulEntry, 'getMessageTemplateFromContentfulEntryAndTemplateName')
+  sandbox.stub(helpers.contentfulEntry, 'getMessageTemplate')
     .returns(Promise.resolve(stubTemplate));
   const result = await campaignHelper.parseCampaignConfig(campaignConfigEntry);
   result.id.should.equal(campaignConfigEntry.sys.id);

test/lib/lib-helpers/contentfulEntry.test.js

@@ -10,6 +10,9 @@ const sinon = require('sinon');
 const contentful = require('../../../lib/contentful');
 const helpers = require('../../../lib/helpers');
 const stubs = require('../../utils/stubs');
+
+const config = require('../../../config/lib/helpers/contentfulEntry');
+
 // Contentful factories
 const askYesNoEntryFactory = require('../../utils/factories/contentful/askYesNo');
 const autoReplyFactory = require('../../utils/factories/contentful/autoReply');
@@ -95,8 +98,8 @@ test('formatSys returns object with properties from contentfulEntry.sys', () =>
   });
 });
 
-// getMessageTemplateFromContentfulEntryAndTemplateName
-test('getMessageTemplateFromContentfulEntryAndTemplateName should call topic.getById to set topic if topic reference field saved', async () => {
+// getMessageTemplate
+test('getMessageTemplate should call topic.getById to set topic if topic reference field saved', async () => {
   const messageTemplate = stubs.getRandomWord();
   const messageText = stubs.getRandomMessageText();
   const messageAttachments = [{ name: 'Tyrion' }, { name: 'Cersei' }];
@@ -108,26 +111,26 @@ test('getMessageTemplateFromContentfulEntryAndTemplateName should call topic.get
     .returns(fetchTopicResult);
 
   const result = await contentfulEntryHelper
-    .getMessageTemplateFromContentfulEntryAndTemplateName(autoReplyBroadcastEntry, messageTemplate);
+    .getMessageTemplate(autoReplyBroadcastEntry, messageTemplate);
   helpers.topic.getById.should.have.been.calledWith(autoReplyBroadcastEntry.fields.topic.sys.id);
   result.text.should.equal(messageText);
   result.attachments.should.deep.equal(messageAttachments);
   result.topic.should.deep.equal(fetchTopicResult);
   result.template.should.equal(messageTemplate);
 });
 
-test('getMessageTemplateFromContentfulEntryAndTemplateName result should be empty object if contentfulEntry undefined', async () => {
+test('getMessageTemplate result should be empty object if contentfulEntry undefined', async () => {
   const result = await contentfulEntryHelper
-    .getMessageTemplateFromContentfulEntryAndTemplateName(null, stubs.getRandomWord());
+    .getMessageTemplate(null, stubs.getRandomWord());
   result.should.deep.equal({});
 });
 
-test('getMessageTemplateFromContentfulEntryAndTemplateName should not call topic.getById if topic reference field undefined', async () => {
+test('getMessageTemplate should not call topic.getById if topic reference field undefined', async () => {
   sandbox.stub(helpers.topic, 'getById')
     .returns(fetchTopicResult);
 
   const result = await contentfulEntryHelper
-    .getMessageTemplateFromContentfulEntryAndTemplateName(askYesNoEntry, stubs.getRandomWord());
+    .getMessageTemplate(askYesNoEntry, stubs.getRandomWord());
   helpers.topic.getById.should.not.have.been.called;
   result.topic.should.deep.equal({});
 });
@@ -171,6 +174,12 @@ test('getTopicTemplates should call topic.getById to set saidYes and saidNo topi
   result.saidNo.topic.should.deep.equal(fetchTopicResult);
 });
 
+// isAskYesNo
+test('isAskYesNo returns whether content type is askYesNo', (t) => {
+  t.truthy(contentfulEntryHelper.isAskYesNo(askYesNoEntry));
+  t.falsy(contentfulEntryHelper.isAskYesNo(autoReplyEntry));
+});
+
 // isAutoReply
 test('isAutoReply returns whether content type is autoReply', (t) => {
   t.falsy(contentfulEntryHelper.isAutoReply(askYesNoEntry));
@@ -194,3 +203,15 @@ test('isMessage returns whether content type is message', (t) => {
   t.truthy(contentfulEntryHelper.isMessage(messageEntry));
   t.falsy(contentfulEntryHelper.isMessage(autoReplyEntry));
 });
+
+// isTransitionTemplate
+test('isTransitionTemplate returns whether contentType config for templateName is a transition field', (t) => {
+  const askVotingPlanStatus = config.contentTypes.askVotingPlanStatus;
+  const autoReply = config.contentTypes.autoReply;
+  let templateName = askVotingPlanStatus.templates.votingPlanStatusVoted.name;
+  t.truthy(contentfulEntryHelper
+    .isTransitionTemplate(askVotingPlanStatus.type, templateName));
+  templateName = autoReply.templates.autoReply.name;
+  t.falsy(contentfulEntryHelper
+    .isTransitionTemplate(autoReply.type, templateName));
+});