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.

Sign up for GitHub

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

Jump to bottom

Jian | BAH-460 | add api for search similar patient using PersonService #29

Closed
wants to merge 2 commits into from
Closed

Jian | BAH-460 | add api for search similar patient using PersonService #29

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a2597d
Jian | BAH-460 | add api for search similar patient using PersonService
Apr 24, 2018
fc8d8f5
Jian | BAH-460 | search patient by name and gender using lucen search
Apr 27, 2018
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
10 changes: 10 additions & 0 deletions .../src/main/java/org/bahmni/module/bahmnicore/contract/patient/PatientSearchParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class PatientSearchParameters {
private Boolean filterPatientsByLocation;
private String identifier;
private String name;
private String gender;
private String addressFieldName;
private String addressFieldValue;
private Integer start;
Expand Down Expand Up @@ -43,6 +44,7 @@ public PatientSearchParameters(RequestContext context) {
} else {
this.setAddressFieldName("city_village");
}
this.setGender(context.getParameter("gender"));
this.setAddressFieldValue(context.getParameter("addressFieldValue"));
Map parameterMap = context.getRequest().getParameterMap();
this.setAddressSearchResultFields((String[]) parameterMap.get("addressSearchResultsConfig"));
Expand Down Expand Up @@ -71,6 +73,14 @@ public void setName(String name) {
this.name = name;
}

public String getGender() {
Copy link

Choose a reason for hiding this comment

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

If we add this field here we also need to make sure it's respected by existing API methods that use PatientSearchParameters.
(It would be okay to skip the gender field when searching for similar patients, if your timeline to finish this is tight.)

return gender;
}

public void setGender(String gender) {
this.gender = gender;
}

public String getAddressFieldName() {
return addressFieldName;
}
Expand Down
4 changes: 3 additions & 1 deletion ...main/java/org/bahmni/module/bahmnicore/contract/patient/mapper/PatientResponseMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ public PatientResponse map(Patient patient, String loginLocationUuid, String[] s
patientResponse.setFamilyName(patient.getFamilyName());
patientResponse.setGender(patient.getGender());
PatientIdentifier primaryIdentifier = patient.getPatientIdentifier();
patientResponse.setIdentifier(primaryIdentifier.getIdentifier());
if(primaryIdentifier != null) {
patientResponse.setIdentifier(primaryIdentifier.getIdentifier());
}
patientResponse.setPatientProgramAttributeValue(programAttributeValue);

mapExtraIdentifiers(patient, primaryIdentifier);
Expand Down
4 changes: 4 additions & 0 deletions bahmnicore-api/src/main/java/org/bahmni/module/bahmnicore/service/BahmniPatientService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.bahmni.module.bahmnicore.service;

import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters;
import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientConfigResponse;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse;
import org.openmrs.Patient;
import org.openmrs.Person;
import org.openmrs.RelationshipType;

import java.util.List;
Expand All @@ -15,6 +17,8 @@ public interface BahmniPatientService {

List<PatientResponse> luceneSearch(PatientSearchParameters searchParameters);

List<PatientResponse> searchSimilarPatients(PatientSearchParameters searchParameters, PatientResponseMapper patientResponseMapper);
Copy link

Choose a reason for hiding this comment

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

I think the input parameter to this should actually be PatientProfile (which is https://github.com/openmrs/openmrs-module-emrapi/blob/master/api/src/main/java/org/openmrs/module/emrapi/patient/PatientProfile.java)
The registration create patient actually posts to PatientProfileResource: https://github.com/Bahmni/openmrs-module-bahmniapps/blob/15d0175b14585c7bca47d63f62e61fbf8a412ceb/ui/app/registration/services/defaultPatientServiceStrategy.js#L41

This method should take the same type, so that the UI that is building up a PatientProfile to submit can just submit a partial version.


public List<Patient> get(String partialIdentifier, boolean shouldMatchExactPatientId);

public List<RelationshipType> getByAIsToB(String aIsToB);
Expand Down
23 changes: 23 additions & 0 deletions ...api/src/main/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
package org.bahmni.module.bahmnicore.service.impl;

import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters;
import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientConfigResponse;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse;
import org.bahmni.module.bahmnicore.dao.PatientDao;
import org.bahmni.module.bahmnicore.service.BahmniPatientService;
import org.openmrs.Concept;
import org.openmrs.Patient;
import org.openmrs.Person;
import org.openmrs.PersonAttributeType;
import org.openmrs.RelationshipType;
import org.openmrs.api.ConceptService;
import org.openmrs.api.PersonService;
import org.openmrs.api.context.Context;
import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

@Service
@Lazy //to get rid of cyclic dependencies
Expand Down Expand Up @@ -83,6 +89,23 @@ public List<PatientResponse> luceneSearch(PatientSearchParameters searchParamete
searchParameters.getFilterPatientsByLocation(), searchParameters.getFilterOnAllIdentifiers());
}

@Override
public List<PatientResponse> searchSimilarPatients(PatientSearchParameters searchParameters, PatientResponseMapper patientResponseMapper) {
Copy link

Choose a reason for hiding this comment

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

If we're going to use PatientResponseMapper in this service we should autowire it in the constructor, not ask for it to be passed in by the caller.

List<PatientResponse> patients = new ArrayList<PatientResponse>();
Set<Person> persons = personService.getSimilarPeople(searchParameters.getName(), null, searchParameters.getGender());
Copy link

Choose a reason for hiding this comment

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

We need to actually use Lucene here. OpenMRS's personService.getSimilarPeople does a handcrafted SQL query which doesn't perform well against large databases: https://github.com/openmrs/openmrs-core/blob/2.1.3/api/src/main/java/org/openmrs/api/db/hibernate/HibernatePersonDAO.java#L82
(Also FYI not every Person is a Patient.)

for (Person person : persons) {
Patient patient = new Patient(person);
PatientResponse patientResponse = patientResponseMapper.map(
patient,
searchParameters.getLoginLocationUuid(),
searchParameters.getPatientSearchResultFields(),
searchParameters.getAddressSearchResultFields(),
patient.getPatientId());
patients.add(patientResponse);
}
return patients;
}

@Override
public List<Patient> get(String partialIdentifier, boolean shouldMatchExactPatientId) {
return patientDao.getPatients(partialIdentifier, shouldMatchExactPatientId);
Expand Down
23 changes: 23 additions & 0 deletions ...src/test/java/org/bahmni/module/bahmnicore/service/impl/BahmniPatientServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
package org.bahmni.module.bahmnicore.service.impl;

import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters;
Copy link

Choose a reason for hiding this comment

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

presumably this change isn't necessary since there's no changes to the class itself

import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientConfigResponse;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse;
import org.bahmni.module.bahmnicore.dao.PatientDao;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.openmrs.Concept;
import org.openmrs.Person;
import org.openmrs.PersonAttributeType;
import org.openmrs.api.ConceptService;
import org.openmrs.api.PersonService;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static junit.framework.Assert.assertEquals;
import static org.mockito.Mockito.anyInt;
Expand All @@ -26,6 +32,10 @@ public class BahmniPatientServiceImplTest {
private ConceptService conceptService;
@Mock
private PatientDao patientDao;
@Mock
private PatientSearchParameters searchParameters;
@Mock
private PatientResponseMapper patientResponseMapper;

private BahmniPatientServiceImpl bahmniPatientService;

Expand Down Expand Up @@ -67,4 +77,17 @@ public void shouldGetPatientByPartialIdentifier() throws Exception {
bahmniPatientService.get("partial_identifier", shouldMatchExactPatientId);
verify(patientDao).getPatients("partial_identifier", shouldMatchExactPatientId);
}

@Test
public void shouldGetSimiliarPatientResponseFromPersonSet() throws Exception {
Set<Person> persons = new HashSet<Person>();
persons.add(new Person());
persons.add(new Person());
when(personService.getSimilarPeople(searchParameters.getName(), null, searchParameters.getGender())).thenReturn(persons);

List<PatientResponse> response = bahmniPatientService.searchSimilarPatients(searchParameters, patientResponseMapper);

assertEquals(response.size(), 2);

}
}
23 changes: 23 additions & 0 deletions ...rg/bahmni/module/bahmnicore/web/v1_0/controller/search/BahmniPatientSearchController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
package org.bahmni.module.bahmnicore.web.v1_0.controller.search;

import org.bahmni.module.bahmnicore.contract.patient.PatientSearchParameters;
import org.bahmni.module.bahmnicore.contract.patient.mapper.PatientResponseMapper;
import org.bahmni.module.bahmnicore.contract.patient.response.PatientResponse;
import org.bahmni.module.bahmnicore.service.BahmniPatientService;
import org.openmrs.Patient;
import org.openmrs.PatientIdentifier;
import org.openmrs.Person;
import org.openmrs.api.context.Context;
import org.openmrs.module.bahmniemrapi.visitlocation.BahmniVisitLocationServiceImpl;
import org.openmrs.module.webservices.rest.web.RequestContext;
import org.openmrs.module.webservices.rest.web.RestConstants;
import org.openmrs.module.webservices.rest.web.RestUtil;
Expand All @@ -15,6 +21,7 @@
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;

import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -66,4 +73,20 @@ public ResponseEntity<AlreadyPaged<PatientResponse>> luceneSearch(HttpServletReq
return new ResponseEntity(RestUtil.wrapErrorResponse(e, e.getMessage()), HttpStatus.BAD_REQUEST);
}
}

@RequestMapping(value="similar", method = RequestMethod.GET)
@ResponseBody
public ResponseEntity<AlreadyPaged<PatientResponse>> searchSimilarPerson(HttpServletRequest request,
HttpServletResponse response) throws ResponseException{
RequestContext requestContext = RestUtil.getRequestContext(request, response);
PatientSearchParameters searchParameters = new PatientSearchParameters(requestContext);
try {
PatientResponseMapper patientResponseMapper = new PatientResponseMapper(Context.getVisitService(),new BahmniVisitLocationServiceImpl(Context.getLocationService()));
List<PatientResponse> patients = bahmniPatientService.searchSimilarPatients(searchParameters, patientResponseMapper);
AlreadyPaged alreadyPaged = new AlreadyPaged(requestContext, patients, false);
return new ResponseEntity(alreadyPaged, HttpStatus.OK);
}catch (IllegalArgumentException e){
return new ResponseEntity(RestUtil.wrapErrorResponse(e, e.getMessage()), HttpStatus.BAD_REQUEST);
}
}
}