-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(imaging): link image with visit #2309
feat(imaging): link image with visit #2309
Conversation
@@ -146,7 +146,7 @@ export default { | |||
label: 'Visits', | |||
startDateTime: 'Start Date', | |||
endDateTime: 'End Date', | |||
type: 'Type', | |||
type: 'Visit Type', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why we decide to add Visit here? Aren't we already looking at Visit* fields? if we have to do it for one should we maybe not also do it for all of them? i.e. Visit Start Date, Visit End Date etc for consistency? Also not sure if this is related to the issue that this PR is meant to be addressing? I stand corrected should it be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I was originally thinking is to create a translation key for the visit type so that it can be used in the new request imaging page
<div className="visits"> {newImagingRequest.patient && visitOption ? ( <SelectWithLabelFormGroup name="visit" label={t('patient.visits.type')}
Maybe it will make more sense to create a new translation key in the imaging locale index.ts file to prevent some confusion. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can keep the translation key in patients since visits are part of the patients module. Maybe just call it Visit rather than Visit Type because visit type makes me think (outpatient/inpatient/emergency etc) where here we are giving the user a list of all visits for this patient regardless of the type of visit it is, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/8a7sca0e8 |
… request imaging page fix HospitalRun#2291
…rontend into link-image-with-visit
…/hospitalrun-frontend into link-image-with-visit
if (patient) { | ||
setNewImagingRequest((previousNewImagingRequest) => ({ | ||
...previousNewImagingRequest, | ||
patient: patient.fullName as string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should store patient id rather than the patient full name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
})) | ||
|
||
const visits = patient.visits?.map((v) => ({ label: v.type, value: v.id })) | ||
setVisitOption(visits) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do the visits need to be a state variable? seems like we could just reference patient.visits
wherever we use the visits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we need the visits to be a state variable. Because we can only fetch the visits data inside the onPatientChange
method using the patient parameter (as we fetch the patient using this line onSearch={async (query: string) => PatientRepository.search(query)}
). Using state variable ensures that the visits data will be re-rendered whenever the patient field value is changed.
{newImagingRequest.patient && visitOption ? ( | ||
<SelectWithLabelFormGroup | ||
name="visit" | ||
label={t('imagings.imaging.visit')} | ||
isRequired | ||
isEditable | ||
options={visitOption} | ||
defaultSelected={visitOption.filter( | ||
({ value }) => value === newImagingRequest.visitId, | ||
)} | ||
onChange={(values) => { | ||
onVisitChange(values[0]) | ||
}} | ||
/> | ||
) : ( | ||
<SelectWithLabelFormGroup | ||
name="visit" | ||
label={t('imagings.imaging.visit')} | ||
isRequired | ||
isEditable={false} | ||
options={[]} | ||
onChange={(values) => { | ||
onVisitChange(values[0]) | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since defaultSelected
takes an undefined, I think we can simplify this logic to the following (or something similar to this):
<SelectWithLabelFormGroup
name="visit"
label={t('imagings.imaging.visit')}
isRequired
isEditable={patient !== undefined}
options={patient.visits?.map((v) => ({ label: v.type, value: v.id })) || []}
onChange={(values) => {
onVisitChange(values[0])
}}
/>
patient: patient.fullName as string, | ||
})) | ||
|
||
const visits = patient.visits?.map((v) => ({ label: v.type, value: v.id })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the label should be something like ${visitType} at
${startDateTime}or
${visitType} (${startDateTime)}`
…the visit div component fix HospitalRun#2291
…/hospitalrun-frontend into link-image-with-visit
Fixes #2291 .
Changes proposed in this pull request: