Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

donaldkibet
Copy link
Member

@donaldkibet donaldkibet commented Jan 13, 2025

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

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

{
    "label": "Patient Identifier",
    "isExpanded": "true",
    "questions": [
        {
            "label": "Unique Patient Number",
            "id": "uniquePatientNumber",
            "questionOptions": {
                "rendering": "text",
                "identifierType": "dfacd928-0370-4315-99d7-6ec1c9f7ae76"
            },
            "type": "patientIdentifier",
            "validators": []
        }
    ]
}

Screenshots

Kapture 2025-01-13 at 17 15 56

Related Issue

Other

@donaldkibet donaldkibet changed the title (feat) : Add patient identifier validator (feat) : Add patient async identifier validator Jan 13, 2025
@donaldkibet donaldkibet force-pushed the feat/patientIdentifierValidator branch from afcf39e to a1d7094 Compare January 13, 2025 15:26
@donaldkibet donaldkibet changed the title (feat) : Add patient async identifier validator (feat) : Add patient identifier validatoion directive Jan 13, 2025
@ojwanganto ojwanganto changed the title (feat) : Add patient identifier validatoion directive (feat) : Add patient identifier validation directive Jan 13, 2025
@ojwanganto
Copy link

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' + '/';
Copy link
Member

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.

Copy link
Member

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);
Copy link
Member

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

Copy link
Member

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.

@ibacher
Copy link
Member

ibacher commented Jan 13, 2025

Also, we need a ticket documenting this change, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants