Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package org.openmrs.module.pihcore.rest;

import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.openmrs.Patient;
import org.openmrs.api.PatientService;
import org.openmrs.api.context.Context;
import org.openmrs.module.webservices.rest.web.RequestContext;
import org.openmrs.module.webservices.rest.web.resource.api.PageableResult;
import org.openmrs.module.webservices.rest.web.resource.api.SearchConfig;
import org.openmrs.module.webservices.rest.web.resource.impl.EmptySearchResult;
import org.openmrs.module.webservices.rest.web.resource.impl.NeedsPaging;
import org.openmrs.module.webservices.rest.web.response.ResponseException;
import org.openmrs.module.webservices.rest.web.v1_0.search.openmrs1_8.PatientByIdentifierSearchHandler1_8;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.openmrs.PatientProgram;
import org.openmrs.Program;
import org.openmrs.api.ProgramWorkflowService;
import org.openmrs.module.pihcore.PihEmrConfigConstants;
import org.openmrs.PatientIdentifier;


import java.util.List;

@Component
public class PihPatientSearchHandler extends PatientByIdentifierSearchHandler1_8 {

protected final Log log = LogFactory.getLog(getClass());

public static final String SEARCH_ID = "pihPatientSearch";

private final PatientService patientService;

private final ProgramWorkflowService programWorkflowService;

public PihPatientSearchHandler(@Autowired PatientService patientService,ProgramWorkflowService programWorkflowService) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both arguments need to be autowired

this.patientService = patientService;
this.programWorkflowService = programWorkflowService;
}

/**
* If multiple search handlers are found for a particular resource and the same supported parameters, the REST
* module will return the one whose id = "default". We configure that here to ensure this search handler is used.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment no longer appears correct, please remove / change it.

*/
@Override
public SearchConfig getSearchConfig() {
SearchConfig p = super.getSearchConfig();
return new SearchConfig(SEARCH_ID, p.getSupportedResource(), p.getSupportedOpenmrsVersions(), p.getSearchQueries());
}

/**
* This has the same logic as the superclass, with the addition of searching by insurance number and phone number
* There are 2 possible parameters that will be passed to this:
* - identifier = search with no white-space
* - q = search with white-space
* This handles both the same way currently
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment is not correct for this, it was simply copied from Rwanda. Please update this to reflect the actual behavior

*/
@Override
public PageableResult search(RequestContext context) throws ResponseException {
String searchString = context.getRequest().getParameter("identifier");
if (StringUtils.isBlank(searchString)) {
searchString = context.getRequest().getParameter("q");
}
if (StringUtils.isNotBlank(searchString)) {
log.trace("Searching patients by: " + searchString);

// The core patient search matches on identifier, name, and attributes based on core configuration(s)
List<Patient> patients = patientService.getPatients(null, searchString, null, true);
log.trace("Found " + patients.size() + " patients from core patient search");

// Retrieve the HIV program from the ProgramWorkflowService
Program hivProgram = programWorkflowService.getProgramByUuid(PihEmrConfigConstants.PROGRAM_HIV_UUID);
for(Patient patient : patients){
// Ensure that the patient object is not null and the HIV program exists
if (patient !=null && hivProgram !=null) {
// Retrieve all patient programs for this patient that are linked to the HIV program
List<PatientProgram> patientPrograms = programWorkflowService.getPatientPrograms(patient, hivProgram, null, null, null, null, false);
if (patientPrograms != null) {
for (PatientProgram pp : patientPrograms) {
for (PatientIdentifier pid : patient.getIdentifiers()) {
// Set the location of the identifier to match the program enrollment's location
pid.setLocation(pp.getLocation());
Copy link
Copy Markdown
Member

@mseaton mseaton Aug 8, 2025

Choose a reason for hiding this comment

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

I am a little concerned about this approach, looking at it, for a few reasons:

  • Setting the location of every identifier for a given patient to their program enrollment location is not correct. This may get us the results we want on the frontend, but it is not the correct place for the data, and although I don't think this would actually save these changes back to the database, I don't like modifying data like this on Hibernate-connected objects and changing data inadvertently
  • I am concerned about how this would perform, if we are iterating over every patient in the search results and making a service call to get all of their HIV program enrollments every time, for every found patient. This seems like it will be slow and/or cause too much load.
  • I'm not convinced that we are getting the right location here - as this query is just getting all HIV enrollments for the patient. We would need to be sure that the results returned are ordered in date ascending order so that we get the location of their most recent enrollment (assuming that is what we want)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To @mseaton 's points:

  • I agree that we need to confirm that the results of patient programs are sorted in date ascending order (or do this oursleves)
  • I agree that we don't want to set the value of the location here

I'm also concerned that we are using the Rwanda logic for patient search instead of what is provided in the core PatientByIdentifierSearchHandler1_8: https://github.com/openmrs/openmrs-module-webservices.rest/blob/master/omod-2.4/src/main/java/org/openmrs/module/webservices/rest/web/v1_0/search/openmrs1_8/PatientByIdentifierSearchHandler1_8.java#L50

This is definitely a complicated one and I've been staring at it for a while now @louidorjp , thanks for all this work and sorry this is so difficult! I wll comment more on the ticket.

}
}
}
}
}

if (!patients.isEmpty()) {
return new NeedsPaging<>(patients, context);
}
}

log.trace("No patients found that match the search criteria");
return new EmptySearchResult();
}

}