Skip to content

fix(notifications): Fix parameters and add tests for participants and subscriptions #57053

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

Merged
merged 10 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/api/serializers/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ def measure_span(op_tag):

if use_notifications_v2:
subscriptions = notifications_service.get_subscriptions_for_projects(
user=user.id,
user_id=user.id,
project_ids=project_ids,
type=NotificationSettingTypes.ISSUE_ALERTS,
)
Expand Down
4 changes: 3 additions & 1 deletion src/sentry/models/groupsubscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from sentry.services.hybrid_cloud.actor import RpcActor
from sentry.services.hybrid_cloud.notifications import notifications_service
from sentry.services.hybrid_cloud.user import RpcUser
from sentry.types.integrations import ExternalProviders

if TYPE_CHECKING:
from sentry.models import Group, Team, User
Expand Down Expand Up @@ -191,12 +192,13 @@ def get_participants(self, group: Group) -> ParticipantMap:
if user.id not in providers_by_recipient:
continue

for provider in providers_by_recipient[user.id]:
for provider_str in providers_by_recipient[user.id]:
reason = (
subscription_option
and subscription_option.reason
or GroupSubscriptionReason.implicit
)
provider = ExternalProviders(provider_str)
result.add(provider, user, reason)
return result

Expand Down
36 changes: 36 additions & 0 deletions src/sentry/notifications/helpers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
from collections import defaultdict
from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Optional

Expand Down Expand Up @@ -49,6 +50,8 @@
User,
)

logger = logging.getLogger(__name__)


def _get_notification_setting_default(
provider: ExternalProviders,
Expand Down Expand Up @@ -89,6 +92,39 @@ def get_provider_defaults() -> list[ExternalProviderEnum]:
return provider_defaults


def get_default_for_provider(
type: NotificationSettingEnum,
provider: ExternalProviderEnum,
) -> NotificationSettingsOptionEnum:
defaults = PROVIDER_DEFAULTS
if provider not in defaults:
return NotificationSettingsOptionEnum.NEVER

# Defaults are defined for the old int enum
_type = [key for key, val in NOTIFICATION_SETTING_TYPES.items() if val == type.value]
if len(_type) != 1 or _type[0] not in NOTIFICATION_SETTINGS_ALL_SOMETIMES_V2:
logger.warning(
"Could not find default for notification type",
extras={"type": type, "_type": _type, "provider": provider},
)
return NotificationSettingsOptionEnum.NEVER

try:
default_value = NOTIFICATION_SETTINGS_ALL_SOMETIMES_V2[_type[0]]
default_enum = NotificationSettingsOptionEnum(
NOTIFICATION_SETTING_OPTION_VALUES[default_value]
)
except KeyError:
# If we don't have a default value for the type, then it's never
return NotificationSettingsOptionEnum.NEVER

if type == NotificationSettingEnum.REPORTS and provider != ExternalProviderEnum.EMAIL:
# Reports are only sent to email
return NotificationSettingsOptionEnum.NEVER

return default_enum or NotificationSettingsOptionEnum.NEVER


def get_type_defaults() -> Mapping[NotificationSettingEnum, NotificationSettingsOptionEnum]:
# this tells us what the default value is for each notification type
type_defaults = {}
Expand Down
22 changes: 12 additions & 10 deletions src/sentry/notifications/notificationcontroller.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from sentry.models.notificationsettingprovider import NotificationSettingProvider
from sentry.models.team import Team
from sentry.notifications.helpers import (
get_provider_defaults,
get_default_for_provider,
get_type_defaults,
recipient_is_team,
recipient_is_user,
Expand Down Expand Up @@ -250,7 +250,6 @@ def _get_layered_setting_providers(
)
scoped_settings = [project_settings, org_settings, user_settings]

defaults = get_provider_defaults()
layered_setting_providers: MutableMapping[
Recipient,
MutableMapping[
Expand All @@ -266,13 +265,18 @@ def _get_layered_setting_providers(
for item in scope_items:
scope = (scope_type, item)
for type in NotificationSettingEnum:
if type == NotificationSettingEnum.DEFAULT:
continue
for provider in ExternalProviderEnum:
most_specific_setting = None
for setting in scoped_settings:
user_or_team_settings = []
if recipient_is_user(recipient):
user_or_team_settings = self._filter_providers(
settings=setting, user_id=recipient.id, type=type.value
settings=setting,
user_id=recipient.id,
type=type.value,
provider=provider.value,
)
elif recipient_is_team(recipient):
user_or_team_settings = self._filter_providers(
Expand All @@ -286,11 +290,7 @@ def _get_layered_setting_providers(
break

if most_specific_setting is None:
most_specific_setting = (
NotificationSettingsOptionEnum.ALWAYS
if provider in defaults
else NotificationSettingsOptionEnum.NEVER
)
most_specific_setting = get_default_for_provider(type, provider)

layered_setting_providers[recipient][scope][type].update(
{provider: most_specific_setting}
Expand Down Expand Up @@ -476,11 +476,13 @@ def get_participants(

actor = RpcActor.from_object(recipient)
for type_map in setting.values():
for provider_map in type_map.values():
for type, provider_map in type_map.items():
if type != self.type:
continue

user_to_providers[actor] = {
EXTERNAL_PROVIDERS_REVERSE[provider]: value
for provider, value in provider_map.items()
if value != NotificationSettingsOptionEnum.NEVER
}

return user_to_providers
Expand Down
9 changes: 6 additions & 3 deletions src/sentry/notifications/utils/participants.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,17 @@ def get_participants_for_release(
if should_use_notifications_v2(organization):
providers_by_recipient = notifications_service.get_participants(
recipients=actors,
project_ids=[project.id for project in projects],
organization_id=organization.id,
type=NotificationSettingTypes.DEPLOY,
type=NotificationSettingEnum.DEPLOY,
)

users_to_reasons_by_provider = ParticipantMap()
for actor in actors:
settings = providers_by_recipient[actor.id]
for provider, val in settings.items():
settings = providers_by_recipient.get(actor.id, {})
for provider_str, val_str in settings.items():
provider = ExternalProviders(provider_str)
val = NotificationSettingsOptionEnum(val_str)
reason = get_reason(actor, val, commited_user_ids)
if reason:
users_to_reasons_by_provider.add(provider, actor, reason)
Expand Down
10 changes: 7 additions & 3 deletions src/sentry/services/hybrid_cloud/notifications/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
NotificationScopeType,
NotificationSettingEnum,
NotificationSettingOptionValues,
NotificationSettingsOptionEnum,
NotificationSettingTypes,
)
from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
Expand Down Expand Up @@ -245,15 +244,20 @@ def get_participants(
project_ids: Optional[List[int]],
organization_id: Optional[int],
type: NotificationSettingEnum,
) -> MutableMapping[int, MutableMapping[ExternalProviders, NotificationSettingsOptionEnum]]:
) -> MutableMapping[
int, MutableMapping[int, str]
]: # { actor_id : { provider_str: value_str } }
controller = NotificationController(
recipients=recipients,
project_ids=project_ids,
organization_id=organization_id,
type=type,
)
participants = controller.get_participants()
return {actor.id: providers for actor, providers in participants.items()}
return {
actor.id: {provider.value: value.value for provider, value in providers.items()}
for actor, providers in participants.items()
}

class _NotificationSettingsQuery(
FilterQueryDatabaseImpl[
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/services/hybrid_cloud/notifications/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from sentry.notifications.types import (
NotificationSettingEnum,
NotificationSettingOptionValues,
NotificationSettingsOptionEnum,
NotificationSettingTypes,
)
from sentry.services.hybrid_cloud.actor import RpcActor
Expand Down Expand Up @@ -155,7 +154,7 @@ def get_participants(
project_ids: Optional[List[int]],
organization_id: Optional[int],
type: NotificationSettingEnum,
) -> MutableMapping[int, MutableMapping[ExternalProviders, NotificationSettingsOptionEnum]]:
) -> MutableMapping[int, MutableMapping[int, str]]:
pass


Expand Down
Loading