Skip to content

Commit 5765d68

Browse files
update typing and tests since list[tuple] is not valid for pydantic
1 parent 2c28e49 commit 5765d68

File tree

4 files changed

+41
-11
lines changed

4 files changed

+41
-11
lines changed

src/sentry/sentry_apps/services/hook/impl.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from sentry.sentry_apps.logic import expand_events
88
from sentry.sentry_apps.models.servicehook import ServiceHook
99
from sentry.sentry_apps.services.hook import HookService, RpcServiceHook
10+
from sentry.sentry_apps.services.hook.model import RpcInstallationOrganizationPair
1011
from sentry.sentry_apps.services.hook.serial import serialize_service_hook
1112
from sentry.sentry_apps.utils.errors import SentryAppSentryError
1213

@@ -148,12 +149,13 @@ def bulk_create_service_hooks_for_app(
148149
region_name: str,
149150
application_id: int,
150151
events: list[str],
151-
installation_organization_ids: list[tuple[int, int]],
152+
installation_organization_ids: list[RpcInstallationOrganizationPair],
152153
url: str,
153154
) -> list[RpcServiceHook]:
154155
with transaction.atomic(router.db_for_write(ServiceHook)):
155156
expanded_events = expand_events(events)
156-
installation_ids = [installation for installation, _ in installation_organization_ids]
157+
# breakpoint()
158+
installation_ids = [pair.installation_id for pair in installation_organization_ids]
157159

158160
# There shouldn't be any existing hooks for this app but in case we don't want to create duplicates
159161
existing_hooks = ServiceHook.objects.filter(
@@ -180,7 +182,10 @@ def bulk_create_service_hooks_for_app(
180182
)
181183

182184
hooks_to_create = []
183-
for installation_id, organization_id in installation_organization_ids:
185+
for pair in installation_organization_ids:
186+
installation_id = pair.installation_id
187+
organization_id = pair.organization_id
188+
184189
if installation_id in existing_installation_ids:
185190
continue
186191

src/sentry/sentry_apps/services/hook/model.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,8 @@ class RpcServiceHook(RpcModel):
2323

2424
def get_audit_log_data(self) -> dict[str, Any]:
2525
return {"url": self.url}
26+
27+
28+
class RpcInstallationOrganizationPair(RpcModel):
29+
installation_id: int
30+
organization_id: int

src/sentry/sentry_apps/services/hook/service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from sentry.hybridcloud.rpc.resolvers import ByOrganizationId, ByRegionName
99
from sentry.hybridcloud.rpc.service import RpcService, regional_rpc_method
1010
from sentry.sentry_apps.services.hook import RpcServiceHook
11+
from sentry.sentry_apps.services.hook.model import RpcInstallationOrganizationPair
1112
from sentry.silo.base import SiloMode
1213

1314

@@ -90,7 +91,7 @@ def bulk_create_service_hooks_for_app(
9091
region_name: str,
9192
application_id: int,
9293
events: list[str],
93-
installation_organization_ids: list[tuple[int, int]],
94+
installation_organization_ids: list[RpcInstallationOrganizationPair],
9495
url: str,
9596
) -> list[RpcServiceHook]:
9697
"""Meant for bulk creating ServiceHooks for all installations of a given Sentry App in a given region"""

tests/sentry/sentry_apps/services/test_hook_service.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from sentry.sentry_apps.logic import consolidate_events, expand_events
22
from sentry.sentry_apps.models.servicehook import ServiceHook
33
from sentry.sentry_apps.services.hook import RpcServiceHook, hook_service
4+
from sentry.sentry_apps.services.hook.model import RpcInstallationOrganizationPair
45
from sentry.sentry_apps.utils.webhooks import EVENT_EXPANSION, SentryAppResourceType
56
from sentry.silo.base import SiloMode
67
from sentry.testutils.cases import TestCase
@@ -355,8 +356,12 @@ def test_bulk_create_service_hooks_for_app_success(self) -> None:
355356

356357
# Prepare installation-organization pairs
357358
installation_org_pairs = [
358-
(installation1.id, self.org.id),
359-
(installation2.id, org2.id),
359+
RpcInstallationOrganizationPair(
360+
installation_id=installation1.id, organization_id=self.org.id
361+
),
362+
RpcInstallationOrganizationPair(
363+
installation_id=installation2.id, organization_id=org2.id
364+
),
360365
]
361366

362367
result = hook_service.bulk_create_service_hooks_for_app(
@@ -402,7 +407,11 @@ def test_bulk_create_service_hooks_for_app_with_event_expansion(self) -> None:
402407
region_name="us",
403408
application_id=self.sentry_app.application.id,
404409
events=["issue", "comment"], # These should expand
405-
installation_organization_ids=[(installation.id, self.org.id)],
410+
installation_organization_ids=[
411+
RpcInstallationOrganizationPair(
412+
installation_id=installation.id, organization_id=self.org.id
413+
)
414+
],
406415
url="https://example.com/webhook",
407416
)
408417

@@ -454,7 +463,11 @@ def test_bulk_create_service_hooks_for_app_ignore_conflicts(self) -> None:
454463
region_name="us",
455464
application_id=self.sentry_app.application.id,
456465
events=["error.created"],
457-
installation_organization_ids=[(installation.id, self.org.id)],
466+
installation_organization_ids=[
467+
RpcInstallationOrganizationPair(
468+
installation_id=installation.id, organization_id=self.org.id
469+
)
470+
],
458471
url="https://different-url.com/webhook",
459472
)
460473

@@ -483,12 +496,16 @@ def test_bulk_create_service_hooks_for_app_large_batch(self) -> None:
483496
installation = self.create_sentry_app_installation(
484497
slug=self.sentry_app.slug, organization=org, user=self.user
485498
)
486-
installation_org_pairs.append((installation.id, org.id))
499+
installation_org_pairs.append(
500+
RpcInstallationOrganizationPair(
501+
installation_id=installation.id, organization_id=org.id
502+
)
503+
)
487504

488505
# Delete existing hooks to test clean bulk creation
489506
with assume_test_silo_mode(SiloMode.REGION):
490507
ServiceHook.objects.filter(
491-
installation_id__in=[pair[0] for pair in installation_org_pairs]
508+
installation_id__in=[pair.installation_id for pair in installation_org_pairs]
492509
).delete()
493510

494511
# Call bulk create
@@ -505,7 +522,9 @@ def test_bulk_create_service_hooks_for_app_large_batch(self) -> None:
505522

506523
# Verify all hooks were created correctly
507524
with assume_test_silo_mode(SiloMode.REGION):
508-
for installation_id, org_id in installation_org_pairs:
525+
for pair in installation_org_pairs:
526+
installation_id = pair.installation_id
527+
org_id = pair.organization_id
509528
hook = ServiceHook.objects.get(
510529
installation_id=installation_id, application_id=self.sentry_app.application.id
511530
)

0 commit comments

Comments
 (0)