diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index 2d671ebd69b5..7e814442adbc 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -415,8 +415,49 @@ def _load_service_hook(organization_id: int | None, installation_id: int) -> Ser ) return service_hook except ServiceHook.DoesNotExist: + # Attempt to repair the hook if the organization_id is missing + return _repair_hook_missing_organization_id(organization_id, installation_id) + + +def _repair_hook_missing_organization_id( + organization_id: int | None, installation_id: int +) -> ServiceHook | None: + """ + Attempt to repair the hook if the organization_id is missing (there was a gap from + between 2025-08-26 and 2026-02-18) + TODO: Remove this once the gap is closed + """ + if organization_id is None: return None + try: + service_hook = ServiceHook.objects.get( + installation_id=installation_id, + organization_id__isnull=True, + ) + except ServiceHook.DoesNotExist: + return None + except ServiceHook.MultipleObjectsReturned: + # We can't tell which hook is live, and guessing would send an org's payloads + # to the wrong url. Fall through to the missing_servicehook halt instead. + logger.warning( + "service_hook.duplicate_hooks_missing_organization_id", + extra={"installation_id": installation_id}, + ) + return None + + service_hook.organization_id = organization_id + service_hook.save(update_fields=["organization_id"]) + logger.info( + "service_hook.repaired_missing_organization_id", + extra={ + "service_hook_id": service_hook.id, + "installation_id": installation_id, + "organization_id": organization_id, + }, + ) + return service_hook + @cache_func_for_models( [(ServiceHookProject, lambda hook_project: (hook_project.service_hook_id,))], diff --git a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py index 1444977dd777..591725d65985 100644 --- a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py +++ b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py @@ -1708,6 +1708,60 @@ def test_does_not_send_if_event_not_in_app_events( mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1 ) + def test_repairs_service_hook_missing_organization_id(self, safe_urlopen: MagicMock) -> None: + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update(organization_id=None) + + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + + ((_, kwargs),) = safe_urlopen.call_args_list + assert kwargs["url"] == self.sentry_app.webhook_url + + with assume_test_silo_mode_of(ServiceHook): + hook = ServiceHook.objects.get(installation_id=self.install.id) + assert hook.organization_id == self.project.organization.id + + def test_does_not_repair_when_multiple_hooks_missing_organization_id( + self, safe_urlopen: MagicMock + ) -> None: + self.create_service_hook( + actor=self.user, + org=self.project.organization, + project_ids=[], + events=["issue.resolved"], + installation_id=self.install.id, + application_id=self.sentry_app.application_id, + url=self.sentry_app.webhook_url, + ) + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update(organization_id=None) + + with pytest.raises(SentryAppSentryError): + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + assert not safe_urlopen.called + + with assume_test_silo_mode_of(ServiceHook): + assert not ServiceHook.objects.filter( + installation_id=self.install.id, organization_id__isnull=False + ).exists() + + def test_does_not_repair_hook_belonging_to_another_organization( + self, safe_urlopen: MagicMock + ) -> None: + other_org = self.create_organization() + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update( + organization_id=other_org.id + ) + + with pytest.raises(SentryAppSentryError): + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + assert not safe_urlopen.called + + with assume_test_silo_mode_of(ServiceHook): + hook = ServiceHook.objects.get(installation_id=self.install.id) + assert hook.organization_id == other_org.id + class TestWebhookRequests(TestCase): def setUp(self) -> None: