From 6f3c6f31ee536d53143f8319e12e6063afe16a87 Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 11 Jul 2016 17:32:07 -0400 Subject: [PATCH 1/6] simplifies slack web api instantiation --- lib/Slack_web_api.js | 460 +++++++++++++++---------------------------- 1 file changed, 163 insertions(+), 297 deletions(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index a74bda592..fe3ddb608 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -1,314 +1,180 @@ var request = require('request'); -module.exports = function(bot, config) { +function noop() {} - // create a nice wrapper for the Slack API +module.exports = function(bot, config) { var slack_api = { - api_url: 'https://slack.com/api/', - // this is a simple function used to call the slack web API - callAPI: function(command, options, cb) { - bot.log('** API CALL: ' + slack_api.api_url + command); - if (!options.token) { - options.token = config.token; - } - bot.debug(command, options); - request.post(this.api_url + command, function(error, response, body) { - bot.debug('Got response', error, body); - if (!error && response.statusCode == 200) { - var json; - try { - json = JSON.parse(body); - } catch (err) { - if (cb) return cb(err || 'Invalid JSON'); - return; - } + api_url: 'https://slack.com/api/' + }; + + // Slack API methods: https://api.slack.com/methods + var slackApiMethods = [ + 'auth.test', + 'oauth.access', + 'channels.archive', + 'channels.create', + 'channels.history', + 'channels.info', + 'channels.invite', + 'channels.join', + 'channels.kick', + 'channels.leave', + 'channels.list', + 'channels.mark', + 'channels.rename', + 'channels.setPurpose', + 'channels.setTopic', + 'channels.unarchive', + 'chat.delete', + 'chat.postMessage', + 'chat.update', + 'emoji.list', + 'files.delete', + 'files.info', + 'files.list', + 'files.upload', + 'groups.archive', + 'groups.close', + 'groups.create', + 'groups.createChild', + 'groups.history', + 'groups.info', + 'groups.invite', + 'groups.kick', + 'groups.leave', + 'groups.list', + 'groups.mark', + 'groups.open', + 'groups.rename', + 'groups.setPurpose', + 'groups.setTopic', + 'groups.unarchive', + 'im.close', + 'im.history', + 'im.list', + 'im.mark', + 'im.open', + 'mpim.close', + 'mpim.history', + 'mpim.list', + 'mpim.mark', + 'mpim.open', + 'pins.add', + 'pins.list', + 'pins.remove', + 'reactions.add', + 'reactions.get', + 'reactions.list', + 'reactions.remove', + 'rtm.start', + 'search.all', + 'search.files', + 'search.messages', + 'stars.list', + 'team.accessLogs', + 'team.info', + 'users.getPresence', + 'users.info', + 'users.list', + 'users.setActive' + ]; - if (json.ok) { - if (cb) cb(null, json); - } else { - if (cb) cb(json.error, json); - } - } else { - if (cb) cb(error || 'Invalid response'); + slack_api.callAPI = function(command, options, cb) { + cb = cb || noop; + + bot.log('** API CALL: ' + slack_api.api_url + command); + if (!options.token) { + options.token = config.token; + } + + bot.debug(command, options); + request.post(slack_api.api_url + command, function(error, response, body) { + bot.debug('Got response', error, body); + if (!error && response.statusCode == 200) { + var json; + try { + json = JSON.parse(body); + } catch (err) { + return cb(err || 'Invalid JSON'); } - }).form(options); - }, - callAPIWithoutToken: function(command, options, cb) { - bot.log('** API CALL: ' + slack_api.api_url + command); - if (!options.client_id) { - options.client_id = bot.config.clientId; - } - if (!options.client_secret) { - options.client_secret = bot.config.clientSecret; - } - if (!options.redirect_uri) { - options.redirect_uri = bot.config.redirectUri; - } - request.post(this.api_url + command, function(error, response, body) { - bot.debug('Got response', error, body); - if (!error && response.statusCode == 200) { - var json; - try { - json = JSON.parse(body); - } catch (err) { - if (cb) return cb(err || 'Invalid JSON'); - return; - } - if (json.ok) { - if (cb) cb(null, json); - } else { - if (cb) cb(json.error, json); - } - } else { - if (cb) cb(error || 'Invalid response'); + if (json.ok) { + return cb(null, json); } - }).form(options); - }, - auth: { - test: function(options, cb) { - slack_api.callAPI('auth.test', options, cb); - } - }, - oauth: { - access: function(options, cb) { - slack_api.callAPIWithoutToken('oauth.access', options, cb); - } - }, - channels: { - archive: function(options, cb) { - slack_api.callAPI('channels.archive', options, cb); - }, - create: function(options, cb) { - slack_api.callAPI('channels.create', options, cb); - }, - history: function(options, cb) { - slack_api.callAPI('channels.history', options, cb); - }, - info: function(options, cb) { - slack_api.callAPI('channels.info', options, cb); - }, - invite: function(options, cb) { - slack_api.callAPI('channels.invite', options, cb); - }, - join: function(options, cb) { - slack_api.callAPI('channels.join', options, cb); - }, - kick: function(options, cb) { - slack_api.callAPI('channels.kick', options, cb); - }, - leave: function(options, cb) { - slack_api.callAPI('channels.leave', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('channels.list', options, cb); - }, - mark: function(options, cb) { - slack_api.callAPI('channels.mark', options, cb); - }, - rename: function(options, cb) { - slack_api.callAPI('channels.rename', options, cb); - }, - setPurpose: function(options, cb) { - slack_api.callAPI('channels.setPurpose', options, cb); - }, - setTopic: function(options, cb) { - slack_api.callAPI('channels.setTopic', options, cb); - }, - unarchive: function(options, cb) { - slack_api.callAPI('channels.unarchive', options, cb); + + return cb(json.error, json); } - }, - chat: { - delete: function(options, cb) { - slack_api.callAPI('chat.delete', options, cb); - }, - postMessage: function(options, cb) { - if (options.attachments && typeof(options.attachments) != 'string') { - options.attachments = JSON.stringify(options.attachments); + + return cb(error || 'Invalid response'); + }).form(options); + }; + + slack_api.callAPIWithoutToken = function(command, options, cb) { + cb = cb || noop; + + bot.log('** API CALL: ' + slack_api.api_url + command); + if (!options.client_id) { + options.client_id = bot.config.clientId; + } + if (!options.client_secret) { + options.client_secret = bot.config.clientSecret; + } + if (!options.redirect_uri) { + options.redirect_uri = bot.config.redirectUri; + } + request.post(slack_api.api_url + command, function(error, response, body) { + bot.debug('Got response', error, body); + if (!error && response.statusCode == 200) { + var json; + try { + json = JSON.parse(body); + } catch (err) { + return cb(err || 'Invalid JSON'); } - slack_api.callAPI('chat.postMessage', options, cb); - }, - update: function(options, cb) { - slack_api.callAPI('chat.update', options, cb); - } - }, - emoji: { - list: function(options, cb) { - slack_api.callAPI('emoji.list', options, cb); - } - }, - files: { - delete: function(options, cb) { - slack_api.callAPI('files.delete', options, cb); - }, - info: function(options, cb) { - slack_api.callAPI('files.info', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('files.list', options, cb); - }, - upload: function(options, cb) { - slack_api.callAPI('files.upload', options, cb); - }, - }, - groups: { - archive: function(options, cb) { - slack_api.callAPI('groups.archive', options, cb); - }, - close: function(options, cb) { - slack_api.callAPI('groups.close', options, cb); - }, - create: function(options, cb) { - slack_api.callAPI('groups.create', options, cb); - }, - createChild: function(options, cb) { - slack_api.callAPI('groups.createChild', options, cb); - }, - history: function(options, cb) { - slack_api.callAPI('groups.history', options, cb); - }, - info: function(options, cb) { - slack_api.callAPI('groups.info', options, cb); - }, - invite: function(options, cb) { - slack_api.callAPI('groups.invite', options, cb); - }, - kick: function(options, cb) { - slack_api.callAPI('groups.kick', options, cb); - }, - leave: function(options, cb) { - slack_api.callAPI('groups.leave', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('groups.list', options, cb); - }, - mark: function(options, cb) { - slack_api.callAPI('groups.mark', options, cb); - }, - open: function(options, cb) { - slack_api.callAPI('groups.open', options, cb); - }, - rename: function(options, cb) { - slack_api.callAPI('groups.rename', options, cb); - }, - setPurpose: function(options, cb) { - slack_api.callAPI('groups.setPurpose', options, cb); - }, - setTopic: function(options, cb) { - slack_api.callAPI('groups.setTopic', options, cb); - }, - unarchive: function(options, cb) { - slack_api.callAPI('groups.unarchive', options, cb); - }, - }, - im: { - close: function(options, cb) { - slack_api.callAPI('im.close', options, cb); - }, - history: function(options, cb) { - slack_api.callAPI('im.history', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('im.list', options, cb); - }, - mark: function(options, cb) { - slack_api.callAPI('im.mark', options, cb); - }, - open: function(options, cb) { - slack_api.callAPI('im.open', options, cb); - } - }, - mpim: { - close: function(options, cb) { - slack_api.callAPI('mpim.close', options, cb); - }, - history: function(options, cb) { - slack_api.callAPI('mpim.history', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('mpim.list', options, cb); - }, - mark: function(options, cb) { - slack_api.callAPI('mpim.mark', options, cb); - }, - open: function(options, cb) { - slack_api.callAPI('mpim.open', options, cb); + + if (json.ok) { + return cb(null, json); + } + return cb(json.error, json); } - }, - pins: { - add: function(options, cb) { - slack_api.callAPI('pins.add', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('pins.list', options, cb); - }, - remove: function(options, cb) { - slack_api.callAPI('pins.remove', options, cb); + return cb(error || 'Invalid response'); + }).form(options); + }; + + + // generate all API methods + slackApiMethods.forEach(function(slackMethod) { + // most slack api methods are only two parts, of the form group.method, e.g. auth.test + // some have three parts: group.subgroup.method, e.g, users.profile.get + // this method loops through all groups in a method, ensures they exist, + // then adds the method to the terminal group + + var groups = slackMethod.split('.'); + var method = groups.pop(); + var currentGroup = slack_api; + var nextGroupName; + + for (var i = 0; i < groups.length; i++) { + nextGroupName = groups[i]; + if (!currentGroup[nextGroupName]) { + currentGroup[nextGroupName] = {}; } - }, - reactions: { - add: function(options, cb) { - slack_api.callAPI('reactions.add', options, cb); - }, - get: function(options, cb) { - slack_api.callAPI('reactions.get', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('reactions.list', options, cb); - }, - remove: function(options, cb) { - slack_api.callAPI('reactions.remove', options, cb); - }, - }, - rtm: { - start: function(options, cb) { - slack_api.callAPI('rtm.start', options, cb); - }, - }, - search: { - all: function(options, cb) { - slack_api.callAPI('search.all', options, cb); - }, - files: function(options, cb) { - slack_api.callAPI('search.files', options, cb); - }, - messages: function(options, cb) { - slack_api.callAPI('search.messages', options, cb); - }, - }, - stars: { - list: function(options, cb) { - slack_api.callAPI('stars.list', options, cb); - }, - }, - team: { - accessLogs: function(options, cb) { - slack_api.callAPI('team.accessLogs', options, cb); - }, - info: function(options, cb) { - slack_api.callAPI('team.info', options, cb); - }, - }, - users: { - getPresence: function(options, cb) { - slack_api.callAPI('users.getPresence', options, cb); - }, - info: function(options, cb) { - slack_api.callAPI('users.info', options, cb); - }, - list: function(options, cb) { - slack_api.callAPI('users.list', options, cb); - }, - setActive: function(options, cb) { - slack_api.callAPI('users.setActive', options, cb); - }, - setPresence: function(options, cb) { - slack_api.callAPI('users.setPresence', options, cb); - }, + currentGroup = currentGroup[nextGroupName]; } + + currentGroup[method] = function(options, cb) { + slack_api.callAPI(slackMethod, options, cb); + }; + + }); + + // overwrite default behavior + slack_api.chat.postMessage = function(options, cb) { + if (options.attachments && typeof(options.attachments) != 'string') { + options.attachments = JSON.stringify(options.attachments); + } + slack_api.callAPI('chat.postMessage', options, cb); }; return slack_api; - }; From eb06b23ae46a80edeff85871a3c38ea2b6deaabb Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 18 Jul 2016 21:02:45 -0400 Subject: [PATCH 2/6] remove unused conditional in error case --- lib/Slack_web_api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index fe3ddb608..e7e7ac042 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -95,7 +95,7 @@ module.exports = function(bot, config) { try { json = JSON.parse(body); } catch (err) { - return cb(err || 'Invalid JSON'); + return cb(err); } if (json.ok) { From aac40a4d22acfcd3f35d30251f3bf8e1a37c9f7d Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Mon, 18 Jul 2016 21:19:14 -0400 Subject: [PATCH 3/6] removes code duplication for callAPI and callAPIWithoutToken, improves error handling --- lib/Slack_web_api.js | 74 ++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index e7e7ac042..2583169e6 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -80,39 +80,15 @@ module.exports = function(bot, config) { ]; slack_api.callAPI = function(command, options, cb) { - cb = cb || noop; - - bot.log('** API CALL: ' + slack_api.api_url + command); if (!options.token) { options.token = config.token; } bot.debug(command, options); - request.post(slack_api.api_url + command, function(error, response, body) { - bot.debug('Got response', error, body); - if (!error && response.statusCode == 200) { - var json; - try { - json = JSON.parse(body); - } catch (err) { - return cb(err); - } - - if (json.ok) { - return cb(null, json); - } - - return cb(json.error, json); - } - - return cb(error || 'Invalid response'); - }).form(options); + postForm(slack_api.api_url + command, options, cb); }; slack_api.callAPIWithoutToken = function(command, options, cb) { - cb = cb || noop; - - bot.log('** API CALL: ' + slack_api.api_url + command); if (!options.client_id) { options.client_id = bot.config.clientId; } @@ -122,23 +98,9 @@ module.exports = function(bot, config) { if (!options.redirect_uri) { options.redirect_uri = bot.config.redirectUri; } - request.post(slack_api.api_url + command, function(error, response, body) { - bot.debug('Got response', error, body); - if (!error && response.statusCode == 200) { - var json; - try { - json = JSON.parse(body); - } catch (err) { - return cb(err || 'Invalid JSON'); - } - if (json.ok) { - return cb(null, json); - } - return cb(json.error, json); - } - return cb(error || 'Invalid response'); - }).form(options); + // DON'T log options: that could expose the client secret! + postForm(slack_api.api_url + command, options, cb); }; @@ -177,4 +139,34 @@ module.exports = function(bot, config) { }; return slack_api; + + + /** + * Makes a POST request as a form to the given url with the options as data + * @param url + * @param options + * @param cb + */ + function postForm(url, options, cb) { + cb = cb || noop; + + bot.log('** API CALL: ' + url); + request.post(url, function(error, response, body) { + bot.debug('Got response', error, body); + if (!error && response.statusCode == 200) { + var json; + try { + json = JSON.parse(body); + } catch (parseError) { + return cb(parseError); + } + + if (json.ok) { + return cb(null, json); + } + return cb(json.error, json); + } + return cb(error || new Error('Invalid response')); + }).form(options); + } }; From fbdfb0f64224219467546bb5d67f856902f93139 Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Tue, 19 Jul 2016 13:51:33 -0400 Subject: [PATCH 4/6] uses forEach instead of for --- lib/Slack_web_api.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index 2583169e6..48562fe04 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -114,15 +114,13 @@ module.exports = function(bot, config) { var groups = slackMethod.split('.'); var method = groups.pop(); var currentGroup = slack_api; - var nextGroupName; - for (var i = 0; i < groups.length; i++) { - nextGroupName = groups[i]; + groups.forEach(function(nextGroupName) { if (!currentGroup[nextGroupName]) { currentGroup[nextGroupName] = {}; } currentGroup = currentGroup[nextGroupName]; - } + }); currentGroup[method] = function(options, cb) { slack_api.callAPI(slackMethod, options, cb); From dc56ba6ba82826d43ac89e85d6bbe7e11c1a0bdc Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Tue, 19 Jul 2016 22:07:06 -0400 Subject: [PATCH 5/6] adds correct jsdoc style docs --- lib/Slack_web_api.js | 57 ++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index 48562fe04..2dfca019b 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -1,7 +1,16 @@ var request = require('request'); +/** + * Does nothing. Takes no params, returns nothing. It's a no-op! + */ function noop() {} +/** + * Returns an interface to the Slack API in the context of the given bot + * @param {Object} bot The botkit bot object + * @param {Object} config A config containing auth credentials. + * @returns {Object} A callback-based Slack API interface. + */ module.exports = function(bot, config) { var slack_api = { api_url: 'https://slack.com/api/' @@ -79,28 +88,40 @@ module.exports = function(bot, config) { 'users.setActive' ]; - slack_api.callAPI = function(command, options, cb) { - if (!options.token) { - options.token = config.token; + /** + * Calls Slack using a Token for authentication/authorization + * @param {string} command The Slack API command to call + * @param {Object} data The data to pass to the API call + * @param {function} cb A NodeJS-style callback + */ + slack_api.callAPI = function(command, data, cb) { + if (!data.token) { + data.token = config.token; } - bot.debug(command, options); - postForm(slack_api.api_url + command, options, cb); + bot.debug(command, data); + postForm(slack_api.api_url + command, data, cb); }; - slack_api.callAPIWithoutToken = function(command, options, cb) { - if (!options.client_id) { - options.client_id = bot.config.clientId; + /** + * Calls Slack using OAuth for authentication/authorization + * @param {string} command The Slack API command to call + * @param {Object} data The data to pass to the API call + * @param {function} cb A NodeJS-style callback + */ + slack_api.callAPIWithoutToken = function(command, data, cb) { + if (!data.client_id) { + data.client_id = bot.config.clientId; } - if (!options.client_secret) { - options.client_secret = bot.config.clientSecret; + if (!data.client_secret) { + data.client_secret = bot.config.clientSecret; } - if (!options.redirect_uri) { - options.redirect_uri = bot.config.redirectUri; + if (!data.redirect_uri) { + data.redirect_uri = bot.config.redirectUri; } // DON'T log options: that could expose the client secret! - postForm(slack_api.api_url + command, options, cb); + postForm(slack_api.api_url + command, data, cb); }; @@ -141,11 +162,11 @@ module.exports = function(bot, config) { /** * Makes a POST request as a form to the given url with the options as data - * @param url - * @param options - * @param cb + * @param {string} url The URL to POST to + * @param {Object} formData The data to POST as a form + * @param {function=} cb An optional NodeJS style callback when the POST completes or errors out. */ - function postForm(url, options, cb) { + function postForm(url, formData, cb) { cb = cb || noop; bot.log('** API CALL: ' + url); @@ -165,6 +186,6 @@ module.exports = function(bot, config) { return cb(json.error, json); } return cb(error || new Error('Invalid response')); - }).form(options); + }).form(formData); } }; From 439a601da4f3def8f66e1f1a55009b2d3b6557da Mon Sep 17 00:00:00 2001 From: Cole Furfaro-Strode Date: Tue, 19 Jul 2016 22:28:13 -0400 Subject: [PATCH 6/6] clear attachments if parsing fails on postMessage --- lib/Slack_web_api.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Slack_web_api.js b/lib/Slack_web_api.js index 2dfca019b..5eeb53d0f 100755 --- a/lib/Slack_web_api.js +++ b/lib/Slack_web_api.js @@ -152,7 +152,12 @@ module.exports = function(bot, config) { // overwrite default behavior slack_api.chat.postMessage = function(options, cb) { if (options.attachments && typeof(options.attachments) != 'string') { - options.attachments = JSON.stringify(options.attachments); + try { + options.attachments = JSON.stringify(options.attachments); + } catch(err) { + delete options.attachments; + bot.log.error('Could not parse attachments', err); + } } slack_api.callAPI('chat.postMessage', options, cb); };