Browse Source

ref(hybrid-cloud): Notifications helper for HC (#46304)

Use RpcActor so that the NotificationSettings object is more usable in
HC. I will remove the `user=None` and `team=None` and use *only*
`actor=` in the future after merging dependent getsentry part.
Zach Collins 1 year ago
parent
commit
fa4f188d49

+ 10 - 16
src/sentry/notifications/helpers.py

@@ -269,6 +269,7 @@ def get_scope_type(type: NotificationSettingTypes) -> NotificationScopeType:
 def get_scope(
 def get_scope(
     user: User | None = None,
     user: User | None = None,
     team: Team | None = None,
     team: Team | None = None,
+    actor: RpcActor | None = None,
     project: Project | None = None,
     project: Project | None = None,
     organization: Organization | None = None,
     organization: Organization | None = None,
 ) -> tuple[NotificationScopeType, int]:
 ) -> tuple[NotificationScopeType, int]:
@@ -276,32 +277,25 @@ def get_scope(
     Figure out the scope from parameters and return it as a tuple.
     Figure out the scope from parameters and return it as a tuple.
     TODO(mgaeta): Make sure the user/team is in the project/organization.
     TODO(mgaeta): Make sure the user/team is in the project/organization.
     """
     """
-
     if project:
     if project:
         return NotificationScopeType.PROJECT, project.id
         return NotificationScopeType.PROJECT, project.id
 
 
     if organization:
     if organization:
         return NotificationScopeType.ORGANIZATION, organization.id
         return NotificationScopeType.ORGANIZATION, organization.id
 
 
-    if team:
-        return NotificationScopeType.TEAM, team.id
-
-    if user:
-        return NotificationScopeType.USER, user.id
+    if user is not None:
+        actor = RpcActor.from_object(user)
+    if team is not None:
+        actor = RpcActor.from_object(team)
+    if actor:
+        if actor.actor_type == ActorType.TEAM:
+            return NotificationScopeType.TEAM, actor.id
+        else:
+            return NotificationScopeType.USER, actor.id
 
 
     raise Exception("scope must be either user, team, organization, or project")
     raise Exception("scope must be either user, team, organization, or project")
 
 
 
 
-def get_target_id(user: User | None = None, team: Team | None = None) -> int:
-    """:returns the actor ID from a User or Team."""
-    if user:
-        return int(user.actor_id)
-    if team:
-        return int(team.actor_id)
-
-    raise Exception("target must be either a user or a team")
-
-
 def get_subscription_from_attributes(
 def get_subscription_from_attributes(
     attrs: Mapping[str, Any]
     attrs: Mapping[str, Any]
 ) -> tuple[bool, Mapping[str, str | bool] | None]:
 ) -> tuple[bool, Mapping[str, str | bool] | None]:

+ 76 - 46
src/sentry/notifications/manager.py

@@ -12,7 +12,6 @@ from sentry.notifications.defaults import NOTIFICATION_SETTINGS_ALL_SOMETIMES
 from sentry.notifications.helpers import (
 from sentry.notifications.helpers import (
     get_scope,
     get_scope,
     get_scope_type,
     get_scope_type,
-    get_target_id,
     transform_to_notification_settings_by_recipient,
     transform_to_notification_settings_by_recipient,
     validate,
     validate,
     where_should_recipient_be_notified,
     where_should_recipient_be_notified,
@@ -46,6 +45,7 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         type: NotificationSettingTypes,
         type: NotificationSettingTypes,
         user: User | None = None,
         user: User | None = None,
         team: Team | None = None,
         team: Team | None = None,
+        actor: RpcActor | None = None,
         project: Project | None = None,
         project: Project | None = None,
         organization: Organization | None = None,
         organization: Organization | None = None,
     ) -> NotificationSettingOptionValues:
     ) -> NotificationSettingOptionValues:
@@ -57,7 +57,18 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         # The `unique_together` constraint should guarantee 0 or 1 rows, but
         # The `unique_together` constraint should guarantee 0 or 1 rows, but
         # using `list()` rather than `.first()` to prevent Django from adding an
         # using `list()` rather than `.first()` to prevent Django from adding an
         # ordering that could make the query slow.
         # ordering that could make the query slow.
-        settings = list(self.find_settings(provider, type, user, team, project, organization))[:1]
+        if actor is None:
+            if user is not None:
+                actor = RpcActor.from_object(user)
+            if team is not None:
+                actor = RpcActor.from_object(team)
+        assert actor
+
+        settings = list(
+            self.find_settings(
+                provider, type, actor=actor, project=project, organization=organization
+            )
+        )[:1]
         return (
         return (
             NotificationSettingOptionValues(settings[0].value)
             NotificationSettingOptionValues(settings[0].value)
             if settings
             if settings
@@ -98,8 +109,9 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         value: NotificationSettingOptionValues,
         value: NotificationSettingOptionValues,
         user: User | None = None,
         user: User | None = None,
         team: Team | None = None,
         team: Team | None = None,
-        project: Project | None = None,
-        organization: Organization | None = None,
+        actor: RpcActor | None = None,
+        project: Project | int | None = None,
+        organization: Organization | int | None = None,
     ) -> None:
     ) -> None:
         """
         """
         Save a target's notification preferences.
         Save a target's notification preferences.
@@ -108,10 +120,19 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
           * Updating a user's per-project preferences
           * Updating a user's per-project preferences
           * Updating a user's per-organization preferences
           * Updating a user's per-organization preferences
         """
         """
-        target_id = get_target_id(user, team)
+
+        if actor is None:
+            if user is not None:
+                actor = RpcActor.from_object(user)
+            if team is not None:
+                actor = RpcActor.from_object(team)
+        assert actor
+
+        target_id = actor.actor_id
+        assert target_id, "None actor_id cannot have settings updated"
         analytics.record(
         analytics.record(
             "notifications.settings_updated",
             "notifications.settings_updated",
-            target_type="user" if user else "team",
+            target_type="user" if actor.actor_type == ActorType.USER else "team",
             actor_id=target_id,
             actor_id=target_id,
         )
         )
 
 
@@ -120,8 +141,7 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
             return self.remove_settings(
             return self.remove_settings(
                 provider,
                 provider,
                 type,
                 type,
-                user=user,
-                team=team,
+                actor=actor,
                 project=project,
                 project=project,
                 organization=organization,
                 organization=organization,
             )
             )
@@ -129,8 +149,15 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         if not validate(type, value):
         if not validate(type, value):
             raise Exception(f"value '{value}' is not valid for type '{type}'")
             raise Exception(f"value '{value}' is not valid for type '{type}'")
 
 
-        scope_type, scope_identifier = get_scope(user, team, project, organization)
-        self._update_settings(provider, type, value, scope_type, scope_identifier, target_id)
+        scope_type, scope_identifier = get_scope(actor, project=project, organization=organization)
+        self._update_settings(
+            provider=provider,
+            type=type,
+            value=value,
+            scope_type=scope_type,
+            scope_identifier=scope_identifier,
+            target_id=target_id,
+        )
 
 
     def remove_settings(
     def remove_settings(
         self,
         self,
@@ -138,15 +165,24 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         type: NotificationSettingTypes,
         type: NotificationSettingTypes,
         user: User | None = None,
         user: User | None = None,
         team: Team | None = None,
         team: Team | None = None,
-        project: Project | None = None,
-        organization: Organization | None = None,
+        actor: RpcActor | None = None,
+        project: Project | int | None = None,
+        organization: Organization | int | None = None,
     ) -> None:
     ) -> None:
         """
         """
         We don't anticipate this function will be used by the API but is useful
         We don't anticipate this function will be used by the API but is useful
         for tests. This can also be called by `update_settings` when attempting
         for tests. This can also be called by `update_settings` when attempting
         to set a notification preference to DEFAULT.
         to set a notification preference to DEFAULT.
         """
         """
-        self.find_settings(provider, type, user, team, project, organization).delete()
+        if actor is None:
+            if user is not None:
+                actor = RpcActor.from_object(user)
+            if team is not None:
+                actor = RpcActor.from_object(team)
+        assert actor
+        self.find_settings(
+            provider, type, actor=actor, project=project, organization=organization
+        ).delete()
 
 
     def _filter(
     def _filter(
         self,
         self,
@@ -214,38 +250,23 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         type: NotificationSettingTypes,
         type: NotificationSettingTypes,
         user: User | None = None,
         user: User | None = None,
         team: Team | None = None,
         team: Team | None = None,
-        project: Project | None = None,
-        organization: Organization | None = None,
+        actor: RpcActor | None = None,
+        project: Project | int | None = None,
+        organization: Organization | int | None = None,
     ) -> QuerySet:
     ) -> QuerySet:
         """Wrapper for .filter that translates object parameters to scopes and targets."""
         """Wrapper for .filter that translates object parameters to scopes and targets."""
-        scope_type, scope_identifier = get_scope(user, team, project, organization)
-        target_id = get_target_id(user, team)
+        if actor is None:
+            if user is not None:
+                actor = RpcActor.from_object(user)
+            if team is not None:
+                actor = RpcActor.from_object(team)
+        assert actor
+
+        scope_type, scope_identifier = get_scope(actor, project=project, organization=organization)
+        target_id = actor.actor_id
+        assert target_id, "Cannot find settings for None actor_id"
         return self._filter(provider, type, scope_type, scope_identifier, [target_id])
         return self._filter(provider, type, scope_type, scope_identifier, [target_id])
 
 
-    def get_for_user_by_projects(
-        self,
-        type: NotificationSettingTypes,
-        user: User,
-        parents: list[Organization | Project],
-    ) -> QuerySet:
-        """
-        Find all of a user's notification settings for a list of projects or
-        organizations. This will include the user's parent-independent setting.
-        """
-        scope_type = get_scope_type(type)
-        return self.filter(
-            Q(
-                scope_type=scope_type.value,
-                scope_identifier__in=[parent.id for parent in parents],
-            )
-            | Q(
-                scope_type=NotificationScopeType.USER.value,
-                scope_identifier=user.id,
-            ),
-            type=type.value,
-            target_id=user.actor_id,
-        )
-
     def get_for_recipient_by_parent(
     def get_for_recipient_by_parent(
         self,
         self,
         type_: NotificationSettingTypes,
         type_: NotificationSettingTypes,
@@ -340,14 +361,23 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
     def update_settings_bulk(
     def update_settings_bulk(
         self,
         self,
         notification_settings: Sequence[NotificationSetting],
         notification_settings: Sequence[NotificationSetting],
-        user: User | None = None,
         team: Team | None = None,
         team: Team | None = None,
+        user: User | None = None,
+        actor: RpcActor | None = None,
     ) -> None:
     ) -> None:
         """
         """
         Given a list of _valid_ notification settings as tuples of column
         Given a list of _valid_ notification settings as tuples of column
         values, save them to the DB. This does not execute as a transaction.
         values, save them to the DB. This does not execute as a transaction.
         """
         """
-        target_id = get_target_id(user, team)
+        if actor is None:
+            if user is not None:
+                actor = RpcActor.from_object(user)
+            if team is not None:
+                actor = RpcActor.from_object(team)
+        assert actor
+
+        target_id = actor.actor_id
+        assert target_id, "Cannot update settings for None actor_id"
         for (provider, type, scope_type, scope_identifier, value) in notification_settings:
         for (provider, type, scope_type, scope_identifier, value) in notification_settings:
             # A missing DB row is equivalent to DEFAULT.
             # A missing DB row is equivalent to DEFAULT.
             if value == NotificationSettingOptionValues.DEFAULT:
             if value == NotificationSettingOptionValues.DEFAULT:
@@ -358,7 +388,7 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
                 )
                 )
         analytics.record(
         analytics.record(
             "notifications.settings_updated",
             "notifications.settings_updated",
-            target_type="user" if user else "team",
+            target_type="user" if actor.actor_type == ActorType.USER else "team",
             actor_id=target_id,
             actor_id=target_id,
         )
         )
 
 
@@ -422,7 +452,7 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
 
 
     def enable_settings_for_user(
     def enable_settings_for_user(
         self,
         self,
-        recipient: User,
+        recipient: User | RpcUser,
         provider: ExternalProviders,
         provider: ExternalProviders,
         types: set[NotificationSettingTypes] | None = None,
         types: set[NotificationSettingTypes] | None = None,
     ) -> None:
     ) -> None:
@@ -431,5 +461,5 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
                 provider=provider,
                 provider=provider,
                 type=type_,
                 type=type_,
                 value=NOTIFICATION_SETTINGS_ALL_SOMETIMES[type_],
                 value=NOTIFICATION_SETTINGS_ALL_SOMETIMES[type_],
-                user=recipient,
+                actor=RpcActor.from_object(recipient),
             )
             )

+ 6 - 2
src/sentry/notifications/utils/__init__.py

@@ -263,7 +263,9 @@ def has_integrations(organization: Organization, project: Project) -> bool:
     from sentry.plugins.base import plugins
     from sentry.plugins.base import plugins
 
 
     project_plugins = plugins.for_project(project, version=1)
     project_plugins = plugins.for_project(project, version=1)
-    organization_integrations = Integration.objects.filter(organizations=organization).first()
+    organization_integrations = Integration.objects.filter(
+        organizationintegration__organization_id=organization.id
+    ).first()
     # TODO: fix because project_plugins is an iterator and thus always truthy
     # TODO: fix because project_plugins is an iterator and thus always truthy
     return bool(project_plugins or organization_integrations)
     return bool(project_plugins or organization_integrations)
 
 
@@ -278,7 +280,9 @@ def has_alert_integration(project: Project) -> bool:
     # check integrations
     # check integrations
     providers = filter(is_alert_rule_integration, list(integrations.all()))
     providers = filter(is_alert_rule_integration, list(integrations.all()))
     provider_keys = map(lambda x: cast(str, x.key), providers)
     provider_keys = map(lambda x: cast(str, x.key), providers)
-    if Integration.objects.filter(organizations=org, provider__in=provider_keys).exists():
+    if Integration.objects.filter(
+        organizationintegration__organization_id=org.id, provider__in=provider_keys
+    ).exists():
         return True
         return True
 
 
     # check plugins
     # check plugins

+ 0 - 5
tests/sentry/notifications/test_utils.py

@@ -12,7 +12,6 @@ from sentry.notifications.helpers import (
     get_scope_type,
     get_scope_type,
     get_settings_by_provider,
     get_settings_by_provider,
     get_subscription_from_attributes,
     get_subscription_from_attributes,
-    get_target_id,
     get_values_by_provider_by_type,
     get_values_by_provider_by_type,
     validate,
     validate,
 )
 )
@@ -154,10 +153,6 @@ class NotificationHelpersTest(TestCase):
             == NotificationScopeType.ORGANIZATION
             == NotificationScopeType.ORGANIZATION
         )
         )
 
 
-    def test_get_target_id(self):
-        assert get_target_id(self.user) == self.user.actor_id
-        assert get_target_id(self.team) == self.team.actor_id
-
     def test_get_subscription_from_attributes(self):
     def test_get_subscription_from_attributes(self):
         attrs = {"subscription": (True, True, None)}
         attrs = {"subscription": (True, True, None)}
         assert get_subscription_from_attributes(attrs) == (True, {"disabled": True})
         assert get_subscription_from_attributes(attrs) == (True, {"disabled": True})