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

notification/slack: update existing messages with status #2004

Merged
merged 16 commits into from
Nov 3, 2021

Conversation

mastercactapus
Copy link
Member

Description:
This PR updates the Slack notifier to include a color bar with alert messages and update the existing message instead of posting status updates in-thread.

Unacked alerts will have a red bar and details included:
unacked

Acked alerts will reflect the latest log entry, a yellow bar, and retain details:
acked

When escalated, the alert summary and link is broadcast to the channel:
escalated

When closed, details of the alert is hidden, bar turns green, and the link remains with the last log entry:
closed

Forfold
Forfold previously approved these changes Nov 1, 2021
Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

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

As a personal preference, I think it could look a bit cleaner without the nested quote line for the details- LGTM!

@mastercactapus
Copy link
Member Author

@Forfold Maybe we can try tweaking it during the two-way interaction work; we'll be changing the rendering logic for that into its final state. I added it so it would stand out since there's no pre-processing for markdown -> slack markdown, but I agree the double color-bar isn't ideal.

@Forfold
Copy link
Contributor

Forfold commented Nov 2, 2021

@mastercactapus That sounds good! Yeah I definitely don't think something like that should hold us back, so it has my approval :)

@Forfold
Copy link
Contributor

Forfold commented Nov 2, 2021

It could be neat to have a button to show the full details (rendered with mkdwn) as a modal within Slack

Copy link
Contributor

@dctalbot dctalbot left a comment

Choose a reason for hiding this comment

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

Code looks good, but seeing some odd behavior...

I created an alert with a summary of "test", and got the following -

Native notification on mac is missing preview:
Screen Shot 2021-11-02 at 2 03 46 PM

There is a stray greater than symbol in the message body:
Screen Shot 2021-11-02 at 2 06 20 PM

It only goes away if I close the alert.

@mastercactapus mastercactapus merged commit d923d5e into master Nov 3, 2021
@mastercactapus mastercactapus deleted the slack-message-updates branch November 3, 2021 15:55
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.

3 participants