diff --git a/config/lib/helpers/contentfulEntry.js b/config/lib/helpers/contentfulEntry.js index 1f3c4e47..14544920 100644 --- a/config/lib/helpers/contentfulEntry.js +++ b/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, }; diff --git a/lib/helpers/broadcast.js b/lib/helpers/broadcast.js index 76e819ea..6778324e 100644 --- a/lib/helpers/broadcast.js +++ b/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), ]); diff --git a/lib/helpers/campaign.js b/lib/helpers/campaign.js index 99411f6f..1759e859 100644 --- a/lib/helpers/campaign.js +++ b/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; } diff --git a/lib/helpers/contentfulEntry.js b/lib/helpers/contentfulEntry.js index ddd9ebc7..7522b653 100644 --- a/lib/helpers/contentfulEntry.js +++ b/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, }; diff --git a/package.json b/package.json index c9636692..2e188526 100644 --- a/package.json +++ b/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": { diff --git a/test/lib/lib-helpers/broadcast.test.js b/test/lib/lib-helpers/broadcast.test.js index 852c7d2f..e1404be4 100644 --- a/test/lib/lib-helpers/broadcast.test.js +++ b/test/lib/lib-helpers/broadcast.test.js @@ -123,13 +123,13 @@ 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); @@ -137,7 +137,7 @@ test('parseBroadcastFromContentfulEntry returns object with message from getMess .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); }); diff --git a/test/lib/lib-helpers/campaign.test.js b/test/lib/lib-helpers/campaign.test.js index 7963d0cf..7bf084c2 100644 --- a/test/lib/lib-helpers/campaign.test.js +++ b/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); diff --git a/test/lib/lib-helpers/contentfulEntry.test.js b/test/lib/lib-helpers/contentfulEntry.test.js index 51c11142..134a117b 100644 --- a/test/lib/lib-helpers/contentfulEntry.test.js +++ b/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,7 +111,7 @@ 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); @@ -116,18 +119,18 @@ test('getMessageTemplateFromContentfulEntryAndTemplateName should call topic.get 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)); +});