-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HSC-287: Configured clinical forms. #22
Conversation
Some forms have an unsupported behavior on the RFE(nested grouping) i.e Historique de santé and Signes vitaux , which I am working on from this PR > openmrs/openmrs-esm-form-engine-lib#421 |
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.
Thanks for the PR @usamaidrsk. Generally looking good. Three things,
1.) Could you update the forms with their encounter types recently added through @kazlaw's recently merged PR
2.) Let's try to maintain the snake-case file names of each form as much as possible
3.) Let's also remove any accents from the names, i.e.
Examen gynécologique.json ➔ examen_gynecologique.json
@@ -2,153 +2,162 @@ | |||
"name": "Vitals and Biometrics", |
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.
The only change that should be part of this PR on the Vitals and Biometrics
form is, 1.) change of file name into snake_case, 2.) attaching of the correct encounter type UUID. And nothing else I believe otherwise the current changes on the form seem to misconfigure it.
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 have reverted the changes!
Thanks @Ruhanga
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.
Thanks, @usamaidrsk. Overall, LGTM. Could you please test these forms on an instance, perhaps via Gitpod? You can use this link to spin up an instance based on the branch from this PR. This is to verify that things like the forms' UUIDs remain consistent and the forms load without issues.
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.
Overral looking good. Thanks @usamaidrsk. One last thing to verify, could you make sure that the concepts' UUIDs (for grouping concept sets, question concepts and answer concepts ) used on the React forms are the very ones that where previously used with the Bahmni forms?
Yeah, for that I was confirming on creation. |
Where do I test this from, looks like this http://hsc-cd04.mekomsolutions.net environment is broken |
It should be up now @usamaidrsk Cc: @Ruhanga |
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.
Looks generally good. One missed update on the concepts generally with an example below.
@@ -618,7 +618,7 @@ | |||
"id": "smokingDuration(Year(S))", | |||
"questionOptions": { | |||
"rendering": "number", | |||
"concept": "159931AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", | |||
"concept": "dbab4772-1814-466f-a4ff-22aceca57200", |
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've changed the concept's UUID but missed updating the label, i.e. instead of being Smoking Duration (Year(s))
it should be Smoking Duration
?
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 have updated all manually appended label units.
This should be done by the RFE, and I created a ticket for this.
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're nearly there. @usamaidrsk, could you verify on a new running instance based on this PR whether the form schema from its file is preserved at runtime when checked through the form builder? If not, you'll have to update the fform's uuid in it's config file.
I've verified that @Ruhanga , and yes the form schemas are preserved. |
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.
LGTM!
https://mekomsolutions.atlassian.net/browse/HSC-287