Skip to content

Commit

Permalink
Merge branch 'master' into production
Browse files Browse the repository at this point in the history
  • Loading branch information
aaronschachter committed Oct 30, 2018
2 parents 0812a76 + 4fd6bcc commit f222010
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 96 deletions.
19 changes: 17 additions & 2 deletions config/lib/helpers/contentfulEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
Expand All @@ -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: {
Expand Down Expand Up @@ -87,6 +90,10 @@ module.exports = {
type: 'autoReplyBroadcast',
broadcastable: true,
},
autoReplyTransition: {
type: 'autoReplyTransition',
transitionable: true,
},
defaultTopicTrigger: {
type: 'defaultTopicTrigger',
},
Expand All @@ -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,
Expand All @@ -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
Expand Down
11 changes: 0 additions & 11 deletions config/lib/helpers/topic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -48,9 +47,6 @@ module.exports = {
startExternalPost: {
fieldName: 'startExternalPostMessage',
},
webStartExternalPost: {
fieldName: 'webStartExternalPostMessage',
},
startExternalPostAutoReply: {
fieldName: 'startExternalPostAutoReplyMessage',
},
Expand All @@ -62,10 +58,6 @@ module.exports = {
fieldName: 'gambitSignupMenuMessage',
defaultText: photoPostDefaultText.startPhotoPost,
},
webStartPhotoPost: {
fieldName: 'externalSignupMenuMessage',
defaultText: photoPostDefaultText.webStartPhotoPost,
},
startPhotoPostAutoReply: {
fieldName: 'invalidSignupMenuCommandMessage',
defaultText: photoPostDefaultText.startPhotoPostAutoReply,
Expand Down Expand Up @@ -112,9 +104,6 @@ module.exports = {
askText: {
fieldName: 'askTextMessage',
},
webAskText: {
fieldName: 'webAskTextMessage',
},
invalidText: {
fieldName: 'invalidTextMessage',
},
Expand Down
12 changes: 11 additions & 1 deletion lib/helpers/contentfulEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -249,6 +258,7 @@ module.exports = {
isDefaultTopicTrigger,
isLegacyBroadcast,
isMessage,
isTransitionable,
isTransitionTemplate,
parseContentfulEntry,
};
56 changes: 22 additions & 34 deletions lib/helpers/defaultTopicTrigger.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,15 @@ function getContentTypes() {
*/
function fetch(query = {}) {
return contentful.fetchByContentTypes(module.exports.getContentTypes(), query)
.then(contentfulRes => Promise.map(contentfulRes.data, module.exports
.parseDefaultTopicTriggerFromContentfulEntry))
.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));
}

/**
* Removes any null items that may be present from parsing draft Contentful entries.
* @see parseDefaultTopicTriggerFromContentfulEntry
* @see parseDefaultTopicTrigger
*
* @param {Array}
* @return {Array}
Expand Down Expand Up @@ -68,67 +67,56 @@ 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.
*
* @param {Object} contentfulEntry - defaultTopicTrigger
* @return {String}
* @return {Promise}
*/
function parseDefaultTopicTriggerFromContentfulEntry(contentfulEntry) {
async function parseDefaultTopicTrigger(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
.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).
// The changeTopicTrigger code in Conversations will be deprecated as well once all entries are
// backfilled with transition entries.
data.topicId = contentful.getContentfulIdFromContentfulEntry(responseEntry);
return helpers.topic.fetchById(data.topicId)
.then((topic) => {
data.topic = topic;
return data;
});
data.topic = await helpers.topic.getById(data.topicId);
return data;
}

module.exports = {
fetch,
getAll,
getAllWithCampaignTopics,
getByTopicId,
getContentTypes,
getTriggersFromDefaultTopicTriggers,
parseDefaultTopicTriggerFromContentfulEntry,
parseDefaultTopicTrigger,
removeInvalidDefaultTopicTriggers,
};
17 changes: 3 additions & 14 deletions lib/middleware/topics/single/topic-get.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
8 changes: 8 additions & 0 deletions test/lib/lib-helpers/contentfulEntry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() };
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit f222010

Please sign in to comment.