diff --git a/src/firetower/incidents/serializers.py b/src/firetower/incidents/serializers.py index 8d92395c..92ce3c12 100644 --- a/src/firetower/incidents/serializers.py +++ b/src/firetower/incidents/serializers.py @@ -4,7 +4,6 @@ from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ValidationError as DjangoValidationError -from django.db import transaction from django.db.models.functions import Lower from rest_framework import serializers @@ -19,7 +18,6 @@ on_visibility_changed, ) from .models import ( - INCIDENT_ID_START, USER_ADDABLE_TAG_TYPES, ExternalLink, ExternalLinkType, @@ -458,16 +456,22 @@ def validate_external_links( return value - def _set_tags_and_links( - self, - incident: Incident, - external_links_data: dict[str, str | None] | None, - affected_service_tag_names: list[str] | None, - affected_region_tag_names: list[str] | None, - root_cause_tag_names: list[str] | None, - impact_type_tag_names: list[str] | None, - ) -> None: - """Set external links and tags on a newly created incident.""" + def create(self, validated_data: dict) -> Incident: + """Create incident with external links and tags""" + external_links_data = validated_data.pop("external_links", None) + affected_service_tag_names = validated_data.pop( + "affected_service_tag_names", None + ) + affected_region_tag_names = validated_data.pop( + "affected_region_tag_names", None + ) + root_cause_tag_names = validated_data.pop("root_cause_tag_names", None) + impact_type_tag_names = validated_data.pop("impact_type_tag_names", None) + + # Create the incident + incident = super().create(validated_data) + + # Create external links if provided if external_links_data: for link_type, url in external_links_data.items(): if url is not None: # Skip null values on create @@ -477,6 +481,7 @@ def _set_tags_and_links( url=url, ) + # Set tags if provided if affected_service_tag_names: tags = Tag.objects.annotate(name_lower=Lower("name")).filter( name_lower__in=[n.lower() for n in affected_service_tag_names], @@ -505,32 +510,9 @@ def _set_tags_and_links( ) incident.impact_type_tags.set(tags) - def create(self, validated_data: dict, skip_hooks: bool = False) -> Incident: - """Create incident with external links and tags""" - external_links_data = validated_data.pop("external_links", None) - affected_service_tag_names = validated_data.pop( - "affected_service_tag_names", None - ) - affected_region_tag_names = validated_data.pop( - "affected_region_tag_names", None - ) - root_cause_tag_names = validated_data.pop("root_cause_tag_names", None) - impact_type_tag_names = validated_data.pop("impact_type_tag_names", None) - - incident = super().create(validated_data) - - self._set_tags_and_links( - incident, - external_links_data, - affected_service_tag_names, - affected_region_tag_names, - root_cause_tag_names, - impact_type_tag_names, - ) - # Runs synchronously — Slack API calls may add latency to the response. # Consider deferring to a background task if this becomes a problem. - if settings.HOOKS_ENABLED and not skip_hooks: + if settings.HOOKS_ENABLED: on_incident_created(incident) return incident @@ -627,50 +609,6 @@ def update(self, instance: Incident, validated_data: dict) -> Incident: return instance -class IncidentImportSerializer(IncidentWriteSerializer): - """Temporary: used for one-time Jira incident migration.""" - - id = serializers.IntegerField(required=True) - created_at = serializers.DateTimeField(required=False) - updated_at = serializers.DateTimeField(required=False) - - class Meta(IncidentWriteSerializer.Meta): - fields = [*IncidentWriteSerializer.Meta.fields, "created_at", "updated_at"] - - def validate_id(self, value: int) -> int: - if value < 1: - raise serializers.ValidationError("ID must be >= 1.") - if value >= INCIDENT_ID_START: - raise serializers.ValidationError( - f"ID must be less than {INCIDENT_ID_START}." - ) - if Incident.objects.filter(pk=value).exists(): - raise serializers.ValidationError( - f"Incident with ID {value} already exists." - ) - return value - - @transaction.atomic - def create(self, validated_data: dict, skip_hooks: bool = False) -> Incident: - created_at = validated_data.pop("created_at", None) - updated_at = validated_data.pop("updated_at", None) - - # Always skip hooks — no Slack channel creation or notifications for - # historical incidents being imported from Jira. - incident = super().create(validated_data, skip_hooks=True) - - timestamp_updates = {} - if created_at is not None: - timestamp_updates["created_at"] = created_at - if updated_at is not None: - timestamp_updates["updated_at"] = updated_at - if timestamp_updates: - Incident.objects.filter(pk=incident.pk).update(**timestamp_updates) - incident.refresh_from_db() - - return incident - - class TagSerializer(serializers.ModelSerializer): class Meta: model = Tag diff --git a/src/firetower/incidents/tests/test_views.py b/src/firetower/incidents/tests/test_views.py index b6ca6606..97f52d51 100644 --- a/src/firetower/incidents/tests/test_views.py +++ b/src/firetower/incidents/tests/test_views.py @@ -8,7 +8,6 @@ from rest_framework.test import APIClient from firetower.incidents.models import ( - INCIDENT_ID_START, ExternalLink, ExternalLinkType, Incident, @@ -1482,126 +1481,3 @@ def test_list_tags_sorted_by_usage(self): assert response.status_code == 200 assert response.data == ["UsedTwice", "UsedOnce", "Unused"] - - -@pytest.mark.django_db -class TestIncidentImportAPIView: - def setup_method(self): - self.client = APIClient() - self.staff_user = User.objects.create_user( - username="admin@example.com", - email="admin@example.com", - password="testpass123", - is_staff=True, - ) - self.regular_user = User.objects.create_user( - username="user@example.com", - email="user@example.com", - password="testpass123", - ) - self.captain = User.objects.create_user( - username="captain@example.com", - email="captain@example.com", - password="testpass123", - ) - self.reporter = User.objects.create_user( - username="reporter@example.com", - email="reporter@example.com", - password="testpass123", - ) - - def _import_data(self, **overrides): - data = { - "id": 1, - "title": "Imported Incident", - "severity": IncidentSeverity.P1, - "captain": self.captain.email, - "reporter": self.reporter.email, - } - data.update(overrides) - return data - - def test_import_with_explicit_id(self): - self.client.force_authenticate(user=self.staff_user) - data = self._import_data(id=42) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 201 - assert Incident.objects.filter(pk=42).exists() - - def test_import_with_timestamp_overrides(self): - self.client.force_authenticate(user=self.staff_user) - data = self._import_data( - created_at="2020-01-15T10:30:00Z", - updated_at="2020-06-20T14:00:00Z", - ) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 201 - incident = Incident.objects.get(pk=data["id"]) - assert incident.created_at.year == 2020 - assert incident.created_at.month == 1 - assert incident.updated_at.year == 2020 - assert incident.updated_at.month == 6 - - def test_import_with_metadata(self): - Tag.objects.create(name="TestService", type=TagType.AFFECTED_SERVICE) - self.client.force_authenticate(user=self.staff_user) - data = self._import_data( - affected_service_tags=["TestService"], - external_links={"jira": "https://jira.example.com/browse/INC-1"}, - ) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 201 - incident = Incident.objects.get(pk=data["id"]) - assert list(incident.affected_service_tags.values_list("name", flat=True)) == [ - "TestService" - ] - assert incident.external_links.filter(type=ExternalLinkType.JIRA).exists() - - def test_rejects_id_at_or_above_threshold(self): - self.client.force_authenticate(user=self.staff_user) - data = self._import_data(id=INCIDENT_ID_START) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 400 - assert "id" in response.data - - def test_rejects_id_below_one(self): - self.client.force_authenticate(user=self.staff_user) - data = self._import_data(id=0) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 400 - assert "id" in response.data - - def test_rejects_duplicate_id(self): - Incident.objects.create( - id=100, - title="Existing", - severity=IncidentSeverity.P1, - status=IncidentStatus.ACTIVE, - ) - self.client.force_authenticate(user=self.staff_user) - data = self._import_data(id=100) - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 400 - assert "id" in response.data - - def test_requires_staff_user(self): - self.client.force_authenticate(user=self.regular_user) - data = self._import_data() - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 403 - - @patch("firetower.incidents.serializers.on_incident_created") - def test_does_not_trigger_on_incident_created(self, mock_hook): - self.client.force_authenticate(user=self.staff_user) - data = self._import_data() - response = self.client.post("/api/incidents/import/", data, format="json") - - assert response.status_code == 201 - mock_hook.assert_not_called() diff --git a/src/firetower/incidents/urls.py b/src/firetower/incidents/urls.py index 7a02567a..42d5406e 100644 --- a/src/firetower/incidents/urls.py +++ b/src/firetower/incidents/urls.py @@ -2,7 +2,6 @@ from .views import ( AvailabilityView, - IncidentImportAPIView, IncidentListCreateAPIView, IncidentRetrieveUpdateAPIView, TagListCreateAPIView, @@ -21,12 +20,6 @@ name="incident-detail-ui", ), # Service API endpoints - # Temporary: one-time Jira migration import endpoint - path( - "incidents/import/", - IncidentImportAPIView.as_view(), - name="incident-import", - ), path( "incidents/", IncidentListCreateAPIView.as_view(), diff --git a/src/firetower/incidents/views.py b/src/firetower/incidents/views.py index c1d7fcd1..604b1efe 100644 --- a/src/firetower/incidents/views.py +++ b/src/firetower/incidents/views.py @@ -7,7 +7,7 @@ from django.db.models import Count, QuerySet from django.shortcuts import get_object_or_404 from django.utils import timezone -from rest_framework import generics, permissions, serializers +from rest_framework import generics, serializers from rest_framework.exceptions import ValidationError from rest_framework.request import Request from rest_framework.response import Response @@ -39,7 +39,6 @@ get_year_periods, ) from .serializers import ( - IncidentImportSerializer, IncidentListUISerializer, IncidentOrRedirectReadSerializer, IncidentReadSerializer, @@ -262,13 +261,6 @@ def get_object(self) -> Incident: return obj -class IncidentImportAPIView(generics.CreateAPIView): - """Temporary: admin-only endpoint for one-time Jira incident migration.""" - - serializer_class = IncidentImportSerializer - permission_classes = [permissions.IsAdminUser] - - class SyncIncidentParticipantsView(generics.GenericAPIView): """ Force sync incident participants from Slack channel.