From cfbe407cae4275a04dd8ee8f5f93b4bce3b835c7 Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 09:47:02 -0700 Subject: [PATCH 1/7] Removes templates deprecated by campaign webSignup --- config/lib/helpers/topic.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/config/lib/helpers/topic.js b/config/lib/helpers/topic.js index 63ad10bf..0b8738bb 100644 --- a/config/lib/helpers/topic.js +++ b/config/lib/helpers/topic.js @@ -15,7 +15,6 @@ const photoPostDefaultText = { completedPhotoPostAutoReply: `${defaultText.invalidInput}\n\n${completeAnotherPhotoPostText}`, startPhotoPost: startPhotoPostText, startPhotoPostAutoReply: `${defaultText.invalidInput}\n\nText ${startCommand} when you're ready to submit a post for {{title}}.`, - webStartPhotoPost: `Hi it's Freddie from DoSomething! ${startPhotoPostText}`, }; module.exports = { @@ -48,9 +47,6 @@ module.exports = { startExternalPost: { fieldName: 'startExternalPostMessage', }, - webStartExternalPost: { - fieldName: 'webStartExternalPostMessage', - }, startExternalPostAutoReply: { fieldName: 'startExternalPostAutoReplyMessage', }, @@ -62,10 +58,6 @@ module.exports = { fieldName: 'gambitSignupMenuMessage', defaultText: photoPostDefaultText.startPhotoPost, }, - webStartPhotoPost: { - fieldName: 'externalSignupMenuMessage', - defaultText: photoPostDefaultText.webStartPhotoPost, - }, startPhotoPostAutoReply: { fieldName: 'invalidSignupMenuCommandMessage', defaultText: photoPostDefaultText.startPhotoPostAutoReply, @@ -112,9 +104,6 @@ module.exports = { askText: { fieldName: 'askTextMessage', }, - webAskText: { - fieldName: 'webAskTextMessage', - }, invalidText: { fieldName: 'invalidTextMessage', }, From e027e5dfd2aced36b823afb1e4fa0ad98302558a Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 10:34:57 -0700 Subject: [PATCH 2/7] Adds support for transition types as defaultTopicTrigger response --- config/lib/helpers/contentfulEntry.js | 19 +++++++++++++++++-- lib/helpers/contentfulEntry.js | 10 ++++++++++ lib/helpers/defaultTopicTrigger.js | 27 +++++++++++++++++---------- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/config/lib/helpers/contentfulEntry.js b/config/lib/helpers/contentfulEntry.js index 14544920..50312ba8 100644 --- a/config/lib/helpers/contentfulEntry.js +++ b/config/lib/helpers/contentfulEntry.js @@ -9,8 +9,9 @@ const templateFieldTypes = { * This maps the fields in our Contentful types into broadcast, topic, and defaultTopicTriggers. * * Content types with either a templates or postType property set are returned as topics. - * If the type contains a templates array, each array item should correspond to a single value text - * field defined on the content type, which is used as a bot reply template in the topic. + * + * If the type contains a templates object, each property defines the Contentful field name and type + * to use as a bot reply template fir the topic. * If the type contains a postType string property instead, its an older content type and its * templates are configured via config/lib/helpers/topic. Ideally we should consolidate, but it'd * take a bit of refactoring as the topic helper config contains default values for certain text @@ -21,6 +22,8 @@ const templateFieldTypes = { * exists, it will include the topic in the outbound message, indicating that the conversation topic * should be updated upon receiving the broadcast (otherwise, the broadcast itself can be used as a * topic if it has templates -- e.g. askYesNo) + * + * A transitionable content type requires a text field named "text" and a reference field "topic". */ module.exports = { contentTypes: { @@ -87,6 +90,10 @@ module.exports = { type: 'autoReplyBroadcast', broadcastable: true, }, + autoReplyTransition: { + type: 'autoReplyTransition', + transitionable: true, + }, defaultTopicTrigger: { type: 'defaultTopicTrigger', }, @@ -102,6 +109,10 @@ module.exports = { // TODO: Refactor photoPostConfig in config/lib/helpers/topic here to DRY. postType: 'photo', }, + photoPostTransition: { + type: 'photoPostTransition', + transitionable: true, + }, textPostBroadcast: { type: 'textPostBroadcast', broadcastable: true, @@ -111,6 +122,10 @@ module.exports = { // TODO: Move textPostConfig in config/lib/helpers/topic here to DRY. postType: 'text', }, + textPostTransition: { + type: 'textPostTransition', + transitionable: true, + }, // Legacy types: // Ideally we'd backfill all legacy entries as their new types, but we likely can't change the // the type of a Contentful entry without changing its id (if that's the case - we'd need to diff --git a/lib/helpers/contentfulEntry.js b/lib/helpers/contentfulEntry.js index 7522b653..734d40a1 100644 --- a/lib/helpers/contentfulEntry.js +++ b/lib/helpers/contentfulEntry.js @@ -224,6 +224,15 @@ function isMessage(contentfulEntry) { return contentful.isContentType(contentfulEntry, config.contentTypes.message.type); } +/** + * @param {Object} contentfulEntry + * @return {Boolean} + */ +function isTransitionable(contentfulEntry) { + const contentType = contentful.getContentTypeFromContentfulEntry(contentfulEntry); + return config.contentTypes[contentType].transitionable; +} + /** * @param {String} contentType * @param {String} templateName @@ -249,6 +258,7 @@ module.exports = { isDefaultTopicTrigger, isLegacyBroadcast, isMessage, + isTransitionable, isTransitionTemplate, parseContentfulEntry, }; diff --git a/lib/helpers/defaultTopicTrigger.js b/lib/helpers/defaultTopicTrigger.js index 17f3def2..027d48b0 100644 --- a/lib/helpers/defaultTopicTrigger.js +++ b/lib/helpers/defaultTopicTrigger.js @@ -91,35 +91,42 @@ function getTriggersFromDefaultTopicTriggers(defaultTopicTriggers) { * triggers on the default topic. * * @param {Object} contentfulEntry - defaultTopicTrigger - * @return {String} + * @return {Promise} */ -function parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry) { +async function parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry) { const data = { id: contentful.getContentfulIdFromContentfulEntry(contentfulEntry), trigger: contentful.getTriggerTextFromDefaultTopicTrigger(contentfulEntry), }; const responseEntry = contentful.getResponseEntryFromDefaultTopicTrigger(contentfulEntry); + // Check for draft entries that may not contain a response reference. if (!responseEntry) { - return Promise.resolve(null); + return null; } if (helpers.contentfulEntry.isDefaultTopicTrigger(responseEntry)) { data.redirect = contentful.getTriggerTextFromDefaultTopicTrigger(responseEntry); - return Promise.resolve(data); + return data; } if (helpers.contentfulEntry.isMessage(responseEntry)) { data.reply = contentful.getTextFromMessage(responseEntry); - return Promise.resolve(data); + return data; } + if (helpers.contentfulEntry.isTransitionable(responseEntry)) { + data.reply = contentful.getTextFromMessage(responseEntry); + data.topic = await helpers.topic + .fetchById(contentful.getContentfulIdFromContentfulEntry(responseEntry.fields.topic)); + return data; + } + + // TODO: This will be removed once all defaultTopicTriggers that reference topic entries (e.g. + // photoPost, textPost) are updated to reference transition entries (e.g. photoPostTransition). data.topicId = contentful.getContentfulIdFromContentfulEntry(responseEntry); - return helpers.topic.fetchById(data.topicId) - .then((topic) => { - data.topic = topic; - return data; - }); + data.topic = await helpers.topic.fetchById(data.topicId); + return data; } module.exports = { From 50968d144d739111a4cbc81dea5819aa1045f455 Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 10:52:51 -0700 Subject: [PATCH 3/7] Adds autoReplyTransition factory --- config/lib/helpers/contentfulEntry.js | 2 +- test/lib/lib-helpers/contentfulEntry.test.js | 8 ++++++++ .../contentful/autoReplyTransition.js | 20 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 test/utils/factories/contentful/autoReplyTransition.js diff --git a/config/lib/helpers/contentfulEntry.js b/config/lib/helpers/contentfulEntry.js index 50312ba8..17ff7083 100644 --- a/config/lib/helpers/contentfulEntry.js +++ b/config/lib/helpers/contentfulEntry.js @@ -11,7 +11,7 @@ const templateFieldTypes = { * Content types with either a templates or postType property set are returned as topics. * * If the type contains a templates object, each property defines the Contentful field name and type - * to use as a bot reply template fir the topic. + * to use as a template within the topic. * If the type contains a postType string property instead, its an older content type and its * templates are configured via config/lib/helpers/topic. Ideally we should consolidate, but it'd * take a bit of refactoring as the topic helper config contains default values for certain text diff --git a/test/lib/lib-helpers/contentfulEntry.test.js b/test/lib/lib-helpers/contentfulEntry.test.js index 134a117b..2a8a6d83 100644 --- a/test/lib/lib-helpers/contentfulEntry.test.js +++ b/test/lib/lib-helpers/contentfulEntry.test.js @@ -17,6 +17,7 @@ const config = require('../../../config/lib/helpers/contentfulEntry'); const askYesNoEntryFactory = require('../../utils/factories/contentful/askYesNo'); const autoReplyFactory = require('../../utils/factories/contentful/autoReply'); const autoReplyBroadcastFactory = require('../../utils/factories/contentful/autoReplyBroadcast'); +const autoReplyTransitionFactory = require('../../utils/factories/contentful/autoReplyTransition'); const defaultTopicTriggerFactory = require('../../utils/factories/contentful/defaultTopicTrigger'); const messageFactory = require('../../utils/factories/contentful/message'); // Parsed factories @@ -26,6 +27,7 @@ const broadcastFactory = require('../../utils/factories/broadcast'); const askYesNoEntry = askYesNoEntryFactory.getValidAskYesNo(); const autoReplyEntry = autoReplyFactory.getValidAutoReply(); const autoReplyBroadcastEntry = autoReplyBroadcastFactory.getValidAutoReplyBroadcast(); +const autoReplyTransitionEntry = autoReplyTransitionFactory.getValidAutoReplyTransition(); const defaultTopicTriggerEntry = defaultTopicTriggerFactory.getValidDefaultTopicTrigger(); const messageEntry = messageFactory.getValidMessage(); const fetchTopicResult = { id: stubs.getContentfulId() }; @@ -204,6 +206,12 @@ test('isMessage returns whether content type is message', (t) => { t.falsy(contentfulEntryHelper.isMessage(autoReplyEntry)); }); +// isTransitionable +test('isTransitionable returns whether content type is transitionable', (t) => { + t.truthy(contentfulEntryHelper.isTransitionable(autoReplyTransitionEntry)); + t.falsy(contentfulEntryHelper.isTransitionable(autoReplyBroadcastEntry)); +}); + // isTransitionTemplate test('isTransitionTemplate returns whether contentType config for templateName is a transition field', (t) => { const askVotingPlanStatus = config.contentTypes.askVotingPlanStatus; diff --git a/test/utils/factories/contentful/autoReplyTransition.js b/test/utils/factories/contentful/autoReplyTransition.js new file mode 100644 index 00000000..40d2f3f5 --- /dev/null +++ b/test/utils/factories/contentful/autoReplyTransition.js @@ -0,0 +1,20 @@ +'use strict'; + +const stubs = require('../../stubs'); +const autoReplyFactory = require('./autoReply'); + +function getValidAutoReplyTransition(date = Date.now()) { + const data = { + sys: stubs.contentful.getSysWithTypeAndDate('autoReplyTransition', date), + fields: { + text: stubs.getRandomMessageText(), + attachments: stubs.contentful.getAttachments(), + topic: autoReplyFactory.getValidAutoReply(), + }, + }; + return data; +} + +module.exports = { + getValidAutoReplyTransition, +}; From 330860a961cd05a8a8010771863e5cb29823612c Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 11:33:20 -0700 Subject: [PATCH 4/7] Renames as parseDefaultTopicTrigger, starts on test --- lib/helpers/contentfulEntry.js | 2 +- lib/helpers/defaultTopicTrigger.js | 11 +++--- .../lib-helpers/defaultTopicTrigger.test.js | 34 ++++++++++++++----- .../contentful/defaultTopicTrigger.js | 16 +++------ 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/lib/helpers/contentfulEntry.js b/lib/helpers/contentfulEntry.js index 734d40a1..317ebe40 100644 --- a/lib/helpers/contentfulEntry.js +++ b/lib/helpers/contentfulEntry.js @@ -80,7 +80,7 @@ function parseContentfulEntry(contentfulEntry) { return helpers.topic.parseTopicFromContentfulEntry(contentfulEntry); } if (helpers.defaultTopicTrigger.getContentTypes().includes(contentType)) { - return helpers.defaultTopicTrigger.parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry); + return helpers.defaultTopicTrigger.parseDefaultTopicTrigger(contentfulEntry); } return Promise.resolve(null); diff --git a/lib/helpers/defaultTopicTrigger.js b/lib/helpers/defaultTopicTrigger.js index 027d48b0..5ad722af 100644 --- a/lib/helpers/defaultTopicTrigger.js +++ b/lib/helpers/defaultTopicTrigger.js @@ -22,7 +22,7 @@ function getContentTypes() { function fetch(query = {}) { return contentful.fetchByContentTypes(module.exports.getContentTypes(), query) .then(contentfulRes => Promise.map(contentfulRes.data, module.exports - .parseDefaultTopicTriggerFromContentfulEntry)) + .parseDefaultTopicTrigger)) // TODO: We need to modify the meta data too, as we're removing data items here. .then(defaultTopicTriggers => module.exports .removeInvalidDefaultTopicTriggers(defaultTopicTriggers)); @@ -30,7 +30,7 @@ function fetch(query = {}) { /** * Removes any null items that may be present from parsing draft Contentful entries. - * @see parseDefaultTopicTriggerFromContentfulEntry + * @see parseDefaultTopicTrigger * * @param {Array} * @return {Array} @@ -93,7 +93,7 @@ function getTriggersFromDefaultTopicTriggers(defaultTopicTriggers) { * @param {Object} contentfulEntry - defaultTopicTrigger * @return {Promise} */ -async function parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry) { +async function parseDefaultTopicTrigger(contentfulEntry) { const data = { id: contentful.getContentfulIdFromContentfulEntry(contentfulEntry), trigger: contentful.getTriggerTextFromDefaultTopicTrigger(contentfulEntry), @@ -118,13 +118,14 @@ async function parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry) { if (helpers.contentfulEntry.isTransitionable(responseEntry)) { data.reply = contentful.getTextFromMessage(responseEntry); data.topic = await helpers.topic - .fetchById(contentful.getContentfulIdFromContentfulEntry(responseEntry.fields.topic)); + .getById(contentful.getContentfulIdFromContentfulEntry(responseEntry.fields.topic)); return data; } // TODO: This will be removed once all defaultTopicTriggers that reference topic entries (e.g. // photoPost, textPost) are updated to reference transition entries (e.g. photoPostTransition). data.topicId = contentful.getContentfulIdFromContentfulEntry(responseEntry); + // Should this have been retrieiving topics from cache? data.topic = await helpers.topic.fetchById(data.topicId); return data; } @@ -136,6 +137,6 @@ module.exports = { getByTopicId, getContentTypes, getTriggersFromDefaultTopicTriggers, - parseDefaultTopicTriggerFromContentfulEntry, + parseDefaultTopicTrigger, removeInvalidDefaultTopicTriggers, }; diff --git a/test/lib/lib-helpers/defaultTopicTrigger.test.js b/test/lib/lib-helpers/defaultTopicTrigger.test.js index e9bd7223..bfa0619a 100644 --- a/test/lib/lib-helpers/defaultTopicTrigger.test.js +++ b/test/lib/lib-helpers/defaultTopicTrigger.test.js @@ -11,9 +11,12 @@ const contentful = require('../../../lib/contentful'); const stubs = require('../../utils/stubs'); const helpers = require('../../../lib/helpers'); const config = require('../../../config/lib/helpers/defaultTopicTrigger'); + const defaultTopicTriggerContentfulFactory = require('../../utils/factories/contentful/defaultTopicTrigger'); const defaultTopicTriggerFactory = require('../../utils/factories/defaultTopicTrigger'); +const topicFactory = require('../../utils/factories/topic'); +const contentfulId = stubs.getContentfulId(); const firstEntry = defaultTopicTriggerContentfulFactory.getValidDefaultTopicTrigger(); const firstDefaultTopicTrigger = defaultTopicTriggerFactory.getValidDefaultTopicTriggerWithReply(); const secondEntry = defaultTopicTriggerContentfulFactory.getValidDefaultTopicTrigger(); @@ -31,10 +34,6 @@ test.afterEach(() => { sandbox.restore(); }); -test.afterEach(() => { - sandbox.restore(); -}); - // fetch test('fetch returns contentful.fetchByContentTypes parsed as defaultTopicTrigger objects', async () => { const entries = [firstEntry, secondEntry]; @@ -44,7 +43,7 @@ test('fetch returns contentful.fetchByContentTypes parsed as defaultTopicTrigger .returns(types); sandbox.stub(contentful, 'fetchByContentTypes') .returns(Promise.resolve(fetchEntriesResult)); - sandbox.stub(defaultTopicTriggerHelper, 'parseDefaultTopicTriggerFromContentfulEntry') + sandbox.stub(defaultTopicTriggerHelper, 'parseDefaultTopicTrigger') .onCall(0) .returns(firstDefaultTopicTrigger) .onCall(1) @@ -53,9 +52,9 @@ test('fetch returns contentful.fetchByContentTypes parsed as defaultTopicTrigger const result = await defaultTopicTriggerHelper.fetch(); contentful.fetchByContentTypes .should.have.been.calledWith(types); - defaultTopicTriggerHelper.parseDefaultTopicTriggerFromContentfulEntry + defaultTopicTriggerHelper.parseDefaultTopicTrigger .should.have.been.calledWith(firstEntry); - defaultTopicTriggerHelper.parseDefaultTopicTriggerFromContentfulEntry + defaultTopicTriggerHelper.parseDefaultTopicTrigger .should.have.been.calledWith(secondEntry); result.should.deep.equal([firstDefaultTopicTrigger, secondDefaultTopicTrigger]); }); @@ -64,7 +63,7 @@ test('fetch throws if contentful.fetchByContentTypes fails', async (t) => { const error = new Error('epic fail'); sandbox.stub(contentful, 'fetchByContentTypes') .returns(Promise.reject(error)); - sandbox.stub(defaultTopicTriggerHelper, 'parseDefaultTopicTriggerFromContentfulEntry') + sandbox.stub(defaultTopicTriggerHelper, 'parseDefaultTopicTrigger') .returns(firstDefaultTopicTrigger); const result = await t.throws(defaultTopicTriggerHelper.fetch()); @@ -111,6 +110,25 @@ test('getTriggersFromDefaultTopicTriggers returns array of trigger properties', result.should.deep.equal([firstDefaultTopicTrigger.trigger, secondDefaultTopicTrigger.trigger]); }); +// parseDefaultTopicTrigger +test('parseDefaultTopicTrigger returns object with reply and topic if entry is transitionable', async () => { + const topic = topicFactory.getValidTopic(); + const triggerText = stubs.getRandomMessageText(); + sandbox.stub(contentful, 'getContentfulIdFromContentfulEntry') + .returns(contentfulId); + sandbox.stub(contentful, 'getTextFromMessage') + .returns(triggerText); + sandbox.stub(helpers.contentfulEntry, 'isTransitionable') + .returns(true); + sandbox.stub(helpers.topic, 'getById') + .returns(Promise.resolve(topic)); + + const result = await defaultTopicTriggerHelper.parseDefaultTopicTrigger(firstEntry); + result.id.should.equal(contentfulId); + result.reply.should.equal(triggerText); + result.topic.should.deep.equal(topic); +}); + // removeInvalidDefaultTopicTriggers test('removeInvalidDefaultTopicTriggers filters any null array items', () => { const parsedItems = [null, firstDefaultTopicTrigger, null, null, secondDefaultTopicTrigger, null]; diff --git a/test/utils/factories/contentful/defaultTopicTrigger.js b/test/utils/factories/contentful/defaultTopicTrigger.js index a9503830..510a2fd0 100644 --- a/test/utils/factories/contentful/defaultTopicTrigger.js +++ b/test/utils/factories/contentful/defaultTopicTrigger.js @@ -1,22 +1,16 @@ 'use strict'; const stubs = require('../../stubs'); +const autoReplyTransitionFactory = require('./autoReplyTransition'); -function getValidDefaultTopicTrigger() { - const data = { - sys: { - id: '51QDjTsMbmCcW804IUGaQg', - contentType: { - sys: { - id: 'defaultTopicTrigger', - }, - }, - }, +function getValidDefaultTopicTrigger(responseEntry) { + return { + sys: stubs.contentful.getSysWithTypeAndDate('defaultTopicTrigger'), fields: { trigger: stubs.getRandomWord(), + response: responseEntry || autoReplyTransitionFactory.getValidAutoReplyTransition(), }, }; - return data; } module.exports = { From f979585a25ee1895baf4f9138b44643d5546b195 Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 12:13:52 -0700 Subject: [PATCH 5/7] Adds tests for draft entries, message responses --- lib/helpers/defaultTopicTrigger.js | 5 ++-- .../lib-helpers/defaultTopicTrigger.test.js | 27 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/helpers/defaultTopicTrigger.js b/lib/helpers/defaultTopicTrigger.js index 5ad722af..77f7e21a 100644 --- a/lib/helpers/defaultTopicTrigger.js +++ b/lib/helpers/defaultTopicTrigger.js @@ -124,9 +124,10 @@ async function parseDefaultTopicTrigger(contentfulEntry) { // TODO: This will be removed once all defaultTopicTriggers that reference topic entries (e.g. // photoPost, textPost) are updated to reference transition entries (e.g. photoPostTransition). + // The changeTopicTrigger code in Conversations will be deprecated as well once all entries are + // backfilled with transition entries. data.topicId = contentful.getContentfulIdFromContentfulEntry(responseEntry); - // Should this have been retrieiving topics from cache? - data.topic = await helpers.topic.fetchById(data.topicId); + data.topic = await helpers.topic.getById(data.topicId); return data; } diff --git a/test/lib/lib-helpers/defaultTopicTrigger.test.js b/test/lib/lib-helpers/defaultTopicTrigger.test.js index bfa0619a..1c168896 100644 --- a/test/lib/lib-helpers/defaultTopicTrigger.test.js +++ b/test/lib/lib-helpers/defaultTopicTrigger.test.js @@ -12,14 +12,15 @@ const stubs = require('../../utils/stubs'); const helpers = require('../../../lib/helpers'); const config = require('../../../config/lib/helpers/defaultTopicTrigger'); -const defaultTopicTriggerContentfulFactory = require('../../utils/factories/contentful/defaultTopicTrigger'); +const defaultTopicTriggerEntryFactory = require('../../utils/factories/contentful/defaultTopicTrigger'); +const messageEntryFactory = require('../../utils/factories/contentful/message'); const defaultTopicTriggerFactory = require('../../utils/factories/defaultTopicTrigger'); const topicFactory = require('../../utils/factories/topic'); const contentfulId = stubs.getContentfulId(); -const firstEntry = defaultTopicTriggerContentfulFactory.getValidDefaultTopicTrigger(); +const firstEntry = defaultTopicTriggerEntryFactory.getValidDefaultTopicTrigger(); const firstDefaultTopicTrigger = defaultTopicTriggerFactory.getValidDefaultTopicTriggerWithReply(); -const secondEntry = defaultTopicTriggerContentfulFactory.getValidDefaultTopicTrigger(); +const secondEntry = defaultTopicTriggerEntryFactory.getValidDefaultTopicTrigger(); const secondDefaultTopicTrigger = defaultTopicTriggerFactory.getValidDefaultTopicTriggerWithReply(); // Module to test @@ -111,6 +112,26 @@ test('getTriggersFromDefaultTopicTriggers returns array of trigger properties', }); // parseDefaultTopicTrigger +test('parseDefaultTopicTrigger returns null if entry response reference is falsy', async (t) => { + sandbox.stub(contentful, 'getResponseEntryFromDefaultTopicTrigger') + .returns(null); + + const result = await defaultTopicTriggerHelper.parseDefaultTopicTrigger(firstEntry); + t.is(result, null); +}); + +test('parseDefaultTopicTrigger returns object without topic if entry response is a message', async (t) => { + const messageEntry = messageEntryFactory.getValidMessage(); + const triggerText = stubs.getRandomMessageText(); + sandbox.stub(contentful, 'getTextFromMessage') + .returns(triggerText); + const triggerEntry = defaultTopicTriggerEntryFactory.getValidDefaultTopicTrigger(messageEntry); + + const result = await defaultTopicTriggerHelper.parseDefaultTopicTrigger(triggerEntry); + result.reply.should.equal(triggerText); + result.should.not.have.property('topic'); +}); + test('parseDefaultTopicTrigger returns object with reply and topic if entry is transitionable', async () => { const topic = topicFactory.getValidTopic(); const triggerText = stubs.getRandomMessageText(); From 35aa3e482218da4b8ec9f0ccd4435d49a3fd688e Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Fri, 26 Oct 2018 12:52:57 -0700 Subject: [PATCH 6/7] Removes getting defaultTopicTriggers by topic --- lib/helpers/defaultTopicTrigger.js | 23 +------------------ lib/middleware/topics/single/topic-get.js | 17 +++----------- .../lib-helpers/defaultTopicTrigger.test.js | 11 +-------- .../topics/single/topic-get.test.js | 5 ---- 4 files changed, 5 insertions(+), 51 deletions(-) diff --git a/lib/helpers/defaultTopicTrigger.js b/lib/helpers/defaultTopicTrigger.js index 77f7e21a..515bc83a 100644 --- a/lib/helpers/defaultTopicTrigger.js +++ b/lib/helpers/defaultTopicTrigger.js @@ -21,8 +21,7 @@ function getContentTypes() { */ function fetch(query = {}) { return contentful.fetchByContentTypes(module.exports.getContentTypes(), query) - .then(contentfulRes => Promise.map(contentfulRes.data, module.exports - .parseDefaultTopicTrigger)) + .then(contentfulRes => Promise.map(contentfulRes.data, module.exports.parseDefaultTopicTrigger)) // TODO: We need to modify the meta data too, as we're removing data items here. .then(defaultTopicTriggers => module.exports .removeInvalidDefaultTopicTriggers(defaultTopicTriggers)); @@ -68,24 +67,6 @@ function getAllWithCampaignTopics() { .filter(trigger => trigger.topic && trigger.topic.campaign)); } -/** - * @param {String} topicId - * @return {Promise} - */ -function getByTopicId(topicId) { - return module.exports.getAll() - .then(defaultTopicTriggers => defaultTopicTriggers - .filter(defaultTopicTrigger => defaultTopicTrigger.topicId === topicId)); -} - -/** - * @param {Array} defaultTopicTriggers - * @return {Array} - */ -function getTriggersFromDefaultTopicTriggers(defaultTopicTriggers) { - return defaultTopicTriggers.map(defaultTopicTrigger => defaultTopicTrigger.trigger); -} - /** * Parses a defaultTopicTrigger Contentful entry as data for writing Rivescript to define * triggers on the default topic. @@ -135,9 +116,7 @@ module.exports = { fetch, getAll, getAllWithCampaignTopics, - getByTopicId, getContentTypes, - getTriggersFromDefaultTopicTriggers, parseDefaultTopicTrigger, removeInvalidDefaultTopicTriggers, }; diff --git a/lib/middleware/topics/single/topic-get.js b/lib/middleware/topics/single/topic-get.js index 79b217de..98bde2ca 100644 --- a/lib/middleware/topics/single/topic-get.js +++ b/lib/middleware/topics/single/topic-get.js @@ -3,18 +3,7 @@ const helpers = require('../../../helpers'); module.exports = function getTopic() { - return (req, res) => { - const topicId = req.params.topicId; - return helpers.topic.getById(topicId, helpers.request.isCacheReset(req)) - .then((topic) => { - req.data = topic; - return helpers.defaultTopicTrigger.getByTopicId(topicId); - }) - .then((defaultTopicTriggers) => { - req.data.triggers = helpers.defaultTopicTrigger - .getTriggersFromDefaultTopicTriggers(defaultTopicTriggers); - return helpers.response.sendData(res, req.data); - }) - .catch(err => helpers.sendErrorResponse(res, err)); - }; + return (req, res) => helpers.topic.getById(req.params.topicId, helpers.request.isCacheReset(req)) + .then(topic => helpers.response.sendData(res, topic)) + .catch(err => helpers.sendErrorResponse(res, err)); }; diff --git a/test/lib/lib-helpers/defaultTopicTrigger.test.js b/test/lib/lib-helpers/defaultTopicTrigger.test.js index 1c168896..12dfca29 100644 --- a/test/lib/lib-helpers/defaultTopicTrigger.test.js +++ b/test/lib/lib-helpers/defaultTopicTrigger.test.js @@ -102,15 +102,6 @@ test('getAll returns fetch results if cache not set', async () => { result.should.deep.equal(fetchResult); }); -// getTriggersFromDefaultTopicTriggers -test('getTriggersFromDefaultTopicTriggers returns array of trigger properties', () => { - const defaultTopicTriggers = [firstDefaultTopicTrigger, secondDefaultTopicTrigger]; - - const result = defaultTopicTriggerHelper - .getTriggersFromDefaultTopicTriggers(defaultTopicTriggers); - result.should.deep.equal([firstDefaultTopicTrigger.trigger, secondDefaultTopicTrigger.trigger]); -}); - // parseDefaultTopicTrigger test('parseDefaultTopicTrigger returns null if entry response reference is falsy', async (t) => { sandbox.stub(contentful, 'getResponseEntryFromDefaultTopicTrigger') @@ -120,7 +111,7 @@ test('parseDefaultTopicTrigger returns null if entry response reference is falsy t.is(result, null); }); -test('parseDefaultTopicTrigger returns object without topic if entry response is a message', async (t) => { +test('parseDefaultTopicTrigger returns object without topic if entry response is a message', async () => { const messageEntry = messageEntryFactory.getValidMessage(); const triggerText = stubs.getRandomMessageText(); sandbox.stub(contentful, 'getTextFromMessage') diff --git a/test/lib/middleware/topics/single/topic-get.test.js b/test/lib/middleware/topics/single/topic-get.test.js index d700b7b2..f161447c 100644 --- a/test/lib/middleware/topics/single/topic-get.test.js +++ b/test/lib/middleware/topics/single/topic-get.test.js @@ -11,12 +11,9 @@ const underscore = require('underscore'); const stubs = require('../../../../utils/stubs'); const helpers = require('../../../../../lib/helpers'); - -const defaultTopicTriggerFactory = require('../../../../utils/factories/defaultTopicTrigger'); const topicFactory = require('../../../../utils/factories/topic'); const topic = topicFactory.getValidTopic(); -const defaultTopicTrigger = defaultTopicTriggerFactory.getValidDefaultTopicTriggerWithTopic(); chai.should(); chai.use(sinonChai); @@ -47,8 +44,6 @@ test('getTopic should inject a topic property set to getById result', async (t) const middleware = getTopic(); sandbox.stub(helpers.topic, 'getById') .returns(Promise.resolve(topic)); - sandbox.stub(helpers.defaultTopicTrigger, 'getByTopicId') - .returns(Promise.resolve([defaultTopicTrigger])); // test await middleware(t.context.req, t.context.res, next); From 4fd6bccd315073441a37303df2181404ec4ebbe4 Mon Sep 17 00:00:00 2001 From: Aaron Schachter Date: Tue, 30 Oct 2018 11:09:23 -0700 Subject: [PATCH 7/7] Bumped version number to 5.15.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2e188526..a67356ed 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gambit-campaigns", - "version": "5.15.0", + "version": "5.15.1", "description": "The DoSomething.org chatbot service for campaigns and their activity.", "license": "MIT", "repository": {