From 709448b4330c02fb2d1c105b457d264c730a1ea8 Mon Sep 17 00:00:00 2001 From: Mat Moore Date: Thu, 30 Apr 2026 14:07:21 +0100 Subject: [PATCH 1/3] Introduce Case model for image reading cases It's useful to have an entity representing a thing to be read, that exists prior to us assigning it to a reader. The models we have now don't quite fit: - A `Reading` is not created until the opinion is given - A `ReadingSessionItem` is not created until it is assigned to a reading session. - A `Study` logically has 2 cases, the first read and the second read We have to ensure that each set of images gets assigned and read exactly 2 times (prior to arbitration, which we haven't implemented yet) I think the simplest way to handle this constraint is to have a separate `Case` queue, push 2 cases into it per study, and then grab the oldest case whenever we need to assign one to a reading session. We also refer to cases in the UI. Later we might decide to give each case a type (1st read/2nd read) so we can do further filtering on the image reading dashboard. When assigning to a reading session, we will link the `ReadingSessionItem` to the `Case`. Note: there is a second constraint that a reader can only read the same study once, and only cases belonging to the current provider will be visible, so it will actually be a filtered subset of this table that is available for assigning to a reading session. --- .../migrations/0010_auto_20260430_1359.py | 51 +++++++++++++++++++ .../dicom/migrations/max_migration.txt | 2 +- manage_breast_screening/dicom/models.py | 51 ++++++++++++++----- .../dicom/tests/factories.py | 23 ++++++++- .../management/commands/seed_demo_data.py | 13 ++++- 5 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 manage_breast_screening/dicom/migrations/0010_auto_20260430_1359.py diff --git a/manage_breast_screening/dicom/migrations/0010_auto_20260430_1359.py b/manage_breast_screening/dicom/migrations/0010_auto_20260430_1359.py new file mode 100644 index 000000000..6899ac1ce --- /dev/null +++ b/manage_breast_screening/dicom/migrations/0010_auto_20260430_1359.py @@ -0,0 +1,51 @@ +# Generated by Django 6.0.4 on 2026-04-30 12:59 + +import uuid + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('dicom', '0009_rename_order_readingsessionitem_reading_order_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='Case', + fields=[ + ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('study', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='cases', to='dicom.study')), + ], + options={ + 'abstract': False, + }, + ), + migrations.RunSQL( + sql='TRUNCATE TABLE dicom_readingsession CASCADE', + reverse_sql='TRUNCATE table dicom_readingsession CASCADE', + ), + migrations.AlterField( + model_name='reading', + name='study', + field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='readings', to='dicom.study'), + ), + migrations.RemoveField( + model_name='readingsessionitem', + name='reading', + ), + migrations.RemoveField( + model_name='readingsessionitem', + name='study', + ), + migrations.AddField( + model_name='readingsessionitem', + name='case', + field=models.OneToOneField(on_delete=django.db.models.deletion.PROTECT, related_name='reading_session_item', to='dicom.case'), + preserve_default=False, + ) + ] diff --git a/manage_breast_screening/dicom/migrations/max_migration.txt b/manage_breast_screening/dicom/migrations/max_migration.txt index 28f2e1f77..4719badf7 100644 --- a/manage_breast_screening/dicom/migrations/max_migration.txt +++ b/manage_breast_screening/dicom/migrations/max_migration.txt @@ -1 +1 @@ -0009_rename_order_readingsessionitem_reading_order_and_more +0010_auto_20260430_1359 diff --git a/manage_breast_screening/dicom/models.py b/manage_breast_screening/dicom/models.py index 5583fffc5..749452954 100644 --- a/manage_breast_screening/dicom/models.py +++ b/manage_breast_screening/dicom/models.py @@ -4,6 +4,7 @@ from django.contrib.postgres.fields import ArrayField from django.core.files.storage import storages from django.db import models +from django.db.models import Exists, OuterRef from manage_breast_screening.core.models import BaseModel from manage_breast_screening.manual_images.models import ( @@ -162,10 +163,10 @@ class BreastOpinions(models.TextChoices): class Reading(BaseModel): """ - One reader's opinion of a study. All of the opinions feed into the consensus read. + One reader's opinion of a study. """ - study = models.ForeignKey(Study, on_delete=models.PROTECT, related_name="opinions") + study = models.ForeignKey(Study, on_delete=models.PROTECT, related_name="readings") reader = models.ForeignKey( settings.AUTH_USER_MODEL, on_delete=models.PROTECT, related_name="readings" ) @@ -217,9 +218,37 @@ class RecallForAssessmentDetails(BaseModel): left_breast_comment = models.CharField(null=False, blank=True, default="") +class CaseQueryset(models.QuerySet): + def unassigned(self): + return self.filter( + ~Exists(ReadingSessionItem.objects.filter(case=OuterRef("id"))) + ) + + def where_same_study_has_not_been_assigned_to_reader(self, reader): + return self.filter( + ~Exists( + ReadingSessionItem.objects.filter( + case__study=OuterRef("study_id"), + session__reader=reader, + ) + ) + ) + + +class Case(BaseModel): + """ + A first or second read that needs doing for a study. These form the queue of cases + that can be picked up by an image reader. + """ + + objects = CaseQueryset.as_manager() + + study = models.ForeignKey(Study, on_delete=models.CASCADE, related_name="cases") + + class ReadingSession(BaseModel): """ - A grouping of studies that are read by a reader in a single session + A grouping of cases to be read by a reader in a single session """ reader = models.ForeignKey( @@ -232,22 +261,20 @@ class ReadingSession(BaseModel): class ReadingSessionItem(BaseModel): """ - Assigns a study to a particular reading session, with an ordering. + An assignment of a pending reading to a reading session """ session = models.ForeignKey( ReadingSession, on_delete=models.CASCADE, related_name="items" ) - study = models.ForeignKey( - Study, on_delete=models.PROTECT, related_name="reading_session_items" + case = models.OneToOneField( + Case, on_delete=models.PROTECT, related_name="reading_session_item" ) reading_order = models.IntegerField() - reading = models.OneToOneField( - Reading, - on_delete=models.PROTECT, - related_name="reading_session_item", - null=True, - ) class Meta: unique_together = [("session", "reading_order")] + + @property + def study(self): + return self.case.study diff --git a/manage_breast_screening/dicom/tests/factories.py b/manage_breast_screening/dicom/tests/factories.py index 43292ee33..4277d6b00 100644 --- a/manage_breast_screening/dicom/tests/factories.py +++ b/manage_breast_screening/dicom/tests/factories.py @@ -16,6 +16,7 @@ class StudyFactory(DjangoModelFactory): class Meta: model = models.Study + skip_postgeneration_save = True study_instance_uid = Sequence(lambda n: f"STUDY{n:04d}") source_message_id = uuid.uuid4() @@ -26,6 +27,15 @@ class Meta: AppointmentFactory, current_status=AppointmentStatusNames.SCREENED ) + case_1 = RelatedFactory( + "manage_breast_screening.dicom.tests.factories.CaseFactory", + factory_related_name="study", + ) + case_2 = RelatedFactory( + "manage_breast_screening.dicom.tests.factories.CaseFactory", + factory_related_name="study", + ) + class StudyWithImagesFactory(StudyFactory): class Meta: @@ -106,11 +116,22 @@ class Params: ) +class CaseFactory(DjangoModelFactory): + class Meta: + model = models.Case + + study = SubFactory(StudyFactory) + + class ReadingSessionItemFactory(DjangoModelFactory): class Meta: model = models.ReadingSessionItem + skip_postgeneration_save = True - study = SubFactory(StudyFactory) + case = SubFactory(CaseFactory) + session = SubFactory( + "manage_breast_screening.dicom.tests.factories.ReadingSessionFactory" + ) reading_order = Sequence(lambda i: i) diff --git a/manage_breast_screening/nonprod/management/commands/seed_demo_data.py b/manage_breast_screening/nonprod/management/commands/seed_demo_data.py index 24977a3f4..500020a69 100644 --- a/manage_breast_screening/nonprod/management/commands/seed_demo_data.py +++ b/manage_breast_screening/nonprod/management/commands/seed_demo_data.py @@ -16,6 +16,7 @@ SettingFactory, UserAssignmentFactory, ) +from manage_breast_screening.dicom.models import Case from manage_breast_screening.dicom.tests.factories import ( ImageFactory as DicomImageFactory, ) @@ -403,7 +404,17 @@ def create_reading_session(self, session_key): else: reading = None - ReadingSessionItemFactory(session=session, reading=reading, **item) + study_id = item.pop("study_id") + case = Case.objects.unassigned().filter(study_id=study_id).first() + + ReadingSessionItemFactory( + session=session, + case=case, + **item, + ) + + case.reading = reading + case.save() def create_reading(self, reader, reading_key): retake_requests = reading_key.pop("retake_requests", []) From 834770d43b24a97d711fb2466dfb82cb65de5dca Mon Sep 17 00:00:00 2001 From: Mat Moore Date: Thu, 30 Apr 2026 14:26:52 +0100 Subject: [PATCH 2/3] Add ReadingSessionService for assigning cases to sessions The first method is to begin a new session and assign a case to it. If there are no available cases, raise an exception, `NoImagesToRead`. This service should ensure the following: 1. A reader can only see cases linked to their current provider 2. The same case should not be assigned more than once At Case creation time, we will ensure that there are exactly 2 cases per study (not implemented yet). To avoid race conditions, I've used `SELECT... FOR UPDATE` to get the most recent item from the Case queue. This holds a lock on the selected table, so that concurrent executions cannot select the same case in between querying the most recent case and using it to create the session. `skip_locked` means that instead of blocking on concurrent access, it will skip over the locked row[1]. [1]. https://www.postgresql.org/docs/current/sql-select.html --- manage_breast_screening/clinics/models.py | 8 ++ .../config/settings/base.py | 2 + .../reading/services/__init__.py | 0 .../services/reading_session_service.py | 34 +++++++ .../services/test_reading_session_service.py | 96 +++++++++++++++++++ 5 files changed, 140 insertions(+) create mode 100644 manage_breast_screening/reading/services/__init__.py create mode 100644 manage_breast_screening/reading/services/reading_session_service.py create mode 100644 manage_breast_screening/reading/tests/services/test_reading_session_service.py diff --git a/manage_breast_screening/clinics/models.py b/manage_breast_screening/clinics/models.py index cf59cf585..c841cd9ab 100644 --- a/manage_breast_screening/clinics/models.py +++ b/manage_breast_screening/clinics/models.py @@ -9,6 +9,8 @@ from django.db import models from django.db.models import OuterRef, Subquery +from manage_breast_screening.dicom.models import Case + from ..auth.models import Role from ..core.models import BaseModel from ..participants.models import Appointment, Participant @@ -34,6 +36,12 @@ def participants(self): screeningepisode__appointment__clinic_slot__clinic__setting__provider=self ).distinct() + @property + def image_reading_cases(self): + return Case.objects.filter( + study__appointment__clinic_slot__clinic__setting__provider=self + ) + def get_config(self) -> "ProviderConfig": config, _ = ProviderConfig.objects.get_or_create(provider=self) return config diff --git a/manage_breast_screening/config/settings/base.py b/manage_breast_screening/config/settings/base.py index 3d1defd76..4c5b13371 100644 --- a/manage_breast_screening/config/settings/base.py +++ b/manage_breast_screening/config/settings/base.py @@ -384,3 +384,5 @@ def list_env(key): } BYPASS_API_TOKEN_AUTH = boolean_env("BYPASS_API_TOKEN_AUTH", default=False) + +READING_SESSION_DEFAULT_SIZE = 50 diff --git a/manage_breast_screening/reading/services/__init__.py b/manage_breast_screening/reading/services/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/manage_breast_screening/reading/services/reading_session_service.py b/manage_breast_screening/reading/services/reading_session_service.py new file mode 100644 index 000000000..315b96569 --- /dev/null +++ b/manage_breast_screening/reading/services/reading_session_service.py @@ -0,0 +1,34 @@ +from django.conf import settings +from django.db import transaction + +from ...dicom.models import ReadingSession, ReadingSessionItem + + +class NoImagesToRead(Exception): + pass + + +class ReadingSessionService: + def __init__(self, reader, provider): + self.reader = reader + self.provider = provider + + @transaction.atomic + def assign_item_to_new_session(self) -> ReadingSessionItem: + queue = ( + self.provider.image_reading_cases.unassigned() + .where_same_study_has_not_been_assigned_to_reader(self.reader) + .order_by("created_at", "id") + .select_for_update(skip_locked=True) + ) + + case = queue.first() + if case is None: + raise NoImagesToRead + + session = ReadingSession.objects.create( + reader=self.reader, session_size=settings.READING_SESSION_DEFAULT_SIZE + ) + item = session.items.create(session=session, case=case, reading_order=1) + + return item diff --git a/manage_breast_screening/reading/tests/services/test_reading_session_service.py b/manage_breast_screening/reading/tests/services/test_reading_session_service.py new file mode 100644 index 000000000..32b66095a --- /dev/null +++ b/manage_breast_screening/reading/tests/services/test_reading_session_service.py @@ -0,0 +1,96 @@ +from datetime import datetime +from datetime import timezone as tz + +import pytest +import time_machine + +from manage_breast_screening.dicom.models import Study +from manage_breast_screening.dicom.tests.factories import ( + ReadingSessionItemFactory, + StudyFactory, +) +from manage_breast_screening.reading.services.reading_session_service import ( + NoImagesToRead, + ReadingSessionService, +) + + +@pytest.mark.django_db +class TestSessionService: + @pytest.fixture + def older_study(self, current_provider): + with time_machine.travel(datetime(2026, 1, 1, 9, 0, 0, tzinfo=tz.utc)): + return StudyFactory.create( + appointment__clinic_slot__clinic__setting__provider=current_provider, + ) + + @pytest.fixture + def newer_study(self, current_provider): + with time_machine.travel(datetime(2026, 1, 1, 10, 0, 0, tzinfo=tz.utc)): + return StudyFactory.create( + appointment__clinic_slot__clinic__setting__provider=current_provider, + ) + + def test_create_with_one_study(self, user, current_provider): + assert Study.objects.count() == 0 + study = StudyFactory.create( + appointment__clinic_slot__clinic__setting__provider=current_provider, + ) + + item = ReadingSessionService( + user, current_provider + ).assign_item_to_new_session() + + assert item.study == study + assert item.session.items.count() == 1 + + def test_create_with_zero_studies(self, user, current_provider): + with pytest.raises(NoImagesToRead): + ReadingSessionService(user, current_provider).assign_item_to_new_session() + + def test_create_with_many_studies( + self, user, current_provider, older_study, newer_study + ): + item = ReadingSessionService( + user, current_provider + ).assign_item_to_new_session() + assert item.study == older_study + + def test_create_excludes_studies_already_assigned_to_me( + self, user, current_provider, older_study, newer_study + ): + """ + A study could be assigned to another session, in which case we should ignore it when starting a new session. + """ + ReadingSessionItemFactory.create( + case=older_study.cases.first(), session__reader=user + ) + + item = ReadingSessionService( + user, current_provider + ).assign_item_to_new_session() + assert item.study == newer_study + + def test_create_includes_studies_with_one_case_assigned( + self, user, current_provider, older_study, newer_study + ): + ReadingSessionItemFactory(case=older_study.cases.first()) + + item = ReadingSessionService( + user, current_provider + ).assign_item_to_new_session() + assert item.study == older_study + + def test_create_excludes_studies_with_all_cases_assigned( + self, user, current_provider, older_study, newer_study + ): + ReadingSessionItemFactory(case=older_study.cases.first()) + ReadingSessionItemFactory(case=older_study.cases.last()) + + item = ReadingSessionService( + user, current_provider + ).assign_item_to_new_session() + assert item.study == newer_study + + +# From da1bc574035f10cadff4f4b1cc5e56f987043fc3 Mon Sep 17 00:00:00 2001 From: Mat Moore Date: Mon, 27 Apr 2026 15:27:32 +0100 Subject: [PATCH 3/3] Give demo Studies/Cases timestamps This will ensure they appear in a consistent order, as we order cases by created_at. --- .../data/image_reading.yml | 30 +++++++++++++++++-- .../management/commands/seed_demo_data.py | 9 +++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/manage_breast_screening/data/image_reading.yml b/manage_breast_screening/data/image_reading.yml index 075f7e097..03c12fc73 100644 --- a/manage_breast_screening/data/image_reading.yml +++ b/manage_breast_screening/data/image_reading.yml @@ -1,5 +1,6 @@ studies: - id: c6ff5cea-a5b8-4823-b307-e5faf587b474 + date_and_time: '2026-04-27 09:00' series: - view_position: CC laterality: R @@ -18,6 +19,7 @@ studies: images: - image_file: set-02/lmlo.png - id: 51a2f9a8-a8d4-4bcc-ad3f-7f20f4d71655 + date_and_time: '2026-04-27 10:00' series: - view_position: CC laterality: R @@ -35,8 +37,8 @@ studies: laterality: L images: - image_file: set-03/lmlo.png - - id: e468ecb0-33f2-4c27-a930-107c3fabd2d4 + date_and_time: '2026-04-27 11:00' series: - view_position: CC laterality: R @@ -63,6 +65,7 @@ studies: - image_file: image-library/repeat-large-b-lmlo.png repeat_count: 0 - id: bedbcf0b-b9d8-4d78-960f-ede6c05563d7 + date_and_time: '2026-04-27 12:00' series: - view_position: CC laterality: R @@ -81,6 +84,7 @@ studies: images: - image_file: set-04/lmlo.png - id: 378f42d7-3361-43b8-bae6-0ce9113cba80 + date_and_time: '2026-04-27 13:00' series: - view_position: CC laterality: R @@ -99,6 +103,7 @@ studies: images: - image_file: set-05/lmlo.png - id: 15564f66-1d9d-4acd-b78f-1a32b4685124 + date_and_time: '2026-04-27 14:00' series: - view_position: CC laterality: R @@ -117,6 +122,7 @@ studies: images: - image_file: set-06/lmlo.png - id: 7dffd1de-88ae-4ac3-93d7-a8d4d0820e6b + date_and_time: '2026-04-27 15:00' series: - view_position: CC laterality: R @@ -142,8 +148,8 @@ studies: repeat_count: 1 repeat_reasons: - POSITIONING_ERROR - - id: 3a447fe4-f4b3-4961-a83c-5bab17a2cda7 + date_and_time: '2026-04-27 16:00' series: - view_position: CC laterality: R @@ -162,6 +168,7 @@ studies: images: - image_file: set-07/lmlo.png - id: 8fc7d736-f2d9-45ad-93cc-f289da5b720b + date_and_time: '2026-04-27 17:00' series: - view_position: CC laterality: R @@ -180,6 +187,7 @@ studies: images: - image_file: set-08/lmlo.png - id: f61faccf-d9ea-4a9b-a284-021680565523 + date_and_time: '2026-04-27 18:00' series: - view_position: CC laterality: R @@ -198,6 +206,7 @@ studies: images: - image_file: set-09/lmlo.png - id: c089a4e6-8f66-4148-a14c-ec039e831c44 + date_and_time: '2026-04-27 19:00' series: - view_position: CC laterality: R @@ -216,6 +225,7 @@ studies: images: - image_file: set-10/lmlo.png - id: 3b25ff17-47b3-417c-9a8a-340c657f6578 + date_and_time: '2026-04-27 20:00' series: - view_position: CC laterality: R @@ -234,6 +244,7 @@ studies: images: - image_file: set-11/lmlo.png - id: 779b425a-b733-4fa9-89d5-fddbb58f09ea + date_and_time: '2026-04-27 21:00' series: - view_position: CC laterality: R @@ -252,6 +263,7 @@ studies: images: - image_file: set-12/lmlo.png - id: 0378ff58-baa5-4484-8afb-258ebfd1e5ef + date_and_time: '2026-04-27 22:00' series: - view_position: CC laterality: R @@ -270,6 +282,7 @@ studies: images: - image_file: set-13/lmlo.png - id: bb2ba1d3-1f72-428a-9c5a-39b50a5d9353 + date_and_time: '2026-04-27 23:00' series: - view_position: CC laterality: R @@ -288,6 +301,7 @@ studies: images: - image_file: set-14/lmlo.png - id: f0c600bf-dab2-44d5-aa5e-3b6481deeb48 + date_and_time: '2026-04-28 00:00' series: - view_position: CC laterality: R @@ -306,6 +320,7 @@ studies: images: - image_file: set-15/lmlo.png - id: bb2187a1-cd82-440f-94b1-46cef5938a23 + date_and_time: '2026-04-28 01:00' series: - view_position: CC laterality: R @@ -324,6 +339,7 @@ studies: images: - image_file: set-16/lmlo.png - id: 9ce6e923-aa30-4118-ac64-725f0fc37698 + date_and_time: '2026-04-28 02:00' series: - view_position: CC laterality: R @@ -342,6 +358,7 @@ studies: images: - image_file: set-17/lmlo.png - id: 9162a72b-3506-4b33-9acc-ef71bce07cb6 + date_and_time: '2026-04-28 03:00' series: - view_position: CC laterality: R @@ -360,6 +377,7 @@ studies: images: - image_file: set-18/lmlo.png - id: 72233799-7179-435f-be6d-e493a8c6ce78 + date_and_time: '2026-04-28 04:00' series: - view_position: CC laterality: R @@ -378,6 +396,7 @@ studies: images: - image_file: set-19/lmlo.png - id: 813a60ef-940c-4d39-bc5b-0167b2dc214c + date_and_time: '2026-04-28 05:00' series: - view_position: CC laterality: R @@ -396,6 +415,7 @@ studies: images: - image_file: set-20/lmlo.png - id: 716f887d-8083-4cc8-9fea-d32213ca8fff + date_and_time: '2026-04-28 06:00' series: - view_position: CC laterality: R @@ -414,6 +434,7 @@ studies: images: - image_file: set-21/lmlo.png - id: 4469102f-b8c6-4cba-9ed7-d77ee4f0ced8 + date_and_time: '2026-04-28 07:00' series: - view_position: CC laterality: R @@ -432,6 +453,7 @@ studies: images: - image_file: set-22/lmlo.png - id: 1ecd140e-c427-4c6d-bf20-d138610b3c0b + date_and_time: '2026-04-28 08:00' series: - view_position: CC laterality: R @@ -450,6 +472,7 @@ studies: images: - image_file: set-23/lmlo.png - id: 4ba01e6b-2c5f-4373-9740-4fa6ca22ca1f + date_and_time: '2026-04-28 09:00' series: - view_position: CC laterality: R @@ -468,6 +491,7 @@ studies: images: - image_file: set-24/lmlo.png - id: f082c62a-25b4-4aef-8fdf-415f31d296a4 + date_and_time: '2026-04-28 10:00' series: - view_position: CC laterality: R @@ -486,6 +510,7 @@ studies: images: - image_file: set-25/lmlo.png - id: d7d2c799-53fe-46c8-a81c-1cfb0e5fd4af + date_and_time: '2026-04-28 11:00' series: - view_position: CC laterality: R @@ -504,6 +529,7 @@ studies: images: - image_file: set-26/lmlo.png - id: ff3fdaa4-9b7d-4d24-b91a-36e45d4c7dfb + date_and_time: '2026-04-28 12:00' series: - view_position: CC laterality: R diff --git a/manage_breast_screening/nonprod/management/commands/seed_demo_data.py b/manage_breast_screening/nonprod/management/commands/seed_demo_data.py index 500020a69..6cdf121bc 100644 --- a/manage_breast_screening/nonprod/management/commands/seed_demo_data.py +++ b/manage_breast_screening/nonprod/management/commands/seed_demo_data.py @@ -375,7 +375,14 @@ def create_study(self, appointment, study_key): ) def create_dicom_study(self, study_key): - study = DicomStudyFactory(id=study_key["id"]) + date_and_time = study_key.get("date_and_time") + study = DicomStudyFactory( + id=study_key["id"], + date_and_time=date_and_time, + created_at=date_and_time, + case_1__created_at=date_and_time, + case_2__created_at=date_and_time, + ) for series_key in study_key["series"]: images = series_key.pop("images") view_position = series_key.pop("view_position")