Skip to content

Commit c570f2e

Browse files
feat: Add comprehensive test mode support for Slack integration
- Handle production vs test Slack user IDs properly - Skip production IDs (alphabetic only) in test mode - Add test channel configuration via environment variables - Improve error handling for user_not_found and already_in_channel - Add slack_user_id parameter passing through the workflow - Update templating to handle test mode user display 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent a389c0b commit c570f2e

File tree

7 files changed

+226
-132
lines changed

7 files changed

+226
-132
lines changed

src/firefighter/slack/models/incident_channel.py

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

33
import logging
4+
import os
45
import textwrap
56
from typing import TYPE_CHECKING, Any
67

@@ -112,7 +113,7 @@ def set_incident_channel_topic(
112113

113114
@slack_client
114115
def invite_users(
115-
self, users_mapped: list[User], client: WebClient = DefaultWebClient
116+
self, users_mapped: list[User], client: WebClient = DefaultWebClient, slack_user_id: str | None = None
116117
) -> None:
117118
"""Invite users to the conversation, if they have a Slack user linked and are active.
118119
@@ -121,7 +122,7 @@ def invite_users(
121122
users_with_slack: list[User] = self._get_active_slack_users(users_mapped)
122123
users_with_slack = list(set(users_with_slack)) # Remove duplicates
123124

124-
user_id_list: set[str] = self._get_slack_id_list(users_with_slack)
125+
user_id_list: set[str] = self._get_slack_id_list(users_with_slack, slack_user_id)
125126
if not user_id_list:
126127
logger.info(f"No users to invite to the conversation {self}.")
127128
return
@@ -161,19 +162,67 @@ def _get_active_slack_users(users_mapped: list[User]) -> list[User]:
161162
return users_with_slack
162163

163164
@staticmethod
164-
def _get_slack_id_list(users_with_slack: list[User]) -> set[str]:
165-
return {u.slack_user.slack_id for u in users_with_slack if u.slack_user}
165+
def _get_slack_id_list(users_with_slack: list[User], slack_user_id: str | None = None) -> set[str]:
166+
from firefighter.firefighter.settings.settings_utils import config # noqa: PLC0415
167+
168+
test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true"
169+
slack_ids = set()
170+
171+
for user in users_with_slack:
172+
if not user:
173+
continue
174+
175+
# In test mode, handle user ID mapping more carefully
176+
if test_mode and hasattr(user, 'slack_user') and user.slack_user and user.slack_user.slack_id:
177+
stored_slack_id = user.slack_user.slack_id
178+
179+
# Skip users with old production IDs that don't exist in test workspace
180+
# Old production IDs are alphabetic only, while test IDs contain numbers
181+
if stored_slack_id.startswith('U') and not any(c.isdigit() for c in stored_slack_id):
182+
logger.info(f"Test mode: Skipping user {user.id} with production slack_id {stored_slack_id}")
183+
continue
184+
else:
185+
# Valid test ID, use it
186+
slack_ids.add(stored_slack_id)
187+
elif hasattr(user, 'slack_user') and user.slack_user and user.slack_user.slack_id:
188+
# Non-test mode: use stored slack_id
189+
slack_ids.add(user.slack_user.slack_id)
190+
191+
# In test mode, ensure the current user (from the event) is included if provided
192+
if test_mode and slack_user_id and slack_user_id not in slack_ids:
193+
logger.info(f"Test mode: Adding current user's Slack ID {slack_user_id} to invitation list")
194+
slack_ids.add(slack_user_id)
195+
196+
return slack_ids
166197

167198
def _invite_users_to_conversation(
168199
self, user_id_list: set[str], client: WebClient
169200
) -> set[str]:
201+
# In test mode, filter out invalid user IDs to prevent errors
202+
from firefighter.firefighter.settings.settings_utils import config # noqa: PLC0415
203+
test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true"
204+
205+
if test_mode:
206+
logger.info(f"Test mode: Attempting to invite users with IDs: {user_id_list}")
207+
170208
try:
171209
done = self._invite_users_with_slack_id(user_id_list, client)
172-
except SlackApiError:
210+
except SlackApiError as e:
173211
logger.warning(
174212
f"Could not batch import Slack users! Slack IDs: {user_id_list}",
175213
exc_info=True,
176214
)
215+
216+
# In test mode, if batch invite fails due to user_not_found, skip individual retries
217+
if test_mode and e.response.get("error") == "user_not_found":
218+
logger.info("Test mode: Skipping individual invitations for user_not_found errors")
219+
return set()
220+
221+
# If batch invite fails due to already_in_channel, consider all users as successfully invited
222+
if e.response.get("error") == "already_in_channel":
223+
logger.debug("All users already in channel - this is expected")
224+
return user_id_list
225+
177226
done = self._invite_users_to_conversation_individually(user_id_list, client)
178227
return done
179228

@@ -193,14 +242,28 @@ def _invite_users_with_slack_id(
193242
def _invite_users_to_conversation_individually(
194243
self, slack_user_ids: set[str], client: WebClient
195244
) -> set[str]:
245+
from firefighter.firefighter.settings.settings_utils import config # noqa: PLC0415
246+
test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true"
247+
196248
done = set()
197249
for slack_user_id in slack_user_ids:
198250
try:
199251
self._invite_users_with_slack_id({slack_user_id}, client)
200252
done.add(slack_user_id)
201-
except SlackApiError:
253+
except SlackApiError as e:
254+
error_type = e.response.get("error", "unknown_error")
255+
256+
if test_mode and error_type == "user_not_found":
257+
logger.info(f"Test mode: Skipping user_not_found error for user ID {slack_user_id}")
258+
continue
259+
260+
if error_type == "already_in_channel":
261+
logger.debug(f"User {slack_user_id} is already in channel - this is expected")
262+
done.add(slack_user_id) # Consider as successfully "invited"
263+
continue
264+
202265
logger.warning(
203-
f"Could not import Slack user! User ID: {slack_user_id}",
266+
f"Could not import Slack user! User ID: {slack_user_id} - {error_type}",
204267
exc_info=True,
205268
)
206269
return done

src/firefighter/slack/signals/create_incident_conversation.py

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from __future__ import annotations
88

99
import logging
10+
import os
1011
from typing import TYPE_CHECKING, Any
1112

1213
from django.dispatch import receiver
@@ -21,6 +22,8 @@
2122
from firefighter.slack.models.conversation import Conversation
2223
from firefighter.slack.models.incident_channel import IncidentChannel
2324
from firefighter.slack.models.user import SlackUser
25+
from firefighter.slack.utils.test_channels import get_or_create_test_conversation
26+
from firefighter.slack.slack_app import DefaultWebClient
2427
from firefighter.slack.rules import (
2528
should_publish_in_general_channel,
2629
should_publish_in_it_deploy_channel,
@@ -35,7 +38,7 @@
3538

3639

3740
@receiver(signal=create_incident_conversation)
38-
def create_incident_slack_conversation(
41+
def create_incident_slack_conversation( # noqa: PLR0912, PLR0915
3942
incident: Incident,
4043
*_args: Any,
4144
**_kwargs: Any,
@@ -73,34 +76,69 @@ def create_incident_slack_conversation(
7376
channel.set_incident_channel_topic()
7477

7578
# Add the person that opened the incident in the channel
79+
invited_creator = False
80+
creator_slack_id = None
7681
if (
7782
incident.created_by
7883
and hasattr(incident.created_by, "slack_user")
7984
and incident.created_by.slack_user
8085
and incident.created_by.slack_user.slack_id
8186
):
87+
creator_slack_id = incident.created_by.slack_user.slack_id
8288
try:
89+
# Check if the invitation was successful by checking if the user is now a member
90+
members_before = set(channel.incident.members.all())
8391
channel.invite_users([incident.created_by])
92+
members_after = set(channel.incident.members.all())
93+
94+
# If the user was added to members, invitation succeeded
95+
if incident.created_by in members_after and incident.created_by not in members_before:
96+
invited_creator = True
97+
logger.info(f"Successfully invited creator {creator_slack_id}")
98+
else:
99+
logger.warning(f"Creator {creator_slack_id} was not added to incident members - invitation may have failed")
100+
84101
except SlackApiError:
85102
logger.warning(
86-
f"Could not import Slack opener user! Slack ID: {incident.created_by.slack_user.slack_id}, User {incident.created_by}, Channel ID {new_channel_id}",
103+
f"Could not import Slack opener user! Slack ID: {creator_slack_id}, User {incident.created_by}, Channel ID {new_channel_id}",
87104
exc_info=True,
88105
)
89106
else:
90107
logger.warning("Could not find user Slack ID for opener_user!")
91108

109+
# In test mode, if creator invitation failed, try to get the Slack user ID from kwargs
110+
test_mode = os.getenv("TEST_MODE", "False").lower() == "true"
111+
slack_user_id = _kwargs.get("slack_user_id")
112+
113+
logger.info(f"Test mode: {test_mode}, invited_creator: {invited_creator}")
114+
logger.info(f"Creator stored slack_id: {creator_slack_id}, event slack_user_id: {slack_user_id}")
115+
if test_mode and not invited_creator and slack_user_id:
116+
logger.info(f"Test mode: Attempting to invite creator using event Slack ID {slack_user_id}")
117+
try:
118+
# Invite directly using the Slack user ID from the event
119+
from firefighter.slack.slack_app import slack_client
120+
@slack_client
121+
def invite_test_user(channel_instance, user_id, client=DefaultWebClient):
122+
return channel_instance._invite_users_to_conversation({user_id}, client)
123+
124+
invite_test_user(channel, slack_user_id)
125+
logger.info(f"Test mode: Successfully invited creator via Slack ID {slack_user_id}")
126+
except SlackApiError:
127+
logger.warning(
128+
f"Test mode: Could not invite creator via Slack ID {slack_user_id}",
129+
exc_info=True,
130+
)
131+
92132
# Send message in the created channel
93133
channel.send_message_and_save(
94-
SlackMessageIncidentDeclaredAnnouncement(incident), pin=True
134+
SlackMessageIncidentDeclaredAnnouncement(incident, slack_user_id=slack_user_id), pin=True
95135
)
96136

97137
# Post in general channel #tech-incidents if needed
98138
if should_publish_in_general_channel(incident, incident_update=None):
99-
announcement_general = SlackMessageIncidentDeclaredAnnouncementGeneral(incident)
139+
announcement_general = SlackMessageIncidentDeclaredAnnouncementGeneral(incident, slack_user_id=slack_user_id)
100140

101-
tech_incidents_conversation = Conversation.objects.get_or_none(
102-
tag="tech_incidents"
103-
)
141+
tech_incidents_conversation = get_or_create_test_conversation("tech_incidents")
104142
if tech_incidents_conversation:
105143
tech_incidents_conversation.send_message_and_save(announcement_general)
106144
else:
@@ -112,14 +150,14 @@ def create_incident_slack_conversation(
112150
users_list: list[User] = incident.build_invite_list()
113151

114152
# Invite all users
115-
incident.conversation.invite_users(users_list)
153+
incident.conversation.invite_users(users_list, slack_user_id=slack_user_id)
116154

117155
# Post in #it-deploy if needed
118156
if should_publish_in_it_deploy_channel(incident):
119157
announcement_it_deploy = SlackMessageDeployWarning(incident)
120158
announcement_it_deploy.id = f"{announcement_it_deploy.id}_{incident.id}"
121159

122-
it_deploy_conversation = Conversation.objects.get_or_none(tag="it_deploy")
160+
it_deploy_conversation = get_or_create_test_conversation("it_deploy")
123161
if it_deploy_conversation:
124162
it_deploy_conversation.send_message_and_save(announcement_it_deploy)
125163
else:

src/firefighter/slack/slack_templating.py

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

33
import logging
4+
import os
45
from functools import cache
56
from textwrap import TextWrapper
67
from typing import TYPE_CHECKING, Any, TypeVar
@@ -61,13 +62,42 @@ def date_time(date: datetime | None) -> str:
6162
return localtime(date).strftime("%Y-%m-%d %H:%M")
6263

6364

64-
def user_slack_handle_or_name(user: User | None) -> str:
65-
"""Returns the Slack handle of the user in Slack MD format (`<@SLACK_ID>`) or the user full name."""
65+
def user_slack_handle_or_name(user: User | None, slack_user_id: str | None = None) -> str:
66+
"""Returns the Slack handle of the user in Slack MD format (`<@SLACK_ID>`) or the user full name.
67+
68+
Args:
69+
user: The user to display
70+
slack_user_id: Optional Slack user ID from current event context (used in TEST_MODE for the current action performer only)
71+
72+
Returns:
73+
Slack handle format '<@USER_ID>' in production or test mode, or user full name as fallback
74+
"""
6675
if user is None:
6776
return "∅"
6877

69-
if hasattr(user, "slack_user") and user.slack_user:
78+
# In test mode: if slack_user_id is provided, it's only for the current action performer
79+
# For other users (assigned to roles), use their stored slack_id if valid or fallback to name
80+
from firefighter.firefighter.settings.settings_utils import config # noqa: PLC0415
81+
test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true"
82+
if test_mode and slack_user_id:
83+
# This is specifically for the "Updated by" context where we use the current action performer's ID
84+
return f"<@{slack_user_id}>"
85+
86+
# In test mode: check if the user has a valid slack_id (not production ID)
87+
if test_mode and hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id:
88+
# Skip production IDs that don't exist in test workspace - fallback to user name
89+
if user.slack_user.slack_id.startswith('U') and len(user.slack_user.slack_id) >= 9:
90+
# This looks like a production ID, fallback to user name in test mode
91+
return user.full_name
92+
else:
93+
# Valid test environment slack_id
94+
return f"<@{user.slack_user.slack_id}>"
95+
96+
# In production: use the stored user.slack_user.slack_id from database
97+
if hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id:
7098
return f"<@{user.slack_user.slack_id}>"
99+
100+
# Fallback to user full name
71101
return user.full_name
72102

73103

0 commit comments

Comments
 (0)