From 4397a7acd476655630ea71ad9a26223713d4d982 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Fri, 10 Oct 2025 07:25:24 +0200 Subject: [PATCH 01/10] feat(integration): Add new endpoint to list channels --- src/sentry/api/urls.py | 8 + .../organization_integration_channels.py | 221 ++++++++++++++ .../test_organization_integration_channels.py | 274 ++++++++++++++++++ 3 files changed, 503 insertions(+) create mode 100644 src/sentry/integrations/api/endpoints/organization_integration_channels.py create mode 100644 tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py diff --git a/src/sentry/api/urls.py b/src/sentry/api/urls.py index 6a214bd1b7b470..c6c5a6b8e674e6 100644 --- a/src/sentry/api/urls.py +++ b/src/sentry/api/urls.py @@ -242,6 +242,9 @@ from sentry.integrations.api.endpoints.organization_config_integrations import ( OrganizationConfigIntegrationsEndpoint, ) +from sentry.integrations.api.endpoints.organization_integration_channels import ( + OrganizationIntegrationChannelsEndpoint, +) from sentry.integrations.api.endpoints.organization_integration_details import ( OrganizationIntegrationDetailsEndpoint, ) @@ -1879,6 +1882,11 @@ def create_group_urls(name_prefix: str) -> list[URLPattern | URLResolver]: OrganizationIntegrationReposEndpoint.as_view(), name="sentry-api-0-organization-integration-repos", ), + re_path( + r"^(?P[^/]+)/integrations/(?P[^/]+)/channels/$", + OrganizationIntegrationChannelsEndpoint.as_view(), + name="sentry-api-0-organization-integration-channels", + ), re_path( r"^(?P[^/]+)/integrations/(?P[^/]+)/issues/$", OrganizationIntegrationIssuesEndpoint.as_view(), diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py new file mode 100644 index 00000000000000..a0c7c21d16418a --- /dev/null +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -0,0 +1,221 @@ +from __future__ import annotations + +from typing import Any + +from rest_framework.request import Request +from rest_framework.response import Response + +from sentry.api.api_owners import ApiOwner +from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import region_silo_endpoint +from sentry.integrations.api.bases.organization_integrations import ( + RegionOrganizationIntegrationBaseEndpoint, +) +from sentry.integrations.discord.client import DiscordClient +from sentry.integrations.msteams.client import MsTeamsClient +from sentry.integrations.services.integration import integration_service +from sentry.integrations.types import IntegrationProviderSlug +from sentry.models.organization import Organization +from sentry.shared_integrations.exceptions import ApiError + + +def _slack_list_channels( + *, integration_id: int, cursor: str | None, limit: int +) -> tuple[list[dict[str, Any]], str | None]: + """ + List Slack channels for a given integration. + + The Slack API supports filtering and pagination. This function fetches only + public and private channels. + """ + + from sentry.integrations.slack.sdk_client import SlackSdkClient + + client = SlackSdkClient(integration_id=integration_id) + params: dict[str, Any] = { + "exclude_archived": True, + "types": "public_channel,private_channel", + "limit": limit, + } + if cursor: + params["cursor"] = cursor + resp = client.conversations_list(**params).data + channels = resp.get("channels", []) or [] + next_cursor: str | None = (resp.get("response_metadata", {}) or {}).get("next_cursor") or None + + results: list[dict[str, Any]] = [] + for ch in channels: + + name = ch.get("name") + results.append( + { + "id": ch.get("id"), + "name": name, # 'name' is always present for public and private channels (required by Slack) + "display": f"#{name}", + "type": "private" if bool(ch.get("is_private")) else "public", + } + ) + + return results, next_cursor + + +def _discord_list_channels( + *, guild_id: str, cursor: str | None, limit: int +) -> tuple[list[dict[str, Any]], str | None]: + """ + List Discord channels for a given guild that can receive alert messages. + + The Discord API returns all guild channels in a single call and does not + support server-side type filtering or pagination. This function filters + for messageable channels and emulates pagination using a cursor offset. + The `cursor` parameter represents the starting offset for slicing the channel list. + """ + + DISCORD_CHANNEL_TYPES = { + 0: "text", + 5: "announcement", + 15: "forum", + } + + client = DiscordClient() + + # Discord API does NOT support server-side type filtering or pagination + channels = ( + client.get_cached( + f"/guilds/{guild_id}/channels", + headers=client.prepare_auth_header(), + cache_time=60, + ) + or [] + ) + + selectable_types = DISCORD_CHANNEL_TYPES.keys() + filtered = [ch for ch in channels if ch.get("type") in selectable_types] + + try: + offset = int(cursor) if cursor else 0 + except ValueError: + offset = 0 + + sliced = filtered[offset : offset + limit] + next_cursor = str(offset + limit) if (offset + limit) < len(filtered) else None + + results: list[dict[str, Any]] = [] + for ch in sliced: + + results.append( + { + "id": ch["id"], + "name": ch["name"], + "display": f"#{ch['name']}", + "type": DISCORD_CHANNEL_TYPES.get(ch["type"], "unknown"), + } + ) + + return results, next_cursor + + +def _msteams_list_channels( + *, integration_id: int, team_id: str, cursor: str | None, limit: int +) -> tuple[list[dict[str, Any]], str | None]: + """ + List Microsoft Teams channels for a team. + + The Teams API returns all channels at once; this function emulates + pagination using a simple offset cursor. + + Only standard and private channels are included. + """ + + integration = integration_service.get_integration(integration_id=integration_id) + client = MsTeamsClient(integration) + + # Cache Teams channel list briefly to avoid re-fetching the full list across pages + channels_resp = client.get_cached(client.CHANNEL_URL % team_id, cache_time=60) or {} + channels = channels_resp.get("conversations") or [] + + try: + offset = int(cursor) if cursor else 0 + except ValueError: + offset = 0 + + sliced = channels[offset : offset + limit] + + next_cursor = str(offset + limit) if (offset + limit) < len(channels) else None + + results: list[dict[str, Any]] = [] + for ch in sliced: + results.append( + { + "id": ch["id"], + "name": ch["displayName"], + "display": ch["displayName"], + "type": ch.get("membershipType", "standard"), # "standard" or "private", + } + ) + + return results, next_cursor + + +@region_silo_endpoint +class OrganizationIntegrationChannelsEndpoint(RegionOrganizationIntegrationBaseEndpoint): + publish_status = { + "GET": ApiPublishStatus.PRIVATE, + } + owner = ApiOwner.TELEMETRY_EXPERIENCE + + def get( + self, + request: Request, + organization: Organization, + integration_id: int, + **kwargs: Any, + ) -> Response: + """ + List messaging channels with pagination for an messaging integration. + + Query params: + - query: optional, currently client-side filtered; reserved + - cursor: provider-specific cursor string (Slack) or integer offset (Discord/Teams) + - limit: page size (default 50, max 200) + """ + limit_raw = request.GET.get("limit") + try: + limit = int(limit_raw) if limit_raw is not None else 50 + except ValueError: + limit = 50 + limit = max(1, min(limit, 200)) + + cursor = request.GET.get("cursor") + + integration = self.get_integration(organization.id, integration_id) + + try: + match integration.provider: + case IntegrationProviderSlug.SLACK.value: + results, next_cursor = _slack_list_channels( + integration_id=integration.id, cursor=cursor, limit=limit + ) + case IntegrationProviderSlug.DISCORD.value: + results, next_cursor = _discord_list_channels( + guild_id=str(integration.external_id), cursor=cursor, limit=limit + ) + case IntegrationProviderSlug.MSTEAMS.value: + results, next_cursor = _msteams_list_channels( + integration_id=integration.id, + team_id=str(integration.external_id), + cursor=cursor, + limit=limit, + ) + case _: + return self.respond( + { + "results": [], + "nextCursor": None, + "warning": f"Channel listing not supported for provider '{integration.provider}'.", + } + ) + except ApiError as e: + return self.respond({"detail": str(e)}, status=400) + + return self.respond({"results": results, "nextCursor": next_cursor}) diff --git a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py new file mode 100644 index 00000000000000..4c7a8f29e56fa9 --- /dev/null +++ b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py @@ -0,0 +1,274 @@ +from unittest.mock import patch + +from sentry.testutils.cases import APITestCase + + +class OrganizationIntegrationChannelsTest(APITestCase): + endpoint = "sentry-api-0-organization-integration-channels" + method = "get" + + def setUp(self) -> None: + super().setUp() + self.login_as(user=self.user) + + +class OrganizationIntegrationChannelsSlackTest(OrganizationIntegrationChannelsTest): + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="slack", + name="Slack Workspace", + external_id="TXXXXXXX1", + metadata={ + "access_token": "xoxb-token", + "installation_type": "born_as_bot", + }, + ) + + @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") + def test_slack_channels_list(self, mock_conversations_list): + """Test listing Slack channels with proper formatting and pagination.""" + channels = [ + {"id": "C123", "name": "general", "is_private": False}, + {"id": "C456", "name": "alerts", "is_private": True}, + ] + mock_conversations_list.return_value.data = { + "ok": True, + "channels": channels, + "response_metadata": {"next_cursor": "cursor123"}, + } + resp = self.get_success_response( + self.organization.slug, self.integration.id, qs_params={"limit": 1} + ) + results = resp.data["results"] + expected_first = channels[0] + assert len(results) == 1 + assert results[0] == { + "id": expected_first["id"], + "name": expected_first["name"], + "display": f"#{expected_first['name']}", + "type": "public", + } + assert resp.data["nextCursor"] == "cursor123" + + @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") + def test_slack_channels_with_cursor(self, mock_conversations_list): + """Test fetching next page of Slack channels with cursor.""" + channel = {"id": "C789", "name": "last-channel", "is_private": False} + mock_conversations_list.return_value.data = { + "ok": True, + "channels": [channel], + "response_metadata": {}, + } + resp = self.get_success_response( + self.organization.slug, + self.integration.id, + qs_params={"cursor": "cursor123", "limit": 2}, + ) + assert resp.data["results"] == [ + { + "id": channel["id"], + "name": channel["name"], + "display": f"#{channel['name']}", + "type": "public", + } + ] + assert resp.data["nextCursor"] is None + call_kwargs = mock_conversations_list.call_args[1] + assert call_kwargs["cursor"] == "cursor123" + + +class OrganizationIntegrationChannelsDiscordTest(OrganizationIntegrationChannelsTest): + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="discord", + name="Discord Server", + external_id="1234567890", + ) + + @patch("sentry.integrations.discord.client.DiscordClient.get_cached") + def test_discord_channels_list(self, mock_get_cached): + """Test listing Discord channels with client-side pagination and caching.""" + mock_channels = [ + {"id": "123456", "name": "general", "type": 0}, + {"id": "789012", "name": "announcements", "type": 5}, + {"id": "345678", "name": "off-topic", "type": 0}, + ] + mock_get_cached.return_value = mock_channels + response = self.get_success_response( + self.organization.slug, self.integration.id, qs_params={"limit": 2} + ) + results = response.data["results"] + assert len(results) == 2 + expected = [ + { + **mock_channels[0], + "display": f"#{mock_channels[0]['name']}", + "type": "text", + }, + { + **mock_channels[1], + "display": f"#{mock_channels[1]['name']}", + "type": "announcement", + }, + ] + assert results == expected + mock_get_cached.assert_called_once() + call_kwargs = mock_get_cached.call_args[1] + assert call_kwargs["cache_time"] == 60 + + @patch("sentry.integrations.discord.client.DiscordClient.get_cached") + def test_discord_channels_pagination(self, mock_get_cached): + """Test Discord channel pagination with offset cursor.""" + mock_get_cached.return_value = [ + {"id": "123456", "name": "general", "type": 0}, + {"id": "789012", "name": "announcements", "type": 5}, + {"id": "345678", "name": "off-topic", "type": 0}, + ] + response = self.get_success_response( + self.organization.slug, self.integration.id, qs_params={"cursor": "2", "limit": 2} + ) + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["id"] == "345678" + assert response.data["nextCursor"] is None + + @patch("sentry.integrations.discord.client.DiscordClient.get_cached") + def test_discord_filters_non_messageable_channels(self, mock_get_cached): + """Test that Discord filters out non-messageable channel types.""" + mock_get_cached.return_value = [ + {"id": "123456", "name": "text-channel", "type": 0}, + {"id": "789012", "name": "voice-channel", "type": 2}, + {"id": "345678", "name": "category", "type": 4}, + {"id": "901234", "name": "announcement", "type": 5}, + ] + + response = self.get_success_response(self.organization.slug, self.integration.id) + + assert len(response.data["results"]) == 2 + assert response.data["results"][0]["type"] == "text" + assert response.data["results"][1]["type"] == "announcement" + + +class OrganizationIntegrationChannelsMsTeamsTest(OrganizationIntegrationChannelsTest): + def setUp(self) -> None: + super().setUp() + self.integration = self.create_integration( + organization=self.organization, + provider="msteams", + name="MS Teams", + external_id="19:team-id@thread.tacv2", + metadata={ + "access_token": "token", + "service_url": "https://smba.trafficmanager.net/amer/", + "expires_at": 9999999999, + }, + ) + + @patch("sentry.integrations.msteams.client.MsTeamsClient.get_cached") + def test_msteams_channels_list(self, mock_get_cached): + """Test listing Microsoft Teams channels with pagination and caching.""" + + mock_channels = { + "conversations": [ + {"id": "19:channel1@thread.tacv2", "displayName": "Development"}, + {"id": "19:channel2@thread.tacv2", "displayName": "General"}, + { + "id": "19:channel3@thread.tacv2", + "displayName": "Testing", + "membershipType": "private", + }, + ] + } + mock_get_cached.return_value = mock_channels + + response = self.get_success_response( + self.organization.slug, self.integration.id, qs_params={"limit": 2} + ) + results = response.data["results"] + + expected = [ + { + "id": ch["id"], + "name": ch["displayName"], + "display": ch["displayName"], + "type": ch.get("membershipType", "standard"), + } + for ch in mock_channels["conversations"][:2] + ] + + assert results == expected + assert response.data["nextCursor"] == "2" + mock_get_cached.assert_called_once() + call_kwargs = mock_get_cached.call_args[1] + assert call_kwargs["cache_time"] == 60 + + @patch("sentry.integrations.msteams.client.MsTeamsClient.get_cached") + def test_msteams_pagination(self, mock_get_cached): + """Test MS Teams channel pagination with offset cursor.""" + mock_get_cached.return_value = { + "conversations": [ + {"id": "19:ch1@thread.tacv2", "displayName": "First"}, + {"id": "19:ch2@thread.tacv2", "displayName": "Second"}, + {"id": "19:ch3@thread.tacv2", "displayName": "Third"}, + ] + } + + response = self.get_success_response( + self.organization.slug, self.integration.id, qs_params={"cursor": "2", "limit": 2} + ) + + assert len(response.data["results"]) == 1 + assert response.data["results"][0]["name"] == "Third" + assert response.data["nextCursor"] is None + + +class OrganizationIntegrationChannelsErrorTest(OrganizationIntegrationChannelsTest): + def test_integration_not_found(self): + """Test 404 when integration doesn't exist.""" + response = self.get_error_response( + self.organization.slug, integration_id=9999, status_code=404 + ) + assert response.status_code == 404 + + def test_unsupported_provider(self): + """Test warning for unsupported integration provider.""" + integration = self.create_integration( + organization=self.organization, + provider="github", # not a messaging integration + name="GitHub", + external_id="github:1", + ) + + response = self.get_success_response(self.organization.slug, integration.id) + + assert response.data["results"] == [] + assert response.data["nextCursor"] is None + assert "not supported" in response.data["warning"] + + def test_limit_validation(self): + """Test limit parameter validation and clamping.""" + integration = self.create_integration( + organization=self.organization, provider="slack", external_id="test" + ) + + # Test max limit clamping + with patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") as m: + m.return_value.data = {"ok": True, "channels": [], "response_metadata": {}} + self.get_success_response( + self.organization.slug, integration.id, qs_params={"limit": 999} + ) + # Should clamp to 200 + call_kwargs = m.call_args[1] + assert call_kwargs["limit"] == 200 + + # Test invalid limit defaults to 50 + with patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") as m: + m.return_value.data = {"ok": True, "channels": [], "response_metadata": {}} + self.get_success_response( + self.organization.slug, integration.id, qs_params={"limit": "invalid"} + ) + call_kwargs = m.call_args[1] + assert call_kwargs["limit"] == 50 From 16769dfedf09a671b723fc69ba6534deee477e6e Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Fri, 10 Oct 2025 08:25:29 +0200 Subject: [PATCH 02/10] fix test --- .../test_organization_integration_channels.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py index 4c7a8f29e56fa9..10bc8acfb8e8c3 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py @@ -237,7 +237,7 @@ def test_unsupported_provider(self): """Test warning for unsupported integration provider.""" integration = self.create_integration( organization=self.organization, - provider="github", # not a messaging integration + provider="github", name="GitHub", external_id="github:1", ) @@ -251,7 +251,14 @@ def test_unsupported_provider(self): def test_limit_validation(self): """Test limit parameter validation and clamping.""" integration = self.create_integration( - organization=self.organization, provider="slack", external_id="test" + organization=self.organization, + provider="slack", + name="Slack Workspace", + external_id="TXXXXXXX1", + metadata={ + "access_token": "xoxb-token", + "installation_type": "born_as_bot", + }, ) # Test max limit clamping @@ -260,7 +267,6 @@ def test_limit_validation(self): self.get_success_response( self.organization.slug, integration.id, qs_params={"limit": 999} ) - # Should clamp to 200 call_kwargs = m.call_args[1] assert call_kwargs["limit"] == 200 From 280d89c28817f863922292438c526a2d6d7060bd Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Fri, 10 Oct 2025 08:39:26 +0200 Subject: [PATCH 03/10] fix tests --- .../test_organization_integration_channels.py | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py index 10bc8acfb8e8c3..f75946c5df3667 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py @@ -28,26 +28,20 @@ def setUp(self) -> None: @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") def test_slack_channels_list(self, mock_conversations_list): - """Test listing Slack channels with proper formatting and pagination.""" - channels = [ - {"id": "C123", "name": "general", "is_private": False}, - {"id": "C456", "name": "alerts", "is_private": True}, - ] + """Test listing Slack channels with proper formatting""" + channel = {"id": "C123", "name": "general", "is_private": False} mock_conversations_list.return_value.data = { "ok": True, - "channels": channels, + "channels": [channel], "response_metadata": {"next_cursor": "cursor123"}, } - resp = self.get_success_response( - self.organization.slug, self.integration.id, qs_params={"limit": 1} - ) + resp = self.get_success_response(self.organization.slug, self.integration.id) results = resp.data["results"] - expected_first = channels[0] assert len(results) == 1 assert results[0] == { - "id": expected_first["id"], - "name": expected_first["name"], - "display": f"#{expected_first['name']}", + "id": channel["id"], + "name": channel["name"], + "display": f"#{channel['name']}", "type": "public", } assert resp.data["nextCursor"] == "cursor123" @@ -64,7 +58,7 @@ def test_slack_channels_with_cursor(self, mock_conversations_list): resp = self.get_success_response( self.organization.slug, self.integration.id, - qs_params={"cursor": "cursor123", "limit": 2}, + qs_params={"cursor": "cursor123"}, ) assert resp.data["results"] == [ { @@ -228,9 +222,7 @@ def test_msteams_pagination(self, mock_get_cached): class OrganizationIntegrationChannelsErrorTest(OrganizationIntegrationChannelsTest): def test_integration_not_found(self): """Test 404 when integration doesn't exist.""" - response = self.get_error_response( - self.organization.slug, integration_id=9999, status_code=404 - ) + response = self.get_error_response(self.organization.slug, 9999, status_code=404) assert response.status_code == 404 def test_unsupported_provider(self): From 6739e23096f671d268316b62ffa33ba89dc1e46b Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Fri, 10 Oct 2025 09:11:54 +0200 Subject: [PATCH 04/10] fix type issue --- .../api/endpoints/organization_integration_channels.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index a0c7c21d16418a..6dc6ae35108770 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -40,8 +40,9 @@ def _slack_list_channels( if cursor: params["cursor"] = cursor resp = client.conversations_list(**params).data - channels = resp.get("channels", []) or [] - next_cursor: str | None = (resp.get("response_metadata", {}) or {}).get("next_cursor") or None + resp_data: dict[str, Any] = resp if isinstance(resp, dict) else {} + channels = resp_data.get("channels", []) or [] + next_cursor: str | None = resp_data.get("response_metadata", {}).get("next_cursor") results: list[dict[str, Any]] = [] for ch in channels: @@ -128,6 +129,9 @@ def _msteams_list_channels( """ integration = integration_service.get_integration(integration_id=integration_id) + if integration is None: + raise ApiError("Microsoft Teams integration not found") + client = MsTeamsClient(integration) # Cache Teams channel list briefly to avoid re-fetching the full list across pages From adec2861ce482cf2f333733cbba562af77df591f Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Mon, 13 Oct 2025 10:32:47 +0200 Subject: [PATCH 05/10] feedback --- .../organization_integration_channels.py | 123 +++------- .../test_organization_integration_channels.py | 229 +++++------------- 2 files changed, 94 insertions(+), 258 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index 6dc6ae35108770..5adbb324ce485f 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -7,7 +7,6 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus -from sentry.api.base import region_silo_endpoint from sentry.integrations.api.bases.organization_integrations import ( RegionOrganizationIntegrationBaseEndpoint, ) @@ -19,57 +18,49 @@ from sentry.shared_integrations.exceptions import ApiError -def _slack_list_channels( - *, integration_id: int, cursor: str | None, limit: int -) -> tuple[list[dict[str, Any]], str | None]: +def _slack_list_channels(*, integration_id: int) -> list[dict[str, Any]]: """ List Slack channels for a given integration. - The Slack API supports filtering and pagination. This function fetches only - public and private channels. + NOTE: We are NOT using pagination here because some organizations have + more than 1000 channels. Searching client-side might miss channels beyond + the first 1000, so we fetch the maximum allowed by Slack and rely on + validation when saving. """ from sentry.integrations.slack.sdk_client import SlackSdkClient client = SlackSdkClient(integration_id=integration_id) - params: dict[str, Any] = { - "exclude_archived": True, - "types": "public_channel,private_channel", - "limit": limit, - } - if cursor: - params["cursor"] = cursor - resp = client.conversations_list(**params).data + resp = client.conversations_list( + exclude_archived=True, + types="public_channel,private_channel", + limit=1000, # Max Slack allows (see https://docs.slack.dev/reference/methods/conversations.list/) + ).data + resp_data: dict[str, Any] = resp if isinstance(resp, dict) else {} channels = resp_data.get("channels", []) or [] - next_cursor: str | None = resp_data.get("response_metadata", {}).get("next_cursor") results: list[dict[str, Any]] = [] for ch in channels: - name = ch.get("name") results.append( { "id": ch.get("id"), - "name": name, # 'name' is always present for public and private channels (required by Slack) + "name": name, "display": f"#{name}", "type": "private" if bool(ch.get("is_private")) else "public", } ) - return results, next_cursor + return results -def _discord_list_channels( - *, guild_id: str, cursor: str | None, limit: int -) -> tuple[list[dict[str, Any]], str | None]: +def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: """ - List Discord channels for a given guild that can receive alert messages. + List Discord channels for a given guild that can receive messages. - The Discord API returns all guild channels in a single call and does not - support server-side type filtering or pagination. This function filters - for messageable channels and emulates pagination using a cursor offset. - The `cursor` parameter represents the starting offset for slicing the channel list. + The Discord API returns all guild channels in a single call. + This function filters for messageable channels only. """ DISCORD_CHANNEL_TYPES = { @@ -79,31 +70,15 @@ def _discord_list_channels( } client = DiscordClient() - - # Discord API does NOT support server-side type filtering or pagination channels = ( - client.get_cached( - f"/guilds/{guild_id}/channels", - headers=client.prepare_auth_header(), - cache_time=60, - ) - or [] + client.get(f"/guilds/{guild_id}/channels", headers=client.prepare_auth_header()) or [] ) selectable_types = DISCORD_CHANNEL_TYPES.keys() filtered = [ch for ch in channels if ch.get("type") in selectable_types] - try: - offset = int(cursor) if cursor else 0 - except ValueError: - offset = 0 - - sliced = filtered[offset : offset + limit] - next_cursor = str(offset + limit) if (offset + limit) < len(filtered) else None - results: list[dict[str, Any]] = [] - for ch in sliced: - + for ch in filtered: results.append( { "id": ch["id"], @@ -113,18 +88,14 @@ def _discord_list_channels( } ) - return results, next_cursor + return results -def _msteams_list_channels( - *, integration_id: int, team_id: str, cursor: str | None, limit: int -) -> tuple[list[dict[str, Any]], str | None]: +def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[str, Any]]: """ List Microsoft Teams channels for a team. - The Teams API returns all channels at once; this function emulates - pagination using a simple offset cursor. - + The Teams API returns all channels at once. Only standard and private channels are included. """ @@ -133,35 +104,23 @@ def _msteams_list_channels( raise ApiError("Microsoft Teams integration not found") client = MsTeamsClient(integration) - - # Cache Teams channel list briefly to avoid re-fetching the full list across pages - channels_resp = client.get_cached(client.CHANNEL_URL % team_id, cache_time=60) or {} + channels_resp = client.get(client.CHANNEL_URL % team_id) or {} channels = channels_resp.get("conversations") or [] - try: - offset = int(cursor) if cursor else 0 - except ValueError: - offset = 0 - - sliced = channels[offset : offset + limit] - - next_cursor = str(offset + limit) if (offset + limit) < len(channels) else None - results: list[dict[str, Any]] = [] - for ch in sliced: + for ch in channels: results.append( { "id": ch["id"], "name": ch["displayName"], "display": ch["displayName"], - "type": ch.get("membershipType", "standard"), # "standard" or "private", + "type": ch.get("membershipType", "standard"), # "standard" or "private" } ) - return results, next_cursor + return results -@region_silo_endpoint class OrganizationIntegrationChannelsEndpoint(RegionOrganizationIntegrationBaseEndpoint): publish_status = { "GET": ApiPublishStatus.PRIVATE, @@ -176,50 +135,30 @@ def get( **kwargs: Any, ) -> Response: """ - List messaging channels with pagination for an messaging integration. - - Query params: - - query: optional, currently client-side filtered; reserved - - cursor: provider-specific cursor string (Slack) or integer offset (Discord/Teams) - - limit: page size (default 50, max 200) + List all messaging channels for an integration. """ - limit_raw = request.GET.get("limit") - try: - limit = int(limit_raw) if limit_raw is not None else 50 - except ValueError: - limit = 50 - limit = max(1, min(limit, 200)) - - cursor = request.GET.get("cursor") integration = self.get_integration(organization.id, integration_id) try: match integration.provider: case IntegrationProviderSlug.SLACK.value: - results, next_cursor = _slack_list_channels( - integration_id=integration.id, cursor=cursor, limit=limit - ) + results = _slack_list_channels(integration_id=integration.id) case IntegrationProviderSlug.DISCORD.value: - results, next_cursor = _discord_list_channels( - guild_id=str(integration.external_id), cursor=cursor, limit=limit - ) + results = _discord_list_channels(guild_id=str(integration.external_id)) case IntegrationProviderSlug.MSTEAMS.value: - results, next_cursor = _msteams_list_channels( + results = _msteams_list_channels( integration_id=integration.id, team_id=str(integration.external_id), - cursor=cursor, - limit=limit, ) case _: return self.respond( { "results": [], - "nextCursor": None, "warning": f"Channel listing not supported for provider '{integration.provider}'.", } ) except ApiError as e: return self.respond({"detail": str(e)}, status=400) - return self.respond({"results": results, "nextCursor": next_cursor}) + return self.respond({"results": results}) diff --git a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py index f75946c5df3667..0dfd15d7d57426 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py @@ -28,49 +28,44 @@ def setUp(self) -> None: @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") def test_slack_channels_list(self, mock_conversations_list): - """Test listing Slack channels with proper formatting""" - channel = {"id": "C123", "name": "general", "is_private": False} + channels = [ + {"id": "C123", "name": "general", "is_private": False}, + {"id": "C124", "name": "random", "is_private": True}, + {"id": "C125", "name": "alerts", "is_private": False}, + ] mock_conversations_list.return_value.data = { "ok": True, - "channels": [channel], - "response_metadata": {"next_cursor": "cursor123"}, + "channels": channels, } resp = self.get_success_response(self.organization.slug, self.integration.id) results = resp.data["results"] - assert len(results) == 1 - assert results[0] == { - "id": channel["id"], - "name": channel["name"], - "display": f"#{channel['name']}", - "type": "public", - } - assert resp.data["nextCursor"] == "cursor123" + assert len(results) == 3 + for i, ch in enumerate(channels): + expected_type = "private" if ch.get("is_private") else "public" + assert results[i] == { + "id": ch["id"], + "name": ch["name"], + "display": f"#{ch['name']}", + "type": expected_type, + } @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") - def test_slack_channels_with_cursor(self, mock_conversations_list): - """Test fetching next page of Slack channels with cursor.""" - channel = {"id": "C789", "name": "last-channel", "is_private": False} + def test_large_slack_channel_list(self, mock_conversations_list): + mock_channels = [ + {"id": f"C{i:04}", "name": f"channel-{i}", "is_private": i % 2 == 0} + for i in range(1, 1001) + ] mock_conversations_list.return_value.data = { "ok": True, - "channels": [channel], - "response_metadata": {}, + "channels": mock_channels, } - resp = self.get_success_response( - self.organization.slug, - self.integration.id, - qs_params={"cursor": "cursor123"}, - ) - assert resp.data["results"] == [ - { - "id": channel["id"], - "name": channel["name"], - "display": f"#{channel['name']}", - "type": "public", - } - ] - assert resp.data["nextCursor"] is None - call_kwargs = mock_conversations_list.call_args[1] - assert call_kwargs["cursor"] == "cursor123" + response = self.get_success_response(self.organization.slug, self.integration.id) + results = response.data["results"] + assert len(results) == 1000 + assert results[0]["id"] == "C0001" + assert results[0]["type"] == "public" + assert results[-1]["id"] == "C1000" + assert results[-1]["type"] == "private" class OrganizationIntegrationChannelsDiscordTest(OrganizationIntegrationChannelsTest): @@ -83,67 +78,33 @@ def setUp(self) -> None: external_id="1234567890", ) - @patch("sentry.integrations.discord.client.DiscordClient.get_cached") - def test_discord_channels_list(self, mock_get_cached): - """Test listing Discord channels with client-side pagination and caching.""" + @patch("sentry.integrations.discord.client.DiscordClient.get") + def test_discord_channels_list(self, mock_get): mock_channels = [ {"id": "123456", "name": "general", "type": 0}, {"id": "789012", "name": "announcements", "type": 5}, {"id": "345678", "name": "off-topic", "type": 0}, ] - mock_get_cached.return_value = mock_channels - response = self.get_success_response( - self.organization.slug, self.integration.id, qs_params={"limit": 2} - ) + mock_get.return_value = mock_channels + response = self.get_success_response(self.organization.slug, self.integration.id) results = response.data["results"] - assert len(results) == 2 - expected = [ - { - **mock_channels[0], - "display": f"#{mock_channels[0]['name']}", - "type": "text", - }, - { - **mock_channels[1], - "display": f"#{mock_channels[1]['name']}", - "type": "announcement", - }, - ] + DISCORD_CHANNEL_TYPES = { + 0: "text", + 5: "announcement", + 15: "forum", + } + expected = [] + for ch in mock_channels: + expected.append( + { + "id": ch["id"], + "name": ch["name"], + "display": f"#{ch['name']}", + "type": DISCORD_CHANNEL_TYPES.get(ch["type"], "unknown"), + } + ) assert results == expected - mock_get_cached.assert_called_once() - call_kwargs = mock_get_cached.call_args[1] - assert call_kwargs["cache_time"] == 60 - - @patch("sentry.integrations.discord.client.DiscordClient.get_cached") - def test_discord_channels_pagination(self, mock_get_cached): - """Test Discord channel pagination with offset cursor.""" - mock_get_cached.return_value = [ - {"id": "123456", "name": "general", "type": 0}, - {"id": "789012", "name": "announcements", "type": 5}, - {"id": "345678", "name": "off-topic", "type": 0}, - ] - response = self.get_success_response( - self.organization.slug, self.integration.id, qs_params={"cursor": "2", "limit": 2} - ) - assert len(response.data["results"]) == 1 - assert response.data["results"][0]["id"] == "345678" - assert response.data["nextCursor"] is None - - @patch("sentry.integrations.discord.client.DiscordClient.get_cached") - def test_discord_filters_non_messageable_channels(self, mock_get_cached): - """Test that Discord filters out non-messageable channel types.""" - mock_get_cached.return_value = [ - {"id": "123456", "name": "text-channel", "type": 0}, - {"id": "789012", "name": "voice-channel", "type": 2}, - {"id": "345678", "name": "category", "type": 4}, - {"id": "901234", "name": "announcement", "type": 5}, - ] - - response = self.get_success_response(self.organization.slug, self.integration.id) - - assert len(response.data["results"]) == 2 - assert response.data["results"][0]["type"] == "text" - assert response.data["results"][1]["type"] == "announcement" + mock_get.assert_called_once() class OrganizationIntegrationChannelsMsTeamsTest(OrganizationIntegrationChannelsTest): @@ -161,10 +122,8 @@ def setUp(self) -> None: }, ) - @patch("sentry.integrations.msteams.client.MsTeamsClient.get_cached") - def test_msteams_channels_list(self, mock_get_cached): - """Test listing Microsoft Teams channels with pagination and caching.""" - + @patch("sentry.integrations.msteams.client.MsTeamsClient.get") + def test_msteams_channels_list(self, mock_get): mock_channels = { "conversations": [ {"id": "19:channel1@thread.tacv2", "displayName": "Development"}, @@ -176,97 +135,35 @@ def test_msteams_channels_list(self, mock_get_cached): }, ] } - mock_get_cached.return_value = mock_channels - - response = self.get_success_response( - self.organization.slug, self.integration.id, qs_params={"limit": 2} - ) + mock_get.return_value = mock_channels + response = self.get_success_response(self.organization.slug, self.integration.id) results = response.data["results"] - - expected = [ - { - "id": ch["id"], - "name": ch["displayName"], - "display": ch["displayName"], - "type": ch.get("membershipType", "standard"), - } - for ch in mock_channels["conversations"][:2] - ] - + expected = [] + for ch in mock_channels["conversations"]: + expected.append( + { + "id": ch["id"], + "name": ch["displayName"], + "display": ch["displayName"], + "type": ch.get("membershipType", "standard"), + } + ) assert results == expected - assert response.data["nextCursor"] == "2" - mock_get_cached.assert_called_once() - call_kwargs = mock_get_cached.call_args[1] - assert call_kwargs["cache_time"] == 60 - - @patch("sentry.integrations.msteams.client.MsTeamsClient.get_cached") - def test_msteams_pagination(self, mock_get_cached): - """Test MS Teams channel pagination with offset cursor.""" - mock_get_cached.return_value = { - "conversations": [ - {"id": "19:ch1@thread.tacv2", "displayName": "First"}, - {"id": "19:ch2@thread.tacv2", "displayName": "Second"}, - {"id": "19:ch3@thread.tacv2", "displayName": "Third"}, - ] - } - - response = self.get_success_response( - self.organization.slug, self.integration.id, qs_params={"cursor": "2", "limit": 2} - ) - - assert len(response.data["results"]) == 1 - assert response.data["results"][0]["name"] == "Third" - assert response.data["nextCursor"] is None + mock_get.assert_called_once() class OrganizationIntegrationChannelsErrorTest(OrganizationIntegrationChannelsTest): def test_integration_not_found(self): - """Test 404 when integration doesn't exist.""" response = self.get_error_response(self.organization.slug, 9999, status_code=404) assert response.status_code == 404 def test_unsupported_provider(self): - """Test warning for unsupported integration provider.""" integration = self.create_integration( organization=self.organization, provider="github", name="GitHub", external_id="github:1", ) - response = self.get_success_response(self.organization.slug, integration.id) - assert response.data["results"] == [] - assert response.data["nextCursor"] is None assert "not supported" in response.data["warning"] - - def test_limit_validation(self): - """Test limit parameter validation and clamping.""" - integration = self.create_integration( - organization=self.organization, - provider="slack", - name="Slack Workspace", - external_id="TXXXXXXX1", - metadata={ - "access_token": "xoxb-token", - "installation_type": "born_as_bot", - }, - ) - - # Test max limit clamping - with patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") as m: - m.return_value.data = {"ok": True, "channels": [], "response_metadata": {}} - self.get_success_response( - self.organization.slug, integration.id, qs_params={"limit": 999} - ) - call_kwargs = m.call_args[1] - assert call_kwargs["limit"] == 200 - - # Test invalid limit defaults to 50 - with patch("sentry.integrations.slack.sdk_client.SlackSdkClient.conversations_list") as m: - m.return_value.data = {"ok": True, "channels": [], "response_metadata": {}} - self.get_success_response( - self.organization.slug, integration.id, qs_params={"limit": "invalid"} - ) - call_kwargs = m.call_args[1] - assert call_kwargs["limit"] == 50 From fb40b9a86dd1a94231a67b8146cdf1e16f704729 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Mon, 13 Oct 2025 13:33:03 +0200 Subject: [PATCH 06/10] fix types --- .../endpoints/organization_integration_channels.py | 12 +++++++----- .../test_organization_integration_channels.py | 9 ++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index 5adbb324ce485f..403b77e2feb3ef 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -7,14 +7,15 @@ from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus +from sentry.api.base import control_silo_endpoint from sentry.integrations.api.bases.organization_integrations import ( - RegionOrganizationIntegrationBaseEndpoint, + OrganizationIntegrationBaseEndpoint, ) from sentry.integrations.discord.client import DiscordClient from sentry.integrations.msteams.client import MsTeamsClient from sentry.integrations.services.integration import integration_service from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.organization import Organization +from sentry.organizations.services.organization import RpcUserOrganizationContext from sentry.shared_integrations.exceptions import ApiError @@ -121,7 +122,8 @@ def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[st return results -class OrganizationIntegrationChannelsEndpoint(RegionOrganizationIntegrationBaseEndpoint): +@control_silo_endpoint +class OrganizationIntegrationChannelsEndpoint(OrganizationIntegrationBaseEndpoint): publish_status = { "GET": ApiPublishStatus.PRIVATE, } @@ -130,7 +132,7 @@ class OrganizationIntegrationChannelsEndpoint(RegionOrganizationIntegrationBaseE def get( self, request: Request, - organization: Organization, + organization_context: RpcUserOrganizationContext, integration_id: int, **kwargs: Any, ) -> Response: @@ -138,7 +140,7 @@ def get( List all messaging channels for an integration. """ - integration = self.get_integration(organization.id, integration_id) + integration = self.get_integration(organization_context.organization.id, integration_id) try: match integration.provider: diff --git a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py index 0dfd15d7d57426..0e9bd9085bd3be 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_integration_channels.py @@ -1,6 +1,8 @@ +from typing import cast from unittest.mock import patch from sentry.testutils.cases import APITestCase +from sentry.testutils.silo import control_silo_test class OrganizationIntegrationChannelsTest(APITestCase): @@ -12,6 +14,7 @@ def setUp(self) -> None: self.login_as(user=self.user) +@control_silo_test class OrganizationIntegrationChannelsSlackTest(OrganizationIntegrationChannelsTest): def setUp(self) -> None: super().setUp() @@ -68,6 +71,7 @@ def test_large_slack_channel_list(self, mock_conversations_list): assert results[-1]["type"] == "private" +@control_silo_test class OrganizationIntegrationChannelsDiscordTest(OrganizationIntegrationChannelsTest): def setUp(self) -> None: super().setUp() @@ -95,18 +99,20 @@ def test_discord_channels_list(self, mock_get): } expected = [] for ch in mock_channels: + channel_type = cast(int, ch["type"]) # mypy: ensure int key for map lookup expected.append( { "id": ch["id"], "name": ch["name"], "display": f"#{ch['name']}", - "type": DISCORD_CHANNEL_TYPES.get(ch["type"], "unknown"), + "type": DISCORD_CHANNEL_TYPES.get(channel_type, "unknown"), } ) assert results == expected mock_get.assert_called_once() +@control_silo_test class OrganizationIntegrationChannelsMsTeamsTest(OrganizationIntegrationChannelsTest): def setUp(self) -> None: super().setUp() @@ -152,6 +158,7 @@ def test_msteams_channels_list(self, mock_get): mock_get.assert_called_once() +@control_silo_test class OrganizationIntegrationChannelsErrorTest(OrganizationIntegrationChannelsTest): def test_integration_not_found(self): response = self.get_error_response(self.organization.slug, 9999, status_code=404) From cfc551c7d69d015d68541e0c28d62bba1ba6ebb5 Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Mon, 13 Oct 2025 14:09:16 +0200 Subject: [PATCH 07/10] add int channels to silo url patterns --- static/app/data/controlsiloUrlPatterns.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/static/app/data/controlsiloUrlPatterns.ts b/static/app/data/controlsiloUrlPatterns.ts index 6b3a90f149ec92..5d85049ce3f38b 100644 --- a/static/app/data/controlsiloUrlPatterns.ts +++ b/static/app/data/controlsiloUrlPatterns.ts @@ -62,6 +62,7 @@ const patterns: RegExp[] = [ new RegExp('^api/0/organizations/[^/]+/audit-logs/$'), new RegExp('^api/0/organizations/[^/]+/integrations/$'), new RegExp('^api/0/organizations/[^/]+/integrations/[^/]+/$'), + new RegExp('^api/0/organizations/[^/]+/integrations/[^/]+/channels/$'), new RegExp('^api/0/organizations/[^/]+/sentry-app-installations/$'), new RegExp('^api/0/organizations/[^/]+/sentry-apps/$'), new RegExp('^api/0/organizations/[^/]+/sentry-app-components/$'), From 1488f2a92ab42405802f1eba3b7122047dffed4e Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 14 Oct 2025 07:00:46 +0200 Subject: [PATCH 08/10] cursor feedback --- .../organization_integration_channels.py | 184 ++++++++++++++---- 1 file changed, 143 insertions(+), 41 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index 403b77e2feb3ef..e53ea15a6d279d 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging from typing import Any from rest_framework.request import Request @@ -18,45 +19,71 @@ from sentry.organizations.services.organization import RpcUserOrganizationContext from sentry.shared_integrations.exceptions import ApiError +logger = logging.getLogger(__name__) + def _slack_list_channels(*, integration_id: int) -> list[dict[str, Any]]: """ List Slack channels for a given integration. - NOTE: We are NOT using pagination here because some organizations have - more than 1000 channels. Searching client-side might miss channels beyond - the first 1000, so we fetch the maximum allowed by Slack and rely on - validation when saving. + Fetches up to the Slack API limit (1000 channels). + Handles authentication via integration context and validates responses. """ - from sentry.integrations.slack.sdk_client import SlackSdkClient - client = SlackSdkClient(integration_id=integration_id) - resp = client.conversations_list( - exclude_archived=True, - types="public_channel,private_channel", - limit=1000, # Max Slack allows (see https://docs.slack.dev/reference/methods/conversations.list/) - ).data + integration = integration_service.get_integration(integration_id=integration_id) + if not integration: + raise ApiError("Slack integration not found") - resp_data: dict[str, Any] = resp if isinstance(resp, dict) else {} - channels = resp_data.get("channels", []) or [] + client = SlackSdkClient(integration=integration) + + try: + response = client.conversations_list( + exclude_archived=True, + types="public_channel,private_channel", + limit=1000, # Max allowed by Slack API + ) + resp_data: dict[str, Any] = response.data if isinstance(response.data, dict) else {} + except Exception as e: + logger.warning("Slack API request failed for integration_id=%s: %s", integration_id, e) + return [] + + # Validate structure + raw_channels = resp_data.get("channels") + if not isinstance(raw_channels, list): + logger.warning( + "Unexpected Slack API response structure for integration_id=%s: %r", + integration_id, + resp_data, + ) + return [] results: list[dict[str, Any]] = [] - for ch in channels: - name = ch.get("name") + for ch in raw_channels: + if not isinstance(ch, dict): + continue + + ch_id = ch.get("id") + ch_name = ch.get("name") + if not ch_id or not ch_name: + continue + + is_private = bool(ch.get("is_private")) + name_str = str(ch_name) + results.append( { - "id": ch.get("id"), - "name": name, - "display": f"#{name}", - "type": "private" if bool(ch.get("is_private")) else "public", + "id": str(ch_id), + "name": name_str, + "display": f"#{name_str}", + "type": "private" if is_private else "public", } ) return results -def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: +def _discord_list_channels(*, integration_id: int, guild_id: str) -> list[dict[str, Any]]: """ List Discord channels for a given guild that can receive messages. @@ -70,22 +97,57 @@ def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: 15: "forum", } - client = DiscordClient() - channels = ( - client.get(f"/guilds/{guild_id}/channels", headers=client.prepare_auth_header()) or [] - ) + integration = integration_service.get_integration(integration_id=integration_id) + if not integration: + raise ApiError("Discord integration not found") + + client = DiscordClient(integration=integration) - selectable_types = DISCORD_CHANNEL_TYPES.keys() - filtered = [ch for ch in channels if ch.get("type") in selectable_types] + try: + raw_resp = client.get( + f"/guilds/{guild_id}/channels", + headers=client.prepare_auth_header(), + ) + except Exception as e: + logger.warning( + "Discord API request failed for integration_id=%s, guild_id=%s: %s", + integration_id, + guild_id, + e, + ) + return [] + + if not isinstance(raw_resp, list): + logger.warning( + "Unexpected Discord API response for integration_id=%s, guild_id=%s: %r", + integration_id, + guild_id, + raw_resp, + ) + return [] + selectable_types = set(DISCORD_CHANNEL_TYPES.keys()) results: list[dict[str, Any]] = [] - for ch in filtered: + + for item in raw_resp: + if not isinstance(item, dict): + continue + + ch_type = item.get("type") + if not isinstance(ch_type, int) or ch_type not in selectable_types: + continue + + ch_id = item.get("id") + ch_name = item.get("name") + if not ch_id or not ch_name: + continue + results.append( { - "id": ch["id"], - "name": ch["name"], - "display": f"#{ch['name']}", - "type": DISCORD_CHANNEL_TYPES.get(ch["type"], "unknown"), + "id": str(ch_id), + "name": str(ch_name), + "display": f"#{ch_name}", + "type": DISCORD_CHANNEL_TYPES.get(ch_type, "unknown"), } ) @@ -94,28 +156,66 @@ def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[str, Any]]: """ - List Microsoft Teams channels for a team. + List Microsoft Teams channels for a given team. The Teams API returns all channels at once. Only standard and private channels are included. """ integration = integration_service.get_integration(integration_id=integration_id) - if integration is None: + if not integration: raise ApiError("Microsoft Teams integration not found") client = MsTeamsClient(integration) - channels_resp = client.get(client.CHANNEL_URL % team_id) or {} - channels = channels_resp.get("conversations") or [] + + try: + raw_resp = client.get(client.CHANNEL_URL % team_id) + except Exception as e: + logger.warning( + "Microsoft Teams API request failed for integration_id=%s, team_id=%s: %s", + integration_id, + team_id, + e, + ) + return [] + + if not isinstance(raw_resp, dict): + logger.warning( + "Unexpected Microsoft Teams API response for integration_id=%s, team_id=%s: %r", + integration_id, + team_id, + raw_resp, + ) + return [] + + raw_channels = raw_resp.get("conversations") + if not isinstance(raw_channels, list): + logger.warning( + "Missing or invalid 'conversations' in Teams API response for integration_id=%s, team_id=%s: %r", + integration_id, + team_id, + raw_resp, + ) + return [] results: list[dict[str, Any]] = [] - for ch in channels: + for item in raw_channels: + if not isinstance(item, dict): + continue + + ch_id = item.get("id") + display_name = item.get("displayName") + if not ch_id or not display_name: + continue + + ch_type = str(item.get("membershipType") or "standard") + results.append( { - "id": ch["id"], - "name": ch["displayName"], - "display": ch["displayName"], - "type": ch.get("membershipType", "standard"), # "standard" or "private" + "id": str(ch_id), + "name": str(display_name), + "display": str(display_name), + "type": ch_type, # "standard" or "private" } ) @@ -147,7 +247,9 @@ def get( case IntegrationProviderSlug.SLACK.value: results = _slack_list_channels(integration_id=integration.id) case IntegrationProviderSlug.DISCORD.value: - results = _discord_list_channels(guild_id=str(integration.external_id)) + results = _discord_list_channels( + integration_id=integration.id, guild_id=str(integration.external_id) + ) case IntegrationProviderSlug.MSTEAMS.value: results = _msteams_list_channels( integration_id=integration.id, From f016f7f7c6286370c422eefdc41bcdd11d831f2c Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 14 Oct 2025 07:41:47 +0200 Subject: [PATCH 09/10] fix integration initialization --- .../organization_integration_channels.py | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index e53ea15a6d279d..717a9b786df9c7 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -29,13 +29,10 @@ def _slack_list_channels(*, integration_id: int) -> list[dict[str, Any]]: Fetches up to the Slack API limit (1000 channels). Handles authentication via integration context and validates responses. """ - from sentry.integrations.slack.sdk_client import SlackSdkClient - integration = integration_service.get_integration(integration_id=integration_id) - if not integration: - raise ApiError("Slack integration not found") + from sentry.integrations.slack.sdk_client import SlackSdkClient - client = SlackSdkClient(integration=integration) + client = SlackSdkClient(integration_id=integration_id) try: response = client.conversations_list( @@ -83,7 +80,7 @@ def _slack_list_channels(*, integration_id: int) -> list[dict[str, Any]]: return results -def _discord_list_channels(*, integration_id: int, guild_id: str) -> list[dict[str, Any]]: +def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: """ List Discord channels for a given guild that can receive messages. @@ -97,11 +94,7 @@ def _discord_list_channels(*, integration_id: int, guild_id: str) -> list[dict[s 15: "forum", } - integration = integration_service.get_integration(integration_id=integration_id) - if not integration: - raise ApiError("Discord integration not found") - - client = DiscordClient(integration=integration) + client = DiscordClient() try: raw_resp = client.get( @@ -110,8 +103,7 @@ def _discord_list_channels(*, integration_id: int, guild_id: str) -> list[dict[s ) except Exception as e: logger.warning( - "Discord API request failed for integration_id=%s, guild_id=%s: %s", - integration_id, + "Discord API request failed for guild_id=%s: %s", guild_id, e, ) @@ -119,8 +111,7 @@ def _discord_list_channels(*, integration_id: int, guild_id: str) -> list[dict[s if not isinstance(raw_resp, list): logger.warning( - "Unexpected Discord API response for integration_id=%s, guild_id=%s: %r", - integration_id, + "Unexpected Discord API response for guild_id=%s: %r", guild_id, raw_resp, ) @@ -247,9 +238,7 @@ def get( case IntegrationProviderSlug.SLACK.value: results = _slack_list_channels(integration_id=integration.id) case IntegrationProviderSlug.DISCORD.value: - results = _discord_list_channels( - integration_id=integration.id, guild_id=str(integration.external_id) - ) + results = _discord_list_channels(guild_id=str(integration.external_id)) case IntegrationProviderSlug.MSTEAMS.value: results = _msteams_list_channels( integration_id=integration.id, From 8785006b97e6a09e6c56f63baabd19918d30c38d Mon Sep 17 00:00:00 2001 From: Priscila Oliveira Date: Tue, 14 Oct 2025 09:21:35 +0200 Subject: [PATCH 10/10] cursor feedback --- .../organization_integration_channels.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/sentry/integrations/api/endpoints/organization_integration_channels.py b/src/sentry/integrations/api/endpoints/organization_integration_channels.py index 717a9b786df9c7..616b9b5caf952e 100644 --- a/src/sentry/integrations/api/endpoints/organization_integration_channels.py +++ b/src/sentry/integrations/api/endpoints/organization_integration_channels.py @@ -13,8 +13,9 @@ OrganizationIntegrationBaseEndpoint, ) from sentry.integrations.discord.client import DiscordClient +from sentry.integrations.models import Integration from sentry.integrations.msteams.client import MsTeamsClient -from sentry.integrations.services.integration import integration_service +from sentry.integrations.services.integration.model import RpcIntegration from sentry.integrations.types import IntegrationProviderSlug from sentry.organizations.services.organization import RpcUserOrganizationContext from sentry.shared_integrations.exceptions import ApiError @@ -145,7 +146,9 @@ def _discord_list_channels(*, guild_id: str) -> list[dict[str, Any]]: return results -def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[str, Any]]: +def _msteams_list_channels( + *, integration: Integration | RpcIntegration, team_id: str +) -> list[dict[str, Any]]: """ List Microsoft Teams channels for a given team. @@ -153,10 +156,6 @@ def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[st Only standard and private channels are included. """ - integration = integration_service.get_integration(integration_id=integration_id) - if not integration: - raise ApiError("Microsoft Teams integration not found") - client = MsTeamsClient(integration) try: @@ -164,7 +163,7 @@ def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[st except Exception as e: logger.warning( "Microsoft Teams API request failed for integration_id=%s, team_id=%s: %s", - integration_id, + integration.id, team_id, e, ) @@ -173,7 +172,7 @@ def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[st if not isinstance(raw_resp, dict): logger.warning( "Unexpected Microsoft Teams API response for integration_id=%s, team_id=%s: %r", - integration_id, + integration.id, team_id, raw_resp, ) @@ -183,7 +182,7 @@ def _msteams_list_channels(*, integration_id: int, team_id: str) -> list[dict[st if not isinstance(raw_channels, list): logger.warning( "Missing or invalid 'conversations' in Teams API response for integration_id=%s, team_id=%s: %r", - integration_id, + integration.id, team_id, raw_resp, ) @@ -241,7 +240,7 @@ def get( results = _discord_list_channels(guild_id=str(integration.external_id)) case IntegrationProviderSlug.MSTEAMS.value: results = _msteams_list_channels( - integration_id=integration.id, + integration=integration, team_id=str(integration.external_id), ) case _: