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

Add slack DM personal notification policy option #3993

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

ravishankar15
Copy link
Contributor

@ravishankar15 ravishankar15 commented Mar 2, 2024

What this PR does

Which issue(s) this PR fixes

Closes #3874

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@ravishankar15 ravishankar15 marked this pull request as ready for review March 2, 2024 13:19
@ravishankar15 ravishankar15 requested a review from a team as a code owner March 2, 2024 13:19
@ravishankar15 ravishankar15 changed the title [DRAFT][MINOR] Add slack dm notification for user [MINOR] Add slack dm notification for user Mar 2, 2024
@Konstantinov-Innokentii
Copy link
Member

Not directly related to pr - maybe we want to completely move slack personal notifications to the DMs instead of thread?

@ravishankar15
Copy link
Contributor Author

@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.

Copy link
Contributor

@joeyorlando joeyorlando left a 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

@joeyorlando joeyorlando changed the title [MINOR] Add slack dm notification for user Add slack DM notification for user Mar 6, 2024
@joeyorlando joeyorlando changed the title Add slack DM notification for user Add slack DM notification personal notification policy option Mar 6, 2024
@joeyorlando joeyorlando added the release:enhancement PR will be added to "Exciting New Features 🎉" section of release notes label Mar 6, 2024
@joeyorlando joeyorlando changed the title Add slack DM notification personal notification policy option Add slack DM personal notification policy option Mar 6, 2024
@BitKickerBHS
Copy link

@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.

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.
-Thanks

@ravishankar15 ravishankar15 requested review from a team as code owners March 13, 2024 15:57
@Konstantinov-Innokentii
Copy link
Member

@BitKickerBHS Yep, that's what I meant. However, it's not something team is working on now, just my vision how it should work!

@ravishankar15
Copy link
Contributor Author

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 ?

Copy link
Contributor

@alyssawada alyssawada left a 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 :)

@joeyorlando
Copy link
Contributor

@Konstantinov-Innokentii wdyt?

@Konstantinov-Innokentii
Copy link
Member

Konstantinov-Innokentii commented May 15, 2024

@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.

@ravishankar15
Copy link
Contributor Author

@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.

@BitKickerBHS
Copy link

BitKickerBHS commented May 21, 2024

@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.

Fix doc

Fix slack test back

Fix diff
@ravishankar15
Copy link
Contributor Author

@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.

Copy link
Contributor

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!

@github-actions github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Jun 25, 2024
@BitKickerBHS
Copy link

@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.

bumping for visibility.

@github-actions github-actions bot removed the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Jul 19, 2024
Copy link
Contributor

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!

@github-actions github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Aug 18, 2024
@BitKickerBHS
Copy link

Hello @ravishankar15 just checking to see if this change is still possible.
-Thanks

@github-actions github-actions bot removed the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Sep 5, 2024
Copy link
Contributor

github-actions bot commented Oct 5, 2024

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!

@github-actions github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action release:enhancement PR will be added to "Exciting New Features 🎉" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the ability to choose between Slack mention or Slack direct message for user notification / ping
6 participants