Slack Events api #480

jonchurch · 2016-11-08T03:40:12Z

Had to resubmit this because I somehow got the other branch into a non-repairable headless state that github support advised I abandon ¯_(ツ)_/¯

Receive Events API events at the webhook setup by createWebhookEndpoints()

EDIT: startTicking() is now called for you when wehbhook endpoints are created
~~Use controller.startTicking() when running bot to enable conversations~~

Would like feedback on Bot Identity management without RTM. I'm saving bot identity to team storage when a user completes oauth flow, and looking it up from storage each time an Events API request is received.

closes #438

…web api when RTM fails

add events_api boolean, event_ts, authed_users

…ntity is not in team storage use identifyBot

…sAPI

…ts api to play nice together start ticking when webhooks are setup

sundeepgupta · 2016-11-09T21:43:31Z

Read the changelog earlier and the latest 0.4.1 release already support the Events API - no?

jonchurch · 2016-11-09T22:15:40Z

Nope! It was mostly small fixes in 4.1
Biggest slack change was defaulting to Web API for sending messages, which is a separate issue from receiving events from Events API at a webhook.

Any feedback @sundeepgupta?
I still need to better explain subscribing to events in the docs. For example, a file shared into a channel the bot is in will generate a message event (assuming the bot is subscribed to messaging events) that has a subtype field with a value of file_share. This is representative of the message slack sends into the channel. Botkit will trigger any message with a subtype as an event of that subtype. So now we can listen to file_share events. However, there is a separate permission in the Events API that is fired whenever a file is shared anywhere on a team. That will fire a file_shared(notice the d) event from the Events API, and botkit will fire file_shared when receiving this event. However the file_shared event is generated under a different permission, and contains different information than file_share(which at this point has much more data about that file than file_shared).

I left it this way because that's how Botkit did it before, but this would likely be very confusing unless explained well in the docs.

sundeepgupta · 2016-11-10T05:15:44Z

@jonchurch I've yet to learn about Events API and haven't looked at your PR, sorry. Thought I'd just check to make sure this wasn't duplicated effort but I obviously mis-understood the changelog.

colestrode

Looks great! I haven't tested this against the API yet, but I left some comments.

colestrode · 2016-11-11T18:40:39Z

examples/slackbutton_bot.js

@@ -27,8 +27,8 @@ This is a sample Slack Button application that adds a bot to one or many slack t
 /* Uses the slack button feature to offer a real time bot to multiple teams */
 var Botkit = require('../lib/Botkit.js');

-if (!process.env.clientId || !process.env.clientSecret || !process.env.port) {
-  console.log('Error: Specify clientId clientSecret and port in environment');
+if (!process.env.clientId || !process.env.clientSecret || !process.env.port || !process.env.redirectUri) {


Why is redirectUri required in this example now? It's optional in the Slack OAuth flow.

This was actually part of a PR from #458, which I realize now was not necessary.
You are correct, redirectUri is not required here, I misunderstood their issue.
Removing

colestrode · 2016-11-11T18:42:57Z

lib/SlackBot.js

@@ -287,15 +358,19 @@ function Slackbot(configuration) {

        slack_botkit.webserver = express();
        slack_botkit.webserver.use(bodyParser.json());
-        slack_botkit.webserver.use(bodyParser.urlencoded({ extended: true }));
+        slack_botkit.webserver.use(bodyParser.urlencoded({


These formatting changes make it a bit hard to understand what changes are relevant to this functionality. Would it be possible to exclude them from this PR?

colestrode · 2016-11-11T18:44:22Z

lib/SlackBot.js

+
+                                            // Save bot identity to team storage for retrieval when handling Events API
+                                            slack_botkit.findTeamById(identity.team_id, function(err, team_data) {
+                                                if (!team_data.bot.identity) {


team_data will probably not be defined if there is an error. I suggest testing for the error immediately, then exiting early if one exists.

colestrode · 2016-11-11T18:52:09Z

.gitignore

 examples/db_slackbutton_bot/
 examples/db_slackbutton_incomingwebhook/
 examples/db_slackbutton_slashcommand/
 examples/db_team_bot/
 .DS_Store
 */.DS_Store
 .env
+.idea


Not sure we need .idea in the gitignore. That seems like it should be in users global .gitignore if they are webstorm users.

colestrode · 2016-11-11T18:52:42Z

.gitignore

@@ -2,10 +2,12 @@ node_modules/
 start.sh
 start_button.sh
 db/
+db_slack_events_api/


why not put this db under either the examples/ directory?

colestrode · 2016-11-11T18:54:00Z

lib/SlackBot.js

+                res.status(200).json({
+                    challenge: req.body.challenge
+                });
+


There are a lot of extra spaces in this file before closing braces. Would you mind removing them to be more consistent with the rest of the file?

colestrode · 2016-11-11T18:54:52Z

lib/SlackBot.js

+
+            // Receive messages and trigger events from the Events API
+            if (req.body.type === 'event_callback') {
+                // respond to events api with a 200


would it be possible to pull all of this logic inside the if into it's own function? this function is getting very long and hard to follow, decomposing it will make it easier to read.

colestrode · 2016-11-11T19:09:42Z

lib/SlackBot.js

+                    if (err || !team) {
+                        slack_botkit.log.error('Received Events API message, but could not load team');
+                        if (err) {
+                            slack_botkit.log.error(err);


if you return here, then you save yourself the if and keeps the indentation flatter.

colestrode · 2016-11-11T19:10:53Z

lib/SlackBot.js

+                            bot.identifyBot(function(err, identity) {
+                                bot.identity = identity;
+
+                                if (req.body.event.type === 'message') {


this logic is duplicated, could we make this a function?

colestrode · 2016-11-11T19:19:33Z

lib/SlackBot.js

+
+                        if (team.bot.identity == undefined || team.bot.identity.id == null) {
+                            // API call to slack to identify bot user
+                            bot.identifyBot(function(err, identity) {


what happens if there is an error identifying the bot?

…eventsApi

benbrown · 2016-11-21T20:57:34Z

This got merged in to #500 and is now released! Thanks @jonchurch and @colestrode and @sundeepgupta and @peterswimm for your attention to this!!

jonchurch added 27 commits October 18, 2016 18:55

handshake with events api

8d1e016

receive callback from slack event_api

4bcbcdb

format events api messages like RTM messages, force send message via …

ccbd228

…web api when RTM fails

check identity of bot, pass identity into bot spawned w/o RTM

74de535

ignore events generated from your bots own actions

0961626

force coreBot to use web api to send messages if no RTM connection

dd78796

handling message_received events in app still not working

4e4c3d7

unwrap events api envelope, follow RTM message format

1d24c81

add events_api boolean, event_ts, authed_users

check for url_verification or event_callback in events_api types

195e871

add example slack event bot, startTicking on event api message received

78d86b1

start ticking when bot is run

6c243d5

emoji edit, lookup channel id to react to emoji edit in a public channel

f093069

testing pin_added, reaction_remove, file_comment_added, star_added

4f71308

team lookup is broken

09c16c4

identifyBot calls auth.test and passes bot identity to cb, if bot ide…

5233aa5

…ntity is not in team storage use identifyBot

beautify

b051ed5

Merge branch 'oldCommit' into eventsAPI

e7d34ad

cleanup

6abb8cd

restore slackbutton_bot_interactive.js

8d585a6

Merge branch 'master' of https://github.com/howdyai/botkit into event…

abf8867

…sAPI

update readme, add semicolons

064c627

reset slackbutton_bot

6b6a556

reset slackbot.js

0612267

add eventsApi config option, silence RTM messages, allow RTM and even…

df1f61a

…ts api to play nice together start ticking when webhooks are setup

undo changes to identifyBot(), typos

131eaea

fix merge conflicts with master

2f6005e

fix merge conflicts

ab52060

jonchurch mentioned this pull request Nov 8, 2016

Slack Events api #459

Closed

update docs

1009c39

peterswimm added the enhancement label Nov 8, 2016

peterswimm added the Slack-related label Nov 8, 2016

colestrode reviewed Nov 11, 2016

View reviewed changes

jonchurch added 3 commits November 14, 2016 15:30

update .gitignore, slackbuton_bot.js

f316ac5

added handleEventsAPI function

a41da9d

Merge branch 'eventsApi' of https://github.com/jonchurch/botkit into …

37d1a5f

…eventsApi

benbrown mentioned this pull request Nov 18, 2016

Events API support #500

Merged

benbrown closed this Nov 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Slack Events api #480

Slack Events api #480

jonchurch commented Nov 8, 2016 •

edited

Loading

sundeepgupta commented Nov 9, 2016 •

edited

Loading

jonchurch commented Nov 9, 2016 •

edited

Loading

sundeepgupta commented Nov 10, 2016

colestrode left a comment

colestrode Nov 11, 2016

jonchurch Nov 14, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

colestrode Nov 11, 2016

benbrown commented Nov 21, 2016

Slack Events api #480

Slack Events api #480

Conversation

jonchurch commented Nov 8, 2016 • edited Loading

sundeepgupta commented Nov 9, 2016 • edited Loading

jonchurch commented Nov 9, 2016 • edited Loading

sundeepgupta commented Nov 10, 2016

colestrode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benbrown commented Nov 21, 2016

jonchurch commented Nov 8, 2016 •

edited

Loading

sundeepgupta commented Nov 9, 2016 •

edited

Loading

jonchurch commented Nov 9, 2016 •

edited

Loading