Skip to content

Commit 49f4da0

Browse files
authored
chore(slack): improve performance of /sentry link command (#104019)
1 parent bb72275 commit 49f4da0

File tree

1 file changed

+50
-17
lines changed

1 file changed

+50
-17
lines changed

src/sentry/integrations/slack/webhooks/command.py

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import logging
4+
from collections.abc import Iterable
45

56
from rest_framework import status
67
from rest_framework.request import Request
@@ -9,7 +10,6 @@
910
from sentry.api.api_owners import ApiOwner
1011
from sentry.api.api_publish_status import ApiPublishStatus
1112
from sentry.api.base import region_silo_endpoint
12-
from sentry.api.helpers.teams import is_team_admin
1313
from sentry.integrations.models.external_actor import ExternalActor
1414
from sentry.integrations.slack.message_builder.disconnected import SlackDisconnectedMessageBuilder
1515
from sentry.integrations.slack.requests.base import SlackDMRequest, SlackRequestError
@@ -18,8 +18,8 @@
1818
from sentry.integrations.slack.views.link_team import build_team_linking_url
1919
from sentry.integrations.slack.views.unlink_team import build_team_unlinking_url
2020
from sentry.integrations.types import ExternalProviders
21-
from sentry.models.organization import Organization
2221
from sentry.models.organizationmember import OrganizationMember
22+
from sentry.models.organizationmemberteam import OrganizationMemberTeam
2323

2424
_logger = logging.getLogger("sentry.integration.slack.bot-commands")
2525

@@ -44,15 +44,27 @@
4444
NO_CHANNEL_ID_MESSAGE = "Could not identify the Slack channel ID. Please try again."
4545

4646

47-
def is_team_linked_to_channel(organization: Organization, slack_request: SlackDMRequest) -> bool:
48-
"""Check if a Slack channel already has a team linked to it"""
49-
return ExternalActor.objects.filter(
50-
organization_id=organization.id,
51-
integration_id=slack_request.integration.id,
52-
provider=ExternalProviders.SLACK.value,
53-
external_name=slack_request.channel_name,
54-
external_id=slack_request.channel_id,
55-
).exists()
47+
def get_orgs_with_teams_linked_to_channel(
48+
organization_ids: list[int], slack_request: SlackDMRequest
49+
) -> set[int]:
50+
"""Get the organizations with teams linked to a Slack channel"""
51+
return set(
52+
ExternalActor.objects.filter(
53+
organization_id__in=organization_ids,
54+
integration_id=slack_request.integration.id,
55+
provider=ExternalProviders.SLACK.value,
56+
external_name=slack_request.channel_name,
57+
external_id=slack_request.channel_id,
58+
).values_list("organization_id", flat=True)
59+
)
60+
61+
62+
def get_team_admin_member_ids(org_members: Iterable[OrganizationMember]) -> set[int]:
63+
return set(
64+
OrganizationMemberTeam.objects.filter(
65+
organizationmember_id__in=[om.id for om in org_members], role="admin"
66+
).values_list("organizationmember_id", flat=True)
67+
)
5668

5769

5870
@region_silo_endpoint
@@ -91,10 +103,17 @@ def link_team(self, slack_request: SlackDMRequest) -> Response:
91103
integration, identity_user
92104
)
93105

106+
# Batch check for team admin roles to avoid N+1 queries
107+
team_admin_member_ids = get_team_admin_member_ids(organization_memberships)
108+
94109
has_valid_role = False
95110
for organization_membership in organization_memberships:
96-
if is_valid_role(organization_membership) or is_team_admin(organization_membership):
111+
if (
112+
is_valid_role(organization_membership)
113+
or organization_membership.id in team_admin_member_ids
114+
):
97115
has_valid_role = True
116+
break
98117

99118
if not has_valid_role:
100119
return self.reply(slack_request, INSUFFICIENT_ROLE_MESSAGE)
@@ -128,15 +147,29 @@ def unlink_team(self, slack_request: SlackDMRequest) -> Response:
128147
integration, identity_user
129148
)
130149

150+
# Batch check which organizations have teams linked to this channel
151+
linked_org_ids = get_orgs_with_teams_linked_to_channel(
152+
[om.organization_id for om in organization_memberships], slack_request
153+
)
154+
155+
if not linked_org_ids:
156+
return self.reply(slack_request, TEAM_NOT_LINKED_MESSAGE)
157+
158+
# Batch check for team admin roles to avoid N+1 queries
159+
team_admin_member_ids = get_team_admin_member_ids(organization_memberships)
160+
161+
# Find an organization where user has both a linked team AND sufficient permissions
131162
found: OrganizationMember | None = None
132163
for organization_membership in organization_memberships:
133-
if is_team_linked_to_channel(organization_membership.organization, slack_request):
134-
found = organization_membership
164+
if organization_membership.organization_id in linked_org_ids:
165+
if (
166+
is_valid_role(organization_membership)
167+
or organization_membership.id in team_admin_member_ids
168+
):
169+
found = organization_membership
170+
break
135171

136172
if not found:
137-
return self.reply(slack_request, TEAM_NOT_LINKED_MESSAGE)
138-
139-
if not is_valid_role(found) and not is_team_admin(found):
140173
return self.reply(slack_request, INSUFFICIENT_ROLE_MESSAGE)
141174

142175
if not slack_request.user_id:

0 commit comments

Comments
 (0)