Make bed names unique for location (#1783)

* make bed names unique for location

* wrap in atomic block

* add tests

* fix migration

* improvements

* make validations lowercase

* add tests

* improve migration queries

* merge migrations

care/facility/api/serializers/bed.py

@@ -5,6 +5,7 @@
 from rest_framework.exceptions import ValidationError
 from rest_framework.serializers import (
     BooleanField,
+    CharField,
     DateTimeField,
     IntegerField,
     ListField,
@@ -35,6 +36,7 @@
 
 class BedSerializer(ModelSerializer):
     id = UUIDField(source="external_id", read_only=True)
+    name = CharField(max_length=1024, required=True)
     bed_type = ChoiceField(choices=BedTypeChoices)
 
     location_object = AssetLocationSerializer(source="location", read_only=True)
@@ -45,6 +47,9 @@ class BedSerializer(ModelSerializer):
 
     number_of_beds = IntegerField(required=False, default=1, write_only=True)
 
+    def validate_name(self, value):
+        return value.strip() if value else value
+
     def validate_number_of_beds(self, value):
         if value > 100:
             raise ValidationError("Cannot create more than 100 beds at once.")

care/facility/api/viewsets/bed.py

@@ -1,4 +1,5 @@
 from django.core.exceptions import ValidationError as DjangoValidationError
+from django.db import IntegrityError, transaction
 from django.db.models import OuterRef, Subquery
 from django_filters import rest_framework as filters
 from drf_spectacular.utils import extend_schema, extend_schema_view
@@ -73,22 +74,29 @@ def get_queryset(self):
             queryset = queryset.filter(facility__id__in=allowed_facilities)
         return queryset
 
+    @transaction.atomic
     def create(self, request, *args, **kwargs):
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
         number_of_beds = serializer.validated_data.pop("number_of_beds", 1)
         # Bulk creating n number of beds
         if number_of_beds > 1:
             data = serializer.validated_data.copy()
-            data.pop("name")
+            name = data.pop("name")
             beds = [
                 Bed(
                     **data,
-                    name=f"{serializer.validated_data['name']} {i+1}",
+                    name=f"{name} {i}",
                 )
-                for i in range(number_of_beds)
+                for i in range(1, number_of_beds + 1)
             ]
-            Bed.objects.bulk_create(beds)
+            try:
+                Bed.objects.bulk_create(beds)
+            except IntegrityError:
+                return Response(
+                    {"detail": "Bed with same name already exists in this location."},
+                    status=status.HTTP_400_BAD_REQUEST,
+                )
             return Response(status=status.HTTP_201_CREATED)
 
         self.perform_create(serializer)

care/facility/migrations/0405_bed_name_constraint.py

@@ -0,0 +1,50 @@
+# Generated by Django 4.2.8 on 2023-12-26 06:17
+
+from django.db import migrations, models
+from django.db.models import F, Window
+from django.db.models.functions import RowNumber
+
+
+def copy_bed_name(apps, schema_editor):
+    Bed = apps.get_model("facility", "Bed")
+    Bed.objects.update(old_name=models.F("name"))
+
+
+def reverse_copy_bed_name(apps, schema_editor):
+    Bed = apps.get_model("facility", "Bed")
+    Bed.objects.filter(old_name__isnull=False).update(name=models.F("old_name"))
+
+
+def fix_duplicate_bed_names(apps, schema_editor):
+    Bed = apps.get_model("facility", "Bed")
+
+    window = Window(
+        expression=RowNumber(),
+        partition_by=[F("location"), F("lname")],
+        order_by=F("id").asc(),
+    )
+
+    beds = Bed.objects.annotate(
+        lname=models.functions.Lower("name"), row_number=window
+    ).filter(row_number__gt=1)
+
+    for bed in beds:
+        bed.name = f"{bed.name} ({bed.row_number - 1})"
+
+    Bed.objects.bulk_update(beds, ["name"], batch_size=2000)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("facility", "0404_merge_20231220_2227"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="bed",
+            name="old_name",
+            field=models.CharField(blank=True, max_length=1024, null=True),
+        ),
+        migrations.RunPython(copy_bed_name, reverse_copy_bed_name),
+        migrations.RunPython(fix_duplicate_bed_names, migrations.RunPython.noop),
+    ]

care/facility/migrations/0406_bed_unique_bed_name_per_location.py

@@ -0,0 +1,21 @@
+# Generated by Django 4.2.8 on 2023-12-27 05:10
+
+import django.db.models.functions.text
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("facility", "0405_bed_name_constraint"),
+    ]
+
+    operations = [
+        migrations.AddConstraint(
+            model_name="bed",
+            constraint=models.UniqueConstraint(
+                django.db.models.functions.text.Lower("name"),
+                models.F("location"),
+                name="unique_bed_name_per_location",
+            ),
+        ),
+    ]

care/facility/migrations/0409_merge_20240210_1510.py

@@ -0,0 +1,14 @@
+# Generated by Django 4.2.8 on 2024-02-10 09:40
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("facility", "0406_bed_unique_bed_name_per_location"),
+        ("facility", "0408_alter_dailyround_resp"),
+        ("facility", "0408_delete_metaicd11diagnosis"),
+        ("facility", "0408_historicalpatientregistration_date_of_delivery_and_more"),
+    ]
+
+    operations = []

care/facility/models/bed.py

@@ -17,6 +17,7 @@
 
 class Bed(BaseModel):
     name = models.CharField(max_length=1024)
+    old_name = models.CharField(max_length=1024, null=True, blank=True)
     description = models.TextField(default="", blank=True)
     bed_type = models.IntegerField(
         choices=BedTypeChoices, default=BedType.REGULAR.value
@@ -30,6 +31,15 @@ class Bed(BaseModel):
         AssetLocation, on_delete=models.PROTECT, null=False, blank=False
     )
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(
+                models.functions.Lower("name"),
+                "location",
+                name="unique_bed_name_per_location",
+            )
+        ]
+
     @property
     def is_occupied(self) -> bool:
         return ConsultationBed.objects.filter(bed=self, end_date__isnull=True).exists()

care/facility/tests/test_bed_create.py

@@ -76,3 +76,33 @@ def test_create_with_more_than_allowed_value(self):
         response = self.client.post("/api/v1/bed/", sample_data, format="json")
         self.assertIs(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(Bed.objects.filter(facility=self.facility).count(), 0)
+
+    def test_create_with_duplicate_name(self):
+        sample_data = {
+            "bed_type": "REGULAR",
+            "description": "Testing creation of beds.",
+            "facility": self.facility.external_id,
+            "location": self.asset_location.external_id,
+            "name": "Test",
+            "number_of_beds": 5,
+        }
+        response = self.client.post("/api/v1/bed/", sample_data, format="json")
+        self.assertIs(response.status_code, status.HTTP_201_CREATED)
+
+        response = self.client.post("/api/v1/bed/", sample_data, format="json")
+        self.assertIs(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data["detail"],
+            "Bed with same name already exists in this location.",
+        )
+
+        # validate case insensitive
+        sample_data["name"] = "test"
+        response = self.client.post("/api/v1/bed/", sample_data, format="json")
+        self.assertIs(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data["detail"],
+            "Bed with same name already exists in this location.",
+        )
+
+        self.assertEqual(Bed.objects.filter(facility=self.facility).count(), 5)