From 1e02158f881635dea8a54a093ea85e92a16afb4b Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Fri, 16 Jan 2026 15:53:09 -0500 Subject: [PATCH] Add table storing next inc id that is lockable to ensure clean single id increments over postgres auto field --- .../migrations/0008_gapless_incident_ids.py | 54 +++++++++++++++++++ src/firetower/incidents/models.py | 35 ++++++++++-- src/firetower/incidents/tests/test_models.py | 30 +++++++++++ 3 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 src/firetower/incidents/migrations/0008_gapless_incident_ids.py diff --git a/src/firetower/incidents/migrations/0008_gapless_incident_ids.py b/src/firetower/incidents/migrations/0008_gapless_incident_ids.py new file mode 100644 index 00000000..19c4ba2c --- /dev/null +++ b/src/firetower/incidents/migrations/0008_gapless_incident_ids.py @@ -0,0 +1,54 @@ +from django.db import migrations, models + + +def seed_counter(apps, schema_editor): + """Seed the counter with MAX(id) + 1 or 2000 if no incidents exist.""" + IncidentCounter = apps.get_model("incidents", "IncidentCounter") + Incident = apps.get_model("incidents", "Incident") + + max_id = Incident.objects.aggregate(max_id=models.Max("id"))["max_id"] + next_id = (max_id + 1) if max_id is not None else 2000 + + IncidentCounter.objects.create(next_id=next_id) + + +def reverse_seed_counter(apps, schema_editor): + """Remove the counter row.""" + IncidentCounter = apps.get_model("incidents", "IncidentCounter") + IncidentCounter.objects.all().delete() + + +class Migration(migrations.Migration): + dependencies = [ + ("incidents", "0007_add_milestone_timestamps"), + ] + + operations = [ + # Change id field from AutoField to PositiveIntegerField + migrations.AlterField( + model_name="incident", + name="id", + field=models.PositiveIntegerField(primary_key=True, serialize=False), + ), + # Create the counter table + migrations.CreateModel( + name="IncidentCounter", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("next_id", models.PositiveIntegerField(default=2000)), + ], + options={ + "db_table": "incidents_incident_counter", + }, + ), + # Seed the counter with the appropriate starting value + migrations.RunPython(seed_counter, reverse_seed_counter), + ] diff --git a/src/firetower/incidents/models.py b/src/firetower/incidents/models.py index ad4a1786..e6d13a87 100644 --- a/src/firetower/incidents/models.py +++ b/src/firetower/incidents/models.py @@ -3,9 +3,30 @@ from django.conf import settings from django.contrib.auth.models import AbstractUser, User from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction from django.db.models import Q, QuerySet +INCIDENT_ID_START = 2000 + + +class IncidentCounter(models.Model): + """Stores the next available incident ID for gapless sequencing.""" + + next_id = models.PositiveIntegerField(default=INCIDENT_ID_START) + + class Meta: + db_table = "incidents_incident_counter" + + +def get_next_incident_id() -> int: + """Atomically get and increment the incident ID counter.""" + with transaction.atomic(): + counter = IncidentCounter.objects.select_for_update().get() + next_id = counter.next_id + counter.next_id += 1 + counter.save() + return next_id + class IncidentStatus(models.TextChoices): ACTIVE = "Active", "Active" @@ -100,7 +121,8 @@ class Incident(models.Model): """ # Primary key - numeric ID (2000, 2001, etc.) - id = models.AutoField(primary_key=True) + # Uses IncidentCounter for gapless sequential IDs + id = models.PositiveIntegerField(primary_key=True) # Core fields title = models.CharField(max_length=500) @@ -235,9 +257,12 @@ def clean(self) -> None: raise ValidationError({"severity": "Severity must be set"}) def save(self, *args: Any, **kwargs: Any) -> None: - """Override save to run validation""" - self.full_clean() - super().save(*args, **kwargs) + """Override save to run validation and assign gapless ID""" + with transaction.atomic(): + if self._state.adding and self.id is None: + self.id = get_next_incident_id() + self.full_clean() + super().save(*args, **kwargs) def __str__(self) -> str: return f"{self.incident_number}: {self.title}" diff --git a/src/firetower/incidents/tests/test_models.py b/src/firetower/incidents/tests/test_models.py index 358a2e46..7fbb284e 100644 --- a/src/firetower/incidents/tests/test_models.py +++ b/src/firetower/incidents/tests/test_models.py @@ -8,6 +8,7 @@ ExternalLink, ExternalLinkType, Incident, + IncidentCounter, IncidentSeverity, IncidentStatus, Tag, @@ -47,6 +48,35 @@ def test_incident_id_starts_at_2000(self): # Should be 2000 or higher (in case other tests ran first) assert incident.id >= 2000 + def test_incident_ids_are_gapless_after_failed_save(self): + """Test that failed saves don't consume IDs (gapless sequence)""" + incident1 = Incident.objects.create( + title="First", + status=IncidentStatus.ACTIVE, + severity=IncidentSeverity.P1, + ) + first_id = incident1.id + + # Attempt to create an incident that will fail validation (empty title) + with pytest.raises(ValidationError): + Incident.objects.create( + title="", # Invalid - will fail validation + status=IncidentStatus.ACTIVE, + severity=IncidentSeverity.P1, + ) + + # The counter should NOT have incremented due to the failed save + counter = IncidentCounter.objects.get() + assert counter.next_id == first_id + 1 + + # Create another valid incident - should be exactly first_id + 1 + incident2 = Incident.objects.create( + title="Second", + status=IncidentStatus.ACTIVE, + severity=IncidentSeverity.P1, + ) + assert incident2.id == first_id + 1 + def test_incident_number_property(self): """Test incident_number property returns correct format""" incident = Incident.objects.create(