diff --git a/src/firetower/config.py b/src/firetower/config.py index eb1f9b35..d0bf44e5 100644 --- a/src/firetower/config.py +++ b/src/firetower/config.py @@ -126,20 +126,6 @@ class LinearConfig: "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 }}. " - "{% if total_action_items == 0 %}There are no action items." - "{% else %}{{ completed_action_items }} of {{ total_action_items }} action " - "item{% if total_action_items != 1 %}s{% endif %} complete.{% endif %}" - ) @deserialize diff --git a/src/firetower/incidents/hooks.py b/src/firetower/incidents/hooks.py index b8cfae6c..fc90cbe4 100644 --- a/src/firetower/incidents/hooks.py +++ b/src/firetower/incidents/hooks.py @@ -18,6 +18,10 @@ IncidentSeverity, IncidentStatus, ) +from firetower.incidents.services import ( + get_linear_parent_issue_state_id, + sync_linear_parent_issue_status, +) from firetower.integrations.services import ( DatadogService, LinearService, @@ -1094,11 +1098,9 @@ def _sync_linear_priority(incident: Incident) -> None: LINEAR_PARENT_DESCRIPTION = ( "Add action items as sub-issues (child issues) of this ticket to have " "them tracked by Firetower. " - "Do not update title, status or captain here, use Firetower for that.\n\n" - "Firetower will mark this ticket as completed once the incident is " - "resolved and all action items are done. " - "Firetower will reopen this ticket if the incident is reopened, or if " - "there are still unfinished action items. " + "Do not update title, status or captain here; use Firetower for those.\n\n" + "This ticket's status mirrors the associated Firetower incident and is " + "independent of its action items. " "If you have questions, please reach out to #team-sre." ) @@ -1151,18 +1153,16 @@ def populate_linear_parent( if not linear_config or not uuid: return - team_id = str(linear_config.get("TEAM_ID", "")) linear_service = _get_linear_service() try: - states = linear_service.get_workflow_states(team_id) if team_id else None - started_state_id = states.get("started") if states else None + state_id = get_linear_parent_issue_state_id(incident, linear_service) captain_linear_id = _resolve_linear_user_id(incident.captain, linear_service) linear_service.update_issue( uuid, title=_linear_issue_title(incident, sync_identifiers=True), description=LINEAR_PARENT_DESCRIPTION, - state_id=started_state_id, + state_id=state_id, assignee_id=captain_linear_id, priority=LINEAR_PRIORITY_BY_SEVERITY[incident.severity], ) @@ -1255,13 +1255,12 @@ def create_linear_parent_issue( ) return - states = linear_service.get_workflow_states(team_id) - started_state_id = states.get("started") if states else None + state_id = get_linear_parent_issue_state_id(incident, linear_service) if not linear_service.update_issue( issue["id"], title=title, description=LINEAR_PARENT_DESCRIPTION, - state_id=started_state_id, + state_id=state_id, assignee_id=captain_linear_id, priority=LINEAR_PRIORITY_BY_SEVERITY[incident.severity], ): @@ -1271,11 +1270,13 @@ def create_linear_parent_issue( ) return else: + state_id = get_linear_parent_issue_state_id(incident, linear_service) issue = linear_service.create_issue( title, LINEAR_PARENT_DESCRIPTION, team_id, project_id, + state_id=state_id, assignee_id=captain_linear_id, priority=LINEAR_PRIORITY_BY_SEVERITY[incident.severity], ) @@ -1801,6 +1802,15 @@ def on_incident_updated( # --- Side effects --- + # Status change: mirror the lifecycle state to the Linear parent ticket. + if old_status is not None: + try: + sync_linear_parent_issue_status(incident) + except Exception: + logger.exception( + f"Failed to sync Linear parent status for incident {incident.id}" + ) + # Status change: trigger slack dump for resolve-like statuses if ( old_status is not None diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index 9d3cf969..ead51cde 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -6,7 +6,6 @@ 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 ( @@ -15,7 +14,6 @@ ) from firetower.incidents.models import ( ActionItem, - ActionItemStatus, ExternalLinkType, Incident, IncidentStatus, @@ -184,104 +182,55 @@ def _resolve_assignees( return resolved -COMPLETED_STATUSES = {ActionItemStatus.DONE, ActionItemStatus.CANCELED} +LINEAR_STATE_BY_INCIDENT_STATUS: dict[str, str] = { + IncidentStatus.ACTIVE: "in_progress", + IncidentStatus.MITIGATED: "in_progress", + IncidentStatus.POSTMORTEM: "in_review", + IncidentStatus.DONE: "done", + IncidentStatus.CANCELED: "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 COMPLETED_STATUSES) - 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( +def get_linear_parent_issue_state_id( incident: Incident, linear_service: LinearService -) -> None: - if not settings.LINEAR or not incident.linear_parent_issue_id: - return - team_id = str(settings.LINEAR.get("TEAM_ID", "")) - if not team_id: - return +) -> str | None: + """Return the Linear workflow state that represents an incident's status.""" + linear_config = settings.LINEAR + if not linear_config: + return None - statuses = list(incident.action_items.values_list("status", flat=True)) - incident_done = incident.status in (IncidentStatus.DONE, IncidentStatus.CANCELED) - all_complete = incident_done and ( - not statuses or all(s in COMPLETED_STATUSES for s in statuses) - ) + team_id = str(linear_config.get("TEAM_ID", "")) + target_state = LINEAR_STATE_BY_INCIDENT_STATUS.get(incident.status) + if not team_id or not target_state: + return None states = linear_service.get_workflow_states(team_id) - if not states: - return + return states.get(target_state) if states else None - target_state = "completed" if all_complete else "started" - parent_issue = linear_service.get_issue(incident.linear_parent_issue_id) - if not parent_issue: +def sync_linear_parent_issue_status(incident: Incident) -> None: + """Set the Linear parent issue state from the incident, never its action items.""" + if not settings.LINEAR or not incident.linear_parent_issue_id: return - # Never override a manually-cancelled parent issue. Firetower only ever - # drives the parent to "started" or "completed", so a "canceled" state - # reflects a deliberate human decision and must not be reopened to - # "started" (or forced to "completed") on subsequent syncs. - current_state_type = parent_issue.get("state_type") - if current_state_type in ("canceled", target_state): + linear_service = _get_linear_service() + target_state = LINEAR_STATE_BY_INCIDENT_STATUS.get(incident.status) + state_id = get_linear_parent_issue_state_id(incident, linear_service) + if not target_state or not state_id: return - state_id = states.get(target_state) - if not state_id: + parent_issue = linear_service.get_issue(incident.linear_parent_issue_id) + if not parent_issue or parent_issue.get("state_id") == state_id: return if linear_service.update_issue(incident.linear_parent_issue_id, state_id=state_id): - _comment_parent_issue_status_change( - incident, linear_service, target_state, statuses - ) return - # The underlying Linear error is logged by LinearService without any - # incident context, so name the incident and its parent here. Without this - # a persistently mislinked parent just emits an anonymous GraphQL error - # every sync, with nothing tying it back to the row that needs fixing. logger.warning( - "Failed to set Linear parent %s to %s for incident %s (team %s)", + "Failed to set Linear parent %s to %s for incident %s", incident.linear_parent_issue_id, target_state, incident.incident_number, - team_id, ) @@ -397,13 +346,6 @@ def sync_action_items_from_linear( incident.action_items_last_synced_at = timezone.now() incident.save(update_fields=["action_items_last_synced_at"]) - try: - _update_parent_issue_status(incident, linear_service) - except Exception: - logger.exception( - f"Failed to update Linear parent issue status for incident {incident.id}" - ) - logger.info( f"Action item sync complete for incident {incident.id}: " f"{stats.created} created, {stats.updated} updated, {stats.deleted} deleted" diff --git a/src/firetower/incidents/tests/test_action_items.py b/src/firetower/incidents/tests/test_action_items.py index 5705c1c8..12c4d873 100644 --- a/src/firetower/incidents/tests/test_action_items.py +++ b/src/firetower/incidents/tests/test_action_items.py @@ -322,80 +322,22 @@ def fake_create_parent(inc): assert stats.created == 1 assert incident.action_items.count() == 1 - def test_auto_completes_parent_when_all_done(self, settings): + def test_syncing_action_items_does_not_change_parent_status(self, settings): settings.LINEAR = {"TEAM_ID": "team-1"} incident = self._make_incident(status=IncidentStatus.DONE) - children = [ _make_linear_issue( - id="id-1", identifier="ENG-1", title="T1", status="Done" - ), - _make_linear_issue( - id="id-2", identifier="ENG-2", title="T2", status="Canceled" - ), - ] - - with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_service = mock_get.return_value - mock_service.get_child_issues.return_value = children - mock_service.get_workflow_states.return_value = { - "completed": "state-done", - "backlog": "state-backlog", - } - mock_service.update_issue.return_value = True - - sync_action_items_from_linear(incident, force=True) - - mock_service.update_issue.assert_any_call( - "parent-issue-id", state_id="state-done" + id="id-1", identifier="ENG-1", title="T1", status="In Progress" ) - - def test_sets_parent_to_started_when_incomplete_items(self, settings): - settings.LINEAR = {"TEAM_ID": "team-1"} - incident = self._make_incident(status=IncidentStatus.DONE) - - children = [ - _make_linear_issue( - id="id-1", identifier="ENG-1", title="T1", status="Done" - ), - _make_linear_issue( - id="id-2", identifier="ENG-2", title="T2", status="In Progress" - ), ] with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_service = mock_get.return_value mock_service.get_child_issues.return_value = children - mock_service.get_workflow_states.return_value = { - "completed": "state-done", - "started": "state-started", - } - mock_service.update_issue.return_value = True sync_action_items_from_linear(incident, force=True) - mock_service.update_issue.assert_any_call( - "parent-issue-id", state_id="state-started" - ) - - def test_completes_parent_when_no_action_items(self, settings): - settings.LINEAR = {"TEAM_ID": "team-1"} - incident = self._make_incident(status=IncidentStatus.DONE) - - with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_service = mock_get.return_value - mock_service.get_child_issues.return_value = [] - mock_service.get_workflow_states.return_value = { - "completed": "state-done", - "backlog": "state-backlog", - } - mock_service.update_issue.return_value = True - - sync_action_items_from_linear(incident, force=True) - - mock_service.update_issue.assert_any_call( - "parent-issue-id", state_id="state-done" - ) + mock_service.update_issue.assert_not_called() def test_does_not_push_parent_assignee_on_sync(self, settings): settings.LINEAR = {"TEAM_ID": "team-1"} @@ -620,6 +562,7 @@ def test_get_workflow_states_caches(self): {"id": "s3", "name": "In Progress", "type": "started"}, {"id": "s4", "name": "Done", "type": "completed"}, {"id": "s5", "name": "Canceled", "type": "canceled"}, + {"id": "s6", "name": "In Review", "type": "started"}, ] } } @@ -629,6 +572,9 @@ def test_get_workflow_states_caches(self): states = service.get_workflow_states("team-1") assert states["completed"] == "s4" assert states["backlog"] == "s1" + assert states["in_progress"] == "s3" + assert states["in_review"] == "s6" + assert states["done"] == "s4" states2 = service.get_workflow_states("team-1") assert states2 is states diff --git a/src/firetower/incidents/tests/test_allocation.py b/src/firetower/incidents/tests/test_allocation.py index 19f44aa7..de896d76 100644 --- a/src/firetower/incidents/tests/test_allocation.py +++ b/src/firetower/incidents/tests/test_allocation.py @@ -486,7 +486,7 @@ def test_updates_by_uuid_never_by_identifier( ): incident = self._incident(settings) linear = mock_get_linear.return_value - linear.get_workflow_states.return_value = {"started": "state-started"} + linear.get_workflow_states.return_value = {"in_progress": "state-in-progress"} linear.update_issue.return_value = True populate_linear_parent( @@ -495,7 +495,7 @@ def test_updates_by_uuid_never_by_identifier( linear.get_issue.assert_not_called() assert linear.update_issue.call_args[0][0] == "uuid-verified" - assert linear.update_issue.call_args[1]["state_id"] == "state-started" + assert linear.update_issue.call_args[1]["state_id"] == "state-in-progress" linear.create_attachment.assert_called_once() assert linear.create_attachment.call_args[0][0] == "uuid-verified" mock_slack.add_bookmark.assert_called_once_with( diff --git a/src/firetower/incidents/tests/test_hooks.py b/src/firetower/incidents/tests/test_hooks.py index a3daebe8..5f18d0f3 100644 --- a/src/firetower/incidents/tests/test_hooks.py +++ b/src/firetower/incidents/tests/test_hooks.py @@ -3522,6 +3522,16 @@ def test_posts_bullet_for_single_change(self, mock_slack): msg = mock_slack.post_message.call_args[0][1] assert "- Status: Active -> Mitigated" in msg + @patch("firetower.incidents.hooks.sync_linear_parent_issue_status") + def test_syncs_linear_parent_status_when_incident_status_changes( + self, mock_sync_status + ): + incident = self._make_incident(status=IncidentStatus.ACTIVE) + + on_incident_updated(incident, old_status=IncidentStatus.MITIGATED) + + mock_sync_status.assert_called_once_with(incident) + @patch("firetower.incidents.hooks._slack_service") def test_includes_actor_attribution(self, mock_slack): mock_slack.parse_channel_id_from_url.return_value = "C12345" diff --git a/src/firetower/incidents/tests/test_services.py b/src/firetower/incidents/tests/test_services.py index e7c3b698..145f3b7d 100644 --- a/src/firetower/incidents/tests/test_services.py +++ b/src/firetower/incidents/tests/test_services.py @@ -1,6 +1,5 @@ -import logging from datetime import timedelta -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest from django.contrib.auth.models import User @@ -8,8 +7,6 @@ from firetower.auth.models import ExternalProfile, ExternalProfileType from firetower.incidents.models import ( - ActionItem, - ActionItemStatus, ExternalLink, ExternalLinkType, Incident, @@ -17,9 +14,8 @@ IncidentStatus, ) from firetower.incidents.services import ( - _comment_parent_issue_status_change, - _update_parent_issue_status, sync_incident_participants_from_slack, + sync_linear_parent_issue_status, ) @@ -417,7 +413,7 @@ def test_skips_inactive_users(self): @pytest.mark.django_db -class TestUpdateParentIssueStatus: +class TestSyncLinearParentIssueStatus: def _make_incident(self, status=IncidentStatus.ACTIVE): return Incident.objects.create( title="Test Incident", @@ -426,351 +422,107 @@ def _make_incident(self, status=IncidentStatus.ACTIVE): linear_parent_issue_id="lin-123", ) - 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", - "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) - svc = self._make_linear_service() - - _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) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-1", - linear_identifier="INC-1", - title="Item 1", - status=ActionItemStatus.DONE, - url="https://linear.app/issue/1", - ) - svc = self._make_linear_service() - - _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): + settings.LINEAR = {"TEAM_ID": "team-1", "API_KEY": "key"} + + @pytest.mark.parametrize( + ("incident_status", "target_state", "state_id"), + [ + (IncidentStatus.ACTIVE, "in_progress", "state-in-progress"), + (IncidentStatus.MITIGATED, "in_progress", "state-in-progress"), + (IncidentStatus.POSTMORTEM, "in_review", "state-in-review"), + (IncidentStatus.DONE, "done", "state-done"), + (IncidentStatus.CANCELED, "canceled", "state-canceled"), + ], + ) + def test_mirrors_incident_status(self, incident_status, target_state, state_id): + incident = self._make_incident(status=incident_status) + + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = { + "in_progress": "state-in-progress", + "in_review": "state-in-review", + "done": "state-done", + "canceled": "state-canceled", + } + service.get_issue.return_value = {"state_id": "state-unstarted"} + service.update_issue.return_value = True + + sync_linear_parent_issue_status(incident) + + service.update_issue.assert_called_once_with("lin-123", state_id=state_id) + + def test_ignores_action_items(self): incident = self._make_incident(status=IncidentStatus.DONE) - svc = self._make_linear_service() - - _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) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-1", - linear_identifier="INC-1", - title="Item 1", - status=ActionItemStatus.DONE, - url="https://linear.app/issue/1", - ) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-2", - linear_identifier="INC-2", - title="Item 2", - status=ActionItemStatus.CANCELED, - url="https://linear.app/issue/2", - ) - svc = self._make_linear_service() - - _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_canceled_incident_all_items_done_sets_completed(self): - incident = self._make_incident(status=IncidentStatus.CANCELED) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-1", - linear_identifier="INC-1", - title="Item 1", - status=ActionItemStatus.DONE, - url="https://linear.app/issue/1", - ) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-2", - linear_identifier="INC-2", - title="Item 2", - status=ActionItemStatus.CANCELED, - url="https://linear.app/issue/2", - ) - svc = self._make_linear_service() + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = {"done": "state-done"} + service.get_issue.return_value = {"state_id": "state-in-progress"} + service.update_issue.return_value = True - _update_parent_issue_status(incident, svc) + sync_linear_parent_issue_status(incident) - svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") - svc.create_comment.assert_called_once() + service.update_issue.assert_called_once_with("lin-123", state_id="state-done") - def test_done_incident_incomplete_items_sets_started(self): - incident = self._make_incident(status=IncidentStatus.DONE) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-1", - linear_identifier="INC-1", - title="Item 1", - status=ActionItemStatus.DONE, - url="https://linear.app/issue/1", - ) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-2", - linear_identifier="INC-2", - title="Item 2", - status=ActionItemStatus.IN_PROGRESS, - url="https://linear.app/issue/2", - ) - svc = self._make_linear_service() - - _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) - ActionItem.objects.create( - incident=incident, - linear_issue_id="li-1", - linear_identifier="INC-1", - title="Item 1", - status=ActionItemStatus.DONE, - url="https://linear.app/issue/1", - ) - svc = self._make_linear_service() - - _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_update_issue_failure_logs_incident_context(self, caplog): - incident = self._make_incident(status=IncidentStatus.DONE) - svc = self._make_linear_service() - svc.update_issue.return_value = False - - # The firetower logger sets propagate=False, so caplog's root handler - # never sees these records; attach it to the logger directly. - logger = logging.getLogger("firetower.incidents.services") - logger.addHandler(caplog.handler) - try: - with caplog.at_level(logging.WARNING, logger=logger.name): - _update_parent_issue_status(incident, svc) - finally: - logger.removeHandler(caplog.handler) - - # The Linear-side error carries no incident context, so the warning must - # name the parent issue and the incident that points at it. - assert len(caplog.records) == 1 - message = caplog.records[0].getMessage() - assert "lin-123" in message - assert incident.incident_number in message - assert "completed" in message - assert "team-1" in message - - def test_skips_update_when_already_in_target_state(self): + def test_reopens_a_canceled_parent_when_incident_reopens(self): incident = self._make_incident(status=IncidentStatus.ACTIVE) - svc = self._make_linear_service(current_state_type="started") - _update_parent_issue_status(incident, svc) + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = { + "in_progress": "state-in-progress" + } + service.get_issue.return_value = {"state_id": "state-canceled"} + service.update_issue.return_value = True - 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_does_not_reopen_manually_cancelled_parent(self): - incident = self._make_incident(status=IncidentStatus.ACTIVE) - svc = self._make_linear_service(current_state_type="canceled") + sync_linear_parent_issue_status(incident) - _update_parent_issue_status(incident, svc) - - svc.update_issue.assert_not_called() - svc.create_comment.assert_not_called() - - def test_does_not_complete_manually_cancelled_parent(self): - incident = self._make_incident(status=IncidentStatus.DONE) - svc = self._make_linear_service(current_state_type="canceled") - - _update_parent_issue_status(incident, svc) - - svc.update_issue.assert_not_called() - svc.create_comment.assert_not_called() - - 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"] + service.update_issue.assert_called_once_with( + "lin-123", state_id="state-in-progress" ) - 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): + def test_skips_update_when_parent_already_matches_incident(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() + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = {"done": "state-done"} + service.get_issue.return_value = {"state_id": "state-done"} - _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + sync_linear_parent_issue_status(incident) - svc.create_comment.assert_not_called() + service.update_issue.assert_not_called() - def test_whitespace_only_template_skips_comment(self, settings): - settings.LINEAR["PARENT_STATUS_COMMENT_STARTED"] = " " + def test_reopens_an_in_review_parent_when_incident_reopens(self): 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() + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = { + "in_progress": "state-in-progress" + } + service.get_issue.return_value = {"state_id": "state-in-review"} + service.update_issue.return_value = True - _comment_parent_issue_status_change(incident, svc, "completed", []) + sync_linear_parent_issue_status(incident) - svc.create_comment.assert_called_once_with( - "lin-123", - f"Set to Completed. Incident {incident.incident_number} is Done. 0/0 done.", + service.update_issue.assert_called_once_with( + "lin-123", state_id="state-in-progress" ) - 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"]) + def test_skips_update_when_parent_cannot_be_found(self): + incident = self._make_incident() - 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() + with patch("firetower.incidents.services._get_linear_service") as mock_get: + service = mock_get.return_value + service.get_workflow_states.return_value = { + "in_progress": "state-in-progress" + } + service.get_issue.return_value = None - _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) + sync_linear_parent_issue_status(incident) - svc.create_comment.assert_not_called() + service.update_issue.assert_not_called() diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 282c99f3..34b13e56 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -44,6 +44,7 @@ url priority state { + id type } assignee { @@ -438,6 +439,7 @@ def get_issue( "identifier": issue["identifier"], "title": issue["title"], "url": issue["url"], + "state_id": (issue.get("state") or {}).get("id", ""), "state_type": (issue.get("state") or {}).get("type", ""), } @@ -622,6 +624,10 @@ def get_workflow_states(self, team_id: str) -> dict[str, str] | None: if state_type not in states: states[state_type] = node["id"] + state_name = node.get("name", "").lower().replace(" ", "_") + if state_name and state_name not in states: + states[state_name] = node["id"] + self._workflow_states_cache = states return states diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index c6741b83..2048eac0 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -884,7 +884,7 @@ def test_returns_none_on_api_failure(self, linear_service): class TestGetIssue: - def test_returns_issue_with_state_type(self, linear_service): + def test_returns_issue_with_state_id_and_type(self, linear_service): mock_response = { "issue": { "id": "issue-123", @@ -892,7 +892,7 @@ def test_returns_issue_with_state_type(self, linear_service): "title": "Fix the thing", "url": "https://linear.app/team/issue/LIN-42", "priority": 1, - "state": {"type": "started"}, + "state": {"id": "state-123", "type": "started"}, "assignee": {"id": "user-1", "email": "alice@example.com"}, } } @@ -905,6 +905,7 @@ def test_returns_issue_with_state_type(self, linear_service): "identifier": "LIN-42", "title": "Fix the thing", "url": "https://linear.app/team/issue/LIN-42", + "state_id": "state-123", "state_type": "started", } @@ -925,6 +926,7 @@ def test_returns_empty_state_type_when_state_is_null(self, linear_service): result = linear_service.get_issue("issue-123") assert result is not None + assert result["state_id"] == "" assert result["state_type"] == "" def test_returns_none_on_api_failure(self, linear_service): diff --git a/src/firetower/settings.py b/src/firetower/settings.py index 80fb3f3a..9545f680 100644 --- a/src/firetower/settings.py +++ b/src/firetower/settings.py @@ -345,8 +345,6 @@ 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