diff --git a/src/firetower/slack_app/handlers/mitigated.py b/src/firetower/slack_app/handlers/mitigated.py index f597d49c..c3cd2c1f 100644 --- a/src/firetower/slack_app/handlers/mitigated.py +++ b/src/firetower/slack_app/handlers/mitigated.py @@ -10,6 +10,7 @@ build_incident_lifecycle_modal, build_incident_update_data, get_incident_from_channel, + notify_submission_error, parse_incident_form_values, validate_lifecycle_form, ) @@ -49,7 +50,8 @@ def handle_mitigated_command(ack: Any, body: dict, command: dict, respond: Any) def handle_mitigated_submission(ack: Any, body: dict, view: dict, client: Any) -> None: - form = parse_incident_form_values(view) + # Parse without resolving tags so validation touches no DB before ack(). + form = parse_incident_form_values(view, resolve_tags=False) channel_id = view.get("private_metadata", "") errors = validate_lifecycle_form(form) @@ -59,39 +61,43 @@ def handle_mitigated_submission(ack: Any, body: dict, view: dict, client: Any) - ack() - incident = get_incident_from_channel(channel_id) - if not incident: - logger.error("Mitigated submission: no incident for channel %s", channel_id) - return + user_id = body["user"]["id"] - captain_user = get_or_create_user_from_slack_id(form["captain_slack_id"]) - if not captain_user: - logger.error( - "Could not resolve Slack user %s to a Firetower user", - form["captain_slack_id"], - ) - client.chat_postMessage( - channel=channel_id, - text="Failed to resolve the selected captain to a Firetower user.", + # Work runs after ack() so the modal closes immediately; any failure is + # reported back to the submitter as an ephemeral message. + try: + incident = get_incident_from_channel(channel_id) + captain_user = ( + get_or_create_user_from_slack_id(form["captain_slack_id"]) + if incident + else None ) - return + if incident is None or captain_user is None: + notify_submission_error(client, channel_id, user_id) + return - data = build_incident_update_data( - form, IncidentStatus.MITIGATED, captain_user.email - ) + # Resolve/create inline tags only after validation passed. + form = parse_incident_form_values(view) - acting_user = get_or_create_user_from_slack_id(body["user"]["id"]) - serializer = IncidentWriteSerializer( - instance=incident, data=data, partial=True, context={"acting_user": acting_user} - ) - if not serializer.is_valid(): - logger.error("Mitigated update failed: %s", serializer.errors) - client.chat_postMessage( - channel=channel_id, - text=f"Failed to update incident: {serializer.errors}", + data = build_incident_update_data( + form, IncidentStatus.MITIGATED, captain_user.email + ) + acting_user = get_or_create_user_from_slack_id(user_id) + serializer = IncidentWriteSerializer( + instance=incident, + data=data, + partial=True, + context={"acting_user": acting_user}, ) + if not serializer.is_valid(): + logger.error("Mitigated update failed: %s", serializer.errors) + notify_submission_error(client, channel_id, user_id) + return + serializer.save() + except Exception: + logger.exception("Failed to process mitigated submission") + notify_submission_error(client, channel_id, user_id) return - serializer.save() incident_url = f"{settings.FIRETOWER_BASE_URL}/{incident.incident_number}" client.chat_postMessage( diff --git a/src/firetower/slack_app/handlers/new_incident.py b/src/firetower/slack_app/handlers/new_incident.py index 0b65bc99..d4234245 100644 --- a/src/firetower/slack_app/handlers/new_incident.py +++ b/src/firetower/slack_app/handlers/new_incident.py @@ -17,6 +17,8 @@ from firetower.integrations.services.slack import escape_slack_text from firetower.slack_app.handlers.utils import ( _DEFAULT_SEVERITY, + CREATE_TAG_PREFIX, + _extract_title, build_incident_form_blocks, parse_incident_form_values, ) @@ -170,6 +172,10 @@ def _build_new_incident_modal(channel_id: str = "", user_id: str = "") -> dict: "affected_region_tags": TagType.AFFECTED_REGION, } +# Tag types users can create inline from the lifecycle modals. impact_type stays +# curated since it powers dashboards, so it is intentionally excluded. +INLINE_CREATABLE_TAG_TYPES = {TagType.AFFECTED_SERVICE, TagType.AFFECTED_REGION} + def handle_tag_options(ack: Any, payload: dict) -> None: close_old_connections() @@ -185,11 +191,31 @@ def handle_tag_options(ack: Any, payload: dict) -> None: if keyword: qs = qs.filter(name__icontains=keyword) + matches = list(qs.order_by("name")[:100]) options = [ {"text": {"type": "plain_text", "text": tag.name}, "value": tag.name} - for tag in qs.order_by("name")[:100] + for tag in matches ] - ack(options=options) + + stripped = keyword.strip() + if tag_type in INLINE_CREATABLE_TAG_TYPES and stripped: + exact_match = Tag.objects.filter(type=tag_type, name__iexact=stripped).exists() + if not exact_match: + # Put "+ Create" first so it survives the 100-option Slack cap even + # when the keyword already matches a full page of existing tags. + options.insert( + 0, + { + "text": { + "type": "plain_text", + "text": f'+ Create "{stripped}"', + }, + "value": f"{CREATE_TAG_PREFIX}{stripped}", + }, + ) + + # Slack rejects block_suggestion responses with more than 100 options. + ack(options=options[:100]) def handle_new_command(ack: Any, body: dict, command: dict, respond: Any) -> None: @@ -259,8 +285,6 @@ def _create_incident_via_db( def handle_new_incident_submission( ack: Any, body: dict, view: dict, client: Any ) -> None: - form = parse_incident_form_values(view) - values = view.get("state", {}).get("values", {}) private_selections = ( values.get("private_block", {}).get("is_private", {}).get("selected_options") @@ -268,7 +292,8 @@ def handle_new_incident_submission( ) is_private = any(opt.get("value") == "private" for opt in private_selections) - if not form["title"]: + title = _extract_title(view) + if not title: ack( response_action="errors", errors={"title_block": "This field is required."}, @@ -280,11 +305,13 @@ def handle_new_incident_submission( slack_user_id = body.get("user", {}).get("id", "") try: + form = parse_incident_form_values(view) incident = _create_incident_via_db(form, slack_user_id, is_private, client) except (OperationalError, InterfaceError): logger.exception( "Database unreachable during incident creation from Slack modal" ) + form = parse_incident_form_values(view, resolve_tags=False) form_data = { "title": form["title"], "severity": form["severity"] or _DEFAULT_SEVERITY.value, diff --git a/src/firetower/slack_app/handlers/resolved.py b/src/firetower/slack_app/handlers/resolved.py index 9b05806c..f83ad089 100644 --- a/src/firetower/slack_app/handlers/resolved.py +++ b/src/firetower/slack_app/handlers/resolved.py @@ -8,6 +8,7 @@ build_incident_lifecycle_modal, build_incident_update_data, get_incident_from_channel, + notify_submission_error, parse_incident_form_values, validate_lifecycle_form, ) @@ -47,7 +48,8 @@ def handle_resolved_command(ack: Any, body: dict, command: dict, respond: Any) - def handle_resolved_submission(ack: Any, body: dict, view: dict, client: Any) -> None: - form = parse_incident_form_values(view) + # Parse without resolving tags so validation touches no DB before ack(). + form = parse_incident_form_values(view, resolve_tags=False) channel_id = view.get("private_metadata", "") errors = validate_lifecycle_form(form) @@ -57,43 +59,47 @@ def handle_resolved_submission(ack: Any, body: dict, view: dict, client: Any) -> ack() - incident = get_incident_from_channel(channel_id) - if not incident: - logger.error("Resolved submission: no incident for channel %s", channel_id) - return + user_id = body["user"]["id"] - captain_user = get_or_create_user_from_slack_id(form["captain_slack_id"]) - if not captain_user: - logger.error( - "Could not resolve Slack user %s to a Firetower user", - form["captain_slack_id"], + # Work runs after ack() so the modal closes immediately; any failure is + # reported back to the submitter as an ephemeral message. + try: + incident = get_incident_from_channel(channel_id) + captain_user = ( + get_or_create_user_from_slack_id(form["captain_slack_id"]) + if incident + else None ) - client.chat_postMessage( - channel=channel_id, - text="Failed to resolve the selected captain to a Firetower user.", - ) - return - - severity = form["severity"] - if severity in ("P0", "P1", "P2"): - target_status = IncidentStatus.POSTMORTEM - else: - target_status = IncidentStatus.DONE - - data = build_incident_update_data(form, target_status, captain_user.email) - - acting_user = get_or_create_user_from_slack_id(body["user"]["id"]) - serializer = IncidentWriteSerializer( - instance=incident, data=data, partial=True, context={"acting_user": acting_user} - ) - if not serializer.is_valid(): - logger.error("Resolved update failed: %s", serializer.errors) - client.chat_postMessage( - channel=channel_id, - text=f"Failed to resolve incident: {serializer.errors}", + if incident is None or captain_user is None: + notify_submission_error(client, channel_id, user_id) + return + + # Resolve/create inline tags only after validation passed. + form = parse_incident_form_values(view) + + severity = form["severity"] + if severity in ("P0", "P1", "P2"): + target_status = IncidentStatus.POSTMORTEM + else: + target_status = IncidentStatus.DONE + + data = build_incident_update_data(form, target_status, captain_user.email) + acting_user = get_or_create_user_from_slack_id(user_id) + serializer = IncidentWriteSerializer( + instance=incident, + data=data, + partial=True, + context={"acting_user": acting_user}, ) + if not serializer.is_valid(): + logger.error("Resolved update failed: %s", serializer.errors) + notify_submission_error(client, channel_id, user_id) + return + serializer.save() + except Exception: + logger.exception("Failed to process resolved submission") + notify_submission_error(client, channel_id, user_id) return - serializer.save() client.chat_postMessage( channel=channel_id, diff --git a/src/firetower/slack_app/handlers/update_incident.py b/src/firetower/slack_app/handlers/update_incident.py index e7a644b5..32c121f6 100644 --- a/src/firetower/slack_app/handlers/update_incident.py +++ b/src/firetower/slack_app/handlers/update_incident.py @@ -7,6 +7,7 @@ from firetower.incidents.serializers import IncidentWriteSerializer from firetower.slack_app.handlers.utils import ( get_incident_from_channel, + notify_submission_error, parse_incident_form_values, ) @@ -232,7 +233,8 @@ def handle_update_command(ack: Any, body: dict, command: dict, respond: Any) -> def handle_update_incident_submission( ack: Any, body: dict, view: dict, client: Any ) -> None: - form = parse_incident_form_values(view) + # Parse without resolving tags so validation touches no DB before ack(). + form = parse_incident_form_values(view, resolve_tags=False) channel_id = view.get("private_metadata", "") values = view.get("state", {}).get("values", {}) @@ -251,48 +253,51 @@ def handle_update_incident_submission( ack() - incident = get_incident_from_channel(channel_id) - if not incident: - logger.error("Update submission: no incident for channel %s", channel_id) - return - - data: dict[str, Any] = { - "title": form["title"], - "description": form["description"], - "impact_summary": form["impact_summary"], - "is_private": is_private, - "impact_type_tags": form["impact_type_tags"], - "affected_service_tags": form["affected_service_tags"], - "affected_region_tags": form["affected_region_tags"], - } - if form["severity"]: - data["severity"] = form["severity"] - - if form["captain_slack_id"]: - captain_user = get_or_create_user_from_slack_id(form["captain_slack_id"]) - if captain_user: - data["captain"] = captain_user.email - - acting_user = get_or_create_user_from_slack_id(body["user"]["id"]) - serializer = IncidentWriteSerializer( - instance=incident, data=data, partial=True, context={"acting_user": acting_user} - ) - if not serializer.is_valid(): - logger.error("Incident update validation failed: %s", serializer.errors) - client.chat_postMessage( - channel=channel_id, - text=f"Failed to update incident: {serializer.errors}", - ) - return + user_id = body["user"]["id"] + # Work runs after ack() so the modal closes immediately; any failure is + # reported back to the submitter as an ephemeral message. try: + incident = get_incident_from_channel(channel_id) + if incident is None: + notify_submission_error(client, channel_id, user_id) + return + + # Resolve/create inline tags only after validation passed. + form = parse_incident_form_values(view) + + data: dict[str, Any] = { + "title": form["title"], + "description": form["description"], + "impact_summary": form["impact_summary"], + "is_private": is_private, + "impact_type_tags": form["impact_type_tags"], + "affected_service_tags": form["affected_service_tags"], + "affected_region_tags": form["affected_region_tags"], + } + if form["severity"]: + data["severity"] = form["severity"] + + if form["captain_slack_id"]: + captain_user = get_or_create_user_from_slack_id(form["captain_slack_id"]) + if captain_user: + data["captain"] = captain_user.email + + acting_user = get_or_create_user_from_slack_id(user_id) + serializer = IncidentWriteSerializer( + instance=incident, + data=data, + partial=True, + context={"acting_user": acting_user}, + ) + if not serializer.is_valid(): + logger.error("Incident update validation failed: %s", serializer.errors) + notify_submission_error(client, channel_id, user_id) + return serializer.save() except Exception: logger.exception("Failed to update incident from Slack modal") - client.chat_postMessage( - channel=channel_id, - text="Something went wrong updating the incident. Please try again.", - ) + notify_submission_error(client, channel_id, user_id) return client.chat_postMessage( diff --git a/src/firetower/slack_app/handlers/utils.py b/src/firetower/slack_app/handlers/utils.py index 16f9a2c4..8da3ada8 100644 --- a/src/firetower/slack_app/handlers/utils.py +++ b/src/firetower/slack_app/handlers/utils.py @@ -1,3 +1,4 @@ +import logging from typing import Any from django.conf import settings @@ -10,10 +11,58 @@ IncidentSeverity, IncidentStatus, ServiceTier, + Tag, + TagType, ) +logger = logging.getLogger(__name__) + _DEFAULT_SEVERITY = IncidentSeverity.P3 +CREATE_TAG_PREFIX = "__create__:" + + +def _resolve_tag_values( + values: list[str], tag_type: TagType, resolve_tags: bool = True +) -> list[str]: + """Resolve selected tag values, creating any prefixed with CREATE_TAG_PREFIX. + + Values prefixed with ``__create__:`` are stripped and get_or_create'd as Tags + of the given type; everything else passes through unchanged. The returned list + preserves order and is de-duplicated case-insensitively. When ``resolve_tags`` + is False, prefixed values are stripped to their raw name without touching the + database (used for degraded-mode fallbacks when the DB is unreachable). + """ + resolved: list[str] = [] + seen: set[str] = set() + for value in values: + name = value + if value.startswith(CREATE_TAG_PREFIX): + name = value[len(CREATE_TAG_PREFIX) :].strip() + if not name: + continue + if resolve_tags: + existing = Tag.objects.filter(type=tag_type, name__iexact=name).first() + if existing: + name = existing.name + else: + # Inline-created tags are intentionally left approved=False + # so they surface for admin audit. Tag.clean() guards + # case-insensitive uniqueness; a rare simultaneous duplicate + # create just errors out and the caller's guard asks the + # user to retry. + tag = Tag.objects.create(name=name, type=tag_type) + logger.info( + "Created %s tag %r from Slack modal", tag_type, tag.name + ) + name = tag.name + key = name.lower() + if key in seen: + continue + seen.add(key) + resolved.append(name) + return resolved + def get_incident_from_channel(channel_id: str) -> Incident | None: link = ( @@ -29,6 +78,19 @@ def get_incident_from_channel(channel_id: str) -> Incident | None: return None +def notify_submission_error( + client: Any, + channel_id: str, + user_id: str, + text: str = "Something went wrong updating the incident. Please try again.", +) -> None: + """Send the submitting user an ephemeral failure notice (visible only to them).""" + try: + client.chat_postEphemeral(channel=channel_id, user=user_id, text=text) + except Exception: + logger.exception("Failed to post ephemeral submission error to %s", channel_id) + + def build_incident_form_blocks(user_id: str = "") -> list[dict[str, Any]]: severity_options = [ { @@ -113,10 +175,15 @@ def build_incident_form_blocks(user_id: str = "") -> list[dict[str, Any]]: ] -def parse_incident_form_values(view: dict) -> dict[str, Any]: +def _extract_title(view: dict) -> str: values = view.get("state", {}).get("values", {}) + return values.get("title_block", {}).get("title", {}).get("value", "").strip() - title = values.get("title_block", {}).get("title", {}).get("value", "").strip() + +def parse_incident_form_values(view: dict, resolve_tags: bool = True) -> dict[str, Any]: + values = view.get("state", {}).get("values", {}) + + title = _extract_title(view) severity_block = values.get("severity_block", {}) selected_option = severity_block.get("severity", {}).get( "selected_option" @@ -150,7 +217,11 @@ def parse_incident_form_values(view: dict) -> dict[str, Any]: .get("selected_options") or [] ) - affected_service_tags = [opt["value"] for opt in affected_service_selections] + affected_service_tags = _resolve_tag_values( + [opt["value"] for opt in affected_service_selections], + TagType.AFFECTED_SERVICE, + resolve_tags=resolve_tags, + ) affected_region_selections = ( values.get("affected_region_block", {}) @@ -158,7 +229,11 @@ def parse_incident_form_values(view: dict) -> dict[str, Any]: .get("selected_options") or [] ) - affected_region_tags = [opt["value"] for opt in affected_region_selections] + affected_region_tags = _resolve_tag_values( + [opt["value"] for opt in affected_region_selections], + TagType.AFFECTED_REGION, + resolve_tags=resolve_tags, + ) captain_slack_id = ( values.get("captain_block", {}).get("captain_select", {}).get("selected_user") diff --git a/src/firetower/slack_app/tests/handlers/test_mitigated.py b/src/firetower/slack_app/tests/handlers/test_mitigated.py index 86a3f07a..003d1a37 100644 --- a/src/firetower/slack_app/tests/handlers/test_mitigated.py +++ b/src/firetower/slack_app/tests/handlers/test_mitigated.py @@ -350,9 +350,9 @@ def test_captain_resolution_failure(self, mock_get_user, incident): handle_mitigated_submission(ack, body, view, client) ack.assert_called_once_with() - client.chat_postMessage.assert_called_once() - msg = client.chat_postMessage.call_args[1]["text"] - assert "Failed to resolve the selected captain" in msg + client.chat_postMessage.assert_not_called() + client.chat_postEphemeral.assert_called_once() + assert client.chat_postEphemeral.call_args[1]["user"] == "U_CAPTAIN" @pytest.mark.django_db 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 371601e7..bc3b70c5 100644 --- a/src/firetower/slack_app/tests/handlers/test_new_incident.py +++ b/src/firetower/slack_app/tests/handlers/test_new_incident.py @@ -14,6 +14,7 @@ handle_new_incident_submission, handle_tag_options, ) +from firetower.slack_app.handlers.utils import _resolve_tag_values @pytest.mark.django_db @@ -78,6 +79,20 @@ def test_new_modal_has_minimal_blocks(self, mock_get_bolt_app): "private_block", } + @patch("firetower.slack_app.bolt.get_bolt_app") + def test_modal_omits_incident_page_hint(self, mock_get_bolt_app): + ack = MagicMock() + body = {"trigger_id": "T12345"} + command = {"text": "new"} + respond = MagicMock() + + handle_new_command(ack, body, command, respond) + + view = mock_get_bolt_app.return_value.client.views_open.call_args[1]["view"] + rendered = str(view["blocks"]) + assert "incident page" not in rendered + assert "Missing a service or region" not in rendered + @pytest.mark.django_db class TestNewIncidentSubmission: @@ -400,9 +415,10 @@ def test_returns_matching_tags(self, _mock_close): ack.assert_called_once() options = ack.call_args[1]["options"] - assert len(options) == 2 + assert len(options) == 3 names = {o["text"]["text"] for o in options} - assert names == {"us-east-1", "us-west-2"} + assert names == {"us-east-1", "us-west-2", '+ Create "us"'} + assert options[0]["value"] == "__create__:us" def test_empty_query_returns_all(self, _mock_close): Tag.objects.create(name="us-east-1", type=TagType.AFFECTED_REGION) @@ -434,6 +450,63 @@ def test_filters_by_tag_type(self, _mock_close): assert len(options) == 1 assert options[0]["text"]["text"] == "us-east-1" + def test_offers_create_option_for_service(self, _mock_close): + ack = MagicMock() + payload = {"action_id": "affected_service_tags", "value": "payments"} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert options[0] == { + "text": {"type": "plain_text", "text": '+ Create "payments"'}, + "value": "__create__:payments", + } + + def test_offers_create_option_for_region(self, _mock_close): + ack = MagicMock() + payload = {"action_id": "affected_region_tags", "value": "ap-south-1"} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert options[0]["value"] == "__create__:ap-south-1" + + def test_no_create_option_for_impact_type(self, _mock_close): + ack = MagicMock() + payload = {"action_id": "impact_type_tags", "value": "Brand New"} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert all(not o["value"].startswith("__create__:") for o in options) + + def test_no_create_option_for_empty_keyword(self, _mock_close): + ack = MagicMock() + payload = {"action_id": "affected_service_tags", "value": " "} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert all(not o["value"].startswith("__create__:") for o in options) + + def test_no_create_option_for_exact_match(self, _mock_close): + Tag.objects.create(name="API", type=TagType.AFFECTED_SERVICE) + + ack = MagicMock() + payload = {"action_id": "affected_service_tags", "value": "api"} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert all(not o["value"].startswith("__create__:") for o in options) + + def test_create_option_first_and_capped_at_100(self, _mock_close): + for i in range(120): + Tag.objects.create(name=f"svc-{i:03d}", type=TagType.AFFECTED_SERVICE) + + ack = MagicMock() + payload = {"action_id": "affected_service_tags", "value": "svc"} + handle_tag_options(ack, payload) + + options = ack.call_args[1]["options"] + assert len(options) == 100 + assert options[0]["value"] == "__create__:svc" + class TestFallbackChannel: def _base_form_data(self): @@ -630,3 +703,67 @@ def test_non_private_posts_to_feed_channel(self, mock_slack_svc, settings): assert len(feed_calls) == 1 assert "degraded mode" in feed_calls[0][0][1] assert "DB is on fire" in feed_calls[0][0][1] + + +@pytest.mark.django_db +class TestResolveTagValues: + def test_collapses_existing_and_create_of_same_name(self): + Tag.objects.create(name="Payments", type=TagType.AFFECTED_SERVICE) + + resolved = _resolve_tag_values( + ["Payments", "__create__:payments"], + TagType.AFFECTED_SERVICE, + ) + + assert resolved == ["Payments"] + + def test_collapses_create_values_differing_only_in_case(self): + resolved = _resolve_tag_values( + ["__create__:Payments", "__create__:payments"], + TagType.AFFECTED_SERVICE, + ) + + assert resolved == ["Payments"] + assert ( + Tag.objects.filter( + type=TagType.AFFECTED_SERVICE, name__iexact="payments" + ).count() + == 1 + ) + + def test_preserves_order(self): + resolved = _resolve_tag_values( + ["__create__:beta", "__create__:alpha"], + TagType.AFFECTED_SERVICE, + ) + + assert resolved == ["beta", "alpha"] + + def test_resolve_false_strips_prefix_without_db(self): + resolved = _resolve_tag_values( + ["__create__:payments", "us-east-1", "__create__: "], + TagType.AFFECTED_SERVICE, + resolve_tags=False, + ) + + assert resolved == ["payments", "us-east-1"] + assert not Tag.objects.exists() + + def test_skips_empty_after_strip(self): + resolved = _resolve_tag_values( + ["__create__: ", "__create__:payments"], + TagType.AFFECTED_SERVICE, + ) + + assert resolved == ["payments"] + + def test_reuses_existing_tag_case_insensitively(self): + Tag.objects.create(name="Payments", type=TagType.AFFECTED_SERVICE) + + resolved = _resolve_tag_values( + ["__create__:payments"], + TagType.AFFECTED_SERVICE, + ) + + assert resolved == ["Payments"] + assert Tag.objects.filter(type=TagType.AFFECTED_SERVICE).count() == 1 diff --git a/src/firetower/slack_app/tests/handlers/test_resolved.py b/src/firetower/slack_app/tests/handlers/test_resolved.py index 6aa45da0..60e08f64 100644 --- a/src/firetower/slack_app/tests/handlers/test_resolved.py +++ b/src/firetower/slack_app/tests/handlers/test_resolved.py @@ -1,6 +1,7 @@ from unittest.mock import MagicMock, patch import pytest +from django.db import OperationalError from firetower.incidents.models import IncidentSeverity, IncidentStatus, Tag, TagType from firetower.slack_app.handlers.resolved import ( @@ -279,6 +280,20 @@ def test_missing_captain_returns_error(self, incident): assert call_kwargs["response_action"] == "errors" assert "captain_block" in call_kwargs["errors"] + def test_rejected_submission_creates_no_inline_tags(self, incident): + ack = MagicMock() + client = MagicMock() + body = {"user": {"id": "U_CAPTAIN"}} + view = _make_resolved_view( + captain=None, + affected_service_tags=("__create__:payments",), + ) + + handle_resolved_submission(ack, body, view, client) + + assert ack.call_args[1]["response_action"] == "errors" + assert not Tag.objects.filter(name__iexact="payments").exists() + @patch("firetower.incidents.serializers.on_incident_updated") @patch("firetower.slack_app.handlers.resolved.get_or_create_user_from_slack_id") def test_missing_description_succeeds( @@ -381,6 +396,38 @@ def test_missing_affected_region_returns_error(self, incident): assert call_kwargs["response_action"] == "errors" assert "affected_region_block" in call_kwargs["errors"] + @patch("firetower.incidents.serializers.on_incident_updated") + @patch("firetower.slack_app.handlers.resolved.get_or_create_user_from_slack_id") + def test_creates_tag_inline_from_resolved_submission( + self, + mock_get_user, + mock_hook, + user, + incident, + impact_type_tag, + ): + mock_get_user.return_value = user + ack = MagicMock() + client = MagicMock() + body = {"user": {"id": "U_CAPTAIN"}} + view = _make_resolved_view( + severity="P1", + affected_service_tags=("__create__:payments",), + affected_region_tags=("__create__:ap-south-1",), + ) + + handle_resolved_submission(ack, body, view, client) + + ack.assert_called_once_with() + service_tag = Tag.objects.get(name="payments", type=TagType.AFFECTED_SERVICE) + assert service_tag.type == TagType.AFFECTED_SERVICE + region_tag = Tag.objects.get(name="ap-south-1", type=TagType.AFFECTED_REGION) + assert region_tag.type == TagType.AFFECTED_REGION + + incident.refresh_from_db() + assert "payments" in incident.affected_service_tag_names + assert "ap-south-1" in incident.affected_region_tag_names + @patch("firetower.slack_app.handlers.resolved.get_or_create_user_from_slack_id") def test_captain_resolution_failure(self, mock_get_user, incident): mock_get_user.return_value = None @@ -392,6 +439,23 @@ def test_captain_resolution_failure(self, mock_get_user, incident): handle_resolved_submission(ack, body, view, client) ack.assert_called_once_with() - client.chat_postMessage.assert_called_once() - msg = client.chat_postMessage.call_args[1]["text"] - assert "Failed to resolve the selected captain" in msg + client.chat_postMessage.assert_not_called() + client.chat_postEphemeral.assert_called_once() + assert client.chat_postEphemeral.call_args[1]["user"] == "U_CAPTAIN" + + @patch("firetower.slack_app.handlers.resolved.get_incident_from_channel") + def test_db_error_posts_ephemeral_failure(self, mock_get_incident): + # A DB failure must surface to the submitter (ephemerally), not vanish + # behind a closed modal. + mock_get_incident.side_effect = OperationalError("db down") + ack = MagicMock() + client = MagicMock() + body = {"user": {"id": "U_CAPTAIN"}} + view = _make_resolved_view() + + handle_resolved_submission(ack, body, view, client) + + ack.assert_called_once_with() + client.chat_postMessage.assert_not_called() + client.chat_postEphemeral.assert_called_once() + assert client.chat_postEphemeral.call_args[1]["user"] == "U_CAPTAIN"