Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

fix(lab): In New lab request make the visit field required #2552

Merged
merged 4 commits into from
Jan 27, 2021

Conversation

SamuelQZQ
Copy link
Contributor

Fixes #2551 .

Changes proposed in this pull request:

  • I make the NewLabRequest use the exact same method to solve the null visit issue. NewImagingRequest.tsx line65
  • My concern is, we use the as Option[] at line 57. So that the type of the variable visits is not right. It should be Option[] | null. Maybe we should do some refactor in NewImagingRequest.tsx and NewLabRequest.tsx to fix this wrong type. Otherwise, it may lead to other bugs in the future.

@vercel
Copy link

vercel bot commented Jan 20, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/8hgo1k2s2
✅ Preview: https://hospitalrun-frontend-git-fork-samuelqzq-lab-request-bug.hospitalrun.now.sh

@matteovivona
Copy link
Contributor

Could you please sign our CLA? Thank you https://cla.js.foundation/HospitalRun/hospitalrun-frontend?pullRequest=2552

@SamuelQZQ
Copy link
Contributor Author

I have signed the CLA

Copy link
Contributor

@blestab blestab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have opened a new session in another chrome instance and I can see the visit option, which is not required, which i guess if why this bug occurs. So if you make the visit required then the issue should be resolved.

Looking good @SamuelQZQ I can see we are no longer crashing if there is no visit specified, however since the visit is required for each lab request we need to make the visit field required as it is on the imaging request.

image

image

@blestab blestab changed the title fix: new lab request sometimes crash fix(lab): in NewLabRequest make the visit field required Jan 25, 2021
@blestab blestab changed the title fix(lab): in NewLabRequest make the visit field required fix(lab): In New lab request make the visit field required Jan 25, 2021
@blestab blestab added in progress indicates that issue/pull request is currently being worked on labs issue/pull request that interacts with labs module LOE - small indicates that the level of effort to complete issue is small (i.e changing the color of a button) 🐛bug issue/pull request that documents/fixes a bug labels Jan 25, 2021
@blestab blestab added this to the v2.0 milestone Jan 25, 2021
Copy link
Contributor

@blestab blestab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @SamuelQZQ

This is now behaving the same as the Request Imaging. I did notice however that Request Imaging (like this latest change) does not prevent saving when visit is not selected, even though it shows visit as required.

Will create separate issues for these though unless you want to deal with the Lab issue in this PR? If you prefer we work on it in a separate issue that's also alright, I can go ahead and approve this one so we can create the new issue?

image

@chadian
Copy link
Contributor

chadian commented Jan 26, 2021

This somewhat relates to my PR #2555 to make sure visits is an empty array when a patient is created. This would help with the "crashing" part but doesn't change the fact if it should be a required field or not (what is covered in this PR).

@SamuelQZQ
Copy link
Contributor Author

@blestab I prefer to work on it in a separate issue. Let's complete this one. I will test more carefully in future work.

@blestab
Copy link
Contributor

blestab commented Jan 27, 2021

@blestab I prefer to work on it in a separate issue. Let's complete this one. I will test more carefully in future work.

Awesome! thanks @SamuelQZQ

@tehkapa this one now looks good.

@blestab blestab self-requested a review January 27, 2021 06:22
@blestab
Copy link
Contributor

blestab commented Jan 27, 2021

This somewhat relates to my PR #2555 to make sure visits is an empty array when a patient is created. This would help with the "crashing" part but doesn't change the fact if it should be a required field or not (what is covered in this PR).

Yeah just noticed that. I have since created a separate issue for enforcing the 'required visit' behavior

@matteovivona matteovivona merged commit eb1fbed into HospitalRun:master Jan 27, 2021
@SamuelQZQ SamuelQZQ deleted the lab-request-bug branch January 27, 2021 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛bug issue/pull request that documents/fixes a bug in progress indicates that issue/pull request is currently being worked on labs issue/pull request that interacts with labs module LOE - small indicates that the level of effort to complete issue is small (i.e changing the color of a button)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lab - In the Request Lab routine make the visit field required
4 participants