diff --git a/src/firetower/incidents/hooks.py b/src/firetower/incidents/hooks.py index 7de9c47a..bb9d9b87 100644 --- a/src/firetower/incidents/hooks.py +++ b/src/firetower/incidents/hooks.py @@ -96,6 +96,7 @@ class ChannelSetupContext: description: str | None = None incident_url: str | None = None incident_number: str | None = None + topic: str | None = None def page_for_channel( @@ -258,6 +259,24 @@ def _get_status_channel_id(incident: Incident) -> str | None: return _slack_service.parse_channel_id_from_url(slack_link.url) +def _set_topic_on_all_channels(incident: Incident, topic: str) -> None: + channel_ids: list[str] = [] + try: + channel_id = _get_channel_id(incident) + if channel_id: + channel_ids.append(channel_id) + except Exception: + logger.exception(f"Failed to get channel id for incident {incident.id}") + try: + status_channel_id = _get_status_channel_id(incident) + if status_channel_id: + channel_ids.append(status_channel_id) + except Exception: + logger.exception(f"Failed to get status channel id for incident {incident.id}") + if channel_ids: + _slack_service.set_all_channel_topics(channel_ids, topic) + + def _invite_user_to_channel( channel_id: str, user: User, slack_user_id: str | None = None ) -> None: @@ -436,7 +455,8 @@ def _save_status_channel_link(incident: Incident, status_channel_id: str) -> Non def _create_status_channel_for_context( - ctx: ChannelSetupContext, slack_service: SlackService + ctx: ChannelSetupContext, + slack_service: SlackService, ) -> str | None: """Create a companion status channel. No DB access. @@ -463,6 +483,14 @@ def _create_status_channel_for_context( ) return None + if ctx.topic: + try: + slack_service.set_channel_topic(status_channel_id, ctx.topic) + except Exception: + logger.exception( + f"Failed to set topic on status channel {status_channel_name}" + ) + label = ctx.incident_number or ctx.channel_name message_lines = [f"This is the status channel for *{label}*."] if ctx.incident_url and ctx.incident_number: @@ -525,6 +553,7 @@ def _create_status_channel(incident: Incident, main_channel_id: str) -> None: reporter_slack_id=reporter_slack_id, incident_url=_build_incident_url(incident), incident_number=incident.incident_number, + topic=build_channel_topic(incident, captain_slack_id), ) status_channel_id = _create_status_channel_for_context(ctx, _slack_service) if status_channel_id: @@ -1247,6 +1276,7 @@ def on_incident_created(incident: Incident) -> None: description=incident.description, incident_url=incident_url, incident_number=incident.incident_number, + topic=build_channel_topic(incident, captain_slack_id), ) status_channel_id = decorate_incident_channel( ctx, @@ -1313,7 +1343,13 @@ def on_severity_changed(incident: Incident, old_severity: str) -> None: try: channel_id = _get_channel_id(incident) if channel_id: - _slack_service.set_channel_topic(channel_id, build_channel_topic(incident)) + try: + topic = build_channel_topic(incident) + _set_topic_on_all_channels(incident, topic) + except Exception: + logger.exception( + f"Failed to set channel topic for incident {incident.id}" + ) incident_url = _build_incident_url(incident) _slack_service.post_message( channel_id, @@ -1377,7 +1413,8 @@ def on_title_changed(incident: Incident) -> None: try: channel_id = _get_channel_id(incident) if channel_id: - _slack_service.set_channel_topic(channel_id, build_channel_topic(incident)) + topic = build_channel_topic(incident) + _set_topic_on_all_channels(incident, topic) except Exception: logger.exception(f"Error in on_title_changed for incident {incident.id}") @@ -1408,7 +1445,8 @@ def on_captain_changed(incident: Incident) -> None: if not channel_id: return - _slack_service.set_channel_topic(channel_id, build_channel_topic(incident)) + topic = build_channel_topic(incident) + _set_topic_on_all_channels(incident, topic) incident_url = _build_incident_url(incident) if incident.captain: @@ -1458,12 +1496,16 @@ def on_incident_updated( old_title is not None or old_severity is not None or captain_changed ): try: - _slack_service.set_channel_topic(channel_id, build_channel_topic(incident)) + topic = build_channel_topic(incident) except Exception: + topic = None logger.exception( - f"Error setting channel topic in on_incident_updated for incident {incident.id}" + f"Error building channel topic in on_incident_updated for incident {incident.id}" ) + if topic: + _set_topic_on_all_channels(incident, topic) + # --- Build combined notification lines --- lines: list[str] = [] diff --git a/src/firetower/incidents/tests/test_hooks.py b/src/firetower/incidents/tests/test_hooks.py index bca3b59a..00fb7ff3 100644 --- a/src/firetower/incidents/tests/test_hooks.py +++ b/src/firetower/incidents/tests/test_hooks.py @@ -403,7 +403,7 @@ def test_posts_status_update_message(self, mock_slack, mock_dump_async): mock_slack.post_message.assert_called_once() assert "Active" in mock_slack.post_message.call_args[0][1] assert "Mitigated" in mock_slack.post_message.call_args[0][1] - mock_slack.set_channel_topic.assert_not_called() + mock_slack.set_all_channel_topics.assert_not_called() @patch("firetower.incidents.hooks._slack_service") def test_noop_without_slack_link(self, mock_slack): @@ -509,7 +509,7 @@ def test_posts_severity_update_message( mock_slack.post_message.assert_called_once() assert "P2" in mock_slack.post_message.call_args[0][1] assert "P0" in mock_slack.post_message.call_args[0][1] - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() @patch("firetower.incidents.hooks._slack_service") def test_noop_without_slack_link(self, mock_slack): @@ -541,7 +541,7 @@ def test_updates_topic(self, mock_slack): on_title_changed(incident) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() @patch("firetower.incidents.hooks._slack_service") def test_noop_without_slack_link(self, mock_slack): @@ -552,7 +552,7 @@ def test_noop_without_slack_link(self, mock_slack): on_title_changed(incident) - mock_slack.set_channel_topic.assert_not_called() + mock_slack.set_all_channel_topics.assert_not_called() @pytest.mark.django_db @@ -642,7 +642,7 @@ def test_updates_topic_and_invites(self, mock_slack): on_captain_changed(incident) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() mock_slack.post_message.assert_called_once() assert "<@U_NEW>" in mock_slack.post_message.call_args[0][1] mock_slack.invite_to_channel.assert_called_once_with("C12345", ["U_NEW"]) @@ -671,7 +671,7 @@ def test_updates_topic_and_posts_name_when_no_slack_profile(self, mock_slack): on_captain_changed(incident) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() mock_slack.post_message.assert_called_once() assert "New Captain" in mock_slack.post_message.call_args[0][1] @@ -692,7 +692,7 @@ def test_updates_topic_only_when_captain_cleared(self, mock_slack): on_captain_changed(incident) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() mock_slack.post_message.assert_not_called() @patch("firetower.incidents.hooks._slack_service") @@ -780,7 +780,7 @@ def test_noop_without_slack_link(self, mock_slack): on_captain_changed(incident) - mock_slack.set_channel_topic.assert_not_called() + mock_slack.set_all_channel_topics.assert_not_called() MOCK_PD_CONFIG = { @@ -3347,7 +3347,7 @@ def test_sets_topic_once_when_severity_and_captain_change(self, mock_slack): incident, old_severity=IncidentSeverity.P4, captain_changed=True ) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() @patch("firetower.incidents.hooks._slack_service") def test_no_message_when_nothing_changed(self, mock_slack): @@ -3397,7 +3397,7 @@ def test_no_captain_line_when_captain_cleared(self, mock_slack): on_incident_updated(incident, captain_changed=True) - mock_slack.set_channel_topic.assert_called_once() + mock_slack.set_all_channel_topics.assert_called_once() mock_slack.post_message.assert_not_called() @patch("firetower.slack_app.handlers.dumpslack.trigger_slack_dump_async") diff --git a/src/firetower/integrations/services/slack.py b/src/firetower/integrations/services/slack.py index 692b0332..165e9cef 100644 --- a/src/firetower/integrations/services/slack.py +++ b/src/firetower/integrations/services/slack.py @@ -227,6 +227,14 @@ def set_channel_topic(self, channel_id: str, topic: str) -> bool: ) return False + def set_all_channel_topics( + self, channel_ids: list[str], topic: str + ) -> dict[str, bool]: + if not self.client: + logger.warning("Cannot set topics - Slack client not initialized") + return dict.fromkeys(channel_ids, False) + return {cid: self.set_channel_topic(cid, topic) for cid in channel_ids} + def invite_to_channel(self, channel_id: str, user_ids: list[str]) -> bool: if not self.client: logger.warning("Cannot invite to channel - Slack client not initialized")