From b0391abe4101f700475fe7e5a338c939cc1053ae Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Mon, 21 Sep 2026 16:58:25 -0700 Subject: [PATCH] ref(cursor-origin): Parse installation events with pydantic Parse `installation.updated` with a model instead of reading the payload by hand. --- .../integrations/cursor_origin/handlers.py | 23 ++++++++------- .../cursor_origin/webhook_types.py | 23 +++++++++++++++ .../cursor_origin/test_handlers.py | 28 ++++++++++++++----- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/sentry/integrations/cursor_origin/handlers.py b/src/sentry/integrations/cursor_origin/handlers.py index 30adfeafb3ef..0df17d1c1b97 100644 --- a/src/sentry/integrations/cursor_origin/handlers.py +++ b/src/sentry/integrations/cursor_origin/handlers.py @@ -6,6 +6,7 @@ from sentry.constants import ObjectStatus from sentry.integrations.cursor_origin.constants import CURSOR_ORIGIN_WEB_BASE_URL +from sentry.integrations.cursor_origin.webhook_types import InstallationEvent from sentry.integrations.services.integration import integration_service from sentry.integrations.services.integration.model import ( RpcIntegration, @@ -156,28 +157,26 @@ def __call__( integration: RpcIntegration, org_integrations: Sequence[RpcOrganizationIntegration], ) -> None: - installation = payload.get("installation") or {} - target = installation.get("target") or {} - name = target.get("slug") + installation = InstallationEvent.from_payload(payload).installation + slug = installation.target.slug changed: dict[str, Any] = { "installation_id": integration.external_id, - "target": target, - "scopes": installation.get("scopes") or [], - "repo_selection_mode": installation.get("repoSelectionMode"), + "target": installation.target.dict(exclude_none=True), + "scopes": installation.scopes, + "repo_selection_mode": installation.repo_selection_mode, + "domain_name": f"{CURSOR_ORIGIN_WEB_BASE_URL}/{slug}", } - if name: - changed["domain_name"] = f"{CURSOR_ORIGIN_WEB_BASE_URL}/{name}" # `update_integration` replaces metadata rather than merging it, which would - # drop the cached access token and, without a slug, `domain_name`. + # drop the cached access token. stored = integration_service.get_integration(integration_id=integration.id) assert stored is not None metadata = {**stored.metadata, **changed} previous_slug = stored.name - if name and name != previous_slug: + if slug != previous_slug: _rename_owner_repositories( - previous_slug, name, integration.id, org_integrations, delivery_id + previous_slug, slug, integration.id, org_integrations, delivery_id ) logger.info( @@ -185,6 +184,6 @@ def __call__( extra={"delivery_id": delivery_id, "integration_id": integration.id}, ) integration_service.update_integration( - integration_id=integration.id, name=name or None, metadata=metadata + integration_id=integration.id, name=slug, metadata=metadata ) _sync_repositories(org_integrations, delivery_id) diff --git a/src/sentry/integrations/cursor_origin/webhook_types.py b/src/sentry/integrations/cursor_origin/webhook_types.py index 765826a0b9f3..5df8623e72b8 100644 --- a/src/sentry/integrations/cursor_origin/webhook_types.py +++ b/src/sentry/integrations/cursor_origin/webhook_types.py @@ -185,3 +185,26 @@ def from_payload(cls, payload: Mapping[str, Any]) -> PullRequestEvent: return cls.parse_obj(payload) except ValidationError as e: raise OriginPayloadError(str(e)) from e + + +class InstallationTarget(OriginModel): + slug: str = Field(min_length=1) + id: str = Field(min_length=1) + type: Literal["team", "user"] | None = None + + +class Installation(OriginModel): + target: InstallationTarget + scopes: list[str] + repo_selection_mode: Literal["all", "selected"] = Field(alias="repoSelectionMode") + + +class InstallationEvent(OriginModel): + installation: Installation + + @classmethod + def from_payload(cls, payload: Mapping[str, Any]) -> InstallationEvent: + try: + return cls.parse_obj(payload) + except ValidationError as e: + raise OriginPayloadError(str(e)) from e diff --git a/tests/sentry/integrations/cursor_origin/test_handlers.py b/tests/sentry/integrations/cursor_origin/test_handlers.py index e77dd9affcfb..6ec554e887cd 100644 --- a/tests/sentry/integrations/cursor_origin/test_handlers.py +++ b/tests/sentry/integrations/cursor_origin/test_handlers.py @@ -7,6 +7,7 @@ from sentry.constants import ObjectStatus from sentry.integrations.cursor_origin.webhook import HANDLERS +from sentry.integrations.cursor_origin.webhook_types import OriginPayloadError from sentry.integrations.models.integration import Integration from sentry.integrations.models.organization_integration import OrganizationIntegration from sentry.integrations.services.integration import integration_service @@ -127,9 +128,10 @@ def test_an_update_keeps_metadata_it_does_not_carry(self) -> None: assert metadata["expires_at"] == "2026-09-16T23:00:00Z" assert metadata["repo_selection_mode"] == "all" - def test_an_update_without_a_slug_keeps_the_domain(self) -> None: - """`source_url_matches` reads `domain_name` directly, so it must not be dropped.""" - self._handle("installation.updated", _installation(target={"id": "ns_01example"})) + def test_an_update_without_a_slug_is_refused(self) -> None: + """Origin documents the target's slug as always present.""" + with pytest.raises(OriginPayloadError, match="installation -> target -> slug"): + self._handle("installation.updated", _installation(target={"id": "ns_01example"})) integration = self._integration() assert integration.metadata["domain_name"] == f"{WEB}/acme" @@ -225,7 +227,10 @@ def test_an_owner_rename_rewrites_every_repository(self) -> None: config={"name": "acme/booster", "default_branch": "main"}, ) - self._handle("installation.updated", _installation(target={"slug": "rocketry"})) + self._handle( + "installation.updated", + _installation(target={"slug": "rocketry", "id": "ns_01example", "type": "team"}), + ) with assume_test_silo_mode_of(Repository): renamed = Repository.objects.get(id=other.id) @@ -243,11 +248,17 @@ def test_a_failed_rename_is_finished_by_the_retry(self) -> None: ), pytest.raises(ValueError), ): - self._handle("installation.updated", _installation(target={"slug": "rocketry"})) + self._handle( + "installation.updated", + _installation(target={"slug": "rocketry", "id": "ns_01example", "type": "team"}), + ) assert self._integration().name == "acme" - self._handle("installation.updated", _installation(target={"slug": "rocketry"})) + self._handle( + "installation.updated", + _installation(target={"slug": "rocketry", "id": "ns_01example", "type": "team"}), + ) assert self._integration().name == "rocketry" with assume_test_silo_mode_of(Repository): @@ -274,7 +285,10 @@ def test_a_repository_outside_the_renamed_owner_is_left_alone(self) -> None: config={"name": "elsewhere/thing"}, ) - self._handle("installation.updated", _installation(target={"slug": "rocketry"})) + self._handle( + "installation.updated", + _installation(target={"slug": "rocketry", "id": "ns_01example", "type": "team"}), + ) with assume_test_silo_mode_of(Repository): assert Repository.objects.get(id=odd.id).name == "elsewhere/thing"