Merge pull request #1380 from NHSDigital/DTOSS-12709-model-changes

Link DICOM Study records to Appointment

Author: MatMoore

.gitignore

@@ -16,7 +16,7 @@ __pycache__
 .venv/
 
 # Python coverage
-.coverage
+.coverage*
 
 # Django
 .env

manage_breast_screening/dicom/dicom_recorder.py

@@ -15,6 +15,13 @@
 logger = logging.getLogger(__name__)
 
 
+def lookup_appointment(source_message_id):
+    try:
+        return GatewayAction.objects.get(id=source_message_id).appointment
+    except GatewayAction.DoesNotExist:
+        return None
+
+
 class DicomProcessingError(Exception):
     """Custom exception for DICOM processing errors."""
 
@@ -26,7 +33,8 @@ class DicomRecorder:
     def get_or_create_records(
         source_message_id: str, dicom_file: File
     ) -> tuple[Study, Series, Image]:
-        if not __class__.appointment_in_progress(source_message_id):
+        appointment = lookup_appointment(source_message_id)
+        if not appointment or not appointment.is_in_progress():
             raise DicomProcessingError(
                 f"Cannot process DICOM file for source_message_id={source_message_id} "
                 "because the associated appointment is not in progress."
@@ -46,6 +54,7 @@ def get_or_create_records(
         )
 
         study, _ = Study.objects.get_or_create(
+            appointment=appointment,
             study_instance_uid=study_uid,
             source_message_id=source_message_id,
             defaults={
@@ -135,10 +144,3 @@ def dataset_to_jpeg(sop_uid: str, ds: pydicom.Dataset) -> InMemoryUploadedFile:
             size=in_memory_file.getbuffer().nbytes,
             charset=None,
         )
-
-    @staticmethod
-    def appointment_in_progress(source_message_id: str) -> bool:
-        gateway_action = GatewayAction.objects.filter(id=source_message_id).first()
-        if not gateway_action or not gateway_action.appointment:
-            return False
-        return gateway_action.appointment.current_status.is_in_progress()

manage_breast_screening/dicom/migrations/0009_rename_order_readingsessionitem_reading_order_and_more.py

@@ -0,0 +1,69 @@
+# Generated by Django 6.0.4 on 2026-04-27 14:38
+
+import django.db.models.deletion
+import django.utils.timezone
+from django.conf import settings
+from django.db import migrations, models
+
+
+def set_appointment(apps, schema_editor):
+    Study = apps.get_model('dicom', 'Study')
+    Appointment = apps.get_model('participants', 'Appointment')
+    GatewayAction = apps.get_model('gateway', 'GatewayAction')
+    for study in Study.objects.all():
+        if study.source_message_id:
+            action = GatewayAction.objects.filter(id=study.source_message_id).first()
+            if action is None and settings.DJANGO_ENV != 'production':
+                # workaround inconsistent demo data by linking orphan studies to arbitrary appointments
+                study.appointment = Appointment.objects.filter(dicom_study=None).first()
+            else:
+                study.appointment = action.appointment
+            study.save()
+
+def unset_appointment(apps, schema_editor):
+    Study = apps.get_model('dicom', 'Study')
+    Study.objects.all().update(appointment=None)
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dicom', '0008_readingsession_reading_recallforassessmentdetails_and_more'),
+        ('participants', '0001_squashed_0067_participantreportedmammogram_created_by_and_more'),
+        ('gateway', '0005_relay_provider_to_setting'),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name='readingsessionitem',
+            old_name='order',
+            new_name='reading_order',
+        ),
+        migrations.AlterUniqueTogether(
+            name='readingsessionitem',
+            unique_together={('session', 'reading_order')},
+        ),
+        migrations.AddField(
+            model_name='study',
+            name='created_at',
+            field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now),
+            preserve_default=False,
+        ),
+        migrations.AddField(
+            model_name='study',
+            name='updated_at',
+            field=models.DateTimeField(auto_now=True),
+        ),
+        migrations.AddField(
+            model_name='study',
+            name='appointment',
+            field=models.OneToOneField(null=True, on_delete=django.db.models.deletion.PROTECT, related_name='dicom_study', to='participants.appointment'),
+        ),
+        migrations.RunPython(code=set_appointment, reverse_code=unset_appointment),
+        migrations.AlterField(
+            model_name='study',
+            name='appointment',
+            field=models.OneToOneField(
+                to='participants.Appointment', on_delete=models.PROTECT, related_name="dicom_study"
+            )
+        )
+    ]

manage_breast_screening/dicom/migrations/max_migration.txt

@@ -1 +1 @@
-0008_readingsession_reading_recallforassessmentdetails_and_more
+0009_rename_order_readingsessionitem_reading_order_and_more

manage_breast_screening/dicom/models.py

@@ -12,13 +12,14 @@
     RepeatType,
     StudyCompleteness,
 )
+from manage_breast_screening.participants.models.appointment import Appointment
 
 
 def dicom_storage():
     return storages["dicom"]
 
 
-class Study(models.Model):
+class Study(BaseModel):
     class Meta:
         indexes = [
             models.Index(fields=["study_instance_uid"]),
@@ -27,6 +28,9 @@ class Meta:
         ]
 
     id = models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True)
+    appointment = models.OneToOneField(
+        Appointment, on_delete=models.PROTECT, related_name="dicom_study"
+    )
     study_instance_uid = models.CharField(max_length=128, unique=True)
     source_message_id = models.CharField(max_length=128)
     patient_id = models.CharField(max_length=10, blank=True)
@@ -53,14 +57,6 @@ def images(self) -> models.QuerySet["Image"]:
             "series__series_number", "instance_number"
         )
 
-    @classmethod
-    def for_appointment(cls, appointment):
-        action = appointment.gateway_actions.first()
-        if not action:
-            return None
-
-        return cls.objects.filter(source_message_id=action.id).first()
-
     def series_with_multiple_images(self):
         return self.series.annotate(image_count=models.Count("images")).filter(
             image_count__gt=1
@@ -245,7 +241,7 @@ class ReadingSessionItem(BaseModel):
     study = models.ForeignKey(
         Study, on_delete=models.PROTECT, related_name="reading_session_items"
     )
-    order = models.IntegerField()
+    reading_order = models.IntegerField()
     reading = models.OneToOneField(
         Reading,
         on_delete=models.PROTECT,
@@ -254,4 +250,4 @@ class ReadingSessionItem(BaseModel):
     )
 
     class Meta:
-        unique_together = [("session", "order")]
+        unique_together = [("session", "reading_order")]

manage_breast_screening/dicom/study_service.py

@@ -22,8 +22,7 @@ def save(
         Save additional details to the Study associated with the appointment's GatewayAction.
         Returns the updated Study, or None if no Study is found.
         """
-        study = Study.for_appointment(self.appointment)
-
+        study = getattr(self.appointment, "dicom_study", None)
         if not study:
             return None
 

manage_breast_screening/dicom/tests/factories.py

@@ -4,6 +4,10 @@
 from factory.django import DjangoModelFactory, FileField
 from factory.helpers import post_generation
 
+from manage_breast_screening.participants.models.appointment import (
+    AppointmentStatusNames,
+)
+from manage_breast_screening.participants.tests.factories import AppointmentFactory
 from manage_breast_screening.users.tests.factories import UserFactory
 
 from .. import models
@@ -18,6 +22,9 @@ class Meta:
     patient_id = Sequence(lambda n: f"999{n:07d}")
     date_and_time = None
     description = "Test Study"
+    appointment = SubFactory(
+        AppointmentFactory, current_status=AppointmentStatusNames.SCREENED
+    )
 
 
 class StudyWithImagesFactory(StudyFactory):
@@ -104,7 +111,7 @@ class Meta:
         model = models.ReadingSessionItem
 
     study = SubFactory(StudyFactory)
-    order = Sequence(lambda i: i)
+    reading_order = Sequence(lambda i: i)
 
 
 class ReadingSessionFactory(DjangoModelFactory):

manage_breast_screening/dicom/tests/test_api.py

@@ -8,10 +8,13 @@
 from ninja.testing import TestClient
 
 from manage_breast_screening.core.api import api
-from manage_breast_screening.dicom.dicom_recorder import DicomRecorder
 from manage_breast_screening.dicom.models import Study
 from manage_breast_screening.gateway.models import GatewayActionStatus
 from manage_breast_screening.gateway.tests.factories import GatewayActionFactory
+from manage_breast_screening.participants.models.appointment import (
+    AppointmentStatusNames,
+)
+from manage_breast_screening.participants.tests.factories import AppointmentFactory
 
 os.environ["NINJA_SKIP_REGISTRY"] = "yes"
 
@@ -28,14 +31,26 @@ def dicom_file(dataset) -> bytes:
         )
 
 
+@pytest.fixture
+def appointment_stub():
+    return AppointmentFactory.stub(
+        is_in_progress=MagicMock(return_value=True),
+    )
+
+
 @pytest.mark.django_db
 def test_upload_success(dataset, dicom_file, monkeypatch):
     monkeypatch.setenv("API_ENABLED", "true")
     monkeypatch.setenv("API_AUTH_TOKEN", "testtoken")
 
-    with patch.object(DicomRecorder, "appointment_in_progress", return_value=True):
+    appointment = AppointmentFactory(current_status=AppointmentStatusNames.IN_PROGRESS)
+
+    with patch(
+        "manage_breast_screening.dicom.dicom_recorder.lookup_appointment",
+        return_value=appointment,
+    ):
         response = client.put(
-            "/dicom/abc123",
+            f"/dicom/{appointment.pk}",
             FILES={"file": dicom_file},
             headers={"Authorization": "Bearer " + os.getenv("API_AUTH_TOKEN", "")},
         )
@@ -47,7 +62,7 @@ def test_upload_success(dataset, dicom_file, monkeypatch):
         assert json["series_instance_uid"] == dataset.SeriesInstanceUID
         assert json["sop_instance_uid"] == dataset.SOPInstanceUID
         assert json["instance_id"] == str(study.images().first().id)
-        assert study.source_message_id == "abc123"
+        assert study.source_message_id == str(appointment.pk)
 
 
 def test_upload_no_file(monkeypatch):
@@ -63,15 +78,18 @@ def test_upload_no_file(monkeypatch):
     assert response.status_code == 422
 
 
-def test_upload_invalid_file(monkeypatch):
+def test_upload_invalid_file(monkeypatch, appointment_stub):
     monkeypatch.setenv("API_ENABLED", "true")
     monkeypatch.setenv("API_AUTH_TOKEN", "testtoken")
 
     invalid_file = SimpleUploadedFile(
         "invalid.dcm", b"not a dicom file", content_type="application/dicom"
     )
 
-    with patch.object(DicomRecorder, "appointment_in_progress", return_value=True):
+    with patch(
+        "manage_breast_screening.dicom.dicom_recorder.lookup_appointment",
+        return_value=appointment_stub,
+    ):
         response = client.put(
             "/dicom/abc123",
             FILES={"file": invalid_file},
@@ -102,7 +120,7 @@ def test_upload_file_thats_too_large(monkeypatch):
     assert response.json()["detail"] == "The file cannot be larger than 100MB"
 
 
-def test_upload_missing_uids(dataset, monkeypatch):
+def test_upload_missing_uids(dataset, monkeypatch, appointment_stub):
     monkeypatch.setenv("API_ENABLED", "true")
     monkeypatch.setenv("API_AUTH_TOKEN", "testtoken")
 
@@ -117,7 +135,10 @@ def test_upload_missing_uids(dataset, monkeypatch):
             "temp.dcm", buffer.read(), content_type="application/dicom"
         )
 
-    with patch.object(DicomRecorder, "appointment_in_progress", return_value=True):
+    with patch(
+        "manage_breast_screening.dicom.dicom_recorder.lookup_appointment",
+        return_value=appointment_stub,
+    ):
         response = client.put(
             "/dicom/abc123",
             FILES={"file": dicom_file},
@@ -133,11 +154,16 @@ def test_upload_missing_uids(dataset, monkeypatch):
     )
 
 
-def test_upload_appointment_not_in_progress(dicom_file, monkeypatch):
+def test_upload_appointment_not_in_progress(dicom_file, monkeypatch, appointment_stub):
     monkeypatch.setenv("API_ENABLED", "true")
     monkeypatch.setenv("API_AUTH_TOKEN", "testtoken")
 
-    with patch.object(DicomRecorder, "appointment_in_progress", return_value=False):
+    appointment_stub.is_in_progress.return_value = False
+
+    with patch(
+        "manage_breast_screening.dicom.dicom_recorder.lookup_appointment",
+        return_value=appointment_stub,
+    ):
         response = client.put(
             "/dicom/abc123",
             FILES={"file": dicom_file},

manage_breast_screening/dicom/tests/test_models.py

@@ -1,12 +1,10 @@
 import pytest
 
-from manage_breast_screening.dicom.models import Study
 from manage_breast_screening.dicom.tests.factories import (
     ImageFactory,
     SeriesFactory,
     StudyFactory,
 )
-from manage_breast_screening.gateway.tests.factories import GatewayActionFactory
 
 
 @pytest.mark.django_db
@@ -34,12 +32,6 @@ def test_study_does_not_have_series_with_multiple_images(self):
 
         assert study.has_series_with_multiple_images() is False
 
-    def test_study_for_appointment(self):
-        study = StudyFactory.create()
-        action = GatewayActionFactory.create(id=study.source_message_id)
-
-        assert Study.for_appointment(action.appointment) == study
-
 
 @pytest.mark.django_db
 class TestSeries: