Open
Conversation
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.
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
This will ensure they appear in a consistent order, as we order cases by created_at.
MatMoore
force-pushed
the
DTOSS-12709-image-reading-session-service
branch
from
ba5046e to
da1bc57
Compare
|
swebberuk
reviewed
May 1, 2026
| 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) |
Contributor
There was a problem hiding this comment.
I think session=session is redundant.
Suggested change
| item = session.items.create(session=session, case=case, reading_order=1) | |
| item = session.items.create(case=case, reading_order=1) |
swebberuk
approved these changes
May 1, 2026
Contributor
There was a problem hiding this comment.
This looks good to me. Would be good to hear if anyone else has thoughts.
We'll need to determine whether a ReadingSessionItem has been completed or skipped. In the prototype a "Previously skipped" tag is displayed when viewing a set of images the user had previously skipped. There isn't a foreign key between ReadingSessionItem and Reading. Could infer this by checking if there is a Reading that references both the same user and study as referenced by the ReadingSessionItem's ReadingSession and Case.
As we continue developing the image reading pages we should get a better idea of if the model is suitable.
swebberuk
reviewed
May 1, 2026
|
|
||
| objects = CaseQueryset.as_manager() | ||
|
|
||
| study = models.ForeignKey(Study, on_delete=models.CASCADE, related_name="cases") |
Contributor
There was a problem hiding this comment.
We're using on_delete=models.CASCADE here but on_delete=models.PROTECT on Reading.
Just mentioning in case that wasn't intentional.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Introduce a service with a method 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:
I've introduced a new
Casemodel so that oneStudycan have manyCases. In practice, we should create exactly 2Caseswhen finalising the mammogram appointment.Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12709
Review notes
We've now got quite a few models that link together:
Studyis created first, with series and imagesCases to the study, representing the first and second read to be doneReadingSessionServiceto createReadingSession/ReadingSessionItem. TheReadingSessionItemlinks to the case, and theReadingSessionlinks to the reader.Readingto theStudy.Let me know if you have any thoughts on how this could be simplified, I found it quite tricky to model.
I'm also thinking we should move these all to the
readingapp rather than thedicomapp, as they're not really related to dicom. I can do this in a follow up PR unless anyone objects.To avoid race conditions, I've used
SELECT... FOR UPDATEon the query to get the most recent item from the Case queue.This holds a lock on the selected rows, so that concurrent executions cannot select the same case in between querying the most recent case and using it to create the session.
skip_lockedmeans that instead of blocking on concurrent access, it will skip over the locked row. (I manually tested an earlier version of the query in two psql sessions to verify this behaviour.)Review checklist
/api/v1/), confirm whether it is a breaking change — if so, a new major version (/api/v2/) is required (see ADR-006)