Make it work with new ruff and mypy requirements

Author: spalmurray

src/firetower/auth/tests/test_services.py

@@ -1,4 +1,4 @@
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch
 
 import pytest
 from django.contrib.auth.models import User

src/firetower/incidents/admin.py

@@ -3,7 +3,7 @@
 from typing import Any
 
 from django.contrib import admin
-from django.db.models import ForeignKey, ManyToManyField
+from django.db.models import ForeignKey, ManyToManyField, QuerySet
 from django.forms import ModelChoiceField, ModelMultipleChoiceField
 from django.http import HttpRequest
 
@@ -72,17 +72,19 @@ def incident_number_display(self, obj: Incident) -> str:
     incident_number_display.admin_order_field = "id"
 
     @admin.action(description="Sync participants from Slack")
-    def sync_participants_from_slack(self, request, queryset):
+    def sync_participants_from_slack(
+        self, request: HttpRequest, queryset: QuerySet[Incident]
+    ) -> None:
         success_count = 0
         skipped_count = 0
         error_count = 0
 
         for incident in queryset:
             try:
                 stats = sync_incident_participants_from_slack(incident, force=True)
-                if stats["errors"]:
+                if stats.errors:
                     error_count += 1
-                elif stats["skipped"]:
+                elif stats.skipped:
                     skipped_count += 1
                 else:
                     success_count += 1

src/firetower/incidents/services.py

@@ -1,17 +1,31 @@
 import logging
+from dataclasses import dataclass, field
 from datetime import timedelta
 
+from django.conf import settings
 from django.utils import timezone
 
 from firetower.auth.services import get_or_create_user_from_slack_id
-from firetower.incidents.models import ExternalLinkType
+from firetower.incidents.models import ExternalLinkType, Incident
 from firetower.integrations.services import SlackService
 
 logger = logging.getLogger(__name__)
 _slack_service = SlackService()
 
 
-def sync_incident_participants_from_slack(incident, force=False):
+@dataclass
+class ParticipantsSyncStats:
+    """Statistics from a participant sync operation."""
+
+    added: int = 0
+    already_existed: int = 0
+    errors: list[str] = field(default_factory=list)
+    skipped: bool = False
+
+
+def sync_incident_participants_from_slack(
+    incident: Incident, force: bool = False
+) -> ParticipantsSyncStats:
     """
     Sync incident participants from Slack channel members.
 
@@ -20,21 +34,9 @@ def sync_incident_participants_from_slack(incident, force=False):
         force: If True, bypass throttle and force sync
 
     Returns:
-        dict with sync stats: {
-            "added": int,
-            "already_existed": int,
-            "errors": list[str],
-            "skipped": bool,
-        }
+        ParticipantsSyncStats dataclass with sync statistics
     """
-    from django.conf import settings
-
-    stats = {
-        "added": 0,
-        "already_existed": 0,
-        "errors": [],
-        "skipped": False,
-    }
+    stats = ParticipantsSyncStats()
 
     if not force and incident.participants_last_synced_at:
         time_since_sync = timezone.now() - incident.participants_last_synced_at
@@ -44,31 +46,31 @@ def sync_incident_participants_from_slack(incident, force=False):
             logger.info(
                 f"Skipping sync for incident {incident.id} - synced {time_since_sync.total_seconds():.0f}s ago"
             )
-            stats["skipped"] = True
+            stats.skipped = True
             return stats
 
     slack_link = incident.external_links.filter(type=ExternalLinkType.SLACK).first()
 
     if not slack_link:
         error_msg = f"No Slack link found for incident {incident.id}"
         logger.warning(error_msg)
-        stats["errors"].append(error_msg)
+        stats.errors.append(error_msg)
         return stats
 
     channel_id = _slack_service.parse_channel_id_from_url(slack_link.url)
 
     if not channel_id:
         error_msg = f"Could not parse channel ID from URL: {slack_link.url}"
         logger.warning(error_msg)
