diff --git a/src/firefighter/slack/models/incident_channel.py b/src/firefighter/slack/models/incident_channel.py index e4feebe2..6e4c4d6e 100644 --- a/src/firefighter/slack/models/incident_channel.py +++ b/src/firefighter/slack/models/incident_channel.py @@ -112,7 +112,7 @@ def set_incident_channel_topic( @slack_client def invite_users( - self, users_mapped: list[User], client: WebClient = DefaultWebClient + self, users_mapped: list[User], client: WebClient = DefaultWebClient, slack_user_id: str | None = None ) -> None: """Invite users to the conversation, if they have a Slack user linked and are active. @@ -121,7 +121,7 @@ def invite_users( users_with_slack: list[User] = self._get_active_slack_users(users_mapped) users_with_slack = list(set(users_with_slack)) # Remove duplicates - user_id_list: set[str] = self._get_slack_id_list(users_with_slack) + user_id_list: set[str] = self._get_slack_id_list(users_with_slack, slack_user_id) if not user_id_list: logger.info(f"No users to invite to the conversation {self}.") return @@ -161,19 +161,70 @@ def _get_active_slack_users(users_mapped: list[User]) -> list[User]: return users_with_slack @staticmethod - def _get_slack_id_list(users_with_slack: list[User]) -> set[str]: - return {u.slack_user.slack_id for u in users_with_slack if u.slack_user} + def _get_slack_id_list(users_with_slack: list[User], slack_user_id: str | None = None) -> set[str]: + from firefighter.firefighter.settings.settings_utils import ( + config, + ) + + test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true" + slack_ids = set() + + for user in users_with_slack: + if not user: + continue + + # In test mode, handle user ID mapping more carefully + if test_mode and hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id: + stored_slack_id = user.slack_user.slack_id + + # Skip users with old production IDs that don't exist in test workspace + # Old production IDs are alphabetic only, while test IDs contain numbers + if stored_slack_id.startswith("U") and not any(c.isdigit() for c in stored_slack_id): + logger.info(f"Test mode: Skipping user {user.id} with production slack_id {stored_slack_id}") + continue + # Valid test ID, use it + slack_ids.add(stored_slack_id) + elif hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id: + # Non-test mode: use stored slack_id + slack_ids.add(user.slack_user.slack_id) + + # In test mode, ensure the current user (from the event) is included if provided + if test_mode and slack_user_id and slack_user_id not in slack_ids: + logger.info(f"Test mode: Adding current user's Slack ID {slack_user_id} to invitation list") + slack_ids.add(slack_user_id) + + return slack_ids def _invite_users_to_conversation( self, user_id_list: set[str], client: WebClient ) -> set[str]: + # In test mode, filter out invalid user IDs to prevent errors + from firefighter.firefighter.settings.settings_utils import ( + config, + ) + test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true" + + if test_mode: + logger.info(f"Test mode: Attempting to invite users with IDs: {user_id_list}") + try: done = self._invite_users_with_slack_id(user_id_list, client) - except SlackApiError: + except SlackApiError as e: logger.warning( f"Could not batch import Slack users! Slack IDs: {user_id_list}", exc_info=True, ) + + # In test mode, if batch invite fails due to user_not_found, skip individual retries + if test_mode and e.response.get("error") == "user_not_found": + logger.info("Test mode: Skipping individual invitations for user_not_found errors") + return set() + + # If batch invite fails due to already_in_channel, consider all users as successfully invited + if e.response.get("error") == "already_in_channel": + logger.debug("All users already in channel - this is expected") + return user_id_list + done = self._invite_users_to_conversation_individually(user_id_list, client) return done @@ -193,14 +244,30 @@ def _invite_users_with_slack_id( def _invite_users_to_conversation_individually( self, slack_user_ids: set[str], client: WebClient ) -> set[str]: + from firefighter.firefighter.settings.settings_utils import ( + config, + ) + test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true" + done = set() for slack_user_id in slack_user_ids: try: self._invite_users_with_slack_id({slack_user_id}, client) done.add(slack_user_id) - except SlackApiError: + except SlackApiError as e: + error_type = e.response.get("error", "unknown_error") + + if test_mode and error_type == "user_not_found": + logger.info(f"Test mode: Skipping user_not_found error for user ID {slack_user_id}") + continue + + if error_type == "already_in_channel": + logger.debug(f"User {slack_user_id} is already in channel - this is expected") + done.add(slack_user_id) # Consider as successfully "invited" + continue + logger.warning( - f"Could not import Slack user! User ID: {slack_user_id}", + f"Could not import Slack user! User ID: {slack_user_id} - {error_type}", exc_info=True, ) return done diff --git a/src/firefighter/slack/signals/create_incident_conversation.py b/src/firefighter/slack/signals/create_incident_conversation.py index 5a6094d8..324819ba 100644 --- a/src/firefighter/slack/signals/create_incident_conversation.py +++ b/src/firefighter/slack/signals/create_incident_conversation.py @@ -7,6 +7,7 @@ from __future__ import annotations import logging +import os from typing import TYPE_CHECKING, Any from django.dispatch import receiver @@ -18,7 +19,6 @@ SlackMessageIncidentDeclaredAnnouncement, SlackMessageIncidentDeclaredAnnouncementGeneral, ) -from firefighter.slack.models.conversation import Conversation from firefighter.slack.models.incident_channel import IncidentChannel from firefighter.slack.models.user import SlackUser from firefighter.slack.rules import ( @@ -26,6 +26,8 @@ should_publish_in_it_deploy_channel, ) from firefighter.slack.signals import incident_channel_done +from firefighter.slack.slack_app import DefaultWebClient +from firefighter.slack.utils.test_channels import get_or_create_test_conversation if TYPE_CHECKING: from firefighter.incidents.models.incident import Incident @@ -35,7 +37,7 @@ @receiver(signal=create_incident_conversation) -def create_incident_slack_conversation( +def create_incident_slack_conversation( # noqa: PLR0912, PLR0915 incident: Incident, *_args: Any, **_kwargs: Any, @@ -73,34 +75,70 @@ def create_incident_slack_conversation( channel.set_incident_channel_topic() # Add the person that opened the incident in the channel + invited_creator = False + creator_slack_id = None if ( incident.created_by and hasattr(incident.created_by, "slack_user") and incident.created_by.slack_user and incident.created_by.slack_user.slack_id ): + creator_slack_id = incident.created_by.slack_user.slack_id try: + # Check if the invitation was successful by checking if the user is now a member + members_before = set(channel.incident.members.all()) channel.invite_users([incident.created_by]) + members_after = set(channel.incident.members.all()) + + # If the user was added to members, invitation succeeded + if incident.created_by in members_after and incident.created_by not in members_before: + invited_creator = True + logger.info(f"Successfully invited creator {creator_slack_id}") + else: + logger.warning(f"Creator {creator_slack_id} was not added to incident members - invitation may have failed") + except SlackApiError: logger.warning( - 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}", + f"Could not import Slack opener user! Slack ID: {creator_slack_id}, User {incident.created_by}, Channel ID {new_channel_id}", exc_info=True, ) else: logger.warning("Could not find user Slack ID for opener_user!") + # In test mode, if creator invitation failed, try to get the Slack user ID from kwargs + test_mode = os.getenv("TEST_MODE", "False").lower() == "true" + slack_user_id = _kwargs.get("slack_user_id") + + logger.info(f"Test mode: {test_mode}, invited_creator: {invited_creator}") + logger.info(f"Creator stored slack_id: {creator_slack_id}, event slack_user_id: {slack_user_id}") + if test_mode and not invited_creator and slack_user_id: + logger.info(f"Test mode: Attempting to invite creator using event Slack ID {slack_user_id}") + try: + # Invite directly using the Slack user ID from the event + from firefighter.slack.slack_app import slack_client + + @slack_client + def invite_test_user(channel_instance, user_id, client=DefaultWebClient): + return channel_instance._invite_users_to_conversation({user_id}, client) + + invite_test_user(channel, slack_user_id) + logger.info(f"Test mode: Successfully invited creator via Slack ID {slack_user_id}") + except SlackApiError: + logger.warning( + f"Test mode: Could not invite creator via Slack ID {slack_user_id}", + exc_info=True, + ) + # Send message in the created channel channel.send_message_and_save( - SlackMessageIncidentDeclaredAnnouncement(incident), pin=True + SlackMessageIncidentDeclaredAnnouncement(incident, slack_user_id=slack_user_id), pin=True ) # Post in general channel #tech-incidents if needed if should_publish_in_general_channel(incident, incident_update=None): - announcement_general = SlackMessageIncidentDeclaredAnnouncementGeneral(incident) + announcement_general = SlackMessageIncidentDeclaredAnnouncementGeneral(incident, slack_user_id=slack_user_id) - tech_incidents_conversation = Conversation.objects.get_or_none( - tag="tech_incidents" - ) + tech_incidents_conversation = get_or_create_test_conversation("tech_incidents") if tech_incidents_conversation: tech_incidents_conversation.send_message_and_save(announcement_general) else: @@ -112,14 +150,14 @@ def create_incident_slack_conversation( users_list: list[User] = incident.build_invite_list() # Invite all users - incident.conversation.invite_users(users_list) + incident.conversation.invite_users(users_list, slack_user_id=slack_user_id) # Post in #it-deploy if needed if should_publish_in_it_deploy_channel(incident): announcement_it_deploy = SlackMessageDeployWarning(incident) announcement_it_deploy.id = f"{announcement_it_deploy.id}_{incident.id}" - it_deploy_conversation = Conversation.objects.get_or_none(tag="it_deploy") + it_deploy_conversation = get_or_create_test_conversation("it_deploy") if it_deploy_conversation: it_deploy_conversation.send_message_and_save(announcement_it_deploy) else: diff --git a/src/firefighter/slack/slack_templating.py b/src/firefighter/slack/slack_templating.py index 14bb1726..2e66cde0 100644 --- a/src/firefighter/slack/slack_templating.py +++ b/src/firefighter/slack/slack_templating.py @@ -61,13 +61,41 @@ def date_time(date: datetime | None) -> str: return localtime(date).strftime("%Y-%m-%d %H:%M") -def user_slack_handle_or_name(user: User | None) -> str: - """Returns the Slack handle of the user in Slack MD format (`<@SLACK_ID>`) or the user full name.""" +def user_slack_handle_or_name(user: User | None, slack_user_id: str | None = None) -> str: + """Returns the Slack handle of the user in Slack MD format (`<@SLACK_ID>`) or the user full name. + + Args: + user: The user to display + slack_user_id: Optional Slack user ID from current event context (used in TEST_MODE for the current action performer only) + + Returns: + Slack handle format '<@USER_ID>' in production or test mode, or user full name as fallback + """ if user is None: return "∅" - if hasattr(user, "slack_user") and user.slack_user: + # In test mode: if slack_user_id is provided, it's only for the current action performer + # For other users (assigned to roles), use their stored slack_id if valid or fallback to name + from firefighter.firefighter.settings.settings_utils import config # noqa: PLC0415 + test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true" + if test_mode and slack_user_id: + # This is specifically for the "Updated by" context where we use the current action performer's ID + return f"<@{slack_user_id}>" + + # In test mode: check if the user has a valid slack_id (not production ID) + if test_mode and hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id: + # Skip production IDs that don't exist in test workspace - fallback to user name + if user.slack_user.slack_id.startswith("U") and len(user.slack_user.slack_id) >= 9: + # This looks like a production ID, fallback to user name in test mode + return user.full_name + # Valid test environment slack_id + return f"<@{user.slack_user.slack_id}>" + + # In production: use the stored user.slack_user.slack_id from database + if hasattr(user, "slack_user") and user.slack_user and user.slack_user.slack_id: return f"<@{user.slack_user.slack_id}>" + + # Fallback to user full name return user.full_name diff --git a/src/firefighter/slack/utils.py b/src/firefighter/slack/utils.py deleted file mode 100644 index 4fb2be68..00000000 --- a/src/firefighter/slack/utils.py +++ /dev/null @@ -1,108 +0,0 @@ -from __future__ import annotations - -import logging -import re -from typing import TYPE_CHECKING, Any, Final - -from django.utils.timezone import localtime -from slack_sdk.errors import SlackApiError - -from firefighter.slack.slack_app import DefaultWebClient, slack_client - -if TYPE_CHECKING: - from collections.abc import Sequence - - from slack_sdk.models.blocks import Block - from slack_sdk.web.client import WebClient - - from firefighter.incidents.models.incident import Incident - -logger = logging.getLogger(__name__) - -NON_ALPHANUMERIC_CHARACTERS: Final[re.Pattern[str]] = re.compile(r"[^\da-zA-Z]+") - - -@slack_client -def respond( - body: dict[str, Any], - text: str = "", - blocks: str | Sequence[dict[str, Any] | Block] | None = None, - client: WebClient = DefaultWebClient, -) -> None: - """Respond to the user, depending on where the message was coming from.""" - user_id: str | None = body.get("user_id", body.get("user", {}).get("id")) - channel_id: str | None = body.get("channel_id", body.get("channel", {}).get("id")) - - if not user_id and not channel_id: - raise ValueError( - "Cannot find user_id (or user.id) or channel_id (or channel.id) in the body. At least one is required." - ) - - # From Direct Message => Always respond on the conv between the user and the bot. - if (body.get("channel_name") == "directmessage" or not channel_id) and user_id: - # We should always be able to respond in this conversation... - client.chat_postMessage(channel=user_id, text=text, blocks=blocks) - return - if not user_id: - raise ValueError("Cannot find user_id (or user.id) in the body.") - - # From channel => post as ephemeral in the channel - try: - _send_ephemeral(text, blocks, client, user_id, channel_id) - - except (SlackApiError, ValueError): - logger.warning( - "Failed to send ephemeral chat message to user! Body: %s", - body, - exc_info=True, - ) - # Fallback to DM - if body.get("type") != "view_submission": - client.chat_postMessage( - channel=user_id, - text=":warning: The bot could not respond in the channel you invoked it. Please add it to this channel or conversation if you want to interact with the bot there. If you believe this is a bug, please tell @pulse.", - ) - client.chat_postMessage(channel=user_id, text=text, blocks=blocks) - - -def _send_ephemeral( - text: str, - blocks: str | Sequence[dict[str, Any] | Block] | None, - client: WebClient, - user_id: str, - channel_id: str | None, -) -> None: - if not channel_id: - raise ValueError("Cannot find channel_id (or channel.id) in the body.") - client.chat_postEphemeral( - user=user_id, channel=channel_id, text=text, blocks=blocks - ) - - -def get_slack_user_id_from_body(body: dict[str, Any]) -> str | None: - """Get the slack user id from the body of a Slack request, in `user_id` or `user.id`.""" - return body.get("user_id", body.get("user", {}).get("id")) - - -def channel_name_from_incident(incident: Incident) -> str: - """Lowercase, truncated at 80 chars, this is obviously the channel #name.""" - if ( - not hasattr(incident, "created_at") - or not hasattr(incident, "id") - or not incident.created_at - or not incident.id - ): - raise RuntimeError( - "Incident must be saved before slack_channel_name can be computed" - ) - date_formatted = localtime(incident.created_at).strftime("%Y%m%d") - if incident.environment is not None and incident.environment.value != "PRD": - topic = f"{date_formatted}-{str(incident.id)[:8]}-{incident.environment.value}-{incident.component.name}" - else: - topic = f"{date_formatted}-{str(incident.id)[:8]}-{incident.component.name}" - - # Strip non-alphanumeric characters, cut at 80 chars - # XXX django.utils.text.slugify should be used instead - topic = topic.replace(" ", "-") - topic = NON_ALPHANUMERIC_CHARACTERS.sub("-", topic) - return topic.lower()[:80] diff --git a/src/firefighter/slack/utils/__init__.py b/src/firefighter/slack/utils/__init__.py new file mode 100644 index 00000000..2dc1b0f1 --- /dev/null +++ b/src/firefighter/slack/utils/__init__.py @@ -0,0 +1 @@ +"""Slack utilities package.""" diff --git a/src/firefighter/slack/utils/test_channels.py b/src/firefighter/slack/utils/test_channels.py new file mode 100644 index 00000000..8ecad0cf --- /dev/null +++ b/src/firefighter/slack/utils/test_channels.py @@ -0,0 +1,54 @@ +"""Utilities for handling test channels.""" + +from __future__ import annotations + +import logging +import os + +from firefighter.slack.models.conversation import Conversation + +logger = logging.getLogger(__name__) + + +def get_or_create_test_conversation(tag: str) -> Conversation | None: + """Get conversation by tag, or use test environment variable if in test mode. + + Args: + tag: The conversation tag to look up + + Returns: + Conversation object if found, None otherwise + """ + test_mode = os.getenv("TEST_MODE", "False").lower() == "true" + + if not test_mode: + # Production mode: get from database + return Conversation.objects.get_or_none(tag=tag) + + # Test mode: try to get test channel ID from environment + env_var_name = f"TEST_{tag.upper()}_CHANNEL_ID" + test_channel_id = os.getenv(env_var_name) + + if test_channel_id: + logger.info(f"Test mode: Using channel ID {test_channel_id} for {tag}") + # Try to find existing conversation or create a temporary one + conversation = Conversation.objects.filter(channel_id=test_channel_id).first() + if conversation: + return conversation + # Create a temporary conversation object for the test channel + conversation = Conversation( + channel_id=test_channel_id, + name=f"test-{tag}", + tag=tag + ) + try: + conversation.save() + logger.info(f"Test mode: Created conversation for {tag} with channel ID {test_channel_id}") + return conversation + except Exception as e: + logger.warning(f"Test mode: Could not save conversation for {tag}: {e}") + return conversation # Return unsaved object for immediate use + else: + logger.warning(f"Test mode: No test channel ID found in {env_var_name} environment variable") + # Fallback to database lookup + return Conversation.objects.get_or_none(tag=tag) diff --git a/src/firefighter/slack/views/modals/base_modal/form_utils.py b/src/firefighter/slack/views/modals/base_modal/form_utils.py index 2f88b7fa..05c7dd5b 100644 --- a/src/firefighter/slack/views/modals/base_modal/form_utils.py +++ b/src/firefighter/slack/views/modals/base_modal/form_utils.py @@ -352,14 +352,31 @@ def _process_model_user_field( raise TypeError(err_msg) if f.initial: + from firefighter.firefighter.settings.settings_utils import ( + config, + ) + initial_user: User | None = SlackUser.objects.add_slack_id_to_user( user=f.initial ) - initial_user_slack_id = ( - initial_user.slack_user.slack_id - if initial_user and initial_user.slack_user - else None - ) + initial_user_slack_id = None + + if initial_user and initial_user.slack_user: + test_mode = config("TEST_MODE", default="False", cast=str).lower() == "true" + + # In test mode: skip production IDs that don't exist in test workspace + if test_mode and initial_user.slack_user.slack_id: + # Skip production IDs that start with 'U' and are 11 characters long + if initial_user.slack_user.slack_id.startswith("U") and len(initial_user.slack_user.slack_id) == 11: + logger.info(f"Test mode: Skipping production slack_id {initial_user.slack_user.slack_id} for user field {field_name}") + initial_user_slack_id = None + else: + # Valid test environment slack_id + initial_user_slack_id = initial_user.slack_user.slack_id + else: + # Production mode: use stored slack_id + initial_user_slack_id = initial_user.slack_user.slack_id + slack_input_kwargs["initial_user"] = initial_user_slack_id field_name = f"{field_name}___{f.initial}{datetime.now().timestamp()}" # noqa: DTZ005