Skip to content

Commit

Permalink
refactor: Pass bot messages by return value
Browse files Browse the repository at this point in the history
* Bot.respondWhenPostback(), Bot.respondWhenImage(),
Bot.respondWhenText() now return the list of bot messages.
* DialogManager.executeClassificationResults() and
DialogManager.executeDialog() now return the list
of bot messages.
* DialogManager.execute() takes the previous bot messages
as its 3rd parameter.
* DialogManager.execute returns an object with keys
dialogs and botMessages.

BREAKING CHANGES:
* Dialog.execute() returns an object with keys action and
botMessage().
  • Loading branch information
Michel Blancard committed Apr 18, 2018
1 parent 656d884 commit 2501685
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 50 deletions.
10 changes: 9 additions & 1 deletion packages/botfuel-dialog/src/adapters/adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,15 @@ class Adapter {
async handleMessage(userMessage) {
logger.debug('handleMessage', userMessage);
await this.addUserIfNecessary(userMessage.user);
return this.bot.handleMessage(this.extendMessage(userMessage));
const botMessages = await this.bot.handleMessage(this.extendMessage(userMessage));

for (const botMessage of botMessages) {
// TODO: Remove this ugly line
const extendedBotMessage = this.extendMessage(botMessage);

// eslint-disable-next-line no-await-in-loop
await this.sendMessage(extendedBotMessage);
}
}

/**
Expand Down
68 changes: 32 additions & 36 deletions packages/botfuel-dialog/src/bot.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,31 @@ class Bot {
* @returns {Promise.<void>}
*/
async handleMessage(userMessage) {
let botMessages = [];

logger.debug('handleMessage', userMessage);
const context = {

const contextIn = {
user: userMessage.user,
brain: this.brain,
userMessage,
config: this.config,
};
await this.middlewareManager.in(context, async () => {
await this.respond(userMessage);

await this.middlewareManager.in(contextIn, async () => {
botMessages = await this.respond(userMessage);
});

const contextOut = {
user: userMessage.user,
brain: this.brain,
botMessages,
config: this.config,
userMessage,
};
await this.middlewareManager.out(contextOut, async () => {});

return botMessages;
}

/**
Expand All @@ -171,18 +186,23 @@ class Bot {
* @returns {Promise.<void>}
*/
async respond(userMessage) {
let botMessages;

logger.debug('respond', userMessage);

switch (userMessage.type) {
case 'postback':
await this.respondWhenPostback(userMessage);
botMessages = await this.respondWhenPostback(userMessage);
break;
case 'image':
await this.respondWhenImage(userMessage);
botMessages = await this.respondWhenImage(userMessage);
break;
case 'text':
default:
await this.respondWhenText(userMessage);
botMessages = await this.respondWhenText(userMessage);
}

return botMessages;
}

/**
Expand All @@ -204,11 +224,12 @@ class Bot {
});

logger.debug('respondWhenText: classificationResults', classificationResults, messageEntities);
await this.dm.executeClassificationResults(
const botMessages = await this.dm.executeClassificationResults(
userMessage,
classificationResults,
messageEntities,
);
return botMessages;
}

/**
Expand All @@ -224,7 +245,8 @@ class Bot {
name: userMessage.payload.value.dialog,
data: { messageEntities: userMessage.payload.value.entities },
};
await this.dm.executeDialog(userMessage, dialog);
const botMessages = await this.dm.executeDialog(userMessage, dialog);
return botMessages;
}

/**
Expand All @@ -240,34 +262,8 @@ class Bot {
name: 'image',
data: { url: userMessage.payload.value },
};
await this.dm.executeDialog(userMessage, dialog);
}

/**
* Iterates over the bot messages and send them to adapter.
* @async
* @param {Object[]} botMessages - the bot messages
* @param {Object} userMessage - the user message
* @returns {Promise.<void>}
*/
async send(botMessages, userMessage) {
logger.debug('send', botMessages);
const context = {
user: userMessage.user,
brain: this.brain,
botMessages,
config: this.config,
userMessage,
};
await this.middlewareManager.out(context, async () => {
for (const botMessage of botMessages) {
// TODO: Remove this ugly line
const extendedBotMessage = this.adapter.extendMessage(botMessage);

// eslint-disable-next-line no-await-in-loop
await this.adapter.sendMessage(extendedBotMessage);
}
});
const botMessages = await this.dm.executeDialog(userMessage, dialog);
return botMessages;
}

/**
Expand Down
31 changes: 24 additions & 7 deletions packages/botfuel-dialog/src/dialog-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,15 @@ class DialogManager extends Resolver {
* @param {Object[]} dialogs - the dialogs data
* @returns {Promise.<Object[]>}
*/
async execute(userMessage, dialogs) {
async execute(userMessage, dialogs, previousBotMessages) {
let botMessages = previousBotMessages;

logger.debug('execute', userMessage, dialogs);
if (dialogs.stack.length === 0) {
return dialogs;
return {
dialogs,
botMessages,
};
}
const dialog = dialogs.stack[dialogs.stack.length - 1];
if (dialog.blocked) {
Expand All @@ -239,15 +244,23 @@ class DialogManager extends Resolver {
data: {},
});
} else {
const action = await this.resolve(dialog.name).execute(userMessage, dialog.data);
const { action, botMessages: newBotMessages } = await this.resolve(dialog.name).execute(
userMessage,
dialog.data,
);
botMessages = botMessages.concat(newBotMessages);

logger.debug('execute: action', action);
if (action.name !== Dialog.ACTION_WAIT) {
dialogs = await this.applyAction(dialogs, action);
} else {
return dialogs;
return {
dialogs,
botMessages,
};
}
}
return this.execute(userMessage, dialogs);
return this.execute(userMessage, dialogs, botMessages);
}

/**
Expand All @@ -262,7 +275,9 @@ class DialogManager extends Resolver {
const userId = userMessage.user;
const dialogs = await this.getDialogs(userId);
this.updateWithClassificationResults(userId, dialogs, classificationResults, messageEntities);
await this.setDialogs(userId, await this.execute(userMessage, dialogs));
const { dialogs: newDialogs, botMessages } = await this.execute(userMessage, dialogs, []);
await this.setDialogs(userId, newDialogs);
return botMessages;
}

/**
Expand All @@ -276,7 +291,9 @@ class DialogManager extends Resolver {
const userId = userMessage.user;
const dialogs = await this.getDialogs(userId);
this.updateWithDialog(dialogs, newDialog);
await this.setDialogs(userId, await this.execute(userMessage, dialogs));
const { dialogs: newDialogs, botMessages } = await this.execute(userMessage, dialogs, []);
await this.setDialogs(userId, newDialogs);
return botMessages;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const classificationDisambiguationDialog = { name: 'classification-disambiguatio

describe('DialogManager', () => {
const bot = new Bot(TEST_CONFIG);
const { brain, dm, adapter } = bot;
const { brain, dm } = bot;

beforeEach(async () => {
await brain.clean();
Expand All @@ -52,8 +52,8 @@ describe('DialogManager', () => {
});

test('should not crash when no intent', async () => {
await dm.executeClassificationResults({ user: TEST_USER }, [], []);
expect(adapter.log).toEqual([new BotTextMessage('Not understood.').toJson(TEST_USER)]);
const botMessages = await dm.executeClassificationResults({ user: TEST_USER }, [], []);
expect(botMessages).toEqual([new BotTextMessage('Not understood.').toJson(TEST_USER)]);
});

test('should keep on the stack a dialog which is waiting', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ describe('ClassificationDisambiguationDialog', () => {
}),
];

expect(
await dialog.execute({ user: 'TEST_USER' }, { classificationResults, messageEntities: [] }),
).toEqual({
const { action } = await dialog.execute(
{ user: 'TEST_USER' },
{ classificationResults, messageEntities: [] },
);
expect(action).toEqual({
name: ClassificationDisambiguationDialog.ACTION_COMPLETE,
});
});
Expand Down

0 comments on commit 2501685

Please sign in to comment.