HAI-736: Display the patient enrollment site during the search #586
Conversation
|
|
||
| /** | ||
| * 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. |
There was a problem hiding this comment.
This comment no longer appears correct, please remove / change it.
| * 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 |
There was a problem hiding this comment.
This comment is not correct for this, it was simply copied from Rwanda. Please update this to reflect the actual behavior
| 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()); |
There was a problem hiding this comment.
I am a little concerned about this approach, looking at it, for a few reasons:
- Setting the
locationof 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)
There was a problem hiding this comment.
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.
|
|
||
| private final ProgramWorkflowService programWorkflowService; | ||
|
|
||
| public PihPatientSearchHandler(@Autowired PatientService patientService,ProgramWorkflowService programWorkflowService) { |
There was a problem hiding this comment.
Both arguments need to be autowired
Please review this PR