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

fix(lab): enforce the visit field when create lab #2559

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(lab): enforce the visit field when create lab #2559

wants to merge 4 commits into from

Conversation

SamuelQZQ
Copy link
Contributor

@SamuelQZQ SamuelQZQ commented Jan 28, 2021 •
edited

Fixes #2557

Changes proposed in this pull request:

  • validate the visit field when saving the Lab.

@vercel
Copy link

vercel bot commented Jan 28, 2021 •
edited

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/Asi5tKLsXTrxsh3FjGBuY1XTqH6s
✅ Preview: https://hospitalrun-frontend-git-fork-samuelqzq-test-hospitalrun.vercel.app

@SamuelQZQ SamuelQZQ changed the title Test fix(lab): enforce the visit field when create lab Jan 28, 2021
@blestab
Copy link
Contributor

blestab commented Jan 28, 2021

Just had a quick peek @SamuelQZQ you might still be busy with this, it looks like the required message does not display yet?
image

See report incident example showing all required fields messages if all of them are not filled in
image

jackcmeyer
jackcmeyer suggested changes Jan 30, 2021
View reviewed changes
src/labs/utils/validate-lab.ts
Comment on lines +36 to +39
if (!lab.visitId) {
labError.visit = 'labs.requests.error.visitRequired'
}

Copy link
Member

Choose a reason for hiding this comment

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

a test should be added to cover this piece of functionality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the validate-lab.test.ts already cover this right?

@jackcmeyer jackcmeyer added labs issue/pull request that interacts with labs module 🚀enhancement an issue/pull request that adds a feature to the application labels Jan 30, 2021
@jackcmeyer jackcmeyer added this to In progress in Version 2.0 via automation Jan 30, 2021
@jackcmeyer jackcmeyer added this to the v2.0 milestone Jan 30, 2021
@SamuelQZQ
Copy link
Contributor Author

@blestab The visit field is implemented with the Select component from @hospitalrun/components. Which does not have the functionality to display the error message. The other input fields are implemented with the TextInput component which can display the error message.

Any suggestion? Should I create a PR for the component Repo?

@CLAassistant
Copy link

CLAassistant commented Jan 30, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@SamuelQZQ
Copy link
Contributor Author

@blestab @jackcmeyer Any response?
The Select component cannot show the error message. Should I make the change in the Component repo first?

@blestab
Copy link
Contributor

blestab commented Feb 2, 2021 •
edited

@blestab @jackcmeyer Any response?
The Select component cannot show the error message. Should I make the change in the Component repo first?

@SamuelQZQ, sorry been offline.
I'll create a new issue in the components repo for you to fix the component to also display an error message when needed. Once that is done you can then resume work on this one to use the components changes? Will that work for you?

@SamuelQZQ
Copy link
Contributor Author

@blestab Sounds good.

@blestab blestab mentioned this pull request Feb 2, 2021
Closed
@blestab
Copy link
Contributor

blestab commented Feb 2, 2021

@blestab Sounds good.

HospitalRun/components#689 has been created for you @SamuelQZQ

@matteovivona
Copy link
Contributor

matteovivona commented Feb 5, 2021 •
edited

@SamuelQZQ select component has been merged HospitalRun/components#690

@SamuelQZQ
Copy link
Contributor Author

@blestab @tehkapa Updated. Please have a look.

@matteovivona matteovivona requested review from jackcmeyer and removed request for blestab February 9, 2021 11:58
@SamuelQZQ
Copy link
Contributor Author

@jackcmeyer Do you have time to have a look

@matteovivona matteovivona added the in progress indicates that issue/pull request is currently being worked on label Oct 2, 2021
@matteovivona matteovivona requested review from a team, morrme and cpondoc and removed request for a team October 2, 2021 09:31
@matteovivona
Copy link
Contributor

@SamuelQZQ sorry about delay, but the project is unfortunately back in a "frozen state". can you fix conflicts? thanks!

@matteovivona matteovivona removed the request for review from a team November 6, 2023 22:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jackcmeyer jackcmeyer Awaiting requested review from jackcmeyer

@morrme morrme Awaiting requested review from morrme morrme was automatically assigned from HospitalRun/core-contributors

@cpondoc cpondoc Awaiting requested review from cpondoc cpondoc was automatically assigned from HospitalRun/core-contributors

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
🚀enhancement an issue/pull request that adds a feature to the application in progress indicates that issue/pull request is currently being worked on labs issue/pull request that interacts with labs module
Projects
Version 2.0
  
In progress
Milestone
v2.0
Development

Successfully merging this pull request may close these issues.

New Lab Request - required visit not enforced
5 participants
@SamuelQZQ @blestab @CLAassistant @matteovivona @jackcmeyer