Update slack notification formatting #34
Conversation
|
@MetcalfeTom I've allocated some time to review your PR this week. |
|
Cool Teams supports a similar formatting feature to blocks called cards. Discord does not have any kind of fancy formats. In all three cases the JSON payloads would still differ |
knockknock/slack_sender.py
Outdated
| dump["blocks"] = _starting_message( | ||
| func_name, host_name, notification, start_time | ||
| ) | ||
| dump["text"] = notification |
There was a problem hiding this comment.
Do you need the notification here too? It seems to me that it's already contained in the block field.
There was a problem hiding this comment.
Yes, removing it from here means that the message text will not display in desktop notifications
knockknock/slack_sender.py
Outdated
| func_name, host_name, notification, start_time | ||
| ) | ||
| dump["text"] = notification | ||
| dump["icon_emoji"] = ":clapper:" |
There was a problem hiding this comment.
I might be missing something on the block builder web demo: I couldn't display this emoji (error with this line it seems).
There was a problem hiding this comment.
Unfortunately I found some bad news about this on the slack webhook docs
You cannot override the default channel (chosen by the user who installed your app), username, or icon when you're using Incoming Webhooks to post messages. Instead, these values will always inherit from the associated Slack app configuration.
I tried some workarounds for this, but it seems that updating the name/emoji are only possible using the new api, which would change the configuration of this sender.
As a result, I removed dump and the channel parameter from this file entirely
There was a problem hiding this comment.
I have difficulties understanding: where is your message sent then (on which channel)? is it encoded in the URL in an identifiable way?
As far as possible, I think it's nice to have an identifiable bot sending messages (instead of yourself to yourself), something that the current version is handling well.
There was a problem hiding this comment.
Is this (https://api.slack.com/bot-users#installing-bot) related?
There was a problem hiding this comment.
Since knockknock only uses the webhook sender for slack, the bot user is set when initialising the app, and the channel is selected specifically for each webhook (as below).
Messages sent to this webhook are still posted as the bot, but only to one channel, and with the same username and profile image every time. This functionality looks like it has been deprecated for webhooks, and is only possible with the new API.
I can update the slack sender to use the new API if you wish, but it will bring it further away from the refactoring you mentioned earlier
There was a problem hiding this comment.
@VictorSanh How would you like to proceed? I believe the choices are:
- Forgo the profile image formatting
- Update this sender with a bot user account, and use
chat.postMessageto specify channel and profile image (and potentially more functionality). This would also mean updating the documentation for the new slack token setup.
|
@MetcalfeTom it looks great! |
|
@VictorSanh Thanks for taking the time to review! 🙌 I followed up on your comments and added your suggestions. Not sure you'd construct the refactor between this, discord and teams, but following that PR I would gladly take a stab at updating the Teams format to use cards too 🚀 |
Happy holidays! 🎅🏼
I thought the slack message formatting looked a little sad, so I updated it. I used the block kit builder to update the messages.
I also updated the
slack_senderto use usernames instead ofuser_ids (perhaps I used this incorrectly, but posting theuser_idnever tagged the right person)The reports look like this now: