-
Notifications
You must be signed in to change notification settings - Fork 288
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
Add slack DM personal notification policy option #3993
Conversation
Not directly related to pr - maybe we want to completely move slack personal notifications to the DMs instead of thread? |
@Konstantinov-Innokentii You mean to replace the thread notification of users to DM ? That would reduce the team from having visibility of who is working on the OnCall ticket I believe. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider updating the documentation around Slack here
The team visibility should still be at the same level. The alert should arrive in the slack channel with a list of invited users correct? But this would just change the individual user notification from a mention that shows up in that users slack thread to a direct message from the Oncall Slack Bot, inviting them to look at the alert in the aforementioned slack channel. Please let me know if I'm not understanding the workflow. |
@BitKickerBHS Yep, that's what I meant. However, it's not something team is working on now, just my vision how it should work! |
Hi Team, Thanks if that behaviour is kind of like a next upgrade can we merge this PR since we don't have any open questions around this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on the documentation front. Will defer to the team for final approval :)
@Konstantinov-Innokentii wdyt? |
@joeyorlando I'm not sure if we need to introduce an additional escalation step and have two very simular one, which might affect UX. I prefer to rework the Notify in Slack step to send notifications in DMs instead, just like @BitKickerBHS described. AFAIK there were no product decisions to do that, but I'm advocating for that to reduce the amount of noise. |
@BitKickerBHS If the alert arrive in the slack channel with a list of invited users by default they will be notified in threads in slack. As part of this change we wanted to send a DM as well with the message correct ? @joeyorlando / @Konstantinov-Innokentii I will go ahead and make the changes with a single step. Let me know if I should defer for a different approach. |
@ravishankar15 imagined the DM message would be instead of the threads notification but if both are necessary that would be fine. |
df377da
to
6fd88ba
Compare
6fd88ba
to
1681bae
Compare
Fix doc Fix slack test back Fix diff
1681bae
to
afd90b2
Compare
@joeyorlando / @Konstantinov-Innokentii I have made the changes. We are already sending a DM notification when the user is not in the alert channel I added the implementation as an extension to the same. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
bumping for visibility. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Hello @ravishankar15 just checking to see if this change is still possible. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
What this PR does
Which issue(s) this PR fixes
Closes #3874
Checklist
pr:no public docs
PR label added if not required)CHANGELOG.md
updated (orpr:no changelog
PR label added if not required)