Skip to content

Commit 081b361

Browse files
authored
feat(notification platform): Handle multiple providers (#25018)
* feat(notification platform): Handle multiple providers
1 parent cb00546 commit 081b361

File tree

12 files changed

+248
-279
lines changed

12 files changed

+248
-279
lines changed

src/sentry/incidents/action_handlers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def get_targets(self):
6969
ExternalProviders.EMAIL,
7070
self.project,
7171
{member.user for member in target.member_set},
72-
)
72+
)[ExternalProviders.EMAIL]
7373
targets = [(user.id, user.email) for user in users]
7474
# TODO: We need some sort of verification system to make sure we're not being
7575
# used as an email relay.

src/sentry/mail/activity/base.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
UserAvatar,
1212
UserOption,
1313
)
14+
from sentry.models.integration import ExternalProviders
1415
from sentry.utils import json
1516
from sentry.utils.assets import get_asset_url
1617
from sentry.utils.avatar import get_email_avatar
@@ -40,7 +41,9 @@ def get_participants(self):
4041
if not self.group:
4142
return []
4243

43-
participants = GroupSubscription.objects.get_participants(group=self.group)
44+
participants = GroupSubscription.objects.get_participants(group=self.group)[
45+
ExternalProviders.EMAIL
46+
]
4447

4548
if self.activity.user is not None and self.activity.user in participants:
4649
receive_own_activity = (

src/sentry/mail/activity/new_processing_issues.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def __init__(self, activity):
2929
def get_participants(self):
3030
users = NotificationSetting.objects.get_notification_recipients(
3131
ExternalProviders.EMAIL, self.project
32-
)
32+
)[ExternalProviders.EMAIL]
3333
return {user: GroupSubscriptionReason.processing_issue for user in users}
3434

3535
def get_context(self):

