Added app.json, to allow for one click installation in Heroku#9
Added app.json, to allow for one click installation in Heroku#9
Conversation
Aded slack event listener for team_join Added information to README about team_join config vars
zkat
left a comment
zkat
left a comment
There was a problem hiding this comment.
Great work so far! I made a few comments for stuff that should probably get addressed before being merged in. Thanks a lot for all this!
lib/bot.js
Outdated
| require('./events/team_join')(token, data, e => { | ||
| console.log('sent welcome message') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
I think I'd prefer this even be tracked through the Events API -- I've enabled the team_join event on both slack apps, so you should be able to add an entry to lib/events
There was a problem hiding this comment.
So basically, in order to switch, you just need to remove this chunk of code -- lib/events will be called automatically by the events API.
There was a problem hiding this comment.
Oh that event wasn't working and I wasn't sure if it was deprecated or something or inactive. Thanks for activating I will change that.
There was a problem hiding this comment.
Let me know if it still doesn't work -- it might need a new permission scope which would be a massive pita
There was a problem hiding this comment.
oh ok :/ yep that's why I opted for this quick and dirty solution, but the proper way is better. I'll have time again tonight and will let you know.
|
|
||
| const cocTxt = process.env.COC_URL ? `<${process.env.COC_URL}|Code of Conduct>` : 'Code of Conduct' | ||
| const welcome_message_text = process.env.WELCOME_MESSAGE_TEXT || `Welcome! Review the ${cocTxt} before participating in the community.` | ||
| const send_welcome_message = process.env.SEND_WELCOME_MESSAGE === 'FALSE' ? false : true |
There was a problem hiding this comment.
We need to figure out a way to make this configurable per-Slack, by Admins. I've been considering adding an admin-only /config command. How do you feel about this part of your patch being left unused until it can be configured? Different Slacks are going to have completely different needs as far as their welcome messages go, too. >_<
There was a problem hiding this comment.
Yep, I realized that later, this would be global config for everyone? how are you currently managing configurations per team, I can change it so it uses that.
There was a problem hiding this comment.
We don't have a way to do that right now, which kinda sucks but was in my mental to-do list for Soon™.
You did the bulk of the work for this feature too so what I meant is that we could merge this with default messages, and then customize it once the feature is up. I'm still not sure what the best way to do that is.
lib/events/team_join.js
Outdated
| const send_welcome_message = process.env.SEND_WELCOME_MESSAGE === 'FALSE' ? false : true | ||
|
|
||
| module.exports.command = 'join-private [channel]' | ||
| module.exports.desc = 'Invites user to the given channel, or lists them.' |
There was a problem hiding this comment.
This seems like you copy-pasted the command and left the command spec stuff in place?
lib/events/team_join.js
Outdated
| module.exports.command = 'join-private [channel]' | ||
| module.exports.desc = 'Invites user to the given channel, or lists them.' | ||
|
|
||
| module.exports = (token, data, cb) => { |
There was a problem hiding this comment.
If you switch to the event API fully (you don't need to move this file, it's fine where it is!), you will only get (data, cb) as arguments.
lib/events/team_join.js
Outdated
| module.exports = (token, data, cb) => { | ||
| console.log('new user joined team: ', data) | ||
| cb() | ||
| console.log(send_welcome_message, welcome_message_text) |
lib/events/team_join.js
Outdated
| cb() | ||
| console.log(send_welcome_message, welcome_message_text) | ||
| if (!send_welcome_message || !welcome_message_text) { | ||
| return (cb ? cb() : undefined) |
There was a problem hiding this comment.
cb() always needs to be called. It will always be passed in by the events handler.
lib/events/team_join.js
Outdated
| channel: data.user.id, | ||
| text: welcome_message_text | ||
| }).then(() => { | ||
| return (cb ? cb() : undefined) |
There was a problem hiding this comment.
see above comment about cb always being available.
| "description": "" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
How do you feel about filling in these descriptions?
There was a problem hiding this comment.
I don't know what exactly goes there, would you mind helping with that text, I don't wanna confuse the users.
There was a problem hiding this comment.
I can separate the things into two different PRs too, they're not really the same feature.
There was a problem hiding this comment.
They're basically application-level tokens. This isn't really a blocker either so don't worry about it. They're used by Slack to identify and authenticate applications, and there verification token is for authenticating individual requests -- these are simply things you grab from slack
|
@zkat added the changes so that it uses the events file and api. Not 100% sure that it works but if the authorizations are made it should work. |
2 participants
Aded slack event listener for team_join
Added information to README about team_join config vars