Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Add /skills slash command support. #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add /skills slash command support. #38

wants to merge 2 commits into from

Conversation

cowboy
Copy link
Contributor

@cowboy cowboy commented Jul 7, 2016

How it works

  1. Type /skills <optional args> from any channel.
  2. This should cause Slack to POST a payload to /command.
  3. Slack should send a message back to you in the channel in which you used /skills.
    • If the channel is a DM with the bot, ti should echo the command back to you.
    • Otherwise, it should instruct you to check your DM with the bot.
  4. The app should simulate a message to the bot from you in a DM.
  5. The bot should respond in the DM.

Notes

  • Slash command responses use the app icon, which is currently the default. Once the app has an icon, it'll look better.
  • I don't think it's possible to have the app NOT respond to a slash command, as far as I can tell, it will always need to display something.

Testing with your own app

As part of Slack app setup, before running the bot:

  1. Click on "App Credentials" in the left nav.
  2. Make note of the Verification Token and add it to your .env file like so:
    • SLACK_VERIFICATION_TOKEN=your_verification_token_value
  3. Click on "Slash Commands" in the left nav.
  4. Click "Create new command".
  5. Enter these settings:
    • Command: /skills
    • Request URL: https://c863a1e3.ngrok.io/command (use your ngrok url + /command)
    • Short Description: Show your skills.
    • Usage hint: [command]
  6. Click "Save".

If you've already added the app to Slack, you'll have to refresh the ngrok server page with the "Add to Slack" button and re-add it, so the new commands scope is added to the integration. This shouldn't require changing or re-importing the database.

@cowboy
Copy link
Contributor Author

cowboy commented Jul 7, 2016

FYI, you should be able to test this right now in our Slack with /skills [command]

@cowboy
Copy link
Contributor Author

cowboy commented Jul 7, 2016

So... is this a worthwhile feature? Should I do anything differently? Skip it entirely?

@@ -36,6 +36,11 @@ export default class BotRunner {
}
}

// Find the first bot for which "fn" returns true.
findBot(fn) {
return Object.keys(this.bots).map(k => this.bots[k]).find(fn);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how well this would scale to a large quantity of bots--seems like we should be making an index by the key we care about for lookups, or bypassing the instantiated bot entirely, somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could easily be optimized later, I wrote it to be as generic as possible for now.

@tkellen
Copy link
Contributor

tkellen commented Jul 7, 2016

This seems like a worthwhile feature if we were going to try to do serverless bots. Outside that context, I'm not sure what value it adds for the end user?

@cowboy
Copy link
Contributor Author

cowboy commented Jul 7, 2016

@tkellen think about it this way, perhaps: How many times will people who haven't read the docs try to type /skills?

@cowboy
Copy link
Contributor Author

cowboy commented Jul 7, 2016

I'm totally happy not landing this, but keeping it in mind in case we decide we want this functionality. It's all fine with me!

@tkellen
Copy link
Contributor

tkellen commented Jul 7, 2016

@tkellen think about it this way, perhaps: How many times will people who haven't read the docs try to type /skills?

👍 that won me over

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants