Browse Source

fix(notifications): Fix enum types for notifications (#56935)

Snigdha Sharma 1 year ago
parent
commit
ca6c7f2889

+ 0 - 2
pyproject.toml

@@ -551,12 +551,10 @@ module = [
     "sentry.notifications.notifications.activity.base",
     "sentry.notifications.notifications.activity.new_processing_issues",
     "sentry.notifications.notifications.activity.release",
-    "sentry.notifications.notifications.base",
     "sentry.notifications.notifications.integration_nudge",
     "sentry.notifications.notifications.rules",
     "sentry.notifications.utils",
     "sentry.notifications.utils.avatar",
-    "sentry.notifications.utils.participants",
     "sentry.onboarding_tasks.backends.organization_onboarding_task",
     "sentry.onboarding_tasks.base",
     "sentry.options.defaults",

+ 5 - 1
src/sentry/notifications/manager.py

@@ -434,7 +434,11 @@ class NotificationsManager(BaseManager["NotificationSetting"]):  # noqa: F821
 
         if should_use_notifications_v2(organization):  # type: ignore
             # We should replace calls to NotificationSettings.get_notification_recipients at the call site - this code should never be reached
-            setting_type = NotificationSettingEnum(NOTIFICATION_SETTING_TYPES[type])
+            setting_type = (
+                NotificationSettingEnum(NOTIFICATION_SETTING_TYPES[type])
+                if type
+                else NotificationSettingEnum.ISSUE_ALERTS
+            )
             controller = NotificationController(
                 recipients=recipient_actors,
                 project_ids=project_ids,

+ 17 - 5
src/sentry/notifications/notifications/base.py

@@ -12,7 +12,12 @@ from sentry.db.models import Model
 from sentry.models import Environment, NotificationSetting
 from sentry.notifications.helpers import should_use_notifications_v2
 from sentry.notifications.notificationcontroller import NotificationController
-from sentry.notifications.types import NotificationSettingTypes, get_notification_setting_type_name
+from sentry.notifications.types import (
+    NOTIFICATION_SETTING_TYPES,
+    NotificationSettingEnum,
+    NotificationSettingTypes,
+    get_notification_setting_type_name,
+)
 from sentry.notifications.utils.actions import MessageAction
 from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
 from sentry.types.integrations import EXTERNAL_PROVIDERS, ExternalProviders
@@ -169,7 +174,7 @@ class BaseNotification(abc.ABC):
                 self.record_analytics(
                     self.analytics_event,
                     self.analytics_instance,
-                    providers=provider.name.lower(),
+                    providers=provider.name.lower() if provider.name else "",
                     **self.get_custom_analytics_params(recipient),
                 )
 
@@ -227,18 +232,25 @@ class BaseNotification(abc.ABC):
     def filter_to_accepting_recipients(
         self, recipients: Iterable[RpcActor]
     ) -> Mapping[ExternalProviders, Iterable[RpcActor]]:
+        setting_type = (
+            NotificationSettingEnum(NOTIFICATION_SETTING_TYPES[self.notification_setting_type])
+            if self.notification_setting_type
+            else NotificationSettingEnum.ISSUE_ALERTS
+        )
         if should_use_notifications_v2(self.organization):
             controller = NotificationController(
                 recipients=recipients,
                 organization_id=self.organization.id,
-                type=self.notification_setting_type,
+                type=setting_type,
             )
-            return controller.get_notification_recipients(type=self.notification_setting_type)
+            return controller.get_notification_recipients(type=setting_type)
 
         accepting_recipients: Mapping[
             ExternalProviders, Iterable[RpcActor]
         ] = NotificationSetting.objects.filter_to_accepting_recipients(
-            self.organization, recipients, self.notification_setting_type
+            self.organization,
+            recipients,
+            self.notification_setting_type or NotificationSettingTypes.ISSUE_ALERTS,
         )
         return accepting_recipients
 

+ 36 - 19
src/sentry/notifications/utils/participants.py

@@ -49,6 +49,7 @@ from sentry.notifications.types import (
     GroupSubscriptionReason,
     NotificationSettingEnum,
     NotificationSettingOptionValues,
+    NotificationSettingsOptionEnum,
     NotificationSettingTypes,
 )
 from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
@@ -170,14 +171,23 @@ def get_participants_for_group(group: Group, user_id: int | None = None) -> Part
 
 
 def get_reason(
-    user: Union[User, RpcActor], value: NotificationSettingOptionValues, user_ids: set[int]
+    user: Union[User, RpcActor],
+    value: NotificationSettingOptionValues | NotificationSettingsOptionEnum,
+    user_ids: set[int],
 ) -> int | None:
     # Members who opt into all deploy emails.
-    if value == NotificationSettingOptionValues.ALWAYS:
+    if value in [NotificationSettingOptionValues.ALWAYS, NotificationSettingsOptionEnum.ALWAYS]:
         return GroupSubscriptionReason.deploy_setting
 
     # Members which have been seen in the commit log.
-    elif value == NotificationSettingOptionValues.COMMITTED_ONLY and user.id in user_ids:
+    elif (
+        value
+        in [
+            NotificationSettingOptionValues.COMMITTED_ONLY,
+            NotificationSettingsOptionEnum.COMMITTED_ONLY,
+        ]
+        and user.id in user_ids
+    ):
         return GroupSubscriptionReason.committed
     return None
 
@@ -215,10 +225,10 @@ def get_participants_for_release(
         users_to_reasons_by_provider = ParticipantMap()
         for actor in actors:
             settings = providers_by_recipient[actor.id]
-            for provider, value in settings.items():
-                reason_option = get_reason(actor, value, commited_user_ids)
-                if reason_option:
-                    users_to_reasons_by_provider.add(provider, actor, reason_option)
+            for provider, val in settings.items():
+                reason = get_reason(actor, val, commited_user_ids)
+                if reason:
+                    users_to_reasons_by_provider.add(provider, actor, reason)
         return users_to_reasons_by_provider
 
     # Get all the involved users' settings for deploy-emails (including
@@ -361,7 +371,7 @@ def determine_eligible_recipients(
     elif target_type == ActionTargetType.MEMBER:
         user = get_user_from_identifier(project, target_identifier)
         if user:
-            return [RpcActor.from_orm_user(user)]
+            return [RpcActor.from_object(user)]
 
     elif target_type == ActionTargetType.TEAM:
         team = get_team_from_identifier(project, target_identifier)
@@ -369,6 +379,9 @@ def determine_eligible_recipients(
             return [RpcActor.from_orm_team(team)]
 
     elif target_type == ActionTargetType.ISSUE_OWNERS:
+        if not event:
+            return []
+
         suggested_assignees, outcome = get_owners(project, event, fallthrough_choice)
 
         # We're adding the current assignee to the list of suggested assignees because
@@ -478,7 +491,9 @@ def get_fallthrough_recipients(
     raise NotImplementedError(f"Unknown fallthrough choice: {fallthrough_choice}")
 
 
-def get_user_from_identifier(project: Project, target_identifier: str | int | None) -> User | None:
+def get_user_from_identifier(
+    project: Project, target_identifier: str | int | None
+) -> RpcUser | None:
     if target_identifier is None:
         return None
 
@@ -542,8 +557,8 @@ def get_users_from_team_fall_back(
 
 
 def combine_recipients_by_provider(
-    teams_by_provider: Mapping[ExternalProviders, set[RpcActor]],
-    users_by_provider: Mapping[ExternalProviders, set[RpcActor]],
+    teams_by_provider: Mapping[ExternalProviders, Iterable[RpcActor]],
+    users_by_provider: Mapping[ExternalProviders, Iterable[RpcActor]],
 ) -> Mapping[ExternalProviders, set[RpcActor]]:
     """TODO(mgaeta): Make this more generic and move it to utils."""
     recipients_by_provider = defaultdict(set)
@@ -568,7 +583,8 @@ def get_recipients_by_provider(
 
     # First evaluate the teams.
     setting_type = NotificationSettingEnum(NOTIFICATION_SETTING_TYPES[notification_type])
-    teams_by_provider = None
+    controller = None
+    teams_by_provider: Mapping[ExternalProviders, Iterable[RpcActor]] = {}
     if should_use_notifications_v2(project.organization):
         controller = NotificationController(
             recipients=users,
@@ -595,14 +611,15 @@ def get_recipients_by_provider(
     users |= set(RpcActor.many_from_object(get_users_from_team_fall_back(teams, teams_by_provider)))
 
     # Repeat for users.
-    users_by_provider = None
+    users_by_provider: Mapping[ExternalProviders, Iterable[RpcActor]] = {}
     if should_use_notifications_v2(project.organization):
-        controller = NotificationController(
-            recipients=users,
-            organization_id=project.organization_id,
-            project_ids=[project.id],
-            type=setting_type,
-        )
+        if not controller:
+            controller = NotificationController(
+                recipients=users,
+                organization_id=project.organization_id,
+                project_ids=[project.id],
+                type=setting_type,
+            )
         users_by_provider = controller.get_notification_recipients(
             type=setting_type, actor_type=ActorType.USER
         )