-
Notifications
You must be signed in to change notification settings - Fork 23
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
(feat) : Add patient identifier validation directive #158
base: main
Are you sure you want to change the base?
Conversation
afcf39e
to
a1d7094
Compare
Thanks, @donaldkibet |
|
||
private validateIdentifier(identifier: string): Observable<any> { | ||
// For testing purposes, change openmrsBase to the OpenMRS server you are using | ||
const baseUrl = window?.['openmrsBase'] + '/ws/rest/v1' + '/'; |
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.
Why are we concatenating /ws/rest/v1
with /
here? These feel like they could be used elsewhere.
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 don't think there's any reason you can't rely on the framework constants here).
const baseUrl = window?.['openmrsBase'] + '/ws/rest/v1' + '/'; | ||
const apiUrl = `${baseUrl}patient`; | ||
|
||
const params = new HttpParams().set('q', identifier); |
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 don't love this mechanism for checking if an ID is duplicated. We really need to use information about the type of identifier being configured. Specifically, OpenMRS IDs, depending on identifier type can be:
- Unique
- Unique per location
- Allow duplicates
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 doing this is a great idea, but rather than doing some query of the backend, we should just add a backend endpoint that can tell us if a specified identifier is valid for an identifier type. That also allows us to check other validation constraints that this mechanism can't support. It wouldn't be too hard to write a REST endpoint that takes a (potential) identifier and identifier type and runs the validation and returns any errors that occur.
Also, we need a ticket documenting this change, please. |
Requirements
Summary
This PR adds an asynchronous patient identifier validator, which validates whether the captured patient identifier has been assigned to another patient. The aim is to perform validation while capturing form fields instead of when doing submission. This validation directive works on text input control
Sample form schema
Screenshots
Related Issue
Other