diff --git a/slack-app-manifest.yaml b/slack-app-manifest.yaml index b9a84a43..87e8e9ae 100644 --- a/slack-app-manifest.yaml +++ b/slack-app-manifest.yaml @@ -6,6 +6,15 @@ features: bot_user: display_name: "Firetower" # test: "Firetower Test" always_online: true + shortcuts: + - name: Declare incident + type: message + callback_id: declare_incident + description: Open a new Firetower incident from this message + - name: Declare incident + type: global + callback_id: declare_incident_global + description: Open a new Firetower incident slash_commands: - command: /ft # test: /ft-test description: Manage incidents - use /ft help for full usage # test: /ft-test diff --git a/src/firetower/slack_app/bolt.py b/src/firetower/slack_app/bolt.py index 747bea0f..1b6ea217 100644 --- a/src/firetower/slack_app/bolt.py +++ b/src/firetower/slack_app/bolt.py @@ -28,6 +28,7 @@ handle_mitigated_submission, ) from firetower.slack_app.handlers.new_incident import ( + handle_declare_incident_shortcut, handle_new_command, handle_new_incident_submission, handle_severity_action, @@ -247,6 +248,12 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: def _register_views(app: App) -> None: """Register view handlers (modals, etc.) on the Bolt app.""" + app.shortcut("declare_incident")( + _with_metrics("declare_incident")(handle_declare_incident_shortcut) + ) + app.shortcut("declare_incident_global")( + _with_metrics("declare_incident_global")(handle_declare_incident_shortcut) + ) app.view("backfill_incident_modal")( _with_metrics("backfill_incident_modal")(handle_backfill_submission) ) diff --git a/src/firetower/slack_app/handlers/help.py b/src/firetower/slack_app/handlers/help.py index 0953760d..6a5011f3 100644 --- a/src/firetower/slack_app/handlers/help.py +++ b/src/firetower/slack_app/handlers/help.py @@ -22,5 +22,8 @@ def handle_help_command(ack: Any, command: dict, respond: Any) -> None: f" `{cmd} severity ` - Change incident severity (alias: `{cmd} sev`)\n" f" `{cmd} statuspage` - Create or update a statuspage post\n" f" `{cmd} subject ` - Change incident title (alias: `{cmd} title`)\n" - f" `{cmd} update` - Interactively update incident metadata (alias: `{cmd} edit`)\n" + f" `{cmd} update` - Interactively update incident metadata (alias: `{cmd} edit`)\n\n" + "Slash commands don't work in threads. To declare an incident from a thread, " + "use *Declare incident* in the message's More actions menu, or pick " + "Firetower's *Declare incident* shortcut from the `/` menu.\n" ) diff --git a/src/firetower/slack_app/handlers/new_incident.py b/src/firetower/slack_app/handlers/new_incident.py index 14c309b8..74c929d5 100644 --- a/src/firetower/slack_app/handlers/new_incident.py +++ b/src/firetower/slack_app/handlers/new_incident.py @@ -1,3 +1,4 @@ +import json import logging import uuid from typing import Any @@ -197,11 +198,33 @@ def _build_options_block( } +def _encode_invoking_context(channel_id: str, thread_ts: str = "") -> str: + if not thread_ts: + return channel_id + return json.dumps({"channel_id": channel_id, "thread_ts": thread_ts}) + + +def _parse_invoking_context(private_metadata: str) -> tuple[str, str]: + """Return ``(channel_id, thread_ts)`` from new-incident modal metadata. + + Slash commands store a bare channel id; the message shortcut stores JSON so + the creation notice can be threaded under the source message. + """ + if not private_metadata.startswith("{"): + return private_metadata, "" + try: + data = json.loads(private_metadata) + except ValueError: + return "", "" + return data.get("channel_id", ""), data.get("thread_ts", "") + + def _build_new_incident_modal( channel_id: str = "", user_id: str = "", severity: str = "", selected_options: set[str] | None = None, + thread_ts: str = "", ) -> dict: blocks = build_incident_form_blocks(user_id=user_id) @@ -220,7 +243,7 @@ def _build_new_incident_modal( "blocks": blocks, } if channel_id: - modal["private_metadata"] = channel_id + modal["private_metadata"] = _encode_invoking_context(channel_id, thread_ts) return modal @@ -294,6 +317,26 @@ def handle_new_command(ack: Any, body: dict, command: dict, respond: Any) -> Non ) +def handle_declare_incident_shortcut(ack: Any, shortcut: dict, client: Any) -> None: + """Open the new incident modal from the message or global shortcut. + + Slack does not deliver custom slash commands from threads, so shortcuts are + the way to declare an incident there. Typing ``/inc`` in a thread surfaces + the global shortcut, which carries no channel or message context. + """ + ack() + message = shortcut.get("message", {}) + thread_ts = message.get("thread_ts") or message.get("ts", "") + client.views_open( + trigger_id=shortcut["trigger_id"], + view=_build_new_incident_modal( + channel_id=shortcut.get("channel", {}).get("id", ""), + user_id=shortcut.get("user", {}).get("id", ""), + thread_ts=thread_ts, + ), + ) + + def handle_severity_action(ack: Any, body: dict, client: Any) -> None: ack() view = body.get("view", {}) @@ -305,7 +348,7 @@ def handle_severity_action(ack: Any, body: dict, client: Any) -> None: values.get("severity_block", {}).get("severity", {}).get("selected_option") ) severity = selected_option.get("value") if selected_option else "" - channel_id = view.get("private_metadata", "") + channel_id, thread_ts = _parse_invoking_context(view.get("private_metadata", "")) user_id = body.get("user", {}).get("id", "") prior_selections = ( @@ -321,6 +364,7 @@ def handle_severity_action(ack: Any, body: dict, client: Any) -> None: user_id=user_id, severity=severity, selected_options=selected_values, + thread_ts=thread_ts, ) client.views_update(view_id=view["id"], view=new_view) @@ -482,7 +526,9 @@ def handle_new_incident_submission( client.chat_postMessage(channel=slack_user_id, text=dm_message) - invoking_channel = view.get("private_metadata", "") + invoking_channel, invoking_thread_ts = _parse_invoking_context( + view.get("private_metadata", "") + ) feed_channel_id = settings.SLACK.get("INCIDENT_FEED_CHANNEL_ID", "") if ( invoking_channel @@ -500,6 +546,10 @@ def handle_new_incident_submission( f"\n\nFor those involved, please join <#{channel_id}>" ) _slack_service.join_channel(invoking_channel) - client.chat_postMessage(channel=invoking_channel, text=channel_message) + client.chat_postMessage( + channel=invoking_channel, + text=channel_message, + thread_ts=invoking_thread_ts or None, + ) except Exception: logger.exception("Failed to send incident creation notifications") diff --git a/src/firetower/slack_app/tests/handlers/test_new_incident.py b/src/firetower/slack_app/tests/handlers/test_new_incident.py index 2f95a573..95e0ea44 100644 --- a/src/firetower/slack_app/tests/handlers/test_new_incident.py +++ b/src/firetower/slack_app/tests/handlers/test_new_incident.py @@ -12,6 +12,8 @@ from firetower.slack_app.handlers.new_incident import ( _build_new_incident_modal, _create_fallback_channel, + _parse_invoking_context, + handle_declare_incident_shortcut, handle_new_command, handle_new_incident_submission, handle_severity_action, @@ -391,6 +393,43 @@ def test_posts_to_invoking_channel(self, mock_get_user, mock_hook, mock_slack_sv assert "C_INVOKE" in channels mock_slack_svc.join_channel.assert_called_once_with("C_INVOKE") + @patch("firetower.slack_app.handlers.new_incident._slack_service") + @patch("firetower.incidents.serializers.on_incident_created") + @patch("firetower.slack_app.handlers.new_incident.get_or_create_user_from_slack_id") + def test_posts_in_thread_when_declared_from_shortcut( + self, mock_get_user, mock_hook, mock_slack_svc + ): + mock_get_user.return_value = self.user + + client = MagicMock() + body = {"user": {"id": "U_TEST"}} + modal = _build_new_incident_modal(channel_id="C_INVOKE", thread_ts="100.0") + view = { + "private_metadata": modal["private_metadata"], + "state": { + "values": { + "title_block": {"title": {"value": "Test Incident"}}, + "severity_block": { + "severity": { + "selected_option": {"value": "P1"}, + } + }, + "options_block": {"incident_options": {"selected_options": []}}, + } + }, + } + + handle_new_incident_submission(MagicMock(), body, view, client) + + channel_calls = [ + c[1] + for c in client.chat_postMessage.call_args_list + if c[1]["channel"] == "C_INVOKE" + ] + assert len(channel_calls) == 1 + assert channel_calls[0]["thread_ts"] == "100.0" + mock_slack_svc.join_channel.assert_called_once_with("C_INVOKE") + @patch("firetower.slack_app.handlers.new_incident._slack_service") @patch("firetower.incidents.serializers.on_incident_created") @patch("firetower.slack_app.handlers.new_incident.get_or_create_user_from_slack_id") @@ -1156,3 +1195,91 @@ def test_reuses_existing_tag_case_insensitively(self): assert resolved == ["Payments"] assert Tag.objects.filter(type=TagType.AFFECTED_SERVICE).count() == 1 + + +class TestDeclareIncidentShortcut: + def _shortcut(self, message: dict) -> dict: + return { + "trigger_id": "T12345", + "channel": {"id": "C_THREAD"}, + "user": {"id": "U_TEST"}, + "message": message, + } + + def test_opens_modal_threaded_under_reply(self): + ack = MagicMock() + client = MagicMock() + shortcut = self._shortcut({"ts": "200.0", "thread_ts": "100.0"}) + + handle_declare_incident_shortcut(ack, shortcut, client) + + ack.assert_called_once_with() + kwargs = client.views_open.call_args[1] + assert kwargs["trigger_id"] == "T12345" + view = kwargs["view"] + assert view["callback_id"] == "new_incident_modal" + assert _parse_invoking_context(view["private_metadata"]) == ( + "C_THREAD", + "100.0", + ) + + def test_top_level_message_threads_under_itself(self): + client = MagicMock() + + handle_declare_incident_shortcut( + MagicMock(), self._shortcut({"ts": "300.0"}), client + ) + + view = client.views_open.call_args[1]["view"] + assert _parse_invoking_context(view["private_metadata"]) == ( + "C_THREAD", + "300.0", + ) + + def test_global_shortcut_opens_modal_without_context(self): + ack = MagicMock() + client = MagicMock() + shortcut = {"trigger_id": "T12345", "user": {"id": "U_TEST"}} + + handle_declare_incident_shortcut(ack, shortcut, client) + + ack.assert_called_once_with() + view = client.views_open.call_args[1]["view"] + assert view["callback_id"] == "new_incident_modal" + assert "private_metadata" not in view + + def test_severity_change_preserves_thread(self): + client = MagicMock() + modal = _build_new_incident_modal(channel_id="C_THREAD", thread_ts="100.0") + body = { + "user": {"id": "U_TEST"}, + "view": { + "id": "V1", + "callback_id": "new_incident_modal", + "private_metadata": modal["private_metadata"], + "state": { + "values": { + "severity_block": { + "severity": {"selected_option": {"value": "P0"}} + } + } + }, + }, + } + + handle_severity_action(MagicMock(), body, client) + + new_view = client.views_update.call_args[1]["view"] + assert new_view["private_metadata"] == modal["private_metadata"] + + +class TestParseInvokingContext: + def test_bare_channel_id(self): + assert _parse_invoking_context("C123") == ("C123", "") + + def test_empty(self): + assert _parse_invoking_context("") == ("", "") + + def test_slash_command_metadata_stays_bare(self): + modal = _build_new_incident_modal(channel_id="C123") + assert modal["private_metadata"] == "C123"