Record images taken (are the images standard or not) #943
Conversation
MatMoore
force-pushed
the
DTOSS-11825-record-images-taken
branch
7 times, most recently
from
d166a22 to
091b00c
Compare
|
The review app at this URL has been deleted: |
|
|
||
| @transaction.atomic | ||
| def remove_existing_study_and_series(self): | ||
| if hasattr(self.appointment, "study"): |
There was a problem hiding this comment.
Should we be creating audit records of the deletes here, just as we do above for the creates?
Something like: self.auditor.audit_bulk_delete(self.appointment.study.series_set.all())
There was a problem hiding this comment.
Yeah I think so - I've added it in.
|
|
||
| match form.cleaned_data["standard_images"]: | ||
| case form.StandardImagesChoices.YES_TWO_CC_AND_TWO_MLO: | ||
| self.mark_workflow_step_complete() |
There was a problem hiding this comment.
On StudyService we've added @transaction.atomic so all database operations in create_with_default_series are executed inside a single transaction. Does that mean the database operation in mark_workflow_step_complete will happen in a different transaction and, if so, does it matter?
There was a problem hiding this comment.
Yeah it will.
I kept the AppointmentWorkflowStepCompletion bit outside of the StudyService as it felt quite unrelated - it belongs to a different app, and we already have other views creating these records from the view class so I thought I'd keep it consistent with that.
It probably doesn't matter, although I've just added @atomic to the view's form_valid method to bring everything into the same transaction. (We have ATOMIC_REQUESTS off so the default will be to autocommit)
swebberuk
left a comment
swebberuk
left a comment
There was a problem hiding this comment.
Looks good to me. I've added separate comments about auditing and transactions, but they're questions rather than suggestions.
Otherwise we get a `study_set` attribute on the appointment rather than a singular `study`.
Previously we add a hint param to our custom form fields for the overall field. In the case of choice fields, we also want to be able to add hints to the individual choices. This gets passed as the hint param within in params.items.
This used to be "awaiting images" but the connectivity with the PACS is coming later, so for now entering the details is completely manual.
Redirects to one of the folliowing: - appointment cannot continue - additional images step - check information Creation of AppointmentWorkflowStepCompletion record Creation of study and series.
This allows AppointmentWorkflowStepCompletion and Study updates to participate in the same database transaction. I decided against moving the step completion code into the StudyService as logically they are unrelated, and they belong to separate apps. Doing the former update in the view is also more consistent with what the other views are already doing.
MatMoore
force-pushed
the
DTOSS-11825-record-images-taken
branch
from
091b00c to
229a4e5
Compare
|
2 participants
Description
This is the first step for recording images. It replaces the awaiting images page, since we're assuming we won't have automatic recording of images ready for the pilot.
Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11825
(note the screenshot in the Jira looks to be out of date - I used the copy from the prototype instead)
Review notes
This is intended to link up with the additional details step to be added in #938. For now I put in a placeholder view.
Because you can go back to this step after reaching the check information page, I've made it so that when the form is submitted, it clears the existing study and series. We might need similar logic on the following forms depending on whether it's possible to revisit them without going back to this form.
Some other things I noticed not specific to this step:
Review checklist