src/sentry/mail/activity/release.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
User,
2121
UserEmail,
2222
)
23-
from sentry.models.integration import ExternalProviders
2423
from sentry.notifications.types import (
2524
NotificationScopeType,
2625
NotificationSettingOptionValues,
@@ -129,7 +128,6 @@ def get_participants(self):
129128
# get all the involved users' settings for deploy-emails (user default
130129
# saved without org set)
131130
notification_settings = NotificationSetting.objects.get_for_users_by_parent(
132-
ExternalProviders.EMAIL,
133131
NotificationSettingTypes.DEPLOY,
134132
users=users,
135133
parent=self.organization,

src/sentry/mail/adapter.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ def get_sendable_user_objects(project):
158158
"""
159159
return NotificationSetting.objects.get_notification_recipients(
160160
ExternalProviders.EMAIL, project
161-
)
161+
)[ExternalProviders.EMAIL]
162162

163163
def get_sendable_user_ids(self, project):
164164
users = self.get_sendable_user_objects(project)
@@ -231,7 +231,6 @@ def get_send_to_owners(self, event, project):
231231
sentry_orgmember_set__organizationmemberteam__team__id__in=teams_to_resolve,
232232
).values_list("id", flat=True)
233233
)
234-
235234
return send_to - self.disabled_users_from_project(project)
236235
else:
237236
metrics.incr(
@@ -247,7 +246,6 @@ def disabled_users_from_project(project: Project) -> Set[int]:
247246
user_ids = project.member_set.values_list("user", flat=True)
248247
users = User.objects.filter(id__in=user_ids)
249248
notification_settings = NotificationSetting.objects.get_for_users_by_parent(
250-
provider=ExternalProviders.EMAIL,
251249
type=NotificationSettingTypes.ISSUE_ALERTS,
252250
parent=project,
253251
users=users,
@@ -263,7 +261,11 @@ def disabled_users_from_project(project: Project) -> Set[int]:
263261
if settings:
264262
# Check per-project settings first, fallback to project-independent settings.
265263
project_setting = settings.get(NotificationScopeType.PROJECT)
264+
if project_setting:
265+
project_setting = project_setting[ExternalProviders.EMAIL]
266266
user_setting = settings.get(NotificationScopeType.USER)
267+
if user_setting:
268+
user_setting = user_setting[ExternalProviders.EMAIL]
267269
if project_setting == NotificationSettingOptionValues.NEVER or (
268270
not project_setting and user_setting == NotificationSettingOptionValues.NEVER
269271
):
@@ -526,6 +528,8 @@ def handle_user_report(self, payload, project, **kwargs):
526528
group = Group.objects.get(id=payload["report"]["issue"]["id"])
527529

528530
participants = GroupSubscription.objects.get_participants(group=group)
531+
if participants:
532+
participants = participants[ExternalProviders.EMAIL]
529533

530534
if not participants:
531535
return

src/sentry/models/groupsubscription.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from typing import Any, Mapping
22

3+
from collections import defaultdict
34
from django.conf import settings
45
from django.db import IntegrityError, models, transaction
56
from django.utils import timezone
@@ -11,9 +12,8 @@
1112
Model,
1213
sane_repr,
1314
)
14-
from sentry.models.integration import ExternalProviders
1515
from sentry.notifications.helpers import (
16-
should_be_participating,
16+
where_should_be_participating,
1717
transform_to_notification_settings_by_user,
1818
)
1919
from sentry.notifications.types import NotificationSettingTypes
@@ -122,31 +122,33 @@ def get_participants(self, group) -> Mapping[Any, GroupSubscriptionReason]:
122122
user_ids = [user.id for user in users]
123123
subscriptions = self.filter(group=group, user_id__in=user_ids)
124124
notification_settings = NotificationSetting.objects.get_for_users_by_parent(
125-
ExternalProviders.EMAIL,
126125
NotificationSettingTypes.WORKFLOW,
127126
users=users,
128127
parent=group.project,
129128
)
130-
131129
subscriptions_by_user_id = {
132130
subscription.user_id: subscription for subscription in subscriptions
133131
}
134132
notification_settings_by_user = transform_to_notification_settings_by_user(
135133
notification_settings, users
136134
)
137-
return {
138-
user: getattr(
139-
subscriptions_by_user_id.get(user.id),
140-
"reason",
141-
GroupSubscriptionReason.implicit,
142-
)
143-
for user in users
144-
if should_be_participating(
135+
result = defaultdict(dict)
136+
137+
for user in users:
138+
providers = where_should_be_participating(
145139
user,
146140
subscriptions_by_user_id,
147141
notification_settings_by_user,
148142
)
149-
}
143+
for provider in providers:
144+
value = getattr(
145+
subscriptions_by_user_id.get(user.id),
146+
"reason",
147+
GroupSubscriptionReason.implicit,
148+
)
149+
result[provider][user] = value
150+
151+
return result
150152

151153

152154
class GroupSubscription(Model):

src/sentry/models/notificationsetting.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ class Meta:
101101
)
102102

103103
__repr__ = sane_repr(
104-
"scope_type",
104+
"scope_str",
105105
"scope_identifier",
106106
"target",
107-
"provider",
108-
"type",
109-
"value",
107+
"provider_str",
108+
"type_str",
109+
"value_str",
110110
)
111111

112112

src/sentry/notifications/helpers.py

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from collections import defaultdict
2-
from typing import Any, Dict, Iterable, Mapping, Optional, Tuple
2+
from typing import Any, Dict, Iterable, List, Mapping, Optional, Tuple
33

4+
from sentry.models.integration import ExternalProviders
45
from sentry.notifications.legacy_mappings import get_legacy_value
56
from sentry.notifications.types import (
67
NotificationScopeType,
@@ -9,94 +10,102 @@
910
)
1011

1112

12-
def _get_setting_value_from_mapping(
13+
def _get_setting_mapping_from_mapping(
1314
notification_settings_by_user: Mapping[
14-
Any, Mapping[NotificationScopeType, NotificationSettingOptionValues]
15+
Any,
16+
Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
1517
],
1618
user: Any,
1719
type: NotificationSettingTypes,
18-
default: NotificationSettingOptionValues,
19-
) -> NotificationSettingOptionValues:
20+
) -> Mapping[ExternalProviders, NotificationSettingOptionValues]:
21+
# XXX(CEO): may not respect granularity of a setting for Slack a setting for email
22+
# but we'll worry about that later since we don't have a FE for it yet
2023
specific_scope = get_scope_type(type)
21-
notification_settings_option = notification_settings_by_user.get(user)
22-
if notification_settings_option:
23-
notification_setting_option = notification_settings_option.get(
24+
notification_settings_mapping = notification_settings_by_user.get(user)
25+
if notification_settings_mapping:
26+
notification_setting_option = notification_settings_mapping.get(
2427
specific_scope
25-
) or notification_settings_option.get(NotificationScopeType.USER)
28+
) or notification_settings_mapping.get(NotificationScopeType.USER)
2629
if notification_setting_option:
2730
return notification_setting_option
28-
return default
31+
return {ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS}
2932

3033

31-
def should_user_be_notified(
34+
def where_should_user_be_notified(
3235
notification_settings_by_user: Mapping[
33-
Any, Mapping[NotificationScopeType, NotificationSettingOptionValues]
36+
Any,
37+
Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
3438
],
3539
user: Any,
36-
) -> bool:
40+
) -> List[ExternalProviders]:
3741
"""
3842
Given a mapping of default and specific notification settings by user,
39-
determine if a user should receive an ISSUE_ALERT notification.
43+
return the list of providers after verifying the user has opted into this notification.
4044
"""
41-
return (
42-
_get_setting_value_from_mapping(
43-
notification_settings_by_user,
44-
user,
45-
NotificationSettingTypes.ISSUE_ALERTS,
46-
NotificationSettingOptionValues.ALWAYS,
47-
)
48-
== NotificationSettingOptionValues.ALWAYS
45+
mapping = _get_setting_mapping_from_mapping(
46+
notification_settings_by_user,
47+
user,
48+
NotificationSettingTypes.ISSUE_ALERTS,
49+
)
50+
return list(
51+
filter(lambda elem: mapping[elem] == NotificationSettingOptionValues.ALWAYS, mapping)
4952
)
5053

5154

52-
def should_be_participating(
55+
def where_should_be_participating(
5356
user: Any,
5457
subscriptions_by_user_id: Mapping[int, Any],
5558
notification_settings_by_user: Mapping[
56-
Any, Mapping[NotificationScopeType, NotificationSettingOptionValues]
59+
Any,
60+
Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
5761
],
58-
) -> bool:
62+
) -> List[ExternalProviders]:
5963
"""
6064
Given a mapping of users to subscriptions and a mapping of default and
61-
specific notification settings by user, determine if a user should receive
65+
specific notification settings by user, determine where a user should receive
6266
a WORKFLOW notification. Unfortunately, this algorithm does not respect
6367
NotificationSettingOptionValues.ALWAYS. If the user is unsubscribed from
6468
the group, that overrides their notification preferences.
6569
"""
66-
value = _get_setting_value_from_mapping(
70+
mapping = _get_setting_mapping_from_mapping(
6771
notification_settings_by_user,
6872
user,
6973
NotificationSettingTypes.WORKFLOW,
70-
NotificationSettingOptionValues.SUBSCRIBE_ONLY,
7174
)
72-
73-
if value == NotificationSettingOptionValues.NEVER:
74-
return False
75-
76-
subscription = subscriptions_by_user_id.get(user.id)
77-
if subscription:
78-
return bool(subscription.is_active)
79-
80-
return value == NotificationSettingOptionValues.ALWAYS
75+
output = []
76+
for provider, value in mapping.items():
77+
subscription = subscriptions_by_user_id.get(user.id)
78+
if (subscription and not subscription.is_active) or (
79+
value == NotificationSettingOptionValues.NEVER
80+
):
81+
continue
82+
if (subscription and subscription.is_active) or (
83+
value == NotificationSettingOptionValues.ALWAYS
84+
):
85+
output.append(provider)
86+
return output
8187

8288

8389
def transform_to_notification_settings_by_user(
8490
notification_settings: Iterable[Any],
8591
users: Iterable[Any],
86-
) -> Mapping[Any, Mapping[NotificationScopeType, NotificationSettingOptionValues]]:
92+
) -> Mapping[
93+
Any, Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]]
94+
]:
8795
"""
8896
Given a unorganized list of notification settings, create a mapping of
8997
users to a map of notification scopes to setting values.
9098
"""
9199
actor_mapping = {user.actor_id: user for user in users}
92100
notification_settings_by_user: Dict[
93-
Any, Dict[NotificationScopeType, NotificationSettingOptionValues]
94-
] = defaultdict(dict)
101+
Any, Dict[NotificationScopeType, Dict[ExternalProviders, NotificationSettingOptionValues]]
102+
] = defaultdict(lambda: defaultdict(dict))
95103
for notification_setting in notification_settings:
96104
user = actor_mapping.get(notification_setting.target_id)
97-
notification_settings_by_user[user][
98-
NotificationScopeType(notification_setting.scope_type)
99-
] = NotificationSettingOptionValues(notification_setting.value)
105+
scope_type = NotificationScopeType(notification_setting.scope_type)
106+
value = NotificationSettingOptionValues(notification_setting.value)
107+
provider = ExternalProviders(notification_setting.provider)
108+
notification_settings_by_user[user][scope_type][provider] = value
100109
return notification_settings_by_user
101110

102111

0 commit comments

Comments
 (0)