Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rich "Attachment" Notifications Implemented #32

Merged
merged 3 commits into from
Feb 17, 2014

Conversation

m9
Copy link
Contributor

@m9 m9 commented Feb 16, 2014

I've finally implemented Attachment notifications, as suggested in #24

After a short discussion on IRC with @technicalpickles we've found probably a best way to implement this without extending core hubot functionality.

I've also wrote an example script that represents, how to use this feature. The code below will result with such message in a chat room:

screenshot1

# Description:
#   Demonstrating Slack Attachments.
#
# Commands:
#   hubot demo-attachment - Demo Attachement

module.exports = (robot) ->
  robot.respond /demo-attachment$/i, (msg) =>
    fields = []
    fields.push
      title: "Field 1: Title"
      value: "Field 1: Value"
      short: true

    fields.push
      title: "Field 2: Title"
      value: "Field 2: Value"
      short: true

    payload = 
      message: msg.message
      content:
        text: "Attachement Demo Text"
        fallback: "Fallback Text"
        pretext: "This is Pretext"
        color: "#FF0000"
        fields: fields

    robot.emit 'slack-attachment', payload

@@ -110,6 +127,9 @@ class Slack extends Adapter
# The star.
###################################################################
run: ->
@robot.on 'slack-attachment', (payload)=>
Copy link
Contributor

Choose a reason for hiding this comment

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

From a readability point of view, I think it makes sense to wait until the options are parsed and possible errors are logged to bind this listener.

@evansolomon
Copy link
Contributor

Looks good to me. Since it's pretty slack-specific, would be good to get @grantmd's opinion.

@m9
Copy link
Contributor Author

m9 commented Feb 16, 2014

Thanks @evansolomon for your comments

grantmd added a commit that referenced this pull request Feb 17, 2014
Rich "Attachment" Notifications Implemented
@grantmd grantmd merged commit 7bb0c95 into slackapi:master Feb 17, 2014
@grantmd
Copy link
Contributor

grantmd commented Feb 17, 2014

Thank you @balbeko for the code and @evansolomon for the reviews!

@technicalpickles
Copy link
Contributor

I'd be curious to see is how you'd be able to write scripts that work for other adapters, while still being able to add slack-specific enhancements.

I'm also not sure if I'm 👍 on naming it custom. If you this is going to be slack-specific, may as well call it attach, because that's what they are called in the API documentation.

@m9
Copy link
Contributor Author

m9 commented Feb 17, 2014

@technicalpickles what's a best way to determine in script, what adapter is used?

@technicalpickles
Copy link
Contributor

@balbeko some discussion about that in hubotio/hubot#647 , still need to put together a PR to make the name accessible though.

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

Successfully merging this pull request may close these issues.

4 participants