search

getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
37.08k stars 3.98k forks source link

fix(slack): Ensure notifications for same issue are seen #70108

Closed ykamo001 closed 2 weeks ago

ykamo001 commented 2 weeks ago

When the issue alert flow gets triggered for the same issue, we should ensure the notification gets seen in the main channel as well. Refactored some code a little to clean it up.

Resolves: https://github.com/getsentry/sentry/issues/70135#event-12684736214

ykamo001 commented 2 weeks ago

do we have a test for this 😅

otherwise lgtm

Unfortunately we mock all slack api calls, so it won't test anything. In general though, it does not look like this class or method has any tests at all 😅

We have used this param for slack in another flow though, so we can be confident it does work

ykamo001 commented 2 weeks ago

do we have a test for this 😅 otherwise lgtm

Unfortunately we mock all slack api calls, so it won't test anything. In general though, it does not look like this class or method has any tests at all 😅

We have used this param for slack in another flow though, so we can be confident it does work

for reference: https://api.slack.com/methods/chat.postMessage#arg_reply_broadcast

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 79.82%. Comparing base (df71fec) to head (101e473). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #70108 +/- ## ========================================== - Coverage 79.82% 79.82% -0.01% ========================================== Files 6504 6504 Lines 289377 289378 +1 Branches 49833 49833 ========================================== - Hits 230989 230982 -7 - Misses 57977 57985 +8 Partials 411 411 ``` | [Files](https://app.codecov.io/gh/getsentry/sentry/pull/70108?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) | Coverage Δ | | |---|---|---| | [.../sentry/integrations/slack/actions/notification.py](https://app.codecov.io/gh/getsentry/sentry/pull/70108?src=pr&el=tree&filepath=src%2Fsentry%2Fintegrations%2Fslack%2Factions%2Fnotification.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry#diff-c3JjL3NlbnRyeS9pbnRlZ3JhdGlvbnMvc2xhY2svYWN0aW9ucy9ub3RpZmljYXRpb24ucHk=) | `80.32% <66.66%> (-0.67%)` | :arrow_down: | ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/getsentry/sentry/pull/70108/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry)