typing

getsentry · Oct 9, 2024 · 301e41a · 301e41a
1 parent 484ae19
commit 301e41a
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 10 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -469,7 +469,6 @@ module = [
     "sentry.issues.update_inbox",
     "sentry.lang.java.processing",
     "sentry.llm.*",
-    "sentry.mediators.sentry_app_installations.installation_notifier",
     "sentry.migrations.*",
     "sentry.models.event",
     "sentry.models.eventattachment",

diff --git a/src/sentry/sentry_apps/api/endpoints/installation_details.py b/src/sentry/sentry_apps/api/endpoints/installation_details.py
@@ -19,6 +19,8 @@
     SentryAppInstallationUpdater,
 )
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
+from sentry.users.models.user import User
+from sentry.users.services.user.model import RpcUser
 from sentry.utils.audit import create_audit_entry
 
 
@@ -41,28 +43,35 @@ def get(self, request: Request, installation) -> Response:
         )
 
     def delete(self, request: Request, installation) -> Response:
-        installation: SentryAppInstallation = SentryAppInstallation.objects.get(id=installation.id)
+        sentry_app_installation: SentryAppInstallation = SentryAppInstallation.objects.get(
+            id=installation.id
+        )
         with transaction.atomic(using=router.db_for_write(SentryAppInstallation)):
             try:
+                assert isinstance(
+                    request.user, (User, RpcUser)
+                ), "User must be a user or rpcuser to delete installation"
                 SentryAppInstallationNotifier(
-                    sentry_app_installation=installation, user=request.user, action="deleted"
+                    sentry_app_installation=sentry_app_installation,
+                    user=request.user,
+                    action="deleted",
                 ).run()
             # if the error is from a request exception, log the error and continue
             except RequestException as exc:
                 sentry_sdk.capture_exception(exc)
-            deletions.exec_sync(installation)
+            deletions.exec_sync(sentry_app_installation)
             create_audit_entry(
                 request=request,
-                organization_id=installation.organization_id,
-                target_object=installation.organization_id,
+                organization_id=sentry_app_installation.organization_id,
+                target_object=sentry_app_installation.organization_id,
                 event=audit_log.get_event_id("SENTRY_APP_UNINSTALL"),
-                data={"sentry_app": installation.sentry_app.name},
+                data={"sentry_app": sentry_app_installation.sentry_app.name},
             )
         analytics.record(
             "sentry_app.uninstalled",
             user_id=request.user.id,
-            organization_id=installation.organization_id,
-            sentry_app=installation.sentry_app.slug,
+            organization_id=sentry_app_installation.organization_id,
+            sentry_app=sentry_app_installation.sentry_app.slug,
         )
         return Response(status=204)
 

diff --git a/src/sentry/sentry_apps/api/endpoints/sentry_app_details.py b/src/sentry/sentry_apps/api/endpoints/sentry_app_details.py
@@ -162,6 +162,9 @@ def delete(self, request: Request, sentry_app) -> Response:
                 for install in sentry_app.installations.all():
                     try:
                         with transaction.atomic(using=router.db_for_write(SentryAppInstallation)):
+                            assert isinstance(
+                                request.user, (User, RpcUser)
+                            ), "User must be a user or rpcuser to delete installation"
                             SentryAppInstallationNotifier(
                                 sentry_app_installation=install, user=request.user, action="deleted"
                             ).run()

diff --git a/src/sentry/sentry_apps/installations.py b/src/sentry/sentry_apps/installations.py
@@ -188,7 +188,7 @@ def sentry_app(self) -> SentryApp:
 @dataclasses.dataclass
 class SentryAppInstallationNotifier:
     sentry_app_installation: SentryAppInstallation
-    user: RpcUser
+    user: User | RpcUser
     action: str
 
     def run(self) -> None:

diff --git a/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py b/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py
@@ -107,6 +107,8 @@ def test_delete_install(self, record):
         responses.add(url="https://example.com/webhook", method=responses.POST, body=b"")
         self.login_as(user=self.user)
         rpc_user = user_service.get_user(user_id=self.user.id)
+        assert rpc_user, "User should exist in test to delete sentry app installation unless noted"
+
         response = self.client.delete(self.url, format="json")
         assert AuditLogEntry.objects.filter(
             event=audit_log.get_event_id("SENTRY_APP_UNINSTALL")

diff --git a/tests/sentry/sentry_apps/test_sentry_app_installation_notifier.py b/tests/sentry/sentry_apps/test_sentry_app_installation_notifier.py
@@ -44,6 +44,7 @@ def setUp(self):
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_task_enqueued(self, safe_urlopen):
+        assert self.rpc_user, "Rpcuser should exist, unless explicitly noted in test"
         SentryAppInstallationNotifier(
             sentry_app_installation=self.install, user=self.rpc_user, action="created"
         ).run()
@@ -75,6 +76,8 @@ def test_task_enqueued(self, safe_urlopen):
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_uninstallation_enqueued(self, safe_urlopen):
+        assert self.rpc_user, "Rpcuser should exist, unless explicitly noted in test"
+
         SentryAppInstallationNotifier(
             sentry_app_installation=self.install, user=self.rpc_user, action="deleted"
         ).run()
@@ -107,6 +110,7 @@ def test_uninstallation_enqueued(self, safe_urlopen):
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen")
     def test_invalid_installation_action(self, safe_urlopen):
         with pytest.raises(APIUnauthorized):
+            assert self.rpc_user, "Rpcuser should exist, unless explicitly noted in test"
             SentryAppInstallationNotifier(
                 sentry_app_installation=self.install, user=self.rpc_user, action="updated"
             ).run()
@@ -115,6 +119,7 @@ def test_invalid_installation_action(self, safe_urlopen):
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_webhook_request_saved(self, safe_urlopen):
+        assert self.rpc_user, "Rpcuser should exist, unless explicitly noted in test"
         SentryAppInstallationNotifier(
             sentry_app_installation=self.install, user=self.rpc_user, action="created"
         ).run()

diff --git a/tests/sentry/tasks/test_sentry_apps.py b/tests/sentry/tasks/test_sentry_apps.py
@@ -437,6 +437,7 @@ def test_sends_installation_notification(self):
         response_body = json.loads(responses.calls[0].request.body)
         assert response_body.get("installation").get("uuid") == self.install.uuid
         assert response_body.get("action") == "created"
+        assert self.rpc_user, "User should exist in test to test installation webhook unless noted"
         assert response_body.get("actor")["id"] == self.rpc_user.id
 
     @responses.activate