diff --git a/src/firetower/config.py b/src/firetower/config.py index cfcbc543..59280e55 100644 --- a/src/firetower/config.py +++ b/src/firetower/config.py @@ -80,19 +80,32 @@ class LinearConfig: action_item_slo_days_high_priority: int = 14 action_item_slo_days_medium_priority: int = 30 action_item_nag_comment_high_priority: str = ( - "{% if days_past_due > 0 %}This action item is **{{ days_past_due }} day(s) " - "past due**. {% endif %}" + "{% if days_past_due > 0 %}This action item is **{{ days_past_due }} " + "day{% if days_past_due != 1 %}s{% endif %} past due**. {% endif %}" "The SLO for completing P0/P1 incident action items is {{ slo_days }} " "days from incident creation. Please prioritize this work or close " "out the issue if it is no longer relevant." ) action_item_nag_comment_medium_priority: str = ( - "{% if days_past_due > 0 %}This action item is **{{ days_past_due }} day(s) " - "past due**. {% endif %}" + "{% if days_past_due > 0 %}This action item is **{{ days_past_due }} " + "day{% if days_past_due != 1 %}s{% endif %} past due**. {% endif %}" "The SLO for completing P2 incident action items is {{ slo_days }} " "days from incident creation. Please prioritize this work or close " "out the issue if it is no longer relevant." ) + parent_status_comment_completed: str = ( + "Firetower set this issue to **Completed**. " + "Incident {{ incident.incident_number }} is {{ incident.status }} " + "and {% if total_action_items == 0 %}there are no action items." + "{% else %}all {{ total_action_items }} action " + "item{% if total_action_items != 1 %}s{% endif %} are complete.{% endif %}" + ) + parent_status_comment_started: str = ( + "Firetower set this issue to **Started**. " + "Incident {{ incident.incident_number }} is {{ incident.status }}. " + "{{ completed_action_items }} of {{ total_action_items }} action " + "item{% if total_action_items != 1 %}s{% endif %} complete." + ) @deserialize diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index 80d14433..16cc0103 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -6,6 +6,7 @@ from django.contrib.auth.models import User from django.db import transaction from django.utils import timezone +from jinja2 import Environment, TemplateError from firetower.auth.models import ExternalProfile, ExternalProfileType from firetower.auth.services import ( @@ -185,6 +186,50 @@ def _resolve_assignees( COMPLETED_STATUSES = {ActionItemStatus.DONE, ActionItemStatus.CANCELED} +_PARENT_STATUS_TEMPLATE_ENV = Environment(autoescape=False) + + +def _comment_parent_issue_status_change( + incident: Incident, + linear_service: LinearService, + target_state: str, + statuses: list[str], +) -> None: + if not settings.LINEAR or not incident.linear_parent_issue_id: + return + + template_key = ( + "PARENT_STATUS_COMMENT_COMPLETED" + if target_state == "completed" + else "PARENT_STATUS_COMMENT_STARTED" + ) + template_source = settings.LINEAR.get(template_key, "") + if not template_source or not template_source.strip(): + return + + completed_action_items = sum( + 1 for s in statuses if s in {ActionItemStatus.DONE, ActionItemStatus.CANCELED} + ) + try: + comment = _PARENT_STATUS_TEMPLATE_ENV.from_string(template_source).render( + incident=incident, + total_action_items=len(statuses), + completed_action_items=completed_action_items, + target_state=target_state, + ) + except TemplateError: + logger.exception( + f"Failed to render parent status comment template for incident {incident.id}" + ) + return + + try: + linear_service.create_comment(incident.linear_parent_issue_id, comment) + except Exception: + logger.exception( + f"Failed to post parent status comment for incident {incident.id}" + ) + def _update_parent_issue_status( incident: Incident, linear_service: LinearService @@ -206,9 +251,18 @@ def _update_parent_issue_status( return target_state = "completed" if all_complete else "started" + + parent_issue = linear_service.get_issue(incident.linear_parent_issue_id) + if not parent_issue or parent_issue.get("state_type") == target_state: + return + state_id = states.get(target_state) - if state_id: - linear_service.update_issue(incident.linear_parent_issue_id, state_id=state_id) + if state_id and linear_service.update_issue( + incident.linear_parent_issue_id, state_id=state_id + ): + _comment_parent_issue_status_change( + incident, linear_service, target_state, statuses + ) def _sync_parent_assignee(incident: Incident, linear_service: LinearService) -> None: diff --git a/src/firetower/incidents/tests/test_services.py b/src/firetower/incidents/tests/test_services.py index f9c64d7b..97c70042 100644 --- a/src/firetower/incidents/tests/test_services.py +++ b/src/firetower/incidents/tests/test_services.py @@ -16,6 +16,7 @@ IncidentStatus, ) from firetower.incidents.services import ( + _comment_parent_issue_status_change, _update_parent_issue_status, sync_incident_participants_from_slack, ) @@ -424,17 +425,24 @@ def _make_incident(self, status=IncidentStatus.ACTIVE): linear_parent_issue_id="lin-123", ) - def _make_linear_service(self): + def _make_linear_service(self, current_state_type="unstarted"): svc = MagicMock() svc.get_workflow_states.return_value = { "started": "state-started", "completed": "state-completed", } + svc.update_issue.return_value = True + svc.get_issue.return_value = {"state_type": current_state_type} return svc @pytest.fixture(autouse=True) def _linear_settings(self, settings): - settings.LINEAR = {"TEAM_ID": "team-1", "API_KEY": "key"} + settings.LINEAR = { + "TEAM_ID": "team-1", + "API_KEY": "key", + "PARENT_STATUS_COMMENT_COMPLETED": "completed comment", + "PARENT_STATUS_COMMENT_STARTED": "started comment", + } def test_active_incident_no_action_items_sets_started(self): incident = self._make_incident(status=IncidentStatus.ACTIVE) @@ -443,6 +451,7 @@ def test_active_incident_no_action_items_sets_started(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-started") + svc.create_comment.assert_called_once() def test_active_incident_all_items_done_sets_started(self): incident = self._make_incident(status=IncidentStatus.ACTIVE) @@ -459,6 +468,7 @@ def test_active_incident_all_items_done_sets_started(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-started") + svc.create_comment.assert_called_once() def test_done_incident_no_action_items_sets_completed(self): incident = self._make_incident(status=IncidentStatus.DONE) @@ -467,6 +477,7 @@ def test_done_incident_no_action_items_sets_completed(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") + svc.create_comment.assert_called_once() def test_done_incident_all_items_done_sets_completed(self): incident = self._make_incident(status=IncidentStatus.DONE) @@ -491,6 +502,7 @@ def test_done_incident_all_items_done_sets_completed(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") + svc.create_comment.assert_called_once() def test_done_incident_incomplete_items_sets_started(self): incident = self._make_incident(status=IncidentStatus.DONE) @@ -515,6 +527,7 @@ def test_done_incident_incomplete_items_sets_started(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-started") + svc.create_comment.assert_called_once() def test_mitigated_incident_all_items_done_sets_started(self): incident = self._make_incident(status=IncidentStatus.MITIGATED) @@ -531,3 +544,165 @@ def test_mitigated_incident_all_items_done_sets_started(self): _update_parent_issue_status(incident, svc) svc.update_issue.assert_called_once_with("lin-123", state_id="state-started") + svc.create_comment.assert_called_once() + + def test_update_issue_failure_skips_comment(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = self._make_linear_service() + svc.update_issue.return_value = False + + _update_parent_issue_status(incident, svc) + + svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") + svc.create_comment.assert_not_called() + + def test_skips_update_when_already_in_target_state(self): + incident = self._make_incident(status=IncidentStatus.ACTIVE) + svc = self._make_linear_service(current_state_type="started") + + _update_parent_issue_status(incident, svc) + + svc.update_issue.assert_not_called() + svc.create_comment.assert_not_called() + + def test_updates_when_in_different_state(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = self._make_linear_service(current_state_type="started") + + _update_parent_issue_status(incident, svc) + + svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") + svc.create_comment.assert_called_once() + + def test_skips_update_when_get_issue_fails(self): + incident = self._make_incident(status=IncidentStatus.ACTIVE) + svc = self._make_linear_service() + svc.get_issue.return_value = None + + _update_parent_issue_status(incident, svc) + + svc.update_issue.assert_not_called() + svc.create_comment.assert_not_called() + + +@pytest.mark.django_db +class TestCommentParentIssueStatusChange: + def _make_incident(self, status=IncidentStatus.ACTIVE): + return Incident.objects.create( + title="Test Incident", + status=status, + severity=IncidentSeverity.P1, + linear_parent_issue_id="lin-123", + ) + + @pytest.fixture(autouse=True) + def _linear_settings(self, settings): + settings.LINEAR = { + "TEAM_ID": "team-1", + "API_KEY": "key", + "PARENT_STATUS_COMMENT_COMPLETED": ( + "Set to Completed. " + "Incident {{ incident.incident_number }} is {{ incident.status }}. " + "{{ completed_action_items }}/{{ total_action_items }} done." + ), + "PARENT_STATUS_COMMENT_STARTED": ( + "Set to Started. " + "Incident {{ incident.incident_number }} is {{ incident.status }}. " + "{{ completed_action_items }}/{{ total_action_items }} done." + ), + } + + def test_posts_completed_comment(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + + _comment_parent_issue_status_change( + incident, svc, "completed", ["Done", "Done"] + ) + + svc.create_comment.assert_called_once_with( + "lin-123", + f"Set to Completed. Incident {incident.incident_number} is Done. 2/2 done.", + ) + + def test_posts_started_comment_with_mixed_statuses(self): + incident = self._make_incident(status=IncidentStatus.ACTIVE) + svc = MagicMock() + + _comment_parent_issue_status_change( + incident, svc, "started", ["Done", "In Progress", "Todo"] + ) + + svc.create_comment.assert_called_once_with( + "lin-123", + f"Set to Started. Incident {incident.incident_number} is Active. 1/3 done.", + ) + + def test_counts_canceled_as_completed(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + + _comment_parent_issue_status_change( + incident, svc, "completed", ["Done", "Canceled"] + ) + + svc.create_comment.assert_called_once_with( + "lin-123", + f"Set to Completed. Incident {incident.incident_number} is Done. 2/2 done.", + ) + + def test_empty_template_skips_comment(self, settings): + settings.LINEAR["PARENT_STATUS_COMMENT_COMPLETED"] = "" + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + + svc.create_comment.assert_not_called() + + def test_whitespace_only_template_skips_comment(self, settings): + settings.LINEAR["PARENT_STATUS_COMMENT_STARTED"] = " " + incident = self._make_incident(status=IncidentStatus.ACTIVE) + svc = MagicMock() + + _comment_parent_issue_status_change(incident, svc, "started", ["Todo"]) + + svc.create_comment.assert_not_called() + + def test_no_action_items(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + + _comment_parent_issue_status_change(incident, svc, "completed", []) + + svc.create_comment.assert_called_once_with( + "lin-123", + f"Set to Completed. Incident {incident.incident_number} is Done. 0/0 done.", + ) + + def test_create_comment_failure_logs_and_continues(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + svc.create_comment.return_value = False + + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + + svc.create_comment.assert_called_once() + + def test_create_comment_exception_logs_and_continues(self): + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + svc.create_comment.side_effect = Exception("API error") + + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + + svc.create_comment.assert_called_once() + + def test_template_render_error_logs_and_continues(self, settings): + settings.LINEAR["PARENT_STATUS_COMMENT_COMPLETED"] = "{{ unterminated" + incident = self._make_incident(status=IncidentStatus.DONE) + svc = MagicMock() + + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + + svc.create_comment.assert_not_called() diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 8666d774..c89cf85d 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -243,6 +243,7 @@ def get_issue(self, issue_id: str) -> dict[str, Any] | None: "identifier": issue["identifier"], "title": issue["title"], "url": issue["url"], + "state_type": (issue.get("state") or {}).get("type", ""), } def get_user_by_email(self, email: str) -> dict[str, str] | None: diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index 63d34c01..390ef7dd 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -657,3 +657,62 @@ def test_returns_none_on_api_failure(self, linear_service): result = linear_service.get_user_by_email("alice@example.com") assert result is None + + +class TestGetIssue: + def test_returns_issue_with_state_type(self, linear_service): + mock_response = { + "issue": { + "id": "issue-123", + "identifier": "LIN-42", + "title": "Fix the thing", + "url": "https://linear.app/team/issue/LIN-42", + "priority": 1, + "state": {"type": "started"}, + "assignee": {"id": "user-1", "email": "alice@example.com"}, + } + } + + with patch.object(linear_service, "_graphql", return_value=mock_response): + result = linear_service.get_issue("issue-123") + + assert result == { + "id": "issue-123", + "identifier": "LIN-42", + "title": "Fix the thing", + "url": "https://linear.app/team/issue/LIN-42", + "state_type": "started", + } + + def test_returns_empty_state_type_when_state_is_null(self, linear_service): + mock_response = { + "issue": { + "id": "issue-123", + "identifier": "LIN-42", + "title": "Fix the thing", + "url": "https://linear.app/team/issue/LIN-42", + "priority": 1, + "state": None, + "assignee": None, + } + } + + with patch.object(linear_service, "_graphql", return_value=mock_response): + result = linear_service.get_issue("issue-123") + + assert result is not None + assert result["state_type"] == "" + + def test_returns_none_on_api_failure(self, linear_service): + with patch.object(linear_service, "_graphql", return_value=None): + result = linear_service.get_issue("issue-123") + + assert result is None + + def test_returns_none_when_issue_not_found(self, linear_service): + mock_response = {"issue": None} + + with patch.object(linear_service, "_graphql", return_value=mock_response): + result = linear_service.get_issue("nonexistent") + + assert result is None diff --git a/src/firetower/settings.py b/src/firetower/settings.py index 11fadea0..9cc6da37 100644 --- a/src/firetower/settings.py +++ b/src/firetower/settings.py @@ -327,6 +327,8 @@ class StatuspageSettings(TypedDict): "ACTION_ITEM_SLO_DAYS_MEDIUM_PRIORITY": config.linear.action_item_slo_days_medium_priority, "ACTION_ITEM_NAG_COMMENT_HIGH_PRIORITY": config.linear.action_item_nag_comment_high_priority, "ACTION_ITEM_NAG_COMMENT_MEDIUM_PRIORITY": config.linear.action_item_nag_comment_medium_priority, + "PARENT_STATUS_COMMENT_COMPLETED": config.linear.parent_status_comment_completed, + "PARENT_STATUS_COMMENT_STARTED": config.linear.parent_status_comment_started, } if config.linear else None