From 301e41ad2da76d52e36b6e4b1db1e2eac7ec5dfe Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 8 Oct 2024 17:16:30 -0700 Subject: [PATCH] typing --- pyproject.toml | 1 - .../api/endpoints/installation_details.py | 25 +++++++++++++------ .../api/endpoints/sentry_app_details.py | 3 +++ src/sentry/sentry_apps/installations.py | 2 +- ...ization_sentry_app_installation_details.py | 2 ++ .../test_sentry_app_installation_notifier.py | 5 ++++ tests/sentry/tasks/test_sentry_apps.py | 1 + 7 files changed, 29 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1436b0401fd20a..a0c6384fa7db61 100644 --- 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 index 54f0bb1c7691e1..8b54e948c83a0e 100644 --- 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 index a3d1703cb9e21c..ae72c462a2d401 100644 --- 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 index 05131eeaeb619d..1cd130a9fe7daf 100644 --- 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 index 6247e184bb8bbe..02e8a57f1e0df5 100644 --- 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 index d9e2fe51f5bcbc..a4590af094beea 100644 --- 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 index 9e30c6dac46ef1..343d725f01513b 100644 --- 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