From e85e946e50882ede14a780bb9db2b213f192c6f1 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 17:17:20 -0500 Subject: [PATCH 01/39] Add interview option This can be opt-in for experimenting at the beginning until we decide the UX is adequate. --- src/direct-message-handler.js | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index bf55c5e..b9c7acf 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -24,7 +24,36 @@ class DirectMessageHandler { var user = this.adapter.getUser(message.user); - if (text === 'info') { + // TODO replace the self-identification request with this + if (text === 'interview') { + channel.postMessage({ + text: DirectMessageHandler.INTERVIEW_INTRODUCTION, + attachments: [{ + title: 'Would you like to self-identify?', + callback_id: 'initial', + color: '#ccc', + attachment_type: 'default', + actions: [ + { + name: 'yes', + text: 'Yes', + type: 'button', + value: 'yes' + }, { + name: 'no', + text: 'No', + type: 'button', + value: 'no' + }, { + name: 'more', + text: 'Tell me more', + type: 'button', + value: 'more' + } + ] + }] + }); + } else if (text === 'info') { this.handleInformationRequest(channel, message); } else if (text === 'help') { this.handleHelpRequest(channel, message); @@ -179,6 +208,7 @@ class DirectMessageHandler { // TODO maybe messages should be collected somewhere central, and parameterised DirectMessageHandler.HELP_MESSAGE = 'You can let me know “I’m not a man”, “I am a person of colour”, “it’s complicated whether I am white” and other such variations, or ask for my current information on you with “info”. View my source at https://github.com/backspace/slack-statsbot'; -DirectMessageHandler.VERBOSE_HELP_MESSAGE = `Hey, I’m a bot that collects statistics on who is taking up space in the channels I’m in. For now, I only track whether or not a participant is a man and/or a person of colour. ${DirectMessageHandler.HELP_MESSAGE}`; +DirectMessageHandler.INTERVIEW_INTRODUCTION = 'Hey, I’m a bot that collects statistics on who is taking up space in the channels I’m in. For now, I only track whether or not a participant is a man and/or a person of colour.'; +DirectMessageHandler.VERBOSE_HELP_MESSAGE = `${DirectMessageHandler.INTERVIEW_INTRODUCTION} ${DirectMessageHandler.HELP_MESSAGE}`; module.exports = DirectMessageHandler; From 1e8319d8e2e2f019d8a90a7ab4d029b29f4eef13 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 17:41:55 -0500 Subject: [PATCH 02/39] Add server with handling of interview rejection --- package.json | 3 +++ src/message-buttons/server.js | 18 ++++++++++++++++++ test/message-buttons/server.js | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 src/message-buttons/server.js create mode 100644 test/message-buttons/server.js diff --git a/package.json b/package.json index a5cb2ac..0d47e9f 100644 --- a/package.json +++ b/package.json @@ -28,12 +28,15 @@ "grunt-tape": "0.0.2", "require-subvert": "^0.1.0", "sinon": "^1.12.2", + "supertest-koa-agent": "^0.2.1", "tape": "^3.4.0" }, "dependencies": { "cli-table": "^0.3.1", "convict": "^0.6.1", "cron-parser": "^0.6.1", + "koa": "^1.2.0", + "koa-router": "^5.4.0", "lodash.every": "^3.2.0", "lodash.find": "^3.2.0", "lodash.map": "^3.1.4", diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js new file mode 100644 index 0000000..1308285 --- /dev/null +++ b/src/message-buttons/server.js @@ -0,0 +1,18 @@ +const koa = require('koa'); +const Router = require('koa-router'); + +module.exports = function() { + const app = koa(); + const router = new Router(); + + router.post('/slack/actions', function* (next) { + this.body = 'Aww!'; + + yield next; + }); + + app.use(router.routes()); + app.use(router.allowedMethods()); + + return app; +}; diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js new file mode 100644 index 0000000..f06bc9e --- /dev/null +++ b/test/message-buttons/server.js @@ -0,0 +1,19 @@ +const startServer = require('../../src/message-buttons/server'); +const agent = require('supertest-koa-agent'); + +const test = require('tape'); +const sinon = require('sinon'); + +test('it handles a rejection of the initial interview question', function(t) { + agent(startServer()) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'initial', + actions: [{ + name: 'no', + value: 'no' + }] + })}) + .expect(200, 'Aww!', t.end); +}); From d7a7cba71ca33c3fcfdc6f682ecdf2a5b93a6d64 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 17:42:36 -0500 Subject: [PATCH 03/39] Add web process --- Procfile | 1 + package.json | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Procfile b/Procfile index 9ebe8e8..06177a2 100644 --- a/Procfile +++ b/Procfile @@ -1 +1,2 @@ +web: npm run web worker: npm start diff --git a/package.json b/package.json index 0d47e9f..edb8ef2 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "scripts": { "start": "node --use-strict index.js", "debug": "node debug --use-strict index.js", - "test": "tap --strict test/*" + "test": "tap --strict test/*", + "web": "node --use-strict web.js" }, "repository": { "type": "git", From 96af731e7b91d4db09b5bde88928f12b51ccbdf2 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 17:46:50 -0500 Subject: [PATCH 04/39] Add handling of more information response --- package.json | 1 + src/message-buttons/server.js | 13 ++++++++++++- test/message-buttons/server.js | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index edb8ef2..94329a6 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "convict": "^0.6.1", "cron-parser": "^0.6.1", "koa": "^1.2.0", + "koa-body": "^1.4.0", "koa-router": "^5.4.0", "lodash.every": "^3.2.0", "lodash.find": "^3.2.0", diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 1308285..ff9fc5a 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -1,12 +1,23 @@ const koa = require('koa'); const Router = require('koa-router'); +const bodyParser = require('koa-body'); + module.exports = function() { const app = koa(); + app.use(bodyParser()); + const router = new Router(); router.post('/slack/actions', function* (next) { - this.body = 'Aww!'; + const payload = JSON.parse(this.request.body.payload); + const action = payload.actions[0]; + + if (action.value === 'more') { + this.body = 'Here is more information.'; + } else { + this.body = 'Aww!'; + } yield next; }); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index f06bc9e..43c1179 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,6 +4,20 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); +test('it handles a more information request from the initial interview question', function(t) { + agent(startServer()) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'initial', + actions: [{ + name: 'more', + value: 'more' + }] + })}) + .expect(200, 'Here is more information.', t.end); +}); + test('it handles a rejection of the initial interview question', function(t) { agent(startServer()) .post('/slack/actions') From f5686b51f4e8d99215476ed8aede315381a46b63 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 17:49:06 -0500 Subject: [PATCH 05/39] Add placeholder handling of interview acceptance --- src/message-buttons/server.js | 4 +++- test/message-buttons/server.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index ff9fc5a..2fd05d0 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -13,7 +13,9 @@ module.exports = function() { const payload = JSON.parse(this.request.body.payload); const action = payload.actions[0]; - if (action.value === 'more') { + if (action.value === 'yes') { + this.body = 'Let us continue.'; + } else if (action.value === 'more') { this.body = 'Here is more information.'; } else { this.body = 'Aww!'; diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 43c1179..2ffd03a 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,6 +4,20 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); +test('it handles acceptance of the initial interview question', function(t) { + agent(startServer()) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'initial', + actions: [{ + name: 'yes', + value: 'yes' + }] + })}) + .expect(200, 'Let us continue.', t.end); +}); + test('it handles a more information request from the initial interview question', function(t) { agent(startServer()) .post('/slack/actions') From 50cf7fe889c11ce739e5c8575ce3298add3648a8 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:05:53 -0500 Subject: [PATCH 06/39] Add partial first attribute question --- src/message-buttons/server.js | 4 ++-- test/message-buttons/server.js | 40 +++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 2fd05d0..1af207a 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -3,7 +3,7 @@ const Router = require('koa-router'); const bodyParser = require('koa-body'); -module.exports = function() { +module.exports = function({attributeConfigurations, questionForAttributeConfiguration} = {}) { const app = koa(); app.use(bodyParser()); @@ -14,7 +14,7 @@ module.exports = function() { const action = payload.actions[0]; if (action.value === 'yes') { - this.body = 'Let us continue.'; + this.body = questionForAttributeConfiguration(attributeConfigurations[0]); } else if (action.value === 'more') { this.body = 'Here is more information.'; } else { diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 2ffd03a..7781b5c 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,8 +4,42 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); -test('it handles acceptance of the initial interview question', function(t) { - agent(startServer()) +const firstAttributeConfiguration = { + name: 'jorts', + interviewQuestion: 'Do you wear jorts?', + values: [{ + value: 'wears jorts', + texts: { + interviewAnswer: 'Yes' + } + }, { + value: 'does not wear jorts', + texts: { + interviewAnswer: 'No' + } + }, { + value: 'sometimes wears jorts', + texts: { + interviewAnswer: 'Sometimes' + } + }, { + value: null, + texts: { + interviewAnswer: 'Decline' + } + }] +}; + +const attributeConfigurations = [ + firstAttributeConfiguration +]; + +function questionForAttributeConfiguration() { + return 'a question'; +} + +test('it handles acceptance of the initial interview question by responding with a question for the first attribute', function(t) { + agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) .post('/slack/actions') .type('form') .send({payload: JSON.stringify({ @@ -15,7 +49,7 @@ test('it handles acceptance of the initial interview question', function(t) { value: 'yes' }] })}) - .expect(200, 'Let us continue.', t.end); + .expect(200, questionForAttributeConfiguration(), t.end); }); test('it handles a more information request from the initial interview question', function(t) { From 39e79b45e4216fcd9f7729d9a34be6cd24a52008 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:14:34 -0500 Subject: [PATCH 07/39] Add attribute-to-question converter --- .../question-for-attribute-configuration.js | 20 ++++++ .../question-for-attribute-configuration.js | 63 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 src/message-buttons/question-for-attribute-configuration.js create mode 100644 test/message-buttons/question-for-attribute-configuration.js diff --git a/src/message-buttons/question-for-attribute-configuration.js b/src/message-buttons/question-for-attribute-configuration.js new file mode 100644 index 0000000..2f83169 --- /dev/null +++ b/src/message-buttons/question-for-attribute-configuration.js @@ -0,0 +1,20 @@ + +module.exports = function questionForAttributeConfiguration(attributeConfiguration) { + return { + text: attributeConfiguration.interviewQuestion, + attachments: [{ + title: attributeConfiguration.interviewQuestion, + callback_id: attributeConfiguration.name, + color: '#ccc', + attachment_type: 'default', + actions: attributeConfiguration.values.map(value => { + return { + name: attributeConfiguration.name, + text: value.texts.interviewAnswer, + type: 'button', + value: value.value || 'decline' + }; + }) + }] + }; +} diff --git a/test/message-buttons/question-for-attribute-configuration.js b/test/message-buttons/question-for-attribute-configuration.js new file mode 100644 index 0000000..37b7c51 --- /dev/null +++ b/test/message-buttons/question-for-attribute-configuration.js @@ -0,0 +1,63 @@ +const test = require('tape'); +const questionForAttributeConfiguration = require('../../src/message-buttons/question-for-attribute-configuration'); + +const attributeConfiguration = { + name: 'jorts', + interviewQuestion: 'Do you wear jorts?', + values: [{ + value: 'wears jorts', + texts: { + interviewAnswer: 'Yes' + } + }, { + value: 'does not wear jorts', + texts: { + interviewAnswer: 'No' + } + }, { + value: 'sometimes wears jorts', + texts: { + interviewAnswer: 'Sometimes' + } + }, { + value: null, + texts: { + interviewAnswer: 'Decline' + } + }] +}; + +test('it translates an attribute configuration into a question', function(t) { + t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), { + text: 'Do you wear jorts?', + attachments: [{ + title: 'Do you wear jorts?', + callback_id: 'jorts', + color: '#ccc', + attachment_type: 'default', + actions: [{ + name: 'jorts', + text: 'Yes', + type: 'button', + value: 'wears jorts' + }, { + name: 'jorts', + text: 'No', + type: 'button', + value: 'does not wear jorts' + }, { + name: 'jorts', + text: 'Sometimes', + type: 'button', + value: 'sometimes wears jorts' + }, { + name: 'jorts', + text: 'Decline', + type: 'button', + value: 'decline' + }] + }] + }); + + t.end(); +}); From 93a9de270d6865413d399f57702c40811d1f1345 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:16:01 -0500 Subject: [PATCH 08/39] Add script to start web server --- web.js | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 web.js diff --git a/web.js b/web.js new file mode 100644 index 0000000..4d5bdd1 --- /dev/null +++ b/web.js @@ -0,0 +1,6 @@ +const attributeConfigurations = require('./attribute-configurations'); +const questionForAttributeConfiguration = require('src/message-buttons/question-for-attribute-configuration'); + +const server = require('./src/message-buttons/server')({attributeConfiguration, questionForAttributeConfiguration}); + +server.listen(process.env.PORT || 3000); From 14b83228cad33d7c63f9655189a2f498d1b0bf8d Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:19:26 -0500 Subject: [PATCH 09/39] Add interview strings for attributes --- attributes/manness.json | 13 +++++++++---- attributes/pocness.json | 13 +++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/attributes/manness.json b/attributes/manness.json index 56e2d72..163f5e4 100644 --- a/attributes/manness.json +++ b/attributes/manness.json @@ -1,5 +1,6 @@ { "name": "manness", + "interviewQuestion": "Are you a man?", "values": [ { "value": "not a man", @@ -14,7 +15,8 @@ "statistics": "notMen", "table": "not-men", "terse": "not-men", - "short": "NM" + "short": "NM", + "interviewAnswer": "Yes" } }, { @@ -29,7 +31,8 @@ "update": "Okay, we have noted that you are a man. If I got it wrong, try saying “I am *not* a man!”", "statistics": "men", "table": "men", - "short": "M" + "short": "M", + "interviewAnswer": "No" } }, { @@ -43,7 +46,8 @@ "update": "We have noted that it’s complicated whether you are a man. If I got it wrong, try saying “I am not a man.”", "statistics": "complicated", "table": "complicated", - "short": "‽" + "short": "‽", + "interviewAnswer": "It’s complicated" } }, { @@ -57,7 +61,8 @@ "update": "Okay, we have erased our record of whether you are a man.", "statistics": "unknown", "table": "unknown", - "short": "?" + "short": "?", + "interviewAnswer": "Decline" } } ], diff --git a/attributes/pocness.json b/attributes/pocness.json index ffba8fc..647568d 100644 --- a/attributes/pocness.json +++ b/attributes/pocness.json @@ -1,5 +1,6 @@ { "name": "pocness", + "interviewQuestion": "Are you a person of colour?", "values": [ { "value": "a PoC", @@ -13,7 +14,8 @@ "statistics": "peopleOfColour", "table": "PoC", "terse": "people of colour", - "short": "PoC" + "short": "PoC", + "interviewAnswer": "Yes" } }, { @@ -27,7 +29,8 @@ "update": "We have noted that you are not a person of colour. If I got it wrong, try saying “I am a person of colour”", "statistics": "nonPeopleOfColour", "table": "not-PoC", - "short": "N" + "short": "N", + "interviewAnswer": "No" } }, { @@ -41,7 +44,8 @@ "update": "We have noted that it’s complicated whether you are a person of colour. If I got it wrong, try saying “I am a person of colour”", "statistics": "complicated", "table": "complicated", - "short": "‽" + "short": "‽", + "interviewAnswer": "It’s complicated" } }, { @@ -55,7 +59,8 @@ "update": "We have erased our record of whether you are a person of colour.", "statistics": "unknown", "table": "unknown", - "short": "?" + "short": "?", + "interviewAnswer": "Decline" } } ], From 80a871084221be5a43f6af26fbd347b3358ecc7b Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:21:11 -0500 Subject: [PATCH 10/39] Correct imports --- web.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web.js b/web.js index 4d5bdd1..3db262f 100644 --- a/web.js +++ b/web.js @@ -1,6 +1,6 @@ -const attributeConfigurations = require('./attribute-configurations'); -const questionForAttributeConfiguration = require('src/message-buttons/question-for-attribute-configuration'); +const attributeConfigurations = require('./src/attribute-configurations'); +const questionForAttributeConfiguration = require('./src/message-buttons/question-for-attribute-configuration'); -const server = require('./src/message-buttons/server')({attributeConfiguration, questionForAttributeConfiguration}); +const server = require('./src/message-buttons/server')({attributeConfigurations, questionForAttributeConfiguration}); server.listen(process.env.PORT || 3000); From 2ada128029d790d6ed4819fc55069b8bc66e8b92 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:34:56 -0500 Subject: [PATCH 11/39] Add handling of response to first question --- src/message-buttons/server.js | 20 +++++++++++++++----- test/message-buttons/server.js | 27 +++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 1af207a..afca10c 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -11,14 +11,19 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur router.post('/slack/actions', function* (next) { const payload = JSON.parse(this.request.body.payload); + const attributeName = payload.callback_id; const action = payload.actions[0]; - if (action.value === 'yes') { - this.body = questionForAttributeConfiguration(attributeConfigurations[0]); - } else if (action.value === 'more') { - this.body = 'Here is more information.'; + if (attributeName === 'initial') { + if (action.value === 'yes') { + this.body = questionForAttributeConfiguration(attributeConfigurations[0]); + } else if (action.value === 'more') { + this.body = 'Here is more information.'; + } else { + this.body = 'Aww!'; + } } else { - this.body = 'Aww!'; + this.body = questionForAttributeConfiguration(getNextAttributeConfiguration(attributeConfigurations, attributeName)); } yield next; @@ -29,3 +34,8 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur return app; }; + +function getNextAttributeConfiguration(attributeConfigurations, attributeName) { + const attributeConfigurationIndex = attributeConfigurations.findIndex(attributeConfiguration => attributeConfiguration.name === attributeName); + return attributeConfigurations[attributeConfigurationIndex + 1]; +} diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 7781b5c..54a1341 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -30,12 +30,17 @@ const firstAttributeConfiguration = { }] }; +const secondAttributeConfiguration = { + name: 'jants' +}; + const attributeConfigurations = [ - firstAttributeConfiguration + firstAttributeConfiguration, + secondAttributeConfiguration ]; -function questionForAttributeConfiguration() { - return 'a question'; +function questionForAttributeConfiguration(attributeConfiguration) { + return `${attributeConfiguration.name}Question`; } test('it handles acceptance of the initial interview question by responding with a question for the first attribute', function(t) { @@ -49,7 +54,21 @@ test('it handles acceptance of the initial interview question by responding with value: 'yes' }] })}) - .expect(200, questionForAttributeConfiguration(), t.end); + .expect(200, 'jortsQuestion', t.end); +}); + +test('it handles a response to the first attribute question by asking the second attribute question', function(t) { + agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'jorts', + actions: [{ + name: 'yes', + value: 'yes' + }] + })}) + .expect(200, 'jantsQuestion', t.end); }); test('it handles a more information request from the initial interview question', function(t) { From 99fb6cabf9b3b93104df72a832b4ffcc871680b0 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:38:21 -0500 Subject: [PATCH 12/39] Add handling for final interview question --- src/message-buttons/server.js | 8 +++++++- test/message-buttons/server.js | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index afca10c..4fa710d 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -23,7 +23,13 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur this.body = 'Aww!'; } } else { - this.body = questionForAttributeConfiguration(getNextAttributeConfiguration(attributeConfigurations, attributeName)); + const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); + + if (nextAttributeConfiguration) { + this.body = questionForAttributeConfiguration(nextAttributeConfiguration); + } else { + this.body = 'Thanks for participating! See you around the Slack.'; + } } yield next; diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 54a1341..b450759 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -71,6 +71,20 @@ test('it handles a response to the first attribute question by asking the second .expect(200, 'jantsQuestion', t.end); }); +test('it handles a response to the last attribute question by thanking and wrapping up', function(t) { + agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'jants', + actions: [{ + name: 'yes', + value: 'yes' + }] + })}) + .expect(200, 'Thanks for participating! See you around the Slack.', t.end); +}); + test('it handles a more information request from the initial interview question', function(t) { agent(startServer()) .post('/slack/actions') From 7a64ae5fa06f497c8bfb25e9e2105791333f4c04 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:43:03 -0500 Subject: [PATCH 13/39] Split wrongness suggestion from update message --- attributes/manness.json | 6 ++++-- attributes/pocness.json | 9 ++++++--- src/direct-message-handler.js | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/attributes/manness.json b/attributes/manness.json index 163f5e4..8e9e387 100644 --- a/attributes/manness.json +++ b/attributes/manness.json @@ -11,7 +11,8 @@ ], "texts": { "information": "you are not a man", - "update": "Okay, we have noted that you are not a man. If I got it wrong, try saying “I am a man”.", + "update": "Okay, we have noted that you are not a man.", + "wrong": "If I got it wrong, try saying “I am a man”.", "statistics": "notMen", "table": "not-men", "terse": "not-men", @@ -28,7 +29,8 @@ ], "texts": { "information": "you are a man", - "update": "Okay, we have noted that you are a man. If I got it wrong, try saying “I am *not* a man!”", + "update": "Okay, we have noted that you are a man. ", + "wrong": "If I got it wrong, try saying “I am *not* a man!”", "statistics": "men", "table": "men", "short": "M", diff --git a/attributes/pocness.json b/attributes/pocness.json index 647568d..3292145 100644 --- a/attributes/pocness.json +++ b/attributes/pocness.json @@ -10,7 +10,8 @@ ], "texts": { "information": "you are a person of colour", - "update": "We have noted that you are a person of colour. If I got it wrong, try saying “I am not a person of colour”", + "update": "We have noted that you are a person of colour.", + "wrong": "If I got it wrong, try saying “I am not a person of colour”", "statistics": "peopleOfColour", "table": "PoC", "terse": "people of colour", @@ -26,7 +27,8 @@ ], "texts": { "information": "you are not a person of colour", - "update": "We have noted that you are not a person of colour. If I got it wrong, try saying “I am a person of colour”", + "update": "We have noted that you are not a person of colour.", + "wrong": "If I got it wrong, try saying “I am a person of colour”", "statistics": "nonPeopleOfColour", "table": "not-PoC", "short": "N", @@ -41,7 +43,8 @@ ], "texts": { "information": "it’s complicated whether you are a person of colour", - "update": "We have noted that it’s complicated whether you are a person of colour. If I got it wrong, try saying “I am a person of colour”", + "update": "We have noted that it’s complicated whether you are a person of colour.", + "wrong": "If I got it wrong, try saying “I am a person of colour”", "statistics": "complicated", "table": "complicated", "short": "‽", diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index b9c7acf..4db480a 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -131,7 +131,7 @@ class DirectMessageHandler { }); if (matchingValue) { - reply = matchingValue.texts.update; + reply = `${matchingValue.texts.update} ${matchingValue.texts.wrong || ''}`; } channel.send(reply); From c47defb00f5ea7e82ea655512e762be2109fe067 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 18:49:58 -0500 Subject: [PATCH 14/39] Add update message before next question --- .../question-for-attribute-configuration.js | 31 +++++------ src/message-buttons/server.js | 15 ++++- .../question-for-attribute-configuration.js | 55 +++++++++---------- test/message-buttons/server.js | 29 +++++++--- 4 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/message-buttons/question-for-attribute-configuration.js b/src/message-buttons/question-for-attribute-configuration.js index 2f83169..8262266 100644 --- a/src/message-buttons/question-for-attribute-configuration.js +++ b/src/message-buttons/question-for-attribute-configuration.js @@ -1,20 +1,17 @@ module.exports = function questionForAttributeConfiguration(attributeConfiguration) { - return { - text: attributeConfiguration.interviewQuestion, - attachments: [{ - title: attributeConfiguration.interviewQuestion, - callback_id: attributeConfiguration.name, - color: '#ccc', - attachment_type: 'default', - actions: attributeConfiguration.values.map(value => { - return { - name: attributeConfiguration.name, - text: value.texts.interviewAnswer, - type: 'button', - value: value.value || 'decline' - }; - }) - }] - }; + return [{ + title: attributeConfiguration.interviewQuestion, + callback_id: attributeConfiguration.name, + color: '#ccc', + attachment_type: 'default', + actions: attributeConfiguration.values.map(value => { + return { + name: attributeConfiguration.name, + text: value.texts.interviewAnswer, + type: 'button', + value: value.value || 'decline' + }; + }) + }]; } diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 4fa710d..5089a58 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -16,19 +16,28 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur if (attributeName === 'initial') { if (action.value === 'yes') { - this.body = questionForAttributeConfiguration(attributeConfigurations[0]); + this.body = { + text: 'Here is the first question???', + attachments: questionForAttributeConfiguration(attributeConfigurations[0]) + }; } else if (action.value === 'more') { this.body = 'Here is more information.'; } else { this.body = 'Aww!'; } } else { + const responseAttributeConfiguration = attributeConfigurations.find(configuration => configuration.name === attributeName); + const responseAttributeValue = responseAttributeConfiguration.values.find(attributeValue => attributeValue.value === action.value); + const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); if (nextAttributeConfiguration) { - this.body = questionForAttributeConfiguration(nextAttributeConfiguration); + this.body = { + text: responseAttributeValue.texts.update, + attachments: questionForAttributeConfiguration(nextAttributeConfiguration) + }; } else { - this.body = 'Thanks for participating! See you around the Slack.'; + this.body = `${responseAttributeValue.texts.update} Thanks for participating! See you around the Slack.`; } } diff --git a/test/message-buttons/question-for-attribute-configuration.js b/test/message-buttons/question-for-attribute-configuration.js index 37b7c51..5f0e862 100644 --- a/test/message-buttons/question-for-attribute-configuration.js +++ b/test/message-buttons/question-for-attribute-configuration.js @@ -28,36 +28,33 @@ const attributeConfiguration = { }; test('it translates an attribute configuration into a question', function(t) { - t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), { - text: 'Do you wear jorts?', - attachments: [{ - title: 'Do you wear jorts?', - callback_id: 'jorts', - color: '#ccc', - attachment_type: 'default', - actions: [{ - name: 'jorts', - text: 'Yes', - type: 'button', - value: 'wears jorts' - }, { - name: 'jorts', - text: 'No', - type: 'button', - value: 'does not wear jorts' - }, { - name: 'jorts', - text: 'Sometimes', - type: 'button', - value: 'sometimes wears jorts' - }, { - name: 'jorts', - text: 'Decline', - type: 'button', - value: 'decline' - }] + t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), [{ + title: 'Do you wear jorts?', + callback_id: 'jorts', + color: '#ccc', + attachment_type: 'default', + actions: [{ + name: 'jorts', + text: 'Yes', + type: 'button', + value: 'wears jorts' + }, { + name: 'jorts', + text: 'No', + type: 'button', + value: 'does not wear jorts' + }, { + name: 'jorts', + text: 'Sometimes', + type: 'button', + value: 'sometimes wears jorts' + }, { + name: 'jorts', + text: 'Decline', + type: 'button', + value: 'decline' }] - }); + }]); t.end(); }); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index b450759..bcca128 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -10,7 +10,8 @@ const firstAttributeConfiguration = { values: [{ value: 'wears jorts', texts: { - interviewAnswer: 'Yes' + interviewAnswer: 'Yes', + update: 'We have noted that you wear jorts.' } }, { value: 'does not wear jorts', @@ -31,7 +32,19 @@ const firstAttributeConfiguration = { }; const secondAttributeConfiguration = { - name: 'jants' + name: 'jants', + interviewQuestion: 'Do you wear jants?', + values: [{ + value: 'wears jants', + texts: { + interviewAnswer: 'Yes' + } + }, { + value: 'does not wear jants', + texts: { + update: 'We have noted that you do not wear jants.' + } + }] }; const attributeConfigurations = [ @@ -54,7 +67,7 @@ test('it handles acceptance of the initial interview question by responding with value: 'yes' }] })}) - .expect(200, 'jortsQuestion', t.end); + .expect(200, {text: 'Here is the first question???', attachments: 'jortsQuestion'}, t.end); }); test('it handles a response to the first attribute question by asking the second attribute question', function(t) { @@ -65,10 +78,10 @@ test('it handles a response to the first attribute question by asking the second callback_id: 'jorts', actions: [{ name: 'yes', - value: 'yes' + value: 'wears jorts' }] })}) - .expect(200, 'jantsQuestion', t.end); + .expect(200, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}, t.end); }); test('it handles a response to the last attribute question by thanking and wrapping up', function(t) { @@ -78,11 +91,11 @@ test('it handles a response to the last attribute question by thanking and wrapp .send({payload: JSON.stringify({ callback_id: 'jants', actions: [{ - name: 'yes', - value: 'yes' + name: 'irrelevant', + value: 'does not wear jants' }] })}) - .expect(200, 'Thanks for participating! See you around the Slack.', t.end); + .expect(200, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.', t.end); }); test('it handles a more information request from the initial interview question', function(t) { From bfcd59c6f596554dc2938c5fdf2d8d45a7ec6c36 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 9 Jul 2016 19:10:57 -0500 Subject: [PATCH 15/39] Update first question lead-in --- src/message-buttons/server.js | 2 +- test/message-buttons/server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 5089a58..7038de9 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -17,7 +17,7 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur if (attributeName === 'initial') { if (action.value === 'yes') { this.body = { - text: 'Here is the first question???', + text: 'Excellent, thank you!', attachments: questionForAttributeConfiguration(attributeConfigurations[0]) }; } else if (action.value === 'more') { diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index bcca128..59ba239 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -67,7 +67,7 @@ test('it handles acceptance of the initial interview question by responding with value: 'yes' }] })}) - .expect(200, {text: 'Here is the first question???', attachments: 'jortsQuestion'}, t.end); + .expect(200, {text: 'Excellent, thank you!', attachments: 'jortsQuestion'}, t.end); }); test('it handles a response to the first attribute question by asking the second attribute question', function(t) { From b0a6c366fa93911799ac35025e9d7db237a35ef0 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 10 Jul 2016 14:37:58 -0500 Subject: [PATCH 16/39] Add database backing for interview questions --- src/message-buttons/server.js | 6 +++++- test/message-buttons/server.js | 34 ++++++++++++++++++++++++++++------ web.js | 6 +++++- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 7038de9..8c89dd5 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -3,7 +3,7 @@ const Router = require('koa-router'); const bodyParser = require('koa-body'); -module.exports = function({attributeConfigurations, questionForAttributeConfiguration} = {}) { +module.exports = function({attributeConfigurations, questionForAttributeConfiguration, userRepository} = {}) { const app = koa(); app.use(bodyParser()); @@ -29,6 +29,10 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur const responseAttributeConfiguration = attributeConfigurations.find(configuration => configuration.name === attributeName); const responseAttributeValue = responseAttributeConfiguration.values.find(attributeValue => attributeValue.value === action.value); + const userID = payload.user.id; + + userRepository.storeAttribute(userID, attributeName, action.value); + const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); if (nextAttributeConfiguration) { diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 59ba239..04bfab2 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,6 +4,10 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); +const fakeUserRepository = { + storeAttribute() {} +}; + const firstAttributeConfiguration = { name: 'jorts', interviewQuestion: 'Do you wear jorts?', @@ -70,32 +74,50 @@ test('it handles acceptance of the initial interview question by responding with .expect(200, {text: 'Excellent, thank you!', attachments: 'jortsQuestion'}, t.end); }); -test('it handles a response to the first attribute question by asking the second attribute question', function(t) { - agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) +test('it handles a response to the first attribute question by storing it and asking the second attribute question', function(t) { + const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); + + agent(startServer({attributeConfigurations, questionForAttributeConfiguration, userRepository: fakeUserRepository})) .post('/slack/actions') .type('form') .send({payload: JSON.stringify({ callback_id: 'jorts', + user: { + id: 'userID' + }, actions: [{ name: 'yes', value: 'wears jorts' }] })}) - .expect(200, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}, t.end); + .expect(200, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}, () => { + t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts')); + storeAttributeStub.restore(); + t.end(); + }); }); -test('it handles a response to the last attribute question by thanking and wrapping up', function(t) { - agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) +test('it handles a response to the last attribute question by storing it and thanking and wrapping up', function(t) { + const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); + + agent(startServer({attributeConfigurations, questionForAttributeConfiguration, userRepository: fakeUserRepository})) .post('/slack/actions') .type('form') .send({payload: JSON.stringify({ callback_id: 'jants', + user: { + id: 'userID' + }, actions: [{ name: 'irrelevant', value: 'does not wear jants' }] })}) - .expect(200, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.', t.end); + .expect(200, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.', () => { + t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants')); + storeAttributeStub.restore(); + t.end(); + }); }); test('it handles a more information request from the initial interview question', function(t) { diff --git a/web.js b/web.js index 3db262f..51920ed 100644 --- a/web.js +++ b/web.js @@ -1,6 +1,10 @@ const attributeConfigurations = require('./src/attribute-configurations'); const questionForAttributeConfiguration = require('./src/message-buttons/question-for-attribute-configuration'); -const server = require('./src/message-buttons/server')({attributeConfigurations, questionForAttributeConfiguration}); +const UserRepository = require('./src/persistence/user-repository'); +const db = require('./models'); +const userRepository = new UserRepository(db.User); + +const server = require('./src/message-buttons/server')({attributeConfigurations, questionForAttributeConfiguration, userRepository}); server.listen(process.env.PORT || 3000); From d7560a7a5e5caf2dcf55beac6eb1577cc36c2160 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 10 Jul 2016 14:43:21 -0500 Subject: [PATCH 17/39] Correct manness interview answers --- attributes/manness.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/attributes/manness.json b/attributes/manness.json index 8e9e387..b7a62ab 100644 --- a/attributes/manness.json +++ b/attributes/manness.json @@ -17,7 +17,7 @@ "table": "not-men", "terse": "not-men", "short": "NM", - "interviewAnswer": "Yes" + "interviewAnswer": "No" } }, { @@ -34,7 +34,7 @@ "statistics": "men", "table": "men", "short": "M", - "interviewAnswer": "No" + "interviewAnswer": "Yes" } }, { From 47781247dd626415550db02c527b9fb4491c1689 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 10 Jul 2016 14:47:53 -0500 Subject: [PATCH 18/39] Add ability to decline attribute identification --- src/message-buttons/server.js | 6 ++++-- test/message-buttons/server.js | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 8c89dd5..5010c8e 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -26,12 +26,14 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur this.body = 'Aww!'; } } else { + const attributeValueToFind = action.value === 'decline' ? null : action.value; + const responseAttributeConfiguration = attributeConfigurations.find(configuration => configuration.name === attributeName); - const responseAttributeValue = responseAttributeConfiguration.values.find(attributeValue => attributeValue.value === action.value); + const responseAttributeValue = responseAttributeConfiguration.values.find(attributeValue => attributeValue.value === attributeValueToFind); const userID = payload.user.id; - userRepository.storeAttribute(userID, attributeName, action.value); + userRepository.storeAttribute(userID, attributeName, attributeValueToFind); const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 04bfab2..1302a57 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -97,6 +97,29 @@ test('it handles a response to the first attribute question by storing it and as }); }); +test('it handles a decline response to the first attribute question by storing it as null', function(t) { + const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); + + agent(startServer({attributeConfigurations, questionForAttributeConfiguration, userRepository: fakeUserRepository})) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + callback_id: 'jorts', + user: { + id: 'userID' + }, + actions: [{ + name: 'irrelevant', + value: 'decline' + }] + })}) + .expect(200, () => { + t.ok(storeAttributeStub.calledWith('userID', 'jorts', null)); + storeAttributeStub.restore(); + t.end(); + }); +}); + test('it handles a response to the last attribute question by storing it and thanking and wrapping up', function(t) { const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); From c84d436b975602fc66f63be860e00375ba07c4ea Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 15:06:13 -0700 Subject: [PATCH 19/39] Add preliminary handling of OAuth request --- package.json | 2 ++ src/message-buttons/server.js | 21 +++++++++++++++++++++ test/message-buttons/server.js | 27 +++++++++++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/package.json b/package.json index 94329a6..268c74d 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "devDependencies": { "grunt": "^0.4.5", "grunt-tape": "0.0.2", + "nock": "^8.0.0", "require-subvert": "^0.1.0", "sinon": "^1.12.2", "supertest-koa-agent": "^0.2.1", @@ -51,6 +52,7 @@ "sequelize": "^3.21.0", "sequelize-cli": "1.1.0", "slack-client": "^1.3.1", + "superagent": "^2.1.0", "tap": "^0.5.0", "validator": "^3.28.0" }, diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 5010c8e..94f9ef6 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -3,6 +3,8 @@ const Router = require('koa-router'); const bodyParser = require('koa-body'); +const request = require('superagent'); + module.exports = function({attributeConfigurations, questionForAttributeConfiguration, userRepository} = {}) { const app = koa(); app.use(bodyParser()); @@ -50,6 +52,25 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur yield next; }); + router.get('/oauth', function* (next) { + const requestPromise = new Promise((resolve) => { + request + .post('https://slack.com/api/oauth.access') + .type('form') + .send({ + client_id: process.env.CLIENT_ID, + client_secret: process.env.CLIENT_SECRET, + code: this.request.query.code + }) + .end((err, res) => { + this.body = {test: res.body.bot.bot_access_token}; + resolve(); + }); + }); + + yield requestPromise; + }); + app.use(router.routes()); app.use(router.allowedMethods()); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 1302a57..0c91232 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -170,3 +170,30 @@ test('it handles a rejection of the initial interview question', function(t) { })}) .expect(200, 'Aww!', t.end); }); + +test('it responds to an OAuth request with the bot access token', function(t) { + const nock = require('nock'); + + process.env.CLIENT_ID = 'client-id'; + process.env.CLIENT_SECRET = 'client-secret'; + + nock('https://slack.com') + .post('/api/oauth.access', 'client_id=client-id&client_secret=client-secret&code=a-code') + .reply(200, { + bot: { + bot_access_token: 'yesthisisthestring' + } + }); + + agent(startServer()) + .get('/oauth') + .type('form') + .query({ + code: 'a-code' + }) + .expect(200, (err, res) => { + t.notOk(err, 'expected no error'); + t.equal(res.body.test, 'yesthisisthestring'); + t.end(); + }); +}); From 75b6732983769213df0b7d2600114d471db4e8fa Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 15:11:45 -0700 Subject: [PATCH 20/39] Add handling for an OAuth request missing code --- src/message-buttons/server.js | 36 +++++++++++++++++++--------------- test/message-buttons/server.js | 11 +++++++++++ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 94f9ef6..8fe79c1 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -53,22 +53,26 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur }); router.get('/oauth', function* (next) { - const requestPromise = new Promise((resolve) => { - request - .post('https://slack.com/api/oauth.access') - .type('form') - .send({ - client_id: process.env.CLIENT_ID, - client_secret: process.env.CLIENT_SECRET, - code: this.request.query.code - }) - .end((err, res) => { - this.body = {test: res.body.bot.bot_access_token}; - resolve(); - }); - }); - - yield requestPromise; + if (this.request.query.code) { + const requestPromise = new Promise((resolve) => { + request + .post('https://slack.com/api/oauth.access') + .type('form') + .send({ + client_id: process.env.CLIENT_ID, + client_secret: process.env.CLIENT_SECRET, + code: this.request.query.code + }) + .end((err, res) => { + this.body = {test: res.body.bot.bot_access_token}; + resolve(); + }); + }); + + yield requestPromise; + } else { + this.status = 422; + } }); app.use(router.routes()); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 0c91232..81e5332 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -194,6 +194,17 @@ test('it responds to an OAuth request with the bot access token', function(t) { .expect(200, (err, res) => { t.notOk(err, 'expected no error'); t.equal(res.body.test, 'yesthisisthestring'); + + nock.cleanAll(); + t.end(); + }); +}); + +test('it responds to an OAuth request lacking a code with an error', function(t) { + agent(startServer()) + .get('/oauth') + .type('form') + .expect(422, (err, res) => { t.end(); }); }); From 3ef6581c67659cd3195a780cc4a96d7d865e6f0f Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 15:39:23 -0700 Subject: [PATCH 21/39] Add handling of an error from the Slack API --- src/message-buttons/server.js | 7 ++++++- test/message-buttons/server.js | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 8fe79c1..e2253d8 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -64,7 +64,12 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur code: this.request.query.code }) .end((err, res) => { - this.body = {test: res.body.bot.bot_access_token}; + if (res.body.ok) { + this.body = {test: res.body.bot.bot_access_token}; + } else { + this.body = {error: res.body.error}; + } + resolve(); }); }); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 81e5332..df4a51f 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -180,6 +180,7 @@ test('it responds to an OAuth request with the bot access token', function(t) { nock('https://slack.com') .post('/api/oauth.access', 'client_id=client-id&client_secret=client-secret&code=a-code') .reply(200, { + ok: true, bot: { bot_access_token: 'yesthisisthestring' } @@ -208,3 +209,26 @@ test('it responds to an OAuth request lacking a code with an error', function(t) t.end(); }); }); + +test('it handles an error from the Slack API', function(t) { + const nock = require('nock'); + + nock('https://slack.com') + .post('/api/oauth.access') + .reply(200, { + ok: false, + error: 'jorts_crisis' + }); + + agent(startServer()) + .get('/oauth') + .type('form') + .query({code: 'a-code'}) + .expect(422, (err, res) => { + t.ok(err, 'expected an error'); + t.equal(res.body.error, 'jorts_crisis'); + + nock.cleanAll(); + t.end(); + }); +}) From f7fe2e864683be5fc0886eaa7c5e84ba61f33cab Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 15:56:57 -0700 Subject: [PATCH 22/39] Rename bot access token container This UX is tragic but will work for now. --- src/message-buttons/server.js | 2 +- test/message-buttons/server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index e2253d8..c7a794d 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -65,7 +65,7 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur }) .end((err, res) => { if (res.body.ok) { - this.body = {test: res.body.bot.bot_access_token}; + this.body = {bot_access_token: res.body.bot.bot_access_token}; } else { this.body = {error: res.body.error}; } diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index df4a51f..d1aaac3 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -194,7 +194,7 @@ test('it responds to an OAuth request with the bot access token', function(t) { }) .expect(200, (err, res) => { t.notOk(err, 'expected no error'); - t.equal(res.body.test, 'yesthisisthestring'); + t.equal(res.body.bot_access_token, 'yesthisisthestring'); nock.cleanAll(); t.end(); From 4527ffc6d9f918982790087062109c352d57ffc6 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 16:44:57 -0700 Subject: [PATCH 23/39] Move manness answers to align with pocness answers The interview had Yes/No on the first question and No/Yes on the second which is confusing. --- attributes/manness.json | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/attributes/manness.json b/attributes/manness.json index b7a62ab..0a2b357 100644 --- a/attributes/manness.json +++ b/attributes/manness.json @@ -2,6 +2,23 @@ "name": "manness", "interviewQuestion": "Are you a man?", "values": [ + { + "value": "a man", + "matcherSets": [ + [{"matches": "true"}], + [{"matches": "man"}, {"doesNotMatch": "not"}, {"doesNotMatch": "woman"}], + [{"matches": "woman"}, {"matches": "not"}] + ], + "texts": { + "information": "you are a man", + "update": "Okay, we have noted that you are a man. ", + "wrong": "If I got it wrong, try saying “I am *not* a man!”", + "statistics": "men", + "table": "men", + "short": "M", + "interviewAnswer": "Yes" + } + }, { "value": "not a man", "matcherSets": [ @@ -20,23 +37,6 @@ "interviewAnswer": "No" } }, - { - "value": "a man", - "matcherSets": [ - [{"matches": "true"}], - [{"matches": "man"}, {"doesNotMatch": "not"}, {"doesNotMatch": "woman"}], - [{"matches": "woman"}, {"matches": "not"}] - ], - "texts": { - "information": "you are a man", - "update": "Okay, we have noted that you are a man. ", - "wrong": "If I got it wrong, try saying “I am *not* a man!”", - "statistics": "men", - "table": "men", - "short": "M", - "interviewAnswer": "Yes" - } - }, { "value": "complicated", "matcherSets": [ From 7a304855e9c6f4f56cd46aa91859322f9c368712 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 22:30:21 -0700 Subject: [PATCH 24/39] Fix response assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the course of adding a summary after the final answer, I found these assertions weren’t doing anything. Though the documentation looks to me to say this is a valid way to assert against the response, it seems that passing a custom function rather than the t.end of other tests means the assertions aren’t made…? --- test/message-buttons/server.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index d1aaac3..5b16dc1 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -90,7 +90,8 @@ test('it handles a response to the first attribute question by storing it and as value: 'wears jorts' }] })}) - .expect(200, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}, () => { + .expect(200, (err, {res: body}) => { + t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}); t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts')); storeAttributeStub.restore(); t.end(); @@ -136,7 +137,8 @@ test('it handles a response to the last attribute question by storing it and tha value: 'does not wear jants' }] })}) - .expect(200, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.', () => { + .expect(200, (err, {res: body}) => { + t.equal(body.text, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.'); t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants')); storeAttributeStub.restore(); t.end(); From 57f19a965473284520375f7e56793a1632940405 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 22:45:55 -0700 Subject: [PATCH 25/39] Extract function to generate information report --- src/direct-message-handler.js | 35 ++++++--------------------------- src/reports/user-information.js | 35 +++++++++++++++++++++++++++++++++ test/message-buttons/server.js | 17 +++++++++++----- 3 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 src/reports/user-information.js diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index 4db480a..67dd261 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -1,12 +1,12 @@ // TODO this should probably be further decomposed // Also should maybe just return a reply which the bot actually sends? -var every = require('lodash.every'); var find = require('lodash.find'); var UpdateParser = require('./update-parser'); var attributeConfigurations = require('./attribute-configurations'); +var userInformation = require('./reports/user-information'); class DirectMessageHandler { constructor({userRepository, channelRepository, adapter}) { @@ -69,34 +69,11 @@ class DirectMessageHandler { } handleInformationRequest(channel, message) { - // FIXME fetch the entire user rather than run multiple queries - Promise.all(this.attributeConfigurations.map(function(configuration) { - return this.userRepository.retrieveAttribute(message.user, configuration.name); - }.bind(this))).then(function(values) { - var reply; - - if (every(values, function(value) {return value == null || value == undefined})) { - reply = `We don’t have you on record! ${DirectMessageHandler.HELP_MESSAGE}`; - } else { - reply = 'Our records indicate that:\n\n'; - - this.attributeConfigurations.forEach(function(attributeConfiguration, index) { - var value = values[index]; - - var valueConfiguration = find(attributeConfiguration.values, function(valueConfiguration) { - return valueConfiguration.value == value; - }); - - if (valueConfiguration) { - reply += `* ${valueConfiguration.texts.information}\n`; - } else { - reply += `* ${attributeConfiguration.unknownValue.texts.information}\n`; - } - }); - } - - channel.send(reply); - }.bind(this)); + userInformation(message.user, { + userRepository: this.userRepository, + helpMessage: DirectMessageHandler.HELP_MESSAGE, + attributeConfigurations: this.attributeConfigurations + }).then(reply => channel.send(reply)); } handleInformationUpdate(channel, message) { diff --git a/src/reports/user-information.js b/src/reports/user-information.js new file mode 100644 index 0000000..2f15c59 --- /dev/null +++ b/src/reports/user-information.js @@ -0,0 +1,35 @@ +// FIXME this seems like a bad name/position? Separate into fetching and generating? + +const every = require('lodash.every'); +const find = require('lodash.find'); + +module.exports = function userInformation(userID, {userRepository, attributeConfigurations, helpMessage}) { + // FIXME fetch the entire user rather than run multiple queries + return Promise.all(attributeConfigurations.map(function(configuration) { + return userRepository.retrieveAttribute(userID, configuration.name); + })).then(function(values) { + var reply; + + if (every(values, function(value) {return value == null || value == undefined})) { + reply = `We don’t have you on record! ${helpMessage}`; + } else { + reply = 'Our records indicate that:\n\n'; + + attributeConfigurations.forEach(function(attributeConfiguration, index) { + var value = values[index]; + + var valueConfiguration = find(attributeConfiguration.values, function(valueConfiguration) { + return valueConfiguration.value == value; + }); + + if (valueConfiguration) { + reply += `* ${valueConfiguration.texts.information}\n`; + } else { + reply += `* ${attributeConfiguration.unknownValue.texts.information}\n`; + } + }); + } + + return reply; + }); +} diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 5b16dc1..a2c7c9a 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -5,7 +5,8 @@ const test = require('tape'); const sinon = require('sinon'); const fakeUserRepository = { - storeAttribute() {} + storeAttribute() {}, + retrieveAttribute() {} }; const firstAttributeConfiguration = { @@ -15,7 +16,8 @@ const firstAttributeConfiguration = { value: 'wears jorts', texts: { interviewAnswer: 'Yes', - update: 'We have noted that you wear jorts.' + update: 'We have noted that you wear jorts.', + information: 'you wear jorts' } }, { value: 'does not wear jorts', @@ -46,7 +48,8 @@ const secondAttributeConfiguration = { }, { value: 'does not wear jants', texts: { - update: 'We have noted that you do not wear jants.' + update: 'We have noted that you do not wear jants.', + information: 'you do not wear jants' } }] }; @@ -121,9 +124,13 @@ test('it handles a decline response to the first attribute question by storing i }); }); -test('it handles a response to the last attribute question by storing it and thanking and wrapping up', function(t) { +test('it handles a response to the last attribute question by storing it, summarising, and thanking', function(t) { const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); + const retrieveAttributeStub = sinon.stub(fakeUserRepository, 'retrieveAttribute'); + retrieveAttributeStub.withArgs('userID', 'jorts').returns(Promise.resolve('wears jorts')); + retrieveAttributeStub.withArgs('userID', 'jants').returns(Promise.resolve('does not wear jants')); + agent(startServer({attributeConfigurations, questionForAttributeConfiguration, userRepository: fakeUserRepository})) .post('/slack/actions') .type('form') @@ -138,7 +145,7 @@ test('it handles a response to the last attribute question by storing it and tha }] })}) .expect(200, (err, {res: body}) => { - t.equal(body.text, 'We have noted that you do not wear jants. Thanks for participating! See you around the Slack.'); + t.equal(body.text, 'We have noted that you do not wear jants. To summarise:\n* you wear jorts* you do not wear jants\n\nThanks for participating! See you around the Slack.'); t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants')); storeAttributeStub.restore(); t.end(); From 3eb185f12f802f1d7698b0fed70e48680a759e33 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sun, 24 Jul 2016 22:53:58 -0700 Subject: [PATCH 26/39] Add summary at end of interview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wording here is pretty awkward, but it’ll do for now! --- src/message-buttons/server.js | 9 ++++++++- test/message-buttons/server.js | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index c7a794d..280088a 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -5,6 +5,8 @@ const bodyParser = require('koa-body'); const request = require('superagent'); +const userInformation = require('../reports/user-information'); + module.exports = function({attributeConfigurations, questionForAttributeConfiguration, userRepository} = {}) { const app = koa(); app.use(bodyParser()); @@ -45,7 +47,12 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur attachments: questionForAttributeConfiguration(nextAttributeConfiguration) }; } else { - this.body = `${responseAttributeValue.texts.update} Thanks for participating! See you around the Slack.`; + yield userInformation(userID, { + userRepository: userRepository, + attributeConfigurations: attributeConfigurations + }).then(reply => { + this.body = `${responseAttributeValue.texts.update} ${reply}\nThanks for participating! See you around the Slack.`; + }); } } diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index a2c7c9a..1b71ffd 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -145,7 +145,7 @@ test('it handles a response to the last attribute question by storing it, summar }] })}) .expect(200, (err, {res: body}) => { - t.equal(body.text, 'We have noted that you do not wear jants. To summarise:\n* you wear jorts* you do not wear jants\n\nThanks for participating! See you around the Slack.'); + t.equal(body.text, 'We have noted that you do not wear jants. Our records indicate that:\n\n* you wear jorts\n* you do not wear jants\n\nThanks for participating! See you around the Slack.'); t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants')); storeAttributeStub.restore(); t.end(); From eba69109154251c8f4c18ddb31eb6f23e07a2965 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 30 Jul 2016 19:05:53 -0700 Subject: [PATCH 27/39] Add checking of verification token --- src/message-buttons/server.js | 7 +++++++ test/message-buttons/server.js | 30 ++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 280088a..55f8569 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -15,6 +15,13 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur router.post('/slack/actions', function* (next) { const payload = JSON.parse(this.request.body.payload); + + if (payload.token !== process.env.SLACK_VERIFICATION_TOKEN) { + this.status = 403; + yield next; + return; + } + const attributeName = payload.callback_id; const action = payload.actions[0]; diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 1b71ffd..86145b8 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,6 +4,8 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); +process.env.SLACK_VERIFICATION_TOKEN = 'a-verification-token'; + const fakeUserRepository = { storeAttribute() {}, retrieveAttribute() {} @@ -72,7 +74,8 @@ test('it handles acceptance of the initial interview question by responding with actions: [{ name: 'yes', value: 'yes' - }] + }], + token: 'a-verification-token' })}) .expect(200, {text: 'Excellent, thank you!', attachments: 'jortsQuestion'}, t.end); }); @@ -91,7 +94,8 @@ test('it handles a response to the first attribute question by storing it and as actions: [{ name: 'yes', value: 'wears jorts' - }] + }], + token: 'a-verification-token' })}) .expect(200, (err, {res: body}) => { t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}); @@ -115,7 +119,8 @@ test('it handles a decline response to the first attribute question by storing i actions: [{ name: 'irrelevant', value: 'decline' - }] + }], + token: 'a-verification-token' })}) .expect(200, () => { t.ok(storeAttributeStub.calledWith('userID', 'jorts', null)); @@ -142,7 +147,8 @@ test('it handles a response to the last attribute question by storing it, summar actions: [{ name: 'irrelevant', value: 'does not wear jants' - }] + }], + token: 'a-verification-token' })}) .expect(200, (err, {res: body}) => { t.equal(body.text, 'We have noted that you do not wear jants. Our records indicate that:\n\n* you wear jorts\n* you do not wear jants\n\nThanks for participating! See you around the Slack.'); @@ -161,7 +167,8 @@ test('it handles a more information request from the initial interview question' actions: [{ name: 'more', value: 'more' - }] + }], + token: 'a-verification-token' })}) .expect(200, 'Here is more information.', t.end); }); @@ -175,11 +182,22 @@ test('it handles a rejection of the initial interview question', function(t) { actions: [{ name: 'no', value: 'no' - }] + }], + token: 'a-verification-token' })}) .expect(200, 'Aww!', t.end); }); +test('it rejects an action without the correct verification token', function(t) { + agent(startServer()) + .post('/slack/actions') + .type('form') + .send({payload: JSON.stringify({ + token: 'an-invalid-token' + })}) + .expect(403, t.end); +}); + test('it responds to an OAuth request with the bot access token', function(t) { const nock = require('nock'); From b05e42425585ea4a2c58747fa15531cc325ac6db Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Fri, 9 Sep 2016 13:26:14 -0400 Subject: [PATCH 28/39] Add disclaimer to interview introduction --- src/direct-message-handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index 67dd261..6c70018 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -185,7 +185,7 @@ class DirectMessageHandler { // TODO maybe messages should be collected somewhere central, and parameterised DirectMessageHandler.HELP_MESSAGE = 'You can let me know “I’m not a man”, “I am a person of colour”, “it’s complicated whether I am white” and other such variations, or ask for my current information on you with “info”. View my source at https://github.com/backspace/slack-statsbot'; -DirectMessageHandler.INTERVIEW_INTRODUCTION = 'Hey, I’m a bot that collects statistics on who is taking up space in the channels I’m in. For now, I only track whether or not a participant is a man and/or a person of colour.'; +DirectMessageHandler.INTERVIEW_INTRODUCTION = 'Hey, I’m a bot that collects statistics on who is taking up space in the channels I’m in. For now, I only track whether or not a participant is a man and/or a person of colour. If you feel comfortable answering questions about this, we can continue.'; DirectMessageHandler.VERBOSE_HELP_MESSAGE = `${DirectMessageHandler.INTERVIEW_INTRODUCTION} ${DirectMessageHandler.HELP_MESSAGE}`; module.exports = DirectMessageHandler; From 999f9683e73c2a7e73890ff79f43c269671b381a Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Fri, 9 Sep 2016 15:30:04 -0400 Subject: [PATCH 29/39] Replace initial interview response with experiment This is a step in having interview questions be replaced with their answers and appending the next question. --- src/message-buttons/server.js | 16 +++++++++++++ test/message-buttons/server.js | 43 +++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 55f8569..9cc608b 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -6,6 +6,7 @@ const bodyParser = require('koa-body'); const request = require('superagent'); const userInformation = require('../reports/user-information'); +const DirectMessageHandler = require('../direct-message-handler'); module.exports = function({attributeConfigurations, questionForAttributeConfiguration, userRepository} = {}) { const app = koa(); @@ -27,6 +28,21 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur if (attributeName === 'initial') { if (action.value === 'yes') { + request.post(payload.response_url).send({ + text: DirectMessageHandler.INTERVIEW_INTRODUCTION, + attachments: [{ + title: 'Would you like to self-identify?', + text: 'Yes' + }], + replace_original: true + }) + .end((err, res) => { + request.post(payload.response_url).send({ + attachments: questionForAttributeConfiguration(attributeConfigurations[0]), + replace_original: false + }).end(); + }); + this.body = { text: 'Excellent, thank you!', attachments: questionForAttributeConfiguration(attributeConfigurations[0]) diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 86145b8..d11fbd3 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -4,6 +4,8 @@ const agent = require('supertest-koa-agent'); const test = require('tape'); const sinon = require('sinon'); +const DirectMessageHandler = require('../../src/direct-message-handler'); + process.env.SLACK_VERIFICATION_TOKEN = 'a-verification-token'; const fakeUserRepository = { @@ -65,7 +67,41 @@ function questionForAttributeConfiguration(attributeConfiguration) { return `${attributeConfiguration.name}Question`; } -test('it handles acceptance of the initial interview question by responding with a question for the first attribute', function(t) { +test('it handles acceptance of the initial interview question by repeating the question and answer and responding with a question for the first attribute', function(t) { + const nock = require('nock'); + + const responses = []; + + nock('https://example.com') + .post('/a-response-url') + .times(2) + .reply((uri, requestBody) => { + responses.push(JSON.parse(requestBody)); + + if (responses.length === 2) { + const firstResponse = responses[0]; + const secondResponse = responses[1]; + + t.deepEqual(firstResponse, { + text: DirectMessageHandler.INTERVIEW_INTRODUCTION, + attachments: [{ + title: 'Would you like to self-identify?', + text: 'Yes' + }], + replace_original: true + }, 'should restate the question with the answer attached'); + + t.deepEqual(secondResponse, { + attachments: 'jortsQuestion', + replace_original: false + }, 'should follow up with the first attribute question'); + + t.end(); + } + + return [200, 'hello']; + }); + agent(startServer({attributeConfigurations, questionForAttributeConfiguration})) .post('/slack/actions') .type('form') @@ -75,9 +111,10 @@ test('it handles acceptance of the initial interview question by responding with name: 'yes', value: 'yes' }], - token: 'a-verification-token' + token: 'a-verification-token', + response_url: 'https://example.com/a-response-url' })}) - .expect(200, {text: 'Excellent, thank you!', attachments: 'jortsQuestion'}, t.end); + .expect(200, () => {}); }); test('it handles a response to the first attribute question by storing it and asking the second attribute question', function(t) { From 388141293f4de604744582f76cd718cdd58df6b6 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 17:18:49 -0400 Subject: [PATCH 30/39] Remove superfluous response --- src/message-buttons/server.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 9cc608b..418d1a0 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -42,11 +42,6 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur replace_original: false }).end(); }); - - this.body = { - text: 'Excellent, thank you!', - attachments: questionForAttributeConfiguration(attributeConfigurations[0]) - }; } else if (action.value === 'more') { this.body = 'Here is more information.'; } else { From 395943f4e12cea15406507eac0a42ada8895e55c Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 17:21:49 -0400 Subject: [PATCH 31/39] Remove array wrapper around question determination This never really made sense. --- src/message-buttons/question-for-attribute-configuration.js | 4 ++-- src/message-buttons/server.js | 4 ++-- test/message-buttons/question-for-attribute-configuration.js | 4 ++-- test/message-buttons/server.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/message-buttons/question-for-attribute-configuration.js b/src/message-buttons/question-for-attribute-configuration.js index 8262266..f9cb97a 100644 --- a/src/message-buttons/question-for-attribute-configuration.js +++ b/src/message-buttons/question-for-attribute-configuration.js @@ -1,6 +1,6 @@ module.exports = function questionForAttributeConfiguration(attributeConfiguration) { - return [{ + return { title: attributeConfiguration.interviewQuestion, callback_id: attributeConfiguration.name, color: '#ccc', @@ -13,5 +13,5 @@ module.exports = function questionForAttributeConfiguration(attributeConfigurati value: value.value || 'decline' }; }) - }]; + }; } diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 418d1a0..09dea90 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -38,7 +38,7 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur }) .end((err, res) => { request.post(payload.response_url).send({ - attachments: questionForAttributeConfiguration(attributeConfigurations[0]), + attachments: [questionForAttributeConfiguration(attributeConfigurations[0])], replace_original: false }).end(); }); @@ -62,7 +62,7 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur if (nextAttributeConfiguration) { this.body = { text: responseAttributeValue.texts.update, - attachments: questionForAttributeConfiguration(nextAttributeConfiguration) + attachments: [questionForAttributeConfiguration(nextAttributeConfiguration)] }; } else { yield userInformation(userID, { diff --git a/test/message-buttons/question-for-attribute-configuration.js b/test/message-buttons/question-for-attribute-configuration.js index 5f0e862..36602f1 100644 --- a/test/message-buttons/question-for-attribute-configuration.js +++ b/test/message-buttons/question-for-attribute-configuration.js @@ -28,7 +28,7 @@ const attributeConfiguration = { }; test('it translates an attribute configuration into a question', function(t) { - t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), [{ + t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), { title: 'Do you wear jorts?', callback_id: 'jorts', color: '#ccc', @@ -54,7 +54,7 @@ test('it translates an attribute configuration into a question', function(t) { type: 'button', value: 'decline' }] - }]); + }); t.end(); }); diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index d11fbd3..513f978 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -92,7 +92,7 @@ test('it handles acceptance of the initial interview question by repeating the q }, 'should restate the question with the answer attached'); t.deepEqual(secondResponse, { - attachments: 'jortsQuestion', + attachments: ['jortsQuestion'], replace_original: false }, 'should follow up with the first attribute question'); @@ -135,7 +135,7 @@ test('it handles a response to the first attribute question by storing it and as token: 'a-verification-token' })}) .expect(200, (err, {res: body}) => { - t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: 'jantsQuestion'}); + t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: ['jantsQuestion']}); t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts')); storeAttributeStub.restore(); t.end(); From 306e089923cf2fda992acc1e8afa49341450c974 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 17:24:26 -0400 Subject: [PATCH 32/39] Add assertion descriptions --- test/message-buttons/server.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 513f978..28d59b1 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -136,7 +136,7 @@ test('it handles a response to the first attribute question by storing it and as })}) .expect(200, (err, {res: body}) => { t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: ['jantsQuestion']}); - t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts')); + t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts'), 'stores the attribute value'); storeAttributeStub.restore(); t.end(); }); @@ -160,7 +160,7 @@ test('it handles a decline response to the first attribute question by storing i token: 'a-verification-token' })}) .expect(200, () => { - t.ok(storeAttributeStub.calledWith('userID', 'jorts', null)); + t.ok(storeAttributeStub.calledWith('userID', 'jorts', null), 'clears the attribute value'); storeAttributeStub.restore(); t.end(); }); @@ -189,7 +189,7 @@ test('it handles a response to the last attribute question by storing it, summar })}) .expect(200, (err, {res: body}) => { t.equal(body.text, 'We have noted that you do not wear jants. Our records indicate that:\n\n* you wear jorts\n* you do not wear jants\n\nThanks for participating! See you around the Slack.'); - t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants')); + t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants'), 'stores the attribute value'); storeAttributeStub.restore(); t.end(); }); From d92b61c5582c0b10fe921a438774e3360dc1adea Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 17:38:49 -0400 Subject: [PATCH 33/39] Add replacement handling for non-last responses --- src/message-buttons/server.js | 17 ++++++++++++---- test/message-buttons/server.js | 37 ++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 09dea90..13727d6 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -60,10 +60,19 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); if (nextAttributeConfiguration) { - this.body = { - text: responseAttributeValue.texts.update, - attachments: [questionForAttributeConfiguration(nextAttributeConfiguration)] - }; + request.post(payload.response_url).send({ + attachments: [{ + title: responseAttributeConfiguration.interviewQuestion, + text: responseAttributeValue.texts.interviewAnswer + }], + replace_original: true + }) + .end((err, res) => { + request.post(payload.response_url).send({ + attachments: [questionForAttributeConfiguration(nextAttributeConfiguration)], + replace_original: false + }).end(); + }); } else { yield userInformation(userID, { userRepository: userRepository, diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 28d59b1..288e978 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -118,6 +118,39 @@ test('it handles acceptance of the initial interview question by repeating the q }); test('it handles a response to the first attribute question by storing it and asking the second attribute question', function(t) { + const nock = require('nock'); + + const responses = []; + + nock('https://example.com') + .post('/a-response-url') + .times(2) + .reply((uri, requestBody) => { + responses.push(JSON.parse(requestBody)); + + if (responses.length === 2) { + const firstResponse = responses[0]; + const secondResponse = responses[1]; + + t.deepEqual(firstResponse, { + attachments: [{ + title: 'Do you wear jorts?', + text: 'Yes' + }], + replace_original: true + }, 'should restate the question with the answer attached'); + + t.deepEqual(secondResponse, { + attachments: ['jantsQuestion'], + replace_original: false + }, 'should follow up with the second attribute question'); + + t.end(); + } + + return [200, 'hello']; + }); + const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); agent(startServer({attributeConfigurations, questionForAttributeConfiguration, userRepository: fakeUserRepository})) @@ -125,6 +158,7 @@ test('it handles a response to the first attribute question by storing it and as .type('form') .send({payload: JSON.stringify({ callback_id: 'jorts', + response_url: 'https://example.com/a-response-url', user: { id: 'userID' }, @@ -135,10 +169,8 @@ test('it handles a response to the first attribute question by storing it and as token: 'a-verification-token' })}) .expect(200, (err, {res: body}) => { - t.deepEqual(body.body, {text: 'We have noted that you wear jorts.', attachments: ['jantsQuestion']}); t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts'), 'stores the attribute value'); storeAttributeStub.restore(); - t.end(); }); }); @@ -150,6 +182,7 @@ test('it handles a decline response to the first attribute question by storing i .type('form') .send({payload: JSON.stringify({ callback_id: 'jorts', + response_url: 'https://example.com/a-response-url', user: { id: 'userID' }, From 505cf83d81319307a158249998ea4be306cf310a Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 17:54:12 -0400 Subject: [PATCH 34/39] Add replacement for ultimate attribute answer --- src/message-buttons/server.js | 14 +++++++++++- test/message-buttons/server.js | 41 ++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 13727d6..fb44612 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -78,7 +78,19 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur userRepository: userRepository, attributeConfigurations: attributeConfigurations }).then(reply => { - this.body = `${responseAttributeValue.texts.update} ${reply}\nThanks for participating! See you around the Slack.`; + request.post(payload.response_url).send({ + attachments: [{ + title: responseAttributeConfiguration.interviewQuestion, + text: responseAttributeValue.texts.interviewAnswer + }], + replace_original: true + }) + .end((err, res) => { + request.post(payload.response_url).send({ + text: 'Thanks for participating. See you around the Slack!', + replace_original: false + }).end(); + }); }); } } diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 288e978..315e101 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -53,7 +53,8 @@ const secondAttributeConfiguration = { value: 'does not wear jants', texts: { update: 'We have noted that you do not wear jants.', - information: 'you do not wear jants' + information: 'you do not wear jants', + interviewAnswer: 'No' } }] }; @@ -199,7 +200,40 @@ test('it handles a decline response to the first attribute question by storing i }); }); -test('it handles a response to the last attribute question by storing it, summarising, and thanking', function(t) { +test('it handles a response to the last attribute question by storing it, repeating the question and answer, and thanking', function(t) { + const nock = require('nock'); + + const responses = []; + + nock('https://example.com') + .post('/a-response-url') + .times(2) + .reply((uri, requestBody) => { + responses.push(JSON.parse(requestBody)); + + if (responses.length === 2) { + const firstResponse = responses[0]; + const secondResponse = responses[1]; + + t.deepEqual(firstResponse, { + attachments: [{ + title: 'Do you wear jants?', + text: 'No' + }], + replace_original: true + }, 'should restate the question with the answer attached'); + + t.deepEqual(secondResponse, { + text: 'Thanks for participating. See you around the Slack!', + replace_original: false + }, 'should end with a farewell'); + + t.end(); + } + + return [200, 'hello']; + }); + const storeAttributeStub = sinon.stub(fakeUserRepository, 'storeAttribute'); const retrieveAttributeStub = sinon.stub(fakeUserRepository, 'retrieveAttribute'); @@ -211,6 +245,7 @@ test('it handles a response to the last attribute question by storing it, summar .type('form') .send({payload: JSON.stringify({ callback_id: 'jants', + response_url: 'https://example.com/a-response-url', user: { id: 'userID' }, @@ -221,10 +256,8 @@ test('it handles a response to the last attribute question by storing it, summar token: 'a-verification-token' })}) .expect(200, (err, {res: body}) => { - t.equal(body.text, 'We have noted that you do not wear jants. Our records indicate that:\n\n* you wear jorts\n* you do not wear jants\n\nThanks for participating! See you around the Slack.'); t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants'), 'stores the attribute value'); storeAttributeStub.restore(); - t.end(); }); }); From dea761546bd2ff7ef250386c9284108787bebe25 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 18:05:44 -0400 Subject: [PATCH 35/39] Send 200 response explicitly --- src/message-buttons/server.js | 6 ++++++ test/message-buttons/server.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index fb44612..2c56d56 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -28,6 +28,8 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur if (attributeName === 'initial') { if (action.value === 'yes') { + this.status = 200; + request.post(payload.response_url).send({ text: DirectMessageHandler.INTERVIEW_INTRODUCTION, attachments: [{ @@ -60,6 +62,8 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur const nextAttributeConfiguration = getNextAttributeConfiguration(attributeConfigurations, attributeName); if (nextAttributeConfiguration) { + this.status = 200; + request.post(payload.response_url).send({ attachments: [{ title: responseAttributeConfiguration.interviewQuestion, @@ -78,6 +82,8 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur userRepository: userRepository, attributeConfigurations: attributeConfigurations }).then(reply => { + this.status = 200; + request.post(payload.response_url).send({ attachments: [{ title: responseAttributeConfiguration.interviewQuestion, diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 315e101..0257385 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -169,7 +169,7 @@ test('it handles a response to the first attribute question by storing it and as }], token: 'a-verification-token' })}) - .expect(200, (err, {res: body}) => { + .expect(200, () => { t.ok(storeAttributeStub.calledWith('userID', 'jorts', 'wears jorts'), 'stores the attribute value'); storeAttributeStub.restore(); }); @@ -255,7 +255,7 @@ test('it handles a response to the last attribute question by storing it, repeat }], token: 'a-verification-token' })}) - .expect(200, (err, {res: body}) => { + .expect(200, () => { t.ok(storeAttributeStub.calledWith('userID', 'jants', 'does not wear jants'), 'stores the attribute value'); storeAttributeStub.restore(); }); From ec028feebae72f6b7b739e8a6ad2e76d2454a602 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 18:16:29 -0400 Subject: [PATCH 36/39] Remove colour-setting for interview questions --- src/message-buttons/question-for-attribute-configuration.js | 1 - test/message-buttons/question-for-attribute-configuration.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/message-buttons/question-for-attribute-configuration.js b/src/message-buttons/question-for-attribute-configuration.js index f9cb97a..7999539 100644 --- a/src/message-buttons/question-for-attribute-configuration.js +++ b/src/message-buttons/question-for-attribute-configuration.js @@ -3,7 +3,6 @@ module.exports = function questionForAttributeConfiguration(attributeConfigurati return { title: attributeConfiguration.interviewQuestion, callback_id: attributeConfiguration.name, - color: '#ccc', attachment_type: 'default', actions: attributeConfiguration.values.map(value => { return { diff --git a/test/message-buttons/question-for-attribute-configuration.js b/test/message-buttons/question-for-attribute-configuration.js index 36602f1..4a1da73 100644 --- a/test/message-buttons/question-for-attribute-configuration.js +++ b/test/message-buttons/question-for-attribute-configuration.js @@ -31,7 +31,6 @@ test('it translates an attribute configuration into a question', function(t) { t.deepEqual(questionForAttributeConfiguration(attributeConfiguration), { title: 'Do you wear jorts?', callback_id: 'jorts', - color: '#ccc', attachment_type: 'default', actions: [{ name: 'jorts', From 05ef3112562df5f22e17f1b185dbc061d98a816c Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 18:19:29 -0400 Subject: [PATCH 37/39] Remove colour from interview request --- src/direct-message-handler.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index 6c70018..c73d941 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -31,7 +31,6 @@ class DirectMessageHandler { attachments: [{ title: 'Would you like to self-identify?', callback_id: 'initial', - color: '#ccc', attachment_type: 'default', actions: [ { From fccd559fe0412067607bc07b81aa5452507ab41f Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 18:35:30 -0400 Subject: [PATCH 38/39] Change interview decline answer --- src/message-buttons/server.js | 2 +- test/message-buttons/server.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/message-buttons/server.js b/src/message-buttons/server.js index 2c56d56..663339c 100644 --- a/src/message-buttons/server.js +++ b/src/message-buttons/server.js @@ -47,7 +47,7 @@ module.exports = function({attributeConfigurations, questionForAttributeConfigur } else if (action.value === 'more') { this.body = 'Here is more information.'; } else { - this.body = 'Aww!'; + this.body = 'Okay!'; } } else { const attributeValueToFind = action.value === 'decline' ? null : action.value; diff --git a/test/message-buttons/server.js b/test/message-buttons/server.js index 0257385..01cc2e8 100644 --- a/test/message-buttons/server.js +++ b/test/message-buttons/server.js @@ -288,7 +288,7 @@ test('it handles a rejection of the initial interview question', function(t) { }], token: 'a-verification-token' })}) - .expect(200, 'Aww!', t.end); + .expect(200, 'Okay!', t.end); }); test('it rejects an action without the correct verification token', function(t) { From 1392b09c1b08fea71498a7cfc1921c5cdd350af3 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Sat, 10 Sep 2016 19:02:52 -0400 Subject: [PATCH 39/39] Add trimming for direct messages --- adapter-repl.js | 10 ++++++++++ src/direct-message-handler.js | 2 +- test/direct-message-handler.js | 5 +++-- 3 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 adapter-repl.js diff --git a/adapter-repl.js b/adapter-repl.js new file mode 100644 index 0000000..91e56f0 --- /dev/null +++ b/adapter-repl.js @@ -0,0 +1,10 @@ +var conf = require('./config'); + +var SlackAdapter = require('./src/slack-adapter'); + +var SlackClient = require('slack-client'); + +var client = new SlackClient(conf.get('slackToken')); +var adapter = new SlackAdapter(client); + +module.exports = adapter; diff --git a/src/direct-message-handler.js b/src/direct-message-handler.js index c73d941..5ae7099 100644 --- a/src/direct-message-handler.js +++ b/src/direct-message-handler.js @@ -20,7 +20,7 @@ class DirectMessageHandler { handle(channel, message) { if (!message.text || message.subtype === 'bot_message') return; - var text = message.text.toLowerCase(); + var text = message.text.toLowerCase().trim(); var user = this.adapter.getUser(message.user); diff --git a/test/direct-message-handler.js b/test/direct-message-handler.js index af0795d..eb3b23f 100755 --- a/test/direct-message-handler.js +++ b/test/direct-message-handler.js @@ -156,8 +156,9 @@ test('DirectMessageHandler handles an information request', function(t) { retrieveAttributeStub.withArgs(person.id, 'manness').returns(Promise.resolve(person.manness)); retrieveAttributeStub.withArgs(person.id, 'pocness').returns(Promise.resolve(person.pocness)); + // The extra spaces are to verify trimming. handler.handle(personIDToChannel[person.id], { - text: 'info', + text: ' info ', user: person.id }); }); @@ -281,7 +282,7 @@ test('DirectMessageHandler updates channel options and reports them', function(t user: admin.id }); - setTimeout(() => { + setTimeout(() => { t.ok(adminDM.send.calledWithMatch(/<#menexplicitid> has no ignored attributes/), 'expected no ignored attributes to be listed'); t.end();