Link DICOM Study records to Appointment#1380
Merged
Conversation
Previously, there was an indirect link from Study to Appointment via GatewayAction, using the source_message_id, but this did not have a foreign key relation to provide integrity and so we couldn't easily make use of the relationship in django. We will now require the appointment to be set when creating the Study. The previous iteration of the Study demo data was using made up source_message_id values, so it's not possible to link historical data to appointments on environments that have run it. As a workaround, the migration just links it to any old appointment, but only on non-prod envirnments (none of this data is on prod yet). This migration also adds created_at, updated_at fields so that studies are orderable, and renames order to reading_order in ReadingSessionItem. "order" is a bad name as it is a SQL keyword.
Make use of the new Study.appointment relation, and get rid of the old method of looking up a Study by appointment id.
|
steventux
approved these changes
Apr 28, 2026
Contributor
steventux
left a comment
steventux
left a comment
There was a problem hiding this comment.
This looks good to me. Can't see anything wrong or missing.
We likely want to relax the appointment in progress logic when recording dicom data but that's for another PR.
Contributor
|
I've tested this locally, migrations and seeding data works the association between dicom Study and Appointment is created as expected |
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
Previously, there was an indirect link from
StudytoAppointmentviaGatewayAction, using thesource_message_id, but this did not have a foreign key relation to provide integrity and so we couldn't easily make use of the relationship in django.We will now require the appointment to be set when creating the study.
The previous iteration of the study demo data was using made up
source_message_idvalues, so it's not possible to link historical data to appointments on environments that have run it. As a workaround, the migration just links it to any old appointment, but only on non-prod environments (none of this data is on prod yet).This migration also adds
created_at,updated_atfields so that studies are orderable, and renamesordertoreading_orderinReadingSessionItem. "order" is a bad name as it is a SQL keyword.Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12709
Review notes
I haven't tested end to end with the gateway yet as I don't have it set up. I'll do that this afternoon unless anyone can vouch for it working in the meantime.
In most of the tests we are not dependent on GatewayAction anymore so I've avoided creating one. There's possibly further simplification that could be done but I didn't want to change things too much at once.
Review checklist
/api/v1/), confirm whether it is a breaking change — if so, a new major version (/api/v2/) is required (see ADR-006)