From a196f7f8113de37f187896331387c143e120f740 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 15:40:02 -0500 Subject: [PATCH 01/15] add a Response object to hold logic for how to reply --- src/app.js | 25 ++++++++++++++++++++++--- test/app.spec.js | 15 +++++++-------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/app.js b/src/app.js index d6175ff..b3905e9 100644 --- a/src/app.js +++ b/src/app.js @@ -24,6 +24,25 @@ const DEFAULT_HELP_REGEX = /^help\b/i; /*:: type Session = {count: number, profile: ?Object} */ +class Response { + /*:: _messenger: Messenger */ + constructor(messenger/*: Messenger */, options/*: Object */) { + Object.assign(this, options); + ['senderId', 'session'].forEach((required) => { + // $FlowFixMe + if (!this[required]) { + throw new Error(`Incomplete Response, missing ${required}: ${JSON.stringify(options)}`); + } + }); + this._messenger = messenger; + } + + send(response) { + // $FlowFixMe + return this._messenger.send(this.senderId, response, this.session._pageId); + } +} + class Messenger extends EventEmitter { /*:: app: Object */ /*:: cache: Object */ @@ -208,7 +227,7 @@ class Messenger extends EventEmitter { } } }; - this.send(senderId, messageData); + this.send(senderId, messageData, pageId); } getPublicProfile(senderId/*: number */, pageId/*: string|void */)/*: Promise */ { @@ -246,7 +265,7 @@ class Messenger extends EventEmitter { const senderId = event.sender.id; // The 'ref' is the data passed through the 'Send to Messenger' call const optinRef = event.optin.ref; - this.emit('auth', {event, senderId, session, optinRef}); + this.emit('auth', new Response(this, {event, senderId, session, optinRef})); debug('onAuth for user:%d with param: %j', senderId, optinRef); } @@ -266,7 +285,7 @@ class Messenger extends EventEmitter { const senderId = event.sender.id; const {message} = event; - this.emit('message', {event, senderId, session, message}); + this.emit('message', new Response(this, {event, senderId, session, message})); debug('onMessage from user:%d with message: %j', senderId, message); const { diff --git a/test/app.spec.js b/test/app.spec.js index 049eeef..a111d10 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -109,7 +109,7 @@ describe('app', () => { } }); - messenger.onAuth(event, {}); + messenger.onAuth(event, session); assert.equal(messenger.send.callCount, 0); }); @@ -159,7 +159,7 @@ describe('app', () => { message: {foo: 'bar'} }); - messenger.onMessage(event); + messenger.onMessage(event, session); }); it('emits "text" event', () => { @@ -168,7 +168,6 @@ describe('app', () => { text: 'message text' } }); - const fakeSession = {}; messenger.once('text', (payload) => { assert.ok(payload.event); assert.equal(payload.senderId, 'senderId'); @@ -176,7 +175,7 @@ describe('app', () => { assert.equal(payload.text, 'message text'); }); - messenger.onMessage(event, fakeSession); + messenger.onMessage(event, session); }); it('emits "quick reply" event', () => { @@ -195,7 +194,7 @@ describe('app', () => { } }); - messenger.onMessage(event, {}); + messenger.onMessage(event, session); }); @@ -218,7 +217,7 @@ describe('app', () => { } }); - messenger.onMessage(event); + messenger.onMessage(event, session); }); it('emits "sticker" event', () => { @@ -238,7 +237,7 @@ describe('app', () => { } }); - messenger.onMessage(event); + messenger.onMessage(event, session); }); it('emits "thumbsup" event', () => { @@ -260,7 +259,7 @@ describe('app', () => { } }); - messenger.onMessage(event); + messenger.onMessage(event, session); }); it('emits "greeting" event', () => { From 9961a4d56762c8f3f39b15dc713d1f796a0d2b4b Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 15:47:05 -0500 Subject: [PATCH 02/15] Convert as many emits to use Response as possible --- src/app.js | 14 +++++++------- test/app.spec.js | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/app.js b/src/app.js index b3905e9..5aa2538 100644 --- a/src/app.js +++ b/src/app.js @@ -300,12 +300,12 @@ class Messenger extends EventEmitter { const surName = session.profile && session.profile.last_name.trim() || ''; const fullName = `${firstName} ${surName}`; - this.emit('text.greeting', {event, senderId, session, firstName, surName, fullName}); + this.emit('text.greeting', new Response(this, {event, senderId, session, firstName, surName, fullName})); return; } if (this.help.test(text)) { - this.emit('text.help', {event, senderId, session}); + this.emit('text.help', new Response(this, {event, senderId, session})); return; } @@ -313,7 +313,7 @@ class Messenger extends EventEmitter { debug('message.quickReply payload: "%s"', quickReply.payload); this.emit('text', {event, senderId, session, source: 'quickReply', text: quickReply.payload}); - this.emit('message.quickReply', {event, senderId, session, payload: quickReply.payload}); + this.emit('message.quickReply', new Response(this, {event, senderId, session, payload: quickReply.payload})); return; } @@ -321,7 +321,7 @@ class Messenger extends EventEmitter { debug('text user:%d text: "%s" count: %s seq: %s', senderId, text, session.count, message.seq); this.emit('text', {event, senderId, session, source: 'text', text: text.toLowerCase().trim()}); - this.emit('message.text', {event, senderId, session, text}); + this.emit('message.text', new Response(this, {event, senderId, session, text})); return; } @@ -345,7 +345,7 @@ class Messenger extends EventEmitter { // - message.video // https://developers.facebook.com/docs/messenger-platform/webhook-reference/message - this.emit(`message.${type}`, {event, senderId, session, attachment, url: attachment.payload.url}); + this.emit(`message.${type}`, new Response(this, {event, senderId, session, attachment, url: attachment.payload.url})); return; } } @@ -359,8 +359,8 @@ class Messenger extends EventEmitter { debug("onPostback for user:%d with payload '%s'", senderId, payload); - this.emit('text', {event, senderId, session, source: 'postback', text: payload}); - this.emit('postback', {event, senderId, session, payload}); + this.emit('text', new Response(this, {event, senderId, session, source: 'postback', text: payload})); + this.emit('postback', new Response(this, {event, senderId, session, payload})); } // HELPERS diff --git a/test/app.spec.js b/test/app.spec.js index a111d10..d3fa67f 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -357,7 +357,7 @@ describe('app', () => { } }); - messenger.onPostback(event); + messenger.onPostback(event, session); }); }); From b496bd65587b491bd77ab319adf893402482e904 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 16:20:02 -0500 Subject: [PATCH 03/15] add coverage to Response object --- src/app.js | 3 ++- test/app.spec.js | 53 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/app.js b/src/app.js index 5aa2538..5318c1b 100644 --- a/src/app.js +++ b/src/app.js @@ -37,7 +37,7 @@ class Response { this._messenger = messenger; } - send(response) { + reply(response) { // $FlowFixMe return this._messenger.send(this.senderId, response, this.session._pageId); } @@ -429,3 +429,4 @@ class Messenger extends EventEmitter { exports.SESSION_TIMEOUT_MS = SESSION_TIMEOUT_MS; exports.Messenger = Messenger; +exports.Response = Response; diff --git a/test/app.spec.js b/test/app.spec.js index d3fa67f..061a3a0 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -5,7 +5,7 @@ const chaiHttp = require('chai-http'); const reqPromise = require('request-promise'); const sinon = require('sinon'); -const { Messenger } = require('../src/app'); +const { Messenger, Response } = require('../src/app'); chai.use(chaiHttp); @@ -18,7 +18,7 @@ describe('app', () => { }); beforeEach(() => { - sinon.stub(messenger, 'send'); + sinon.stub(messenger, 'send').resolves({}); session = { profile: { first_name: ' Guy ', @@ -32,6 +32,55 @@ describe('app', () => { messenger.send.restore && messenger.send.restore(); }); + describe('Response', () => { + let options; + + beforeEach(() => { + options = { + senderId: 1234, + session + }; + }); + + it('constructs', () => { + const response = new Response(messenger, options); + assert.ok(response); + }); + + + it('throws when passed an object without "senderId"', () => { + try { + delete options.senderId; + new Response(messenger, options); + assert.ok(false, 'This path should not run'); + } catch (err) { + assert.ok(err); + } + }); + + it('throws when passed an object without "senderId"', () => { + try { + delete options.session; + new Response(messenger, options); + assert.ok(false, 'This path should not run'); + } catch (err) { + assert.ok(err); + } + }); + + it('reply calls .send', () => { + options.session._pageId = 1337; + const response = new Response(messenger, options); + + response.reply('message back to user'); + + const args = messenger.send.args[0]; + assert.equal(args[0], options.senderId); + assert.equal(args[1], 'message back to user'); + assert.equal(args[2], options.session._pageId); + }); + }); + describe('constructor', () => { it('can use a supplied cache instead of the default', () => { const fakeCache = {}; From 281e8b4ac7595026b4ab4fd22dd0cff5bc54aa46 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 17:13:59 -0500 Subject: [PATCH 04/15] Document new reply() syntax in readme --- README.md | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 64b1aaf..db7a85e 100644 --- a/README.md +++ b/README.md @@ -40,10 +40,10 @@ We emit a variety of events. Attach listeners like: // messenger.on(eventName, ({dataItem1, dataItem2}) => {}); const { Messenger, Text, Image } = require('launch-vehicle-fbm'); const messenger = new Messenger(); -messenger.on('text', ({senderId, text}) => { +messenger.on('text', ({reply, text}) => { if (text.includes('corgis')) { - messenger.send(senderId, new Text('aRf aRf!')) - .then(() => messenger.send(senderId, new Image('https://i.imgur.com/izwcQLS.jpg'))); + reply(new Text('aRf aRf!')) + .then(() => reply(new Image('https://i.imgur.com/izwcQLS.jpg'))); } }); messenger.start(); @@ -52,17 +52,20 @@ messenger.start(); The event name and what's in the `data` for each event handler: * `message` Any kind of message event. This is sent in addition to the events for specific message types. + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `message` Direct access to `event.message` * `text` Text message + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `source` One of `quickReply`, `postback`, `text` * `text` Message content, `event.message.text` for text events, `payload` for `postback` and `quickReply` events * `text.greeting` (optional, defaults to enabled) Text messages that match common greetings + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate @@ -70,34 +73,41 @@ The event name and what's in the `data` for each event handler: * `surName` Trimmed first name from the user's public Facebook profile * `fullName` Concatenating of `firstName` and `surName` with a single, separating space * `text.help` (optional, defaults to enabled) Text messages that match requests for assistance + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `message.image` Image (both attached and from user's camera) + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `url` Direct access to `event.message.attachments[0].payload.url` for the url of the image * `message.sticker` Sticker + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `message.thumbsup` User clicked the "thumbsup"/"like" button + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `message.text` For conversation, use the `text` event + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `text` Message content, `event.message.text` for text events * `message.quickReply` For conversation, use the `text` event, this is for the raw message sent via a quick reply button + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `session` [A Session object](#the-session-object) you can mutate * `source` One of `quickReply`, `postback`, `text` * `payload` Quick reply content, `event.quick_reply.payload` * `postback` For conversation, use the `text` event, this is for the raw message sent via a postback + * `reply: Function` Reply back to the user with the arguments * `event` The raw event * `senderId` The ID of the sender * `payload` Direct access to `event.postback.payload` @@ -117,11 +127,13 @@ messages too. You'll need to examine `event.message.is_echo` in your handlers. ### Sending responses to the user -Send responses back to the user like: +You're given a `reply` in event emitters (see above): - messenger.send(senderId, responseObject, [pageId]) + reply(responseObject) -But this syntax will be deprecated for a simpler version in the future. +or you can manually send responses back to the user like: + + messenger.send(senderId, responseObject, pageId) Some factories for generating `responseObject` are available at the top level and are also available in a `responses` object if you need a namespace: From 79756724b6664a58c8607b1af444195ccab45f28 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 17:42:18 -0500 Subject: [PATCH 05/15] add failing test case --- test/app.spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/app.spec.js b/test/app.spec.js index 061a3a0..206fdee 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -79,6 +79,18 @@ describe('app', () => { assert.equal(args[1], 'message back to user'); assert.equal(args[2], options.session._pageId); }); + + it('reply calls .send when called without context', () => { + options.session._pageId = 1337; + const { reply } = new Response(messenger, options); + + reply('message back to user'); + + const args = messenger.send.args[0]; + assert.equal(args[0], options.senderId); + assert.equal(args[1], 'message back to user'); + assert.equal(args[2], options.session._pageId); + }); }); describe('constructor', () => { From 07b4103ec10cb9150111a870dfc458fa771b2030 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 17:42:52 -0500 Subject: [PATCH 06/15] get reply to work even when destructured --- src/app.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app.js b/src/app.js index 5318c1b..e8f009b 100644 --- a/src/app.js +++ b/src/app.js @@ -35,6 +35,7 @@ class Response { } }); this._messenger = messenger; + this.reply = this.reply.bind(this); } reply(response) { From f302e29cec465918014c9a61f9952a12a61703e7 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 17:50:11 -0500 Subject: [PATCH 07/15] Make send argument more logical while keeping backwards compatibility --- src/app.js | 17 +++++++++++++++-- test/app.spec.js | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/app.js b/src/app.js index e8f009b..0956dd3 100644 --- a/src/app.js +++ b/src/app.js @@ -35,6 +35,7 @@ class Response { } }); this._messenger = messenger; + // $FlowFixMe this.reply = this.reply.bind(this); } @@ -228,7 +229,7 @@ class Messenger extends EventEmitter { } } }; - this.send(senderId, messageData, pageId); + this.send(pageId, senderId, messageData); } getPublicProfile(senderId/*: number */, pageId/*: string|void */)/*: Promise */ { @@ -375,7 +376,19 @@ class Messenger extends EventEmitter { return this.cache.set(session._key, session); } - send(recipientId/*: string|number */, messageData/*: Object */, pageId/*: string|void */)/* Promise */ { + send(arg1/*: string|number */, arg2/*: string|number|Object */, arg3/*: Object|void */)/* Promise */ { + let recipientId; + let messageData; + let pageId; + // DELETEME use simpler logic once all three args are required + if (arg3) { + pageId = arg1; + recipientId = arg2; + messageData = arg3; + } else { + recipientId = arg1; + messageData = arg2; + } let pageAccessToken; if (!pageId) { // This will be deprecated in the future in favor of finding the token from `this.pages` diff --git a/test/app.spec.js b/test/app.spec.js index 206fdee..2b6f62f 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -445,7 +445,7 @@ describe('app', () => { it('passes sender id and message', () => { const myMessenger = new Messenger({pages: {1337: '1337accesstoken'}}); - return myMessenger.send('senderId', {foo: 'bar'}, 1337) + return myMessenger.send(1337, 'senderId', {foo: 'bar'}) .then(() => { assert.equal(reqPromise.post.args[0][0].qs.access_token, '1337accesstoken'); assert.equal(reqPromise.post.args[0][0].json.recipient.id, 'senderId'); @@ -462,7 +462,7 @@ describe('app', () => { }); it('passes sender id and message with deprecated config', () => { - return messenger.send('senderId', {foo: 'bar'}, 1029384756) // from example.env + return messenger.send(1029384756, 'senderId', {foo: 'bar'}) // from example.env .then(() => { assert.equal(reqPromise.post.args[0][0].json.recipient.id, 'senderId'); assert.deepEqual(reqPromise.post.args[0][0].json.message, {foo: 'bar'}); From 510345fe69dafd3f4489d2b86fc278c34a46a939 Mon Sep 17 00:00:00 2001 From: crccheck Date: Thu, 13 Apr 2017 22:52:45 -0500 Subject: [PATCH 08/15] cleanup --- README.md | 6 +++++- src/app.js | 24 +++++++++++------------- test/app.spec.js | 1 - 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index db7a85e..f2419ef 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,11 @@ You're given a `reply` in event emitters (see above): or you can manually send responses back to the user like: - messenger.send(senderId, responseObject, pageId) + messenger.send(pageId, senderId, responseObject) + +or you may be using the original, now deprecated syntax: + + messenger.send(senderId, responseObject) Some factories for generating `responseObject` are available at the top level and are also available in a `responses` object if you need a namespace: diff --git a/src/app.js b/src/app.js index 0956dd3..8811ea4 100644 --- a/src/app.js +++ b/src/app.js @@ -202,6 +202,7 @@ class Messenger extends EventEmitter { .then((session) => this.saveSession(session)); } + // WIP doLogin(senderId/*: number */, pageId/*: string */) { // Open question: is building the event object worth it for the 'emit'? const event = { @@ -275,6 +276,7 @@ class Messenger extends EventEmitter { This is not an event triggered by Messenger, it is the post-back from the static Facebook login page that is made to look similar to an 'event' */ + // WIP onLink(event) { const senderId = event.sender.id; const fbData = event.facebook; @@ -314,7 +316,7 @@ class Messenger extends EventEmitter { if (quickReply) { debug('message.quickReply payload: "%s"', quickReply.payload); - this.emit('text', {event, senderId, session, source: 'quickReply', text: quickReply.payload}); + this.emit('text', new Response(this, {event, senderId, session, source: 'quickReply', text: quickReply.payload})); this.emit('message.quickReply', new Response(this, {event, senderId, session, payload: quickReply.payload})); return; } @@ -322,7 +324,7 @@ class Messenger extends EventEmitter { if (text) { debug('text user:%d text: "%s" count: %s seq: %s', senderId, text, session.count, message.seq); - this.emit('text', {event, senderId, session, source: 'text', text: text.toLowerCase().trim()}); + this.emit('text', new Response(this, {event, senderId, session, source: 'text', text: text.toLowerCase().trim()})); this.emit('message.text', new Response(this, {event, senderId, session, text})); return; } @@ -377,28 +379,24 @@ class Messenger extends EventEmitter { } send(arg1/*: string|number */, arg2/*: string|number|Object */, arg3/*: Object|void */)/* Promise */ { - let recipientId; + // DELETEME use simpler logic once all three args are required let messageData; + let pageAccessToken; let pageId; - // DELETEME use simpler logic once all three args are required + let recipientId; if (arg3) { pageId = arg1; recipientId = arg2; messageData = arg3; - } else { - recipientId = arg1; - messageData = arg2; - } - let pageAccessToken; - if (!pageId) { - // This will be deprecated in the future in favor of finding the token from `this.pages` - pageAccessToken = config.get('messenger.pageAccessToken'); - } else { pageAccessToken = this.pages[pageId]; // eslint-disable-next-line eqeqeq if (!pageAccessToken && pageId != config.get('facebook.pageId')) { throw new Error(`Tried accessing a profile for page ${pageId} but the page config is missing`); } + } else { + recipientId = arg1; + messageData = arg2; + pageAccessToken = config.get('messenger.pageAccessToken'); } const options = { uri: 'https://graph.facebook.com/v2.8/me/messages', diff --git a/test/app.spec.js b/test/app.spec.js index 2b6f62f..4003d6a 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -47,7 +47,6 @@ describe('app', () => { assert.ok(response); }); - it('throws when passed an object without "senderId"', () => { try { delete options.senderId; From 84f6376512717e5658a7097290c048c655499ea8 Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 12:13:04 -0500 Subject: [PATCH 09/15] make a new .pageSend that legacy .send wraps --- src/app.js | 33 +++++++++++++-------------------- test/app.spec.js | 38 +++++++++++++++++++------------------- 2 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/app.js b/src/app.js index 8811ea4..719a22c 100644 --- a/src/app.js +++ b/src/app.js @@ -41,7 +41,7 @@ class Response { reply(response) { // $FlowFixMe - return this._messenger.send(this.senderId, response, this.session._pageId); + return this._messenger.pageSend(this.session._pageId, this.senderId, response); } } @@ -230,7 +230,7 @@ class Messenger extends EventEmitter { } } }; - this.send(pageId, senderId, messageData); + this.pageSend(pageId, senderId, messageData); } getPublicProfile(senderId/*: number */, pageId/*: string|void */)/*: Promise */ { @@ -378,25 +378,18 @@ class Messenger extends EventEmitter { return this.cache.set(session._key, session); } - send(arg1/*: string|number */, arg2/*: string|number|Object */, arg3/*: Object|void */)/* Promise */ { - // DELETEME use simpler logic once all three args are required - let messageData; - let pageAccessToken; - let pageId; - let recipientId; - if (arg3) { - pageId = arg1; - recipientId = arg2; - messageData = arg3; - pageAccessToken = this.pages[pageId]; - // eslint-disable-next-line eqeqeq - if (!pageAccessToken && pageId != config.get('facebook.pageId')) { - throw new Error(`Tried accessing a profile for page ${pageId} but the page config is missing`); + send(recipientId/*: number */, messageData/*: Object */) { + return this.pageSend(config.get('facebook.pageId'), recipientId, messageData); + } + + pageSend(pageId/*: string|number */, recipientId/*: string|number */, messageData/*: Object */)/* Promise */ { + let pageAccessToken = this.pages[pageId]; + if (!pageAccessToken) { + if (pageId === config.get('facebook.pageId')) { + pageAccessToken = config.get('messenger.pageAccessToken'); + } else { + throw new Error(`Missing page config for: ${pageId}`); } - } else { - recipientId = arg1; - messageData = arg2; - pageAccessToken = config.get('messenger.pageAccessToken'); } const options = { uri: 'https://graph.facebook.com/v2.8/me/messages', diff --git a/test/app.spec.js b/test/app.spec.js index 4003d6a..9386c2a 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -18,7 +18,7 @@ describe('app', () => { }); beforeEach(() => { - sinon.stub(messenger, 'send').resolves({}); + sinon.stub(messenger, 'pageSend').resolves({}); session = { profile: { first_name: ' Guy ', @@ -29,7 +29,7 @@ describe('app', () => { afterEach(() => { // TODO investigate making the suite mock `reqPromise.post` instead of `send` - messenger.send.restore && messenger.send.restore(); + messenger.pageSend.restore && messenger.pageSend.restore(); }); describe('Response', () => { @@ -67,28 +67,28 @@ describe('app', () => { } }); - it('reply calls .send', () => { + it('reply calls .pageSend', () => { options.session._pageId = 1337; const response = new Response(messenger, options); response.reply('message back to user'); - const args = messenger.send.args[0]; - assert.equal(args[0], options.senderId); - assert.equal(args[1], 'message back to user'); - assert.equal(args[2], options.session._pageId); + const args = messenger.pageSend.args[0]; + assert.equal(args[0], options.session._pageId); + assert.equal(args[1], options.senderId); + assert.equal(args[2], 'message back to user'); }); - it('reply calls .send when called without context', () => { + it('reply calls .pageSend when called without context', () => { options.session._pageId = 1337; const { reply } = new Response(messenger, options); reply('message back to user'); - const args = messenger.send.args[0]; - assert.equal(args[0], options.senderId); - assert.equal(args[1], 'message back to user'); - assert.equal(args[2], options.session._pageId); + const args = messenger.pageSend.args[0]; + assert.equal(args[0], options.session._pageId); + assert.equal(args[1], options.senderId); + assert.equal(args[2], 'message back to user'); }); }); @@ -110,7 +110,7 @@ describe('app', () => { messenger.doLogin('narf'); - assert.equal(messenger.send.callCount, 1); + assert.equal(messenger.pageSend.callCount, 1); }); }); @@ -171,7 +171,7 @@ describe('app', () => { messenger.onAuth(event, session); - assert.equal(messenger.send.callCount, 0); + assert.equal(messenger.pageSend.callCount, 0); }); }); @@ -425,7 +425,7 @@ describe('app', () => { let postStub; beforeEach(() => { - messenger.send.restore(); + messenger.pageSend.restore(); postStub = sinon.stub(reqPromise, 'post').resolves({}); }); @@ -435,16 +435,16 @@ describe('app', () => { it('throws if messenger is missing page configuration', () => { try { - messenger.send('senderId', {foo: 'bar'}, 1337); + messenger.pageSend(1337, 'senderId', {foo: 'bar'}); assert.ok(false, 'This path should not execute'); } catch (err) { - assert.equal(err.message.substr(0, 15), 'Tried accessing'); + assert.equal(err.message.substr(0, 19), 'Missing page config'); } }); it('passes sender id and message', () => { const myMessenger = new Messenger({pages: {1337: '1337accesstoken'}}); - return myMessenger.send(1337, 'senderId', {foo: 'bar'}) + return myMessenger.pageSend(1337, 'senderId', {foo: 'bar'}) .then(() => { assert.equal(reqPromise.post.args[0][0].qs.access_token, '1337accesstoken'); assert.equal(reqPromise.post.args[0][0].json.recipient.id, 'senderId'); @@ -461,7 +461,7 @@ describe('app', () => { }); it('passes sender id and message with deprecated config', () => { - return messenger.send(1029384756, 'senderId', {foo: 'bar'}) // from example.env + return messenger.send('senderId', {foo: 'bar'}) .then(() => { assert.equal(reqPromise.post.args[0][0].json.recipient.id, 'senderId'); assert.deepEqual(reqPromise.post.args[0][0].json.message, {foo: 'bar'}); From 9aed7a8d202acf75c57e4d2aa79e3bc9bf90923a Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 14:22:42 -0500 Subject: [PATCH 10/15] align warnings --- src/app.js | 4 +++- test/app.spec.js | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app.js b/src/app.js index 719a22c..8427413 100644 --- a/src/app.js +++ b/src/app.js @@ -242,7 +242,7 @@ class Messenger extends EventEmitter { pageAccessToken = this.pages[pageId]; // eslint-disable-next-line eqeqeq if (!pageAccessToken && pageId != config.get('facebook.pageId')) { - throw new Error(`Tried accessing a profile for page ${pageId} but the page config is missing`); + throw new Error(`Missing page config for: ${pageId}`); } pageAccessToken = config.get('messenger.pageAccessToken'); } @@ -379,6 +379,7 @@ class Messenger extends EventEmitter { } send(recipientId/*: number */, messageData/*: Object */) { + debug('DEPRECATED instead of `.send`, use `reply` or `.pageSend`'); return this.pageSend(config.get('facebook.pageId'), recipientId, messageData); } @@ -386,6 +387,7 @@ class Messenger extends EventEmitter { let pageAccessToken = this.pages[pageId]; if (!pageAccessToken) { if (pageId === config.get('facebook.pageId')) { + // DELETEME after page config is out of config pageAccessToken = config.get('messenger.pageAccessToken'); } else { throw new Error(`Missing page config for: ${pageId}`); diff --git a/test/app.spec.js b/test/app.spec.js index 9386c2a..f8bf197 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -135,7 +135,7 @@ describe('app', () => { messenger.getPublicProfile(12345, 1337); assert.ok(false, 'This path should not execute'); } catch (err) { - assert.equal(err.message.substr(0, 15), 'Tried accessing'); + assert.equal(err.message.substr(0, 19), 'Missing page config'); } }); From 332de7ef37dc2ae2bd856f7b4360b8f5442f5cf6 Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 15:28:32 -0500 Subject: [PATCH 11/15] undeprecate setting a config --- package.json | 4 ++-- src/app.js | 32 +++++++++++++------------------- test.env | 10 ++++++++++ test/app.spec.js | 27 +++++++++++++++++++++++++-- 4 files changed, 50 insertions(+), 23 deletions(-) create mode 100644 test.env diff --git a/package.json b/package.json index a9128fa..d982096 100644 --- a/package.json +++ b/package.json @@ -6,8 +6,8 @@ "scripts": { "lint": "eslint --ignore-path .gitignore .", "pretest": "npm run lint", - "tdd": "NODE_ENV=test env $(cat example.env | xargs) mocha -R dot --recursive --watch", - "test": "NODE_ENV=test env $(cat example.env | xargs) istanbul cover _mocha -- --recursive" + "tdd": "NODE_ENV=test env $(cat test.env | xargs) mocha -R dot --recursive --watch", + "test": "NODE_ENV=test env $(cat test.env | xargs) istanbul cover _mocha -- --recursive" }, "engines": { "node": ">6.0" diff --git a/src/app.js b/src/app.js index 8427413..fa7cc65 100644 --- a/src/app.js +++ b/src/app.js @@ -82,7 +82,15 @@ class Messenger extends EventEmitter { this.cache = new Cacheman('sessions', {ttl: SESSION_TIMEOUT_MS / 1000}); } - this.pages = pages; + if (pages && Object.keys(pages).length) { + this.pages = pages; + } else if (config.has('messenger.pageAccessToken') && config.has('facebook.pageId')) { + this.pages = {}; + this.pages[config.get('facebook.pageId')] = config.get('messenger.pageAccessToken'); + } else { + this.pages = {}; + debug("MISSING options.pages; you won't be able to reply or get profile information"); + } this.app = express(); this.app.engine('handlebars', exphbs({defaultLayout: 'main'})); @@ -234,17 +242,9 @@ class Messenger extends EventEmitter { } getPublicProfile(senderId/*: number */, pageId/*: string|void */)/*: Promise */ { - let pageAccessToken; - if (!pageId) { - // This will be deprecated in the future in favor of finding the token from `this.pages` - pageAccessToken = config.get('messenger.pageAccessToken'); - } else { - pageAccessToken = this.pages[pageId]; - // eslint-disable-next-line eqeqeq - if (!pageAccessToken && pageId != config.get('facebook.pageId')) { - throw new Error(`Missing page config for: ${pageId}`); - } - pageAccessToken = config.get('messenger.pageAccessToken'); + const pageAccessToken = this.pages[pageId || config.get('facebook.pageId')]; + if (!pageAccessToken) { + throw new Error(`Missing page config for: ${pageId || ''}`); } const options = { json: true, @@ -379,19 +379,13 @@ class Messenger extends EventEmitter { } send(recipientId/*: number */, messageData/*: Object */) { - debug('DEPRECATED instead of `.send`, use `reply` or `.pageSend`'); return this.pageSend(config.get('facebook.pageId'), recipientId, messageData); } pageSend(pageId/*: string|number */, recipientId/*: string|number */, messageData/*: Object */)/* Promise */ { let pageAccessToken = this.pages[pageId]; if (!pageAccessToken) { - if (pageId === config.get('facebook.pageId')) { - // DELETEME after page config is out of config - pageAccessToken = config.get('messenger.pageAccessToken'); - } else { - throw new Error(`Missing page config for: ${pageId}`); - } + throw new Error(`Missing page config for: ${pageId}`); } const options = { uri: 'https://graph.facebook.com/v2.8/me/messages', diff --git a/test.env b/test.env new file mode 100644 index 0000000..0ab6d0d --- /dev/null +++ b/test.env @@ -0,0 +1,10 @@ +FACEBOOK_APP_ID=1234567890123456 +FACEBOOK_PAGE_ID=1029384756 +MESSENGER_APP_SECRET=0123456789abcdef0123456789abcdef +MESSENGER_VALIDATION_TOKEN=validate_me +MESSENGER_PAGE_ACCESS_TOKEN=ThatsAReallyLongStringYouGotThere +SERVER_URL=https://localhost/ +SLACK_WEBHOOK_URL=https://hooks.slack.com/services/T123456/B33Z/Pajamas +SLACK_CHANNEL=channel +LOG_FILE=/var/log/my-bot/chat.log +ALLOW_CONFIG_MUTATIONS=Y diff --git a/test/app.spec.js b/test/app.spec.js index f8bf197..b4e218d 100644 --- a/test/app.spec.js +++ b/test/app.spec.js @@ -6,6 +6,7 @@ const reqPromise = require('request-promise'); const sinon = require('sinon'); const { Messenger, Response } = require('../src/app'); +const config = require('../src/config'); chai.use(chaiHttp); @@ -98,6 +99,28 @@ describe('app', () => { const messenger = new Messenger({cache: fakeCache}); assert.strictEqual(messenger.cache, fakeCache); }); + + it('sets .pages based on config if none supplied', () => { + const messenger = new Messenger(); + // based on fixture in `test.env` + assert.strictEqual(messenger.pages[1029384756], 'ThatsAReallyLongStringYouGotThere'); + }); + + it('sets .pages based on options', () => { + const messenger = new Messenger({pages: {1337: '1337accesstoken'}}); + assert.strictEqual(messenger.pages[1337], '1337accesstoken'); + }); + + it('allows you to not pass in pages config at all', () => { + const originalpageId = config.facebook.pageId; + delete config.facebook.pageId; + + const messenger = new Messenger(); + + assert.deepEqual(messenger.pages, {}); + + config.facebook.pageId = originalpageId; + }); }); describe('doLogin', function () { @@ -139,8 +162,8 @@ describe('app', () => { } }); - it('gets public profile with missing page configuration with deprecated config', () => { - return messenger.getPublicProfile(12345, 1029384756) // from example.env + it('gets public profile with missing page configuration with 1page config', () => { + return messenger.getPublicProfile(12345, 1029384756) // from test.env .then((profile) => { assert.ok(profile); }); From f6a2da179590f97b1fe9092bc008c3a4c71c8da9 Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 15:40:34 -0500 Subject: [PATCH 12/15] update readme to match code --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f2419ef..c61ea79 100644 --- a/README.md +++ b/README.md @@ -131,13 +131,13 @@ You're given a `reply` in event emitters (see above): reply(responseObject) -or you can manually send responses back to the user like: +The original syntax will also work: - messenger.send(pageId, senderId, responseObject) + messenger.send(senderId, responseObject) -or you may be using the original, now deprecated syntax: +or if you have multiple Pages, you can send responses like: - messenger.send(senderId, responseObject) + messenger.pageSend(pageId, senderId, responseObject) Some factories for generating `responseObject` are available at the top level and are also available in a `responses` object if you need a namespace: From 7a3d910865f411fcd49bdd707c0b7b61b3c483c7 Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 15:52:37 -0500 Subject: [PATCH 13/15] PR feedback: more context on WIP --- src/app.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app.js b/src/app.js index fa7cc65..fdb3e3c 100644 --- a/src/app.js +++ b/src/app.js @@ -210,7 +210,7 @@ class Messenger extends EventEmitter { .then((session) => this.saveSession(session)); } - // WIP + // TODO flesh these out later doLogin(senderId/*: number */, pageId/*: string */) { // Open question: is building the event object worth it for the 'emit'? const event = { @@ -242,6 +242,7 @@ class Messenger extends EventEmitter { } getPublicProfile(senderId/*: number */, pageId/*: string|void */)/*: Promise */ { + // TODO make `pageId` required, then simplify. `getPublicProfile` is only internal right now const pageAccessToken = this.pages[pageId || config.get('facebook.pageId')]; if (!pageAccessToken) { throw new Error(`Missing page config for: ${pageId || ''}`); @@ -276,7 +277,7 @@ class Messenger extends EventEmitter { This is not an event triggered by Messenger, it is the post-back from the static Facebook login page that is made to look similar to an 'event' */ - // WIP + // TODO flesh these out later onLink(event) { const senderId = event.sender.id; const fbData = event.facebook; From add7f2027ffb8e2e3a638045828f1b040232048e Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 18:44:32 -0500 Subject: [PATCH 14/15] delint a simple FlowFixMe --- package.json | 2 +- src/app.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index d982096..58efad1 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "chai-http": "^3.0.0", "eslint": "^3.19.0", "eslint-plugin-import": "^2.2.0", - "flow-bin": "^0.43.1", + "flow-bin": "^0.44.0", "istanbul": "^0.4.5", "mocha": "^3.2.0", "sinon": "^2.1.0" diff --git a/src/app.js b/src/app.js index fdb3e3c..8e0aedc 100644 --- a/src/app.js +++ b/src/app.js @@ -22,10 +22,12 @@ const SESSION_TIMEOUT_MS = 3600 * 1000; // 1 hour const DEFAULT_GREETINGS_REGEX = /^(get started|good(morning|afternoon)|hello|hey|hi|hola|what's up)/i; const DEFAULT_HELP_REGEX = /^help\b/i; -/*:: type Session = {count: number, profile: ?Object} */ +/*:: type Session = {_pageId: string|number, count: number, profile: ?Object} */ class Response { /*:: _messenger: Messenger */ + /*:: senderId: string|number */ + /*:: session: Session */ constructor(messenger/*: Messenger */, options/*: Object */) { Object.assign(this, options); ['senderId', 'session'].forEach((required) => { @@ -40,7 +42,6 @@ class Response { } reply(response) { - // $FlowFixMe return this._messenger.pageSend(this.session._pageId, this.senderId, response); } } From 5c87489f9cc6fed33c2da3104d891edaf51d8617 Mon Sep 17 00:00:00 2001 From: crccheck Date: Fri, 14 Apr 2017 19:01:23 -0500 Subject: [PATCH 15/15] save 1 LOC with fancy esnext syntax I can't remember --- src/app.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/app.js b/src/app.js index 8e0aedc..058b7b6 100644 --- a/src/app.js +++ b/src/app.js @@ -86,8 +86,7 @@ class Messenger extends EventEmitter { if (pages && Object.keys(pages).length) { this.pages = pages; } else if (config.has('messenger.pageAccessToken') && config.has('facebook.pageId')) { - this.pages = {}; - this.pages[config.get('facebook.pageId')] = config.get('messenger.pageAccessToken'); + this.pages = {[config.get('facebook.pageId')]: config.get('messenger.pageAccessToken')}; } else { this.pages = {}; debug("MISSING options.pages; you won't be able to reply or get profile information");