-        stats["errors"].append(error_msg)
+        stats.errors.append(error_msg)
         return stats
 
     slack_member_ids = _slack_service.get_channel_members(channel_id)
 
     if slack_member_ids is None:
         error_msg = f"Failed to fetch channel members for {channel_id}"
         logger.error(error_msg)
-        stats["errors"].append(error_msg)
+        stats.errors.append(error_msg)
         return stats
 
     logger.info(
@@ -88,14 +90,14 @@ def sync_incident_participants_from_slack(incident, force=False):
         if not user:
             error_msg = f"Could not get/create user for Slack ID: {slack_user_id}"
             logger.warning(error_msg)
-            stats["errors"].append(error_msg)
+            stats.errors.append(error_msg)
             continue
 
         if user.id in existing_participant_ids:
-            stats["already_existed"] += 1
+            stats.already_existed += 1
         else:
             new_participants.append(user)
-            stats["added"] += 1
+            stats.added += 1
 
     if new_participants:
         incident.participants.add(*new_participants)
@@ -107,8 +109,8 @@ def sync_incident_participants_from_slack(incident, force=False):
     incident.save(update_fields=["participants_last_synced_at"])
 
     logger.info(
-        f"Sync complete for incident {incident.id}: {stats['added']} added, "
-        f"{stats['already_existed']} already existed, {len(stats['errors'])} errors"
+        f"Sync complete for incident {incident.id}: {stats.added} added, "
+        f"{stats.already_existed} already existed, {len(stats.errors)} errors"
     )
 
     return stats

src/firetower/incidents/tests/test_admin.py

@@ -8,6 +8,7 @@
 
 from firetower.incidents.admin import IncidentAdmin
 from firetower.incidents.models import Incident, IncidentSeverity, IncidentStatus
+from firetower.incidents.services import ParticipantsSyncStats
 
 
 @pytest.mark.django_db
@@ -54,12 +55,10 @@ def test_sync_participants_action_calls_sync_function(self):
         with patch(
             "firetower.incidents.admin.sync_incident_participants_from_slack"
         ) as mock_sync:
-            mock_sync.return_value = {
-                "added": 5,
-                "already_existed": 2,
-                "errors": [],
-                "skipped": False,
-            }
+            mock_sync.return_value = ParticipantsSyncStats(
+                added=5,
+                already_existed=2,
+            )
 
             self.incident_admin.sync_participants_from_slack(request, queryset)
 
@@ -97,19 +96,14 @@ def test_sync_participants_action_reports_stats(self):
 
             def side_effect(incident, force):
                 if incident.id == incident1.id:
-                    return {
-                        "added": 5,
-                        "already_existed": 2,
-                        "errors": [],
-                        "skipped": False,
-                    }
+                    return ParticipantsSyncStats(
+                        added=5,
+                        already_existed=2,
+                    )
                 elif incident.id == incident2.id:
-                    return {
-                        "added": 0,
-                        "already_existed": 0,
-                        "errors": ["Some error"],
-                        "skipped": False,
-                    }
+                    return ParticipantsSyncStats(
+                        errors=["Some error"],
+                    )
                 else:
                     raise Exception("Sync failed")
 

src/firetower/incidents/tests/test_services.py

@@ -68,10 +68,10 @@ def test_syncs_participants_from_slack_channel(self):
 
                 stats = sync_incident_participants_from_slack(incident)
 
-                assert stats["added"] == 2
-                assert stats["already_existed"] == 0
-                assert stats["errors"] == []
-                assert stats["skipped"] is False
+                assert stats.added == 2
+                assert stats.already_existed == 0
+                assert stats.errors == []
+                assert stats.skipped is False
 
                 assert incident.participants.count() == 3
                 assert slack_user1 in incident.participants.all()
@@ -139,8 +139,8 @@ def test_throttle_skips_recent_sync(self):
 
         stats = sync_incident_participants_from_slack(incident)
 
-        assert stats["skipped"] is True
-        assert stats["added"] == 0
+        assert stats.skipped is True
+        assert stats.added == 0
 
     def test_force_bypasses_throttle(self):
         incident = Incident.objects.create(
@@ -177,8 +177,8 @@ def test_force_bypasses_throttle(self):
 
                 stats = sync_incident_participants_from_slack(incident, force=True)
 
-                assert stats["skipped"] is False
-                assert stats["added"] == 1
+                assert stats.skipped is False
+                assert stats.added == 1
 
     def test_handles_missing_slack_link(self):
         incident = Incident.objects.create(
@@ -189,9 +189,9 @@ def test_handles_missing_slack_link(self):
 
         stats = sync_incident_participants_from_slack(incident)
 
-        assert stats["added"] == 0
-        assert len(stats["errors"]) == 1
-        assert "No Slack link" in stats["errors"][0]
+        assert stats.added == 0
+        assert len(stats.errors) == 1
+        assert "No Slack link" in stats.errors[0]
 
     def test_handles_invalid_channel_url(self):
         incident = Incident.objects.create(
@@ -212,9 +212,9 @@ def test_handles_invalid_channel_url(self):
 
             stats = sync_incident_participants_from_slack(incident)
 
-            assert stats["added"] == 0
-            assert len(stats["errors"]) == 1
-            assert "Could not parse channel ID" in stats["errors"][0]
+            assert stats.added == 0
+            assert len(stats.errors) == 1
+            assert "Could not parse channel ID" in stats.errors[0]
 
     def test_handles_slack_api_failure(self):
         incident = Incident.objects.create(
@@ -240,9 +240,9 @@ def test_handles_slack_api_failure(self):
 
                 stats = sync_incident_participants_from_slack(incident)
 
-                assert stats["added"] == 0
-                assert len(stats["errors"]) == 1
-                assert "Failed to fetch channel members" in stats["errors"][0]
+                assert stats.added == 0
+                assert len(stats.errors) == 1
+                assert "Failed to fetch channel members" in stats.errors[0]
 
     def test_handles_user_creation_failure(self):
         incident = Incident.objects.create(
@@ -273,9 +273,9 @@ def test_handles_user_creation_failure(self):
 
                     stats = sync_incident_participants_from_slack(incident)
 
-                    assert stats["added"] == 0
-                    assert len(stats["errors"]) == 1
-                    assert "Could not get/create user" in stats["errors"][0]
+                    assert stats.added == 0
+                    assert len(stats.errors) == 1
+                    assert "Could not get/create user" in stats.errors[0]
 
     def test_counts_already_existed_participants(self):
         incident = Incident.objects.create(
@@ -312,8 +312,8 @@ def test_counts_already_existed_participants(self):
 
                 stats = sync_incident_participants_from_slack(incident)
 
-                assert stats["added"] == 0
-                assert stats["already_existed"] == 1
+                assert stats.added == 0
+                assert stats.already_existed == 1
                 assert incident.participants.count() == 1
 
     def test_skips_bots(self):
@@ -357,5 +357,5 @@ def test_skips_bots(self):
 
                     assert mock_get_user.call_count == 1
                     mock_get_user.assert_called_once_with("U11111")
-                    assert stats["added"] == 1
-                    assert stats["errors"] == []
+                    assert stats.added == 1
+                    assert stats.errors == []

src/firetower/incidents/tests/test_views.py

@@ -14,6 +14,7 @@
     Tag,
     TagType,
 )
+from firetower.incidents.services import ParticipantsSyncStats
 
 
 @pytest.mark.django_db
@@ -323,12 +324,10 @@ def test_sync_participants_endpoint(self):
         with patch(
             "firetower.incidents.views.sync_incident_participants_from_slack"
         ) as mock_sync:
-            mock_sync.return_value = {
-                "added": 3,
-                "already_existed": 5,
-                "errors": [],
-                "skipped": False,
-            }
+            mock_sync.return_value = ParticipantsSyncStats(
+                added=3,
+                already_existed=5,
+            )
 
             self.client.force_authenticate(user=self.user)
             response = self.client.post(