diff --git a/src/firetower/config.py b/src/firetower/config.py index ea36fab5..99194f8b 100644 --- a/src/firetower/config.py +++ b/src/firetower/config.py @@ -100,6 +100,13 @@ class LinearConfig: "{% 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 a2db11c0..723a17d9 100644 --- a/src/firetower/incidents/hooks.py +++ b/src/firetower/incidents/hooks.py @@ -998,11 +998,13 @@ def _sync_linear_assignee(incident: Incident) -> None: LINEAR_PARENT_DESCRIPTION = ( - "Relate action items to this ticket to have them tracked by Firetower. " - "Child issues or other relations (related, blocking, etc.) will all work. " + "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 completed once the incident is resolved and " - "all action items are done. " + "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. " "If you have questions, please reach out to #team-sre." ) diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index f3767d3b..ac7f9df8 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -189,26 +189,31 @@ def _resolve_assignees( _PARENT_STATUS_TEMPLATE_ENV = Environment(autoescape=False) -def _comment_parent_issue_completed( +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_source = settings.LINEAR.get("PARENT_STATUS_COMMENT_COMPLETED", "") + 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} - ) + 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( @@ -239,22 +244,31 @@ def _update_parent_issue_status( not statuses or all(s in COMPLETED_STATUSES for s in statuses) ) - if not all_complete: - return - states = linear_service.get_workflow_states(team_id) if not states: 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") == "completed": + if not parent_issue: + 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): return - state_id = states.get("completed") + state_id = states.get(target_state) if state_id and linear_service.update_issue( incident.linear_parent_issue_id, state_id=state_id ): - _comment_parent_issue_completed(incident, linear_service, statuses) + _comment_parent_issue_status_change( + incident, linear_service, target_state, statuses + ) def sync_action_items_from_linear( @@ -297,21 +311,9 @@ def sync_action_items_from_linear( incident.save(update_fields=["action_items_last_synced_at"]) return stats - related = linear_service.get_related_issues(parent_id) - if related is None: - error_msg = f"Failed to fetch related issues for incident {incident.id}" - logger.warning(error_msg) - stats.errors.append(error_msg) - incident.action_items_last_synced_at = timezone.now() - incident.save(update_fields=["action_items_last_synced_at"]) - return stats - all_issues: dict[str, dict] = {} for issue in children: all_issues[issue["id"]] = issue - for issue in related: - if issue["id"] not in all_issues: - all_issues[issue["id"]] = issue logger.info(f"Syncing {len(all_issues)} Linear issues to incident {incident.id}") diff --git a/src/firetower/incidents/tests/test_action_items.py b/src/firetower/incidents/tests/test_action_items.py index d504ee36..6f36ba66 100644 --- a/src/firetower/incidents/tests/test_action_items.py +++ b/src/firetower/incidents/tests/test_action_items.py @@ -88,7 +88,6 @@ def test_creates_action_items_from_children(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) @@ -99,61 +98,25 @@ def test_creates_action_items_from_children(self): assert incident.action_items.count() == 2 assert incident.action_items_last_synced_at is not None - def test_creates_action_items_from_relations(self): - incident = self._make_incident() - - related = [ - _make_linear_issue( - id="id-r1", - identifier="ENG-10", - title="Related task", - relation_type="related", - ), - _make_linear_issue( - id="id-r2", - identifier="ENG-11", - title="Blocking task", - relation_type="blocks", - ), - ] - - with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_get.return_value.get_child_issues.return_value = [] - mock_get.return_value.get_related_issues.return_value = related - mock_get.return_value.get_workflow_states.return_value = None - - stats = sync_action_items_from_linear(incident, force=True) - - assert stats.created == 2 - item_r1 = incident.action_items.get(linear_issue_id="id-r1") - assert item_r1.relation_type == ActionItemRelationType.RELATED - item_r2 = incident.action_items.get(linear_issue_id="id-r2") - assert item_r2.relation_type == ActionItemRelationType.BLOCKS - - def test_combines_children_and_relations(self): + def test_ignores_related_issues(self): incident = self._make_incident() children = [ _make_linear_issue(id="id-1", identifier="ENG-1", title="Child task"), ] - related = [ - _make_linear_issue( - id="id-2", - identifier="ENG-2", - title="Related task", - relation_type="related", - ), - ] with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = related - mock_get.return_value.get_workflow_states.return_value = None + mock_service = mock_get.return_value + mock_service.get_child_issues.return_value = children + mock_service.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) - assert stats.created == 2 - assert incident.action_items.count() == 2 + assert stats.created == 1 + assert incident.action_items.count() == 1 + item = incident.action_items.get(linear_issue_id="id-1") + assert item.relation_type == ActionItemRelationType.CHILD + mock_service.get_related_issues.assert_not_called() def test_same_issue_on_multiple_incidents(self): incident_a = self._make_incident(linear_parent_issue_id="parent-a") @@ -165,7 +128,6 @@ def test_same_issue_on_multiple_incidents(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = shared_issue - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None sync_action_items_from_linear(incident_a, force=True) @@ -175,32 +137,6 @@ def test_same_issue_on_multiple_incidents(self): assert incident_b.action_items.filter(linear_issue_id="shared-1").exists() assert ActionItem.objects.filter(linear_issue_id="shared-1").count() == 2 - def test_deduplicates_children_and_relations(self): - incident = self._make_incident() - - children = [ - _make_linear_issue(id="id-1", identifier="ENG-1", title="Task"), - ] - related = [ - _make_linear_issue( - id="id-1", - identifier="ENG-1", - title="Task", - relation_type="related", - ), - ] - - with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = related - mock_get.return_value.get_workflow_states.return_value = None - - stats = sync_action_items_from_linear(incident, force=True) - - assert stats.created == 1 - item = incident.action_items.get(linear_issue_id="id-1") - assert item.relation_type == ActionItemRelationType.CHILD - def test_updates_existing_action_items(self): incident = self._make_incident() ActionItem.objects.create( @@ -223,7 +159,6 @@ def test_updates_existing_action_items(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) @@ -251,7 +186,6 @@ def test_deletes_stale_action_items(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) @@ -277,7 +211,6 @@ def test_force_bypasses_throttle(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = [] - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) @@ -298,20 +231,6 @@ def test_handles_children_api_failure(self): incident.refresh_from_db() assert incident.action_items_last_synced_at is not None - def test_handles_relations_api_failure(self): - incident = self._make_incident() - - with patch("firetower.incidents.services._get_linear_service") as mock_get: - mock_get.return_value.get_child_issues.return_value = [] - mock_get.return_value.get_related_issues.return_value = None - - stats = sync_action_items_from_linear(incident, force=True) - - assert len(stats.errors) == 1 - assert "related issues" in stats.errors[0] - incident.refresh_from_db() - assert incident.action_items_last_synced_at is not None - def test_resolves_assignee_by_email(self): user = User.objects.create_user( username="dev@example.com", @@ -332,7 +251,6 @@ def test_resolves_assignee_by_email(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None sync_action_items_from_linear(incident, force=True) @@ -359,7 +277,6 @@ def test_creates_user_for_unknown_assignee_email(self): with patch("firetower.incidents.services._get_linear_service") as mock_get: mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None sync_action_items_from_linear(incident, force=True) @@ -396,7 +313,6 @@ def fake_create_parent(inc): patch("firetower.incidents.services._get_linear_service") as mock_get, ): mock_get.return_value.get_child_issues.return_value = children - mock_get.return_value.get_related_issues.return_value = [] mock_get.return_value.get_workflow_states.return_value = None stats = sync_action_items_from_linear(incident, force=True) @@ -422,7 +338,6 @@ def test_auto_completes_parent_when_all_done(self, settings): 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_related_issues.return_value = [] mock_service.get_workflow_states.return_value = { "completed": "state-done", "backlog": "state-backlog", @@ -435,7 +350,7 @@ def test_auto_completes_parent_when_all_done(self, settings): "parent-issue-id", state_id="state-done" ) - def test_does_not_change_parent_state_when_incomplete_items(self, settings): + def test_sets_parent_to_started_when_incomplete_items(self, settings): settings.LINEAR = {"TEAM_ID": "team-1"} incident = self._make_incident(status=IncidentStatus.DONE) @@ -451,7 +366,6 @@ def test_does_not_change_parent_state_when_incomplete_items(self, settings): 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_related_issues.return_value = [] mock_service.get_workflow_states.return_value = { "completed": "state-done", "started": "state-started", @@ -460,8 +374,9 @@ def test_does_not_change_parent_state_when_incomplete_items(self, settings): sync_action_items_from_linear(incident, force=True) - for call in mock_service.update_issue.call_args_list: - assert "state_id" not in call.kwargs + 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"} @@ -470,7 +385,6 @@ def test_completes_parent_when_no_action_items(self, settings): 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_related_issues.return_value = [] mock_service.get_workflow_states.return_value = { "completed": "state-done", "backlog": "state-backlog", @@ -499,7 +413,6 @@ def test_does_not_push_parent_assignee_on_sync(self, settings): 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_related_issues.return_value = [] mock_service.get_workflow_states.return_value = None mock_service.update_issue.return_value = True @@ -626,180 +539,6 @@ def test_get_child_issues_maps_state_types(self): assert issues[1]["status"] == "Done" assert issues[1]["assignee_email"] == "dev@example.com" - def test_get_related_issues_maps_relation_types(self): - forward_response = { - "issue": { - "relations": { - "nodes": [ - { - "type": "related", - "relatedIssue": { - "id": "id-1", - "identifier": "ENG-1", - "title": "Related task", - "url": "https://linear.app/t/ENG-1", - "state": {"type": "unstarted"}, - "assignee": None, - }, - }, - { - "type": "blocks", - "relatedIssue": { - "id": "id-2", - "identifier": "ENG-2", - "title": "Blocking task", - "url": "https://linear.app/t/ENG-2", - "state": {"type": "started"}, - "assignee": None, - }, - }, - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - inverse_response = { - "issue": { - "inverseRelations": { - "nodes": [], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - - with patch("firetower.integrations.services.linear.settings") as mock_settings: - mock_settings.LINEAR = { - "CLIENT_ID": "test-id", - "CLIENT_SECRET": "test-secret", - } - service = LinearService() - - with patch.object( - service, "_graphql", side_effect=[forward_response, inverse_response] - ): - issues = service.get_related_issues("parent-id") - - assert issues is not None - assert len(issues) == 2 - assert issues[0]["relation_type"] == "related" - assert issues[1]["relation_type"] == "blocks" - - def test_get_related_issues_includes_inverse_relations(self): - forward_response = { - "issue": { - "relations": { - "nodes": [ - { - "type": "related", - "relatedIssue": { - "id": "id-1", - "identifier": "ENG-1", - "title": "Forward relation", - "url": "https://linear.app/t/ENG-1", - "state": {"type": "unstarted"}, - "assignee": None, - }, - }, - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - inverse_response = { - "issue": { - "inverseRelations": { - "nodes": [ - { - "type": "related", - "issue": { - "id": "id-2", - "identifier": "INF-100", - "title": "Inverse relation", - "url": "https://linear.app/t/INF-100", - "state": {"type": "started"}, - "assignee": None, - }, - }, - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - - with patch("firetower.integrations.services.linear.settings") as mock_settings: - mock_settings.LINEAR = { - "CLIENT_ID": "test-id", - "CLIENT_SECRET": "test-secret", - } - service = LinearService() - - with patch.object( - service, "_graphql", side_effect=[forward_response, inverse_response] - ): - issues = service.get_related_issues("parent-id") - - assert issues is not None - assert len(issues) == 2 - assert issues[0]["identifier"] == "ENG-1" - assert issues[1]["identifier"] == "INF-100" - - def test_get_related_issues_deduplicates_across_directions(self): - forward_response = { - "issue": { - "relations": { - "nodes": [ - { - "type": "related", - "relatedIssue": { - "id": "id-1", - "identifier": "ENG-1", - "title": "Same issue", - "url": "https://linear.app/t/ENG-1", - "state": {"type": "unstarted"}, - "assignee": None, - }, - }, - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - inverse_response = { - "issue": { - "inverseRelations": { - "nodes": [ - { - "type": "related", - "issue": { - "id": "id-1", - "identifier": "ENG-1", - "title": "Same issue", - "url": "https://linear.app/t/ENG-1", - "state": {"type": "unstarted"}, - "assignee": None, - }, - }, - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - - with patch("firetower.integrations.services.linear.settings") as mock_settings: - mock_settings.LINEAR = { - "CLIENT_ID": "test-id", - "CLIENT_SECRET": "test-secret", - } - service = LinearService() - - with patch.object( - service, "_graphql", side_effect=[forward_response, inverse_response] - ): - issues = service.get_related_issues("parent-id") - - assert issues is not None - assert len(issues) == 1 - def test_update_issue(self): with patch("firetower.integrations.services.linear.settings") as mock_settings: mock_settings.LINEAR = { diff --git a/src/firetower/incidents/tests/test_services.py b/src/firetower/incidents/tests/test_services.py index 27291dea..1cf8730e 100644 --- a/src/firetower/incidents/tests/test_services.py +++ b/src/firetower/incidents/tests/test_services.py @@ -16,7 +16,7 @@ IncidentStatus, ) from firetower.incidents.services import ( - _comment_parent_issue_completed, + _comment_parent_issue_status_change, _update_parent_issue_status, sync_incident_participants_from_slack, ) @@ -441,60 +441,87 @@ def _linear_settings(self, settings): "TEAM_ID": "team-1", "API_KEY": "key", "PARENT_STATUS_COMMENT_COMPLETED": "completed comment", + "PARENT_STATUS_COMMENT_STARTED": "started comment", } - def _add_item(self, incident, status, suffix="1"): - return ActionItem.objects.create( - incident=incident, - linear_issue_id=f"li-{suffix}", - linear_identifier=f"INC-{suffix}", - title=f"Item {suffix}", - status=status, - url=f"https://linear.app/issue/{suffix}", - ) - - def test_active_incident_no_action_items_does_nothing(self): + 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_not_called() - svc.create_comment.assert_not_called() + 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_does_nothing(self): + def test_active_incident_all_items_done_sets_started(self): incident = self._make_incident(status=IncidentStatus.ACTIVE) - self._add_item(incident, ActionItemStatus.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", + ) svc = self._make_linear_service() _update_parent_issue_status(incident, svc) - svc.update_issue.assert_not_called() - svc.create_comment.assert_not_called() + 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_does_nothing(self): - incident = self._make_incident(status=IncidentStatus.MITIGATED) - self._add_item(incident, ActionItemStatus.DONE) + def test_done_incident_no_action_items_sets_completed(self): + incident = self._make_incident(status=IncidentStatus.DONE) svc = self._make_linear_service() _update_parent_issue_status(incident, svc) - svc.update_issue.assert_not_called() - svc.create_comment.assert_not_called() + 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_does_nothing(self): + def test_done_incident_all_items_done_sets_completed(self): incident = self._make_incident(status=IncidentStatus.DONE) - self._add_item(incident, ActionItemStatus.DONE, "1") - self._add_item(incident, ActionItemStatus.IN_PROGRESS, "2") + 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_not_called() - svc.create_comment.assert_not_called() + svc.update_issue.assert_called_once_with("lin-123", state_id="state-completed") + svc.create_comment.assert_called_once() - def test_done_incident_no_action_items_sets_completed(self): - incident = self._make_incident(status=IncidentStatus.DONE) + 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() _update_parent_issue_status(incident, svc) @@ -502,25 +529,46 @@ def test_done_incident_no_action_items_sets_completed(self): 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): + def test_done_incident_incomplete_items_sets_started(self): incident = self._make_incident(status=IncidentStatus.DONE) - self._add_item(incident, ActionItemStatus.DONE, "1") - self._add_item(incident, ActionItemStatus.CANCELED, "2") + 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-completed") + svc.update_issue.assert_called_once_with("lin-123", state_id="state-started") svc.create_comment.assert_called_once() - def test_canceled_incident_all_items_done_sets_completed(self): - incident = self._make_incident(status=IncidentStatus.CANCELED) - self._add_item(incident, ActionItemStatus.DONE) + 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-completed") + 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): @@ -533,9 +581,9 @@ def test_update_issue_failure_skips_comment(self): 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_completed(self): - incident = self._make_incident(status=IncidentStatus.DONE) - svc = self._make_linear_service(current_state_type="completed") + 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) @@ -551,21 +599,28 @@ def test_updates_when_in_different_state(self): 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.DONE) - svc = self._make_linear_service() - svc.get_issue.return_value = None + 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") _update_parent_issue_status(incident, svc) svc.update_issue.assert_not_called() svc.create_comment.assert_not_called() - def test_never_reopens_when_item_reopens_after_completion(self): + def test_does_not_complete_manually_cancelled_parent(self): incident = self._make_incident(status=IncidentStatus.DONE) - self._add_item(incident, ActionItemStatus.DONE, "1") - self._add_item(incident, ActionItemStatus.IN_PROGRESS, "2") - svc = self._make_linear_service(current_state_type="completed") + 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) @@ -574,8 +629,8 @@ def test_never_reopens_when_item_reopens_after_completion(self): @pytest.mark.django_db -class TestCommentParentIssueCompleted: - def _make_incident(self, status=IncidentStatus.DONE): +class TestCommentParentIssueStatusChange: + def _make_incident(self, status=IncidentStatus.ACTIVE): return Incident.objects.create( title="Test Incident", status=status, @@ -593,24 +648,46 @@ def _linear_settings(self, settings): "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_completed(incident, svc, ["Done", "Done"]) + _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_completed(incident, svc, ["Done", "Canceled"]) + _comment_parent_issue_status_change( + incident, svc, "completed", ["Done", "Canceled"] + ) svc.create_comment.assert_called_once_with( "lin-123", @@ -622,16 +699,16 @@ def test_empty_template_skips_comment(self, settings): incident = self._make_incident(status=IncidentStatus.DONE) svc = MagicMock() - _comment_parent_issue_completed(incident, svc, ["Done"]) + _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_COMPLETED"] = " " - incident = self._make_incident(status=IncidentStatus.DONE) + settings.LINEAR["PARENT_STATUS_COMMENT_STARTED"] = " " + incident = self._make_incident(status=IncidentStatus.ACTIVE) svc = MagicMock() - _comment_parent_issue_completed(incident, svc, ["Done"]) + _comment_parent_issue_status_change(incident, svc, "started", ["Todo"]) svc.create_comment.assert_not_called() @@ -639,7 +716,7 @@ def test_no_action_items(self): incident = self._make_incident(status=IncidentStatus.DONE) svc = MagicMock() - _comment_parent_issue_completed(incident, svc, []) + _comment_parent_issue_status_change(incident, svc, "completed", []) svc.create_comment.assert_called_once_with( "lin-123", @@ -651,7 +728,7 @@ def test_create_comment_failure_logs_and_continues(self): svc = MagicMock() svc.create_comment.return_value = False - _comment_parent_issue_completed(incident, svc, ["Done"]) + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) svc.create_comment.assert_called_once() @@ -660,7 +737,7 @@ def test_create_comment_exception_logs_and_continues(self): svc = MagicMock() svc.create_comment.side_effect = Exception("API error") - _comment_parent_issue_completed(incident, svc, ["Done"]) + _comment_parent_issue_status_change(incident, svc, "completed", ["Done"]) svc.create_comment.assert_called_once() @@ -669,6 +746,6 @@ def test_template_render_error_logs_and_continues(self, settings): incident = self._make_incident(status=IncidentStatus.DONE) svc = MagicMock() - _comment_parent_issue_completed(incident, svc, ["Done"]) + _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 c89cf85d..a4b6de78 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -23,13 +23,6 @@ "canceled": "Canceled", } -LINEAR_RELATION_TYPE_MAP = { - "related": "related", - "blocks": "blocks", - "blocked": "blocked_by", - "duplicate": "duplicate", -} - TOKEN_LIFETIME = timedelta(days=30) TOKEN_REFRESH_BUFFER = timedelta(days=1) @@ -471,81 +464,3 @@ def get_child_issues(self, issue_id: str) -> list[dict[str, Any]] | None: break return issues - - def _fetch_relations( - self, - issue_id: str, - field: str, - issue_key: str, - seen_ids: set[str], - ) -> list[dict[str, Any]] | None: - query = f""" - query($issueId: String!, $after: String) {{ - issue(id: $issueId) {{ - {field}(first: 50, after: $after) {{ - nodes {{ - type - {issue_key} {{ - {ISSUE_FIELDS} - }} - }} - pageInfo {{ - hasNextPage - endCursor - }} - }} - }} - }} - """ - - issues: list[dict[str, Any]] = [] - cursor: str | None = None - max_pages = 25 - - for _ in range(max_pages): - variables: dict[str, Any] = {"issueId": issue_id} - if cursor is not None: - variables["after"] = cursor - - data = self._graphql(query, variables) - if data is None: - return None - - issue = data.get("issue") - if not issue: - return None - - relations = issue.get(field, {}) - for node in relations.get("nodes", []): - related_issue = node.get(issue_key) - if not related_issue or "id" not in related_issue: - continue - if related_issue["id"] in seen_ids: - continue - seen_ids.add(related_issue["id"]) - - linear_type = node.get("type", "").lower() - relation_type = LINEAR_RELATION_TYPE_MAP.get(linear_type, "related") - issues.append(self._parse_issue(related_issue, relation_type)) - - page_info = relations.get("pageInfo", {}) - if not page_info.get("hasNextPage"): - break - cursor = page_info.get("endCursor") - if cursor is None: - break - - return issues - - def get_related_issues(self, issue_id: str) -> list[dict[str, Any]] | None: - seen_ids: set[str] = set() - - forward = self._fetch_relations(issue_id, "relations", "relatedIssue", seen_ids) - if forward is None: - return None - - inverse = self._fetch_relations(issue_id, "inverseRelations", "issue", seen_ids) - if inverse is None: - return None - - return forward + inverse diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index 390ef7dd..dbeebf99 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -537,98 +537,6 @@ def test_paginates_through_multiple_pages(self, linear_service): assert second_call_vars["after"] == "cursor1" -class TestGetRelatedIssues: - def test_combines_forward_and_inverse_relations(self, linear_service): - forward_response = { - "issue": { - "relations": { - "nodes": [ - { - "type": "related", - "relatedIssue": { - "id": "r1", - "identifier": "E-1", - "title": "Related", - "url": "u1", - "state": {"type": "started"}, - "assignee": None, - }, - } - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - inverse_response = { - "issue": { - "inverseRelations": { - "nodes": [ - { - "type": "blocks", - "issue": { - "id": "r2", - "identifier": "E-2", - "title": "Blocker", - "url": "u2", - "state": {"type": "unstarted"}, - "assignee": None, - }, - } - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - - with patch.object( - linear_service, - "_graphql", - side_effect=[forward_response, inverse_response], - ): - result = linear_service.get_related_issues("issue-1") - - assert len(result) == 2 - assert result[0]["identifier"] == "E-1" - assert result[0]["relation_type"] == "related" - assert result[1]["identifier"] == "E-2" - assert result[1]["relation_type"] == "blocks" - - def test_deduplicates_across_forward_and_inverse(self, linear_service): - shared_issue = { - "id": "same-id", - "identifier": "E-1", - "title": "Same", - "url": "u", - "state": {"type": "started"}, - "assignee": None, - } - forward_response = { - "issue": { - "relations": { - "nodes": [{"type": "related", "relatedIssue": shared_issue}], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - inverse_response = { - "issue": { - "inverseRelations": { - "nodes": [{"type": "related", "issue": shared_issue}], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - } - } - } - - with patch.object( - linear_service, - "_graphql", - side_effect=[forward_response, inverse_response], - ): - result = linear_service.get_related_issues("issue-1") - - assert len(result) == 1 - - class TestGetUserByEmail: def test_returns_user_when_found(self, linear_service): mock_response = { diff --git a/src/firetower/settings.py b/src/firetower/settings.py index c675c6a8..9cc6da37 100644 --- a/src/firetower/settings.py +++ b/src/firetower/settings.py @@ -328,6 +328,7 @@ class StatuspageSettings(TypedDict): "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