-
Notifications
You must be signed in to change notification settings - Fork 143
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
EA-207: Implement an API for Fetching Visits with Notes and Diagnosis #250
base: master
Are you sure you want to change the base?
Changes from 9 commits
876bdee
efa8cb7
539eb15
612175d
0828b67
2ede16b
a1dd8a0
1aea583
875b405
8b20d2d
7f8fe94
a23b7c0
f2fa209
cade351
494e9b9
e8c79b1
b155ca9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
package org.openmrs.module.emrapi.db; | ||
|
||
import org.hibernate.Query; | ||
import org.hibernate.SessionFactory; | ||
import org.openmrs.Diagnosis; | ||
import org.openmrs.Patient; | ||
import org.openmrs.Visit; | ||
import org.openmrs.module.emrapi.visit.VisitWithDiagnoses; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Repository; | ||
|
||
import java.util.Comparator; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.stream.Collectors; | ||
|
||
@Repository | ||
public class HibernateVisitDAO implements VisitDAO { | ||
|
||
@Autowired | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually we do not autowire into a DAO AFAIK, but we use xml. There are definitely performance issues to using Autowired in services, and I'm not sure if those carry over to DAOs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch and 100% agree... it's annoying, but we need to manually wire services and daos using xml... there should be examples in this module and Core. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They likely don't, but I'd still favour XML over autowiring. |
||
private SessionFactory sessionFactory; | ||
|
||
public List<VisitWithDiagnoses> getVisitsWithNotesAndDiagnosesByPatient(Patient patient, int startIndex, int limit) { | ||
|
||
String visitNoteEncounterTypeUuid = "d7151f82-c1f3-4152-a605-2f9ea7414a79"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of hardcoding the uuid here, we should be fetching this based on the encounter that has the "emr.visitNoteEncounterType" metadata mapping, see: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And actually, why are we including this subquery at all? Do we only want to include visits that have visit notes (this seems wrong)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We’re including this to get only note encounters with the visit, but we also need visits that don’t have encounters. I’ve updated the query with a left join and modified the test accordingly. |
||
|
||
String hqlVisit="SELECT DISTINCT v FROM Visit v " + | ||
"LEFT JOIN FETCH v.encounters enc " + | ||
"LEFT JOIN enc.encounterType et " + | ||
"WHERE v.patient.id = :patientId " + | ||
"AND (et.uuid = :encounterTypeUuid OR enc IS NULL) " + | ||
"ORDER BY v.startDatetime DESC"; | ||
|
||
Query visitQuery = sessionFactory.getCurrentSession().createQuery(hqlVisit); | ||
|
||
visitQuery.setParameter("patientId", patient.getId()); | ||
visitQuery.setParameter("encounterTypeUuid", visitNoteEncounterTypeUuid); | ||
visitQuery.setFirstResult(startIndex); | ||
visitQuery.setMaxResults(limit); | ||
|
||
List<Visit> visits = visitQuery.list(); | ||
|
||
String hqlDiagnosis = "SELECT DISTINCT diag FROM Diagnosis diag " + | ||
"JOIN diag.encounter e " + | ||
"WHERE e.visit.id IN :visitIds"; | ||
|
||
List<Integer> visitIds = visits.stream() | ||
.map(Visit::getId) | ||
.collect(Collectors.toList()); | ||
|
||
List<Diagnosis> diagnoses = sessionFactory.getCurrentSession() | ||
.createQuery(hqlDiagnosis) | ||
.setParameterList("visitIds", visitIds) | ||
.list(); | ||
|
||
Map<Visit, Set<Diagnosis>> visitToDiagnosesMap = new HashMap<>(); | ||
for (Diagnosis diagnosis : diagnoses) { | ||
Visit visit = diagnosis.getEncounter().getVisit(); | ||
visitToDiagnosesMap | ||
.computeIfAbsent(visit, v -> new HashSet<>()) | ||
.add(diagnosis); | ||
} | ||
|
||
List<VisitWithDiagnoses> visitWithDiagnoses = visits.stream() | ||
.sorted(Comparator.comparing(Visit::getStartDatetime).reversed()) | ||
.map(visit -> new VisitWithDiagnoses(visit, visitToDiagnosesMap.getOrDefault(visit, new HashSet<>()))) | ||
.collect(Collectors.toList()); | ||
|
||
return visitWithDiagnoses; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package org.openmrs.module.emrapi.db; | ||
|
||
import org.openmrs.Patient; | ||
import org.openmrs.module.emrapi.visit.VisitWithDiagnoses; | ||
|
||
import java.util.List; | ||
|
||
public interface VisitDAO { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need an entirely new DAO class we have to wire up and maintain? Can't we just move to having one DAO for the entire module? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm fine with using the existing "EmrApiDAO" for this... there are some other DAOs already in the EMR API module with obsolete names ("EmrEncounterDAO"), but we could move away from these in favor of a single DAO. |
||
|
||
List<VisitWithDiagnoses> getVisitsWithNotesAndDiagnosesByPatient(Patient patient, int startIndex, int limit); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package org.openmrs.module.emrapi.visit; | ||
|
||
import lombok.Getter; | ||
import lombok.Setter; | ||
import org.openmrs.Diagnosis; | ||
import org.openmrs.Visit; | ||
|
||
import java.util.Set; | ||
|
||
@Setter | ||
@Getter | ||
public class VisitWithDiagnoses extends Visit { | ||
|
||
|
||
public VisitWithDiagnoses(Visit visit, Set<Diagnosis> diagnoses) { | ||
jayasanka-sack marked this conversation as resolved.
Show resolved
Hide resolved
|
||
super(); | ||
this.setVisitId(visit.getVisitId()); | ||
this.setPatient(visit.getPatient()); | ||
this.setVisitType(visit.getVisitType()); | ||
this.setIndication(visit.getIndication()); | ||
this.setLocation(visit.getLocation()); | ||
this.setStartDatetime(visit.getStartDatetime()); | ||
this.setStopDatetime(visit.getStopDatetime()); | ||
this.setEncounters(visit.getEncounters()); | ||
this.uuid = visit.getUuid(); | ||
this.diagnoses = diagnoses; | ||
} | ||
|
||
private String uuid; | ||
private Set<Diagnosis> diagnoses; | ||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package org.openmrs.module.emrapi.visit; | ||
|
||
import org.openmrs.Patient; | ||
|
||
import java.util.List; | ||
|
||
public interface VisitWithDiagnosesService { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's a very good idea! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! Let's move towards having fewer distinct DAOs and Services in this module |
||
/** | ||
* Fetches visits with note encounters and diagnoses of a patient by patient ID. | ||
* | ||
* @param patientUuid the UUID of the patient | ||
* @return a list of visits that has note encounters and diagnoses | ||
*/ | ||
List<VisitWithDiagnoses> getVisitsByPatientId(String patientUuid, int startIndex, int limit); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment here, let's make this "getVisitsWithDiagnosesByPatient" (add "WithDiagnoses" and remove "Id") |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package org.openmrs.module.emrapi.visit; | ||
|
||
import org.hibernate.ObjectNotFoundException; | ||
import org.openmrs.Patient; | ||
import org.openmrs.api.PatientService; | ||
import org.openmrs.api.impl.BaseOpenmrsService; | ||
import org.openmrs.module.emrapi.db.VisitDAO; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.stereotype.Service; | ||
|
||
import java.util.List; | ||
|
||
@Service | ||
public class VisitWithDiagnosesServiceImpl extends BaseOpenmrsService implements VisitWithDiagnosesService { | ||
|
||
@Autowired | ||
PatientService patientService; | ||
|
||
@Autowired | ||
VisitDAO visitDAO; | ||
|
||
@Override | ||
public List<VisitWithDiagnoses> getVisitsByPatientId(String patientUuid, int startIndex, int limit) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's have this take a Patient patient, not a String patientUuid (see my further comments below) |
||
|
||
Patient patient = patientService.getPatientByUuid(patientUuid); | ||
|
||
if (patient == null) { | ||
throw new ObjectNotFoundException("No patient found with uuid " + patientUuid, Patient.class.getName()); | ||
} | ||
|
||
return visitDAO.getVisitsWithNotesAndDiagnosesByPatient(patient, startIndex, limit); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package org.openmrs.module.emrapi.db; | ||
|
||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.openmrs.Diagnosis; | ||
import org.openmrs.Encounter; | ||
import org.openmrs.Patient; | ||
import org.openmrs.module.emrapi.EmrApiContextSensitiveTest; | ||
import org.openmrs.module.emrapi.visit.VisitWithDiagnoses; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
|
||
import java.util.List; | ||
import java.util.Set; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertNotNull; | ||
|
||
public class VisitDAOTest extends EmrApiContextSensitiveTest { | ||
|
||
@Autowired | ||
private VisitDAO visitDAO; | ||
|
||
@Before | ||
public void setup() { | ||
executeDataSet("baseTestDataset.xml"); | ||
executeDataSet("pastVisitSetup.xml"); | ||
} | ||
|
||
@Test | ||
public void shouldFetchVisitsByPatientId() { | ||
String visitNoteEncounterTypeUuid = "d7151f82-c1f3-4152-a605-2f9ea7414a79"; | ||
|
||
Patient patient = new Patient(); | ||
patient.setPatientId(109); | ||
|
||
List<VisitWithDiagnoses> visits = visitDAO.getVisitsWithNotesAndDiagnosesByPatient(patient,0,10); | ||
assertNotNull(visits); | ||
assert visits.size() == 3; | ||
|
||
VisitWithDiagnoses firstVisit = visits.get(2); | ||
Set<Encounter> firstVisitEncounters = firstVisit.getEncounters(); | ||
Set<Diagnosis> firstVisitDiagnoses = firstVisit.getDiagnoses(); | ||
|
||
assert firstVisit.getId() == 1014; | ||
assert firstVisit.getPatient().getPatientId() == 109; | ||
assert firstVisitEncounters.size() == 2; | ||
assert firstVisitDiagnoses.size() == 3; | ||
|
||
for (Encounter encounter : firstVisitEncounters) { | ||
assert encounter.getEncounterType().getUuid().equals(visitNoteEncounterTypeUuid); | ||
} | ||
|
||
VisitWithDiagnoses secondVisit = visits.get(1); | ||
Set<Encounter> secondVisitEncounters = secondVisit.getEncounters(); | ||
Set<Diagnosis> secondVisitDiagnoses = secondVisit.getDiagnoses(); | ||
|
||
assert secondVisit.getId() == 1015; | ||
assert secondVisit.getPatient().getPatientId() == 109; | ||
assert secondVisitEncounters.size() == 1; | ||
assert secondVisitDiagnoses.size() == 2; | ||
|
||
for (Encounter encounter : secondVisitEncounters) { | ||
assert encounter.getEncounterType().getUuid().equals(visitNoteEncounterTypeUuid); | ||
} | ||
|
||
VisitWithDiagnoses thirdVisit = visits.get(0); | ||
Set<Encounter> thirdVisitEncounters = thirdVisit.getEncounters(); | ||
Set<Diagnosis> thirdVisitDiagnoses = thirdVisit.getDiagnoses(); | ||
|
||
assert thirdVisit.getId() == 1017; | ||
assert thirdVisit.getPatient().getPatientId() == 109; | ||
assert thirdVisitEncounters.isEmpty(); | ||
assert thirdVisitDiagnoses.isEmpty(); | ||
|
||
} | ||
|
||
@Test | ||
public void shouldFetchVisitsByPatientIdWithPagination() { | ||
Patient patient = new Patient(); | ||
patient.setPatientId(109); | ||
|
||
List<VisitWithDiagnoses> visits = visitDAO.getVisitsWithNotesAndDiagnosesByPatient(patient,0,1); | ||
assertNotNull(visits); | ||
assert visits.size() == 1; | ||
|
||
VisitWithDiagnoses mostRecentVisit = visits.get(0); | ||
assert mostRecentVisit.getId() == 1017; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?xml version='1.0' encoding='UTF-8'?> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this class being used, or is everything handled in the other "pastVisitSetup" class below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both files are used in tests in both modules. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the distinction between them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the organization is a little weird. It looks like there may have been some convention for all the metadata to go into a file like this and the data into a file like the other one, but both of them mix data and metadata, so it's confusing why we need the two. |
||
|
||
<dataset> | ||
|
||
<encounter_type encounter_type_id="1003" name="Visit Note" description="Visit note" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d7151f82-c1f3-4152-a605-2f9ea7414a79"/> | ||
<encounter_type encounter_type_id="1004" name="Vitals" description="For capturing vital signs" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d92ca5dc-7fd0-4fa2-b726-ab16fbc53fc1"/> | ||
|
||
<!-- Person (Patient) Information --> | ||
<person person_id="109" gender="M" dead="false" creator="1" birthdate_estimated="0" | ||
date_created="2023-10-15 10:30:00.0" voided="false" void_reason="" | ||
uuid="8604d42e-3ca8-11e3-bf2b-0d0c09861e97"/> | ||
<person_name person_name_id="109" person_id="109" preferred="true" prefix="" given_name="Scott" middle_name="" | ||
family_name="Clark" family_name_suffix="" creator="1" date_created="2023-10-15 10:31:00.0" | ||
voided="false" void_reason="" uuid="8f9e3a7b-6482-487d-9abe-c07b123c4f08"/> | ||
|
||
<!-- Patient Record --> | ||
<patient patient_id="109" creator="1" date_created="2023-10-15 10:32:00.0" voided="false" /> | ||
|
||
<!-- Location Information --> | ||
<location location_id="1901" name="Location tagged to visit" creator="1" date_created="2023-10-15 10:35:00.0" retired="false" uuid="f1771d8e-bf1f-4dc5-957f-9d40a5eebf08"/> | ||
|
||
<!-- Visit Information --> | ||
<visit visit_id="1014" patient_id="109" visit_type_id="1" date_started="2023-10-20 09:00:00.0" date_stopped="2023-10-20 09:30:00.0" location_id="1901" creator="1" date_created="2023-10-20 09:05:00.0" voided="0" uuid="1esd5218-6b78-11e0-93c3-18a905e044dc" /> | ||
|
||
<!-- Encounter Information (Visit Note) --> | ||
<encounter encounter_id="2001" visit_id="1014" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:10:00.0" creator="1" date_created="2023-10-20 09:12:00.0" voided="false" uuid="a123d653-444b-4118-9c83-a3715b82d4ac"/> | ||
<encounter encounter_id="2002" visit_id="1014" encounter_type="1004" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:10:00.0" creator="1" date_created="2023-10-20 09:12:00.0" voided="false" uuid="a9a41017-2593-442a-bb88-c80358692d1e"/> | ||
|
||
</dataset> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
<?xml version='1.0' encoding='UTF-8'?> | ||
|
||
<dataset> | ||
|
||
<!-- Encounter Types --> | ||
<encounter_type encounter_type_id="1003" name="Visit Note" description="Visit note" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d7151f82-c1f3-4152-a605-2f9ea7414a79"/> | ||
<encounter_type encounter_type_id="1004" name="Consultation" description="Consultation encounter" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d92ca5dc-7fd0-4fa2-b726-ab16fbc53fc1"/> | ||
|
||
<!-- Patient 1: Scott Clark --> | ||
<person person_id="109" gender="M" dead="false" creator="1" birthdate_estimated="0" date_created="2023-10-15 10:30:00.0" voided="false" void_reason="" uuid="8604d42e-3ca8-11e3-bf2b-0d0c09861e97"/> | ||
<person_name person_name_id="109" person_id="109" preferred="true" prefix="" given_name="Scott" middle_name="" family_name="Clark" family_name_suffix="" creator="1" date_created="2023-10-15 10:31:00.0" voided="false" void_reason="" uuid="8f9e3a7b-6482-487d-9abe-c07b123c4f08"/> | ||
<patient patient_id="109" creator="1" date_created="2023-10-15 10:32:00.0" voided="false" /> | ||
|
||
<!-- Location Information --> | ||
<location location_id="1901" name="Location tagged to visit" creator="1" date_created="2023-10-15 10:35:00.0" retired="false" uuid="f1771d8e-bf1f-4dc5-957f-9d40a5eebf08"/> | ||
|
||
<!-- First Visit for Scott Clark --> | ||
<visit visit_id="1014" patient_id="109" visit_type_id="1" date_started="2023-10-20 09:00:00.0" date_stopped="2023-10-20 09:30:00.0" location_id="1901" creator="1" date_created="2023-10-20 09:05:00.0" voided="0" uuid="1esd5218-6b78-11e0-93c3-18a905e044dc" /> | ||
|
||
<!-- First Visit - Encounters for Scott Clark (3 encounters) --> | ||
<encounter encounter_id="2001" visit_id="1014" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:10:00.0" creator="1" date_created="2023-10-20 09:12:00.0" voided="false" uuid="a123d653-444b-4118-9c83-a3715b82d4ac"/> | ||
<encounter encounter_id="2002" visit_id="1014" encounter_type="1004" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:20:00.0" creator="1" date_created="2023-10-20 09:22:00.0" voided="false" uuid="a9a41017-2593-442a-bb88-c80358692d1e"/> | ||
<encounter encounter_id="2008" visit_id="1014" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:25:00.0" creator="1" date_created="2023-10-20 09:27:00.0" voided="false" uuid="b2f67239-8a69-4f68-8fbe-6a248efc2b44"/> | ||
|
||
<!-- Diagnoses for First Visit --> | ||
<encounter_diagnosis diagnosis_id="1" encounter_id="2001" patient_id="109" creator="1" date_created="2023-10-20 09:15:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="47dd2d5e-5cfe-4fdd-b284-3147e60d4d25"/> | ||
<encounter_diagnosis diagnosis_id="2" encounter_id="2002" patient_id="109" creator="1" date_created="2023-10-20 09:25:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="b9dd36c2-3bf7-4a20-8ddc-88ccf75ea4f0"/> | ||
<encounter_diagnosis diagnosis_id="8" encounter_id="2008" patient_id="109" creator="1" date_created="2023-10-20 09:27:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="28c583be-d9ae-4d0f-8d88-df7fcffb2d22"/> | ||
|
||
<!-- Second Visit for Scott Clark --> | ||
<visit visit_id="1015" patient_id="109" visit_type_id="1" date_started="2023-11-01 10:00:00.0" date_stopped="2023-11-01 10:30:00.0" location_id="1901" creator="1" date_created="2023-11-01 10:05:00.0" voided="0" uuid="1c72e1ac-9b18-11e0-93c3-18a905e044dc" /> | ||
|
||
<!-- Second Visit - Encounters for Scott Clark (2 encounters) --> | ||
<encounter encounter_id="2003" visit_id="1015" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-11-01 10:10:00.0" creator="1" date_created="2023-11-01 10:12:00.0" voided="false" uuid="b1f847e9-444c-4a8c-bba5-cc8b8dc1b123"/> | ||
<encounter encounter_id="2004" visit_id="1015" encounter_type="1004" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-11-01 10:20:00.0" creator="1" date_created="2023-11-01 10:25:00.0" voided="false" uuid="d58e3e83-dbd4-4ba8-a61f-b4ff5c1bc1e8"/> | ||
|
||
<!-- Diagnoses for Second Visit --> | ||
<encounter_diagnosis diagnosis_id="3" encounter_id="2003" patient_id="109" creator="1" date_created="2023-11-01 10:15:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="aef38d76-ffde-47a1-8717-5c74fddc2c35"/> | ||
<encounter_diagnosis diagnosis_id="4" encounter_id="2004" patient_id="109" creator="1" date_created="2023-11-01 10:25:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="b3241d9c-5d78-4d9c-bef6-0c264dfa3892"/> | ||
|
||
<!-- Third Visit for Scott Clark --> | ||
<visit visit_id="1017" patient_id="109" visit_type_id="1" date_started="2023-12-05 14:00:00.0" date_stopped="2023-12-05 14:30:00.0" location_id="1901" creator="1" date_created="2023-12-05 14:05:00.0" voided="0" uuid="3c72f2bc-9b18-11e0-93c3-18a905e044ec" /> | ||
|
||
<!-- Patient 2: John Doe --> | ||
<person person_id="110" gender="M" dead="false" creator="1" birthdate_estimated="0" date_created="2023-10-20 09:50:00.0" voided="false" void_reason="" uuid="c672b5ee-452d-4c97-b2d2-8b5ea84c8d0b"/> | ||
<person_name person_name_id="110" person_id="110" preferred="true" prefix="" given_name="John" middle_name="" family_name="Doe" family_name_suffix="" creator="1" date_created="2023-10-20 09:51:00.0" voided="false" void_reason="" uuid="5dc9b93f-4ebf-489e-98c6-fc52b362d12b"/> | ||
<patient patient_id="110" creator="1" date_created="2023-10-20 09:52:00.0" voided="false" /> | ||
|
||
<!-- Visit Information for John Doe --> | ||
<visit visit_id="1016" patient_id="110" visit_type_id="1" date_started="2023-10-25 08:30:00.0" date_stopped="2023-10-25 09:00:00.0" location_id="1901" creator="1" date_created="2023-10-25 08:35:00.0" voided="0" uuid="2a1c9274-791b-41d3-82f9-65482d22df65" /> | ||
|
||
<!-- Encounters for John Doe (3 encounters) --> | ||
<encounter encounter_id="2005" visit_id="1016" encounter_type="1003" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-10-25 08:40:00.0" creator="1" date_created="2023-10-25 08:42:00.0" voided="false" uuid="d14f3c6a-4cf6-48d4-97be-97f5f73d6c16"/> | ||
<encounter encounter_id="2006" visit_id="1016" encounter_type="1004" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-10-25 08:45:00.0" creator="1" date_created="2023-10-25 08:47:00.0" voided="false" uuid="e354f65a-8b36-4eaf-9a54-1342f84faabc"/> | ||
<encounter encounter_id="2007" visit_id="1016" encounter_type="1003" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-11-05 09:00:00.0" creator="1" date_created="2023-11-05 09:05:00.0" voided="false" uuid="f6f8c39b-76f9-4879-8712-4cd178d2c2dc"/> | ||
|
||
<!-- Diagnoses for John Doe --> | ||
<encounter_diagnosis diagnosis_id="5" encounter_id="2005" patient_id="110" creator="1" date_created="2023-10-25 08:42:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="c0a9f874-2b47-4f58-93fc-d8a0d233b77f"/> | ||
<encounter_diagnosis diagnosis_id="6" encounter_id="2006" patient_id="110" creator="1" date_created="2023-10-25 08:47:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="0b75c6d4-45a5-477a-bf7a-f5a0476a47d8"/> | ||
<encounter_diagnosis diagnosis_id="7" encounter_id="2007" patient_id="110" creator="1" date_created="2023-11-05 09:05:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="33a9f72c-d911-42b9-950f-4f1a6201a20a"/> | ||
|
||
</dataset> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,6 +149,12 @@ | |
<version>1.9.13</version> | ||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.junit.jupiter</groupId> | ||
<artifactId>junit-jupiter</artifactId> | ||
<version>RELEASE</version> | ||
<scope>test</scope> | ||
</dependency> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not necessarily totally opposed to this, but what Junit functionality is this added that we don't already have available in this module? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Jupiter is JUnit 5's base engine, so I expect this is to be able to write tests in JUnit 5? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We either should choose to use Junit 4 or Junit 5 for the entire module. I don't want us to have to try to support both in one module, there are too many annoying gotchas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if it's problematic to support both JUnit 4 and JUnit 5 in a single module, let's just go with JUnit 4 until we feel motivated to update all the tests. |
||
</dependencies> | ||
|
||
<build> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
package org.openmrs.module.emrapi.web.controller; | ||
|
||
import org.hibernate.ObjectNotFoundException; | ||
import org.openmrs.Diagnosis; | ||
import org.openmrs.Patient; | ||
import org.openmrs.module.emrapi.visit.VisitWithDiagnoses; | ||
import org.openmrs.module.emrapi.visit.VisitWithDiagnosesService; | ||
import org.openmrs.module.webservices.rest.SimpleObject; | ||
import org.openmrs.module.webservices.rest.web.ConversionUtil; | ||
import org.openmrs.module.webservices.rest.web.RequestContext; | ||
import org.openmrs.module.webservices.rest.web.RestUtil; | ||
import org.openmrs.module.webservices.rest.web.representation.Representation; | ||
import org.openmrs.module.webservices.rest.web.resource.impl.NeedsPaging; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.stereotype.Controller; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestMethod; | ||
|
||
import javax.servlet.http.HttpServletRequest; | ||
import javax.servlet.http.HttpServletResponse; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
@Controller | ||
public class VisitController { | ||
|
||
@Autowired | ||
VisitWithDiagnosesService visitWithDiagnosesService; | ||
|
||
@RequestMapping(method = RequestMethod.GET, value = "/rest/**/emrapi/patient/{patientUuid}/visitWithNotesAndDiagnoses") | ||
public ResponseEntity<?> getVisitsByPatientId( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's make this "getVisitsWithDiagnosesByPatient" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And the endpoint should not be |
||
HttpServletRequest request, | ||
HttpServletResponse response, | ||
@PathVariable("patientUuid") String patientUuid) { | ||
RequestContext context = RestUtil.getRequestContext(request, response, Representation.DEFAULT); | ||
List<VisitWithDiagnoses> visits; | ||
visits = visitWithDiagnosesService.getVisitsByPatientId(patientUuid, context.getStartIndex(), context.getLimit()); | ||
|
||
// Convert the visits and diagnoses to SimpleObjects | ||
List<SimpleObject> convertedVisits = new ArrayList<>(); | ||
for (VisitWithDiagnoses visit : visits) { | ||
SimpleObject visitObject = (SimpleObject) ConversionUtil.convertToRepresentation(visit, context.getRepresentation()); | ||
List<SimpleObject> convertedDiagnoses = new ArrayList<>(); | ||
|
||
for (Diagnosis diagnosis : visit.getDiagnoses()) { | ||
convertedDiagnoses.add((SimpleObject) ConversionUtil.convertToRepresentation(diagnosis, context.getRepresentation())); | ||
} | ||
visitObject.put("diagnoses", convertedDiagnoses); | ||
|
||
convertedVisits.add(visitObject); | ||
} | ||
|
||
return new ResponseEntity<>(new NeedsPaging<>(convertedVisits, context), HttpStatus.OK); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
package org.openmrs.module.emrapi.web.controller; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.openmrs.web.test.BaseModuleWebContextSensitiveTest; | ||
import org.springframework.beans.factory.ObjectFactory; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.http.MediaType; | ||
import org.springframework.test.web.servlet.MockMvc; | ||
import org.springframework.test.web.servlet.MvcResult; | ||
import org.springframework.test.web.servlet.setup.MockMvcBuilders; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import static org.junit.Assert.assertNotNull; | ||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; | ||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
|
||
public class VisitControllerTest extends BaseModuleWebContextSensitiveTest { | ||
|
||
@Autowired | ||
private ObjectFactory<VisitController> controllerFactory; | ||
|
||
private MockMvc mockMvc; | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
executeDataSet("baseMetaData.xml"); | ||
executeDataSet("pastVisitSetup.xml"); | ||
mockMvc = MockMvcBuilders.standaloneSetup(controllerFactory.getObject()).build(); | ||
} | ||
|
||
@Test | ||
public void shouldGetVisitsByPatientId() throws Exception { | ||
|
||
String visitNoteEncounterTypeUuid = "d7151f82-c1f3-4152-a605-2f9ea7414a79"; | ||
String patientUuid = "8604d42e-3ca8-11e3-bf2b-0d0c09861e97"; | ||
String firstVisitUuid = "1esd5218-6b78-11e0-93c3-18a905e044dc"; | ||
String secondVisitUuid = "1c72e1ac-9b18-11e0-93c3-18a905e044dc"; | ||
String thirdVisitUuid = "3c72f2bc-9b18-11e0-93c3-18a905e044ec"; | ||
|
||
MvcResult response = mockMvc.perform(get("/rest/v1/emrapi/patient/" + patientUuid+"/visitWithNotesAndDiagnoses") | ||
.contentType(MediaType.APPLICATION_JSON)) | ||
.andExpect(status().isOk()) | ||
.andReturn(); | ||
|
||
String jsonResponse = response.getResponse().getContentAsString(); | ||
ObjectMapper objectMapper = new ObjectMapper(); | ||
Map<String, Object> result = objectMapper.readValue(jsonResponse, Map.class); | ||
|
||
assertNotNull(result); | ||
assert result.get("totalCount").equals(3); | ||
|
||
List<Map<String, Object>> visits = (List<Map<String, Object>>) result.get("pageOfResults"); | ||
assert visits.size() == 3; | ||
|
||
// extract the first visit and check its properties | ||
Map<String, Object> firstVisit = visits.get(2); | ||
List<Map<String, Object>> firstVisitEncounters = (List<Map<String, Object>>) firstVisit.get("encounters"); | ||
List<Map<String, Object>> firstVisitDiagnoses = (List<Map<String, Object>>) firstVisit.get("diagnoses"); | ||
|
||
assert firstVisit.get("uuid").equals(firstVisitUuid); | ||
assert firstVisitEncounters.size() == 2; | ||
assert firstVisitDiagnoses.size() == 3; | ||
|
||
for (Map<String, Object> encounter : firstVisitEncounters) { | ||
assert ((String) encounter.get("display")).startsWith("Visit Note"); | ||
} | ||
|
||
// extract the second visit and check its properties | ||
Map<String, Object> secondVisit = visits.get(1); | ||
List<Map<String, Object>> secondVisitEncounters = (List<Map<String, Object>>) secondVisit.get("encounters"); | ||
List<Map<String, Object>> secondVisitDiagnoses = (List<Map<String, Object>>) secondVisit.get("diagnoses"); | ||
|
||
assert secondVisit.get("uuid").equals(secondVisitUuid); | ||
assert secondVisitEncounters.size() == 1; | ||
assert secondVisitDiagnoses.size() == 2; | ||
|
||
for (Map<String, Object> encounter : secondVisitEncounters) { | ||
assert ((String) encounter.get("display")).startsWith("Visit Note"); | ||
} | ||
|
||
// extract the third visit and check its properties (no notes or diagnoses) | ||
Map<String, Object> thirdVisit = visits.get(0); | ||
List<Map<String, Object>> thirdVisitEncounters = (List<Map<String, Object>>) thirdVisit.get("encounters"); | ||
List<Map<String, Object>> thirdVisitDiagnoses = (List<Map<String, Object>>) thirdVisit.get("diagnoses"); | ||
|
||
assert thirdVisit.get("uuid").equals(thirdVisitUuid); | ||
assert thirdVisitEncounters.isEmpty(); | ||
assert thirdVisitDiagnoses.isEmpty(); | ||
} | ||
|
||
@Test | ||
public void shouldGetVisitsByPatientIdWithPagination() throws Exception { | ||
|
||
String patientUuid = "8604d42e-3ca8-11e3-bf2b-0d0c09861e97"; | ||
String mostRecentVisitUuid = "1c72e1ac-9b18-11e0-93c3-18a905e044dc"; | ||
|
||
MvcResult response = mockMvc.perform(get("/rest/v1/emrapi/patient/" + patientUuid + "/visit") | ||
.param("startIndex", "0") | ||
.param("limit", "1") | ||
.contentType(MediaType.APPLICATION_JSON)) | ||
.andExpect(status().isOk()) | ||
.andReturn(); | ||
|
||
String jsonResponse = response.getResponse().getContentAsString(); | ||
ObjectMapper objectMapper = new ObjectMapper(); | ||
Map<String, Object> result = objectMapper.readValue(jsonResponse, Map.class); | ||
|
||
assertNotNull(result); | ||
assert result.get("totalCount").equals(1); | ||
|
||
List<Map<String, Object>> visits = (List<Map<String, Object>>) result.get("pageOfResults"); | ||
assert visits.size() == 1; | ||
|
||
Map<String, Object> recentVisit = visits.get(0); | ||
assert recentVisit.get("uuid").equals(mostRecentVisitUuid); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
<?xml version='1.0' encoding='UTF-8'?> | ||
|
||
<dataset> | ||
|
||
<!-- Encounter Types --> | ||
<encounter_type encounter_type_id="1003" name="Visit Note" description="Visit note" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d7151f82-c1f3-4152-a605-2f9ea7414a79"/> | ||
<encounter_type encounter_type_id="1004" name="Consultation" description="Consultation encounter" creator="1" date_created="2023-10-15 10:00:00.0" retired="false" uuid="d92ca5dc-7fd0-4fa2-b726-ab16fbc53fc1"/> | ||
|
||
<!-- Patient 1: Scott Clark --> | ||
<person person_id="109" gender="M" dead="false" creator="1" birthdate_estimated="0" date_created="2023-10-15 10:30:00.0" voided="false" void_reason="" uuid="8604d42e-3ca8-11e3-bf2b-0d0c09861e97"/> | ||
<person_name person_name_id="109" person_id="109" preferred="true" prefix="" given_name="Scott" middle_name="" family_name="Clark" family_name_suffix="" creator="1" date_created="2023-10-15 10:31:00.0" voided="false" void_reason="" uuid="8f9e3a7b-6482-487d-9abe-c07b123c4f08"/> | ||
<patient patient_id="109" creator="1" date_created="2023-10-15 10:32:00.0" voided="false" /> | ||
|
||
<!-- Location Information --> | ||
<location location_id="1901" name="Location tagged to visit" creator="1" date_created="2023-10-15 10:35:00.0" retired="false" uuid="f1771d8e-bf1f-4dc5-957f-9d40a5eebf08"/> | ||
|
||
<!-- First Visit for Scott Clark --> | ||
<visit visit_id="1014" patient_id="109" visit_type_id="1" date_started="2023-10-20 09:00:00.0" date_stopped="2023-10-20 09:30:00.0" location_id="1901" creator="1" date_created="2023-10-20 09:05:00.0" voided="0" uuid="1esd5218-6b78-11e0-93c3-18a905e044dc" /> | ||
|
||
<!-- First Visit - Encounters for Scott Clark (3 encounters) --> | ||
<encounter encounter_id="2001" visit_id="1014" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:10:00.0" creator="1" date_created="2023-10-20 09:12:00.0" voided="false" uuid="a123d653-444b-4118-9c83-a3715b82d4ac"/> | ||
<encounter encounter_id="2002" visit_id="1014" encounter_type="1004" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:20:00.0" creator="1" date_created="2023-10-20 09:22:00.0" voided="false" uuid="a9a41017-2593-442a-bb88-c80358692d1e"/> | ||
<encounter encounter_id="2008" visit_id="1014" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-10-20 09:25:00.0" creator="1" date_created="2023-10-20 09:27:00.0" voided="false" uuid="b2f67239-8a69-4f68-8fbe-6a248efc2b44"/> | ||
|
||
<!-- Diagnoses for First Visit --> | ||
<encounter_diagnosis diagnosis_id="1" encounter_id="2001" patient_id="109" creator="1" date_created="2023-10-20 09:15:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="47dd2d5e-5cfe-4fdd-b284-3147e60d4d25"/> | ||
<encounter_diagnosis diagnosis_id="2" encounter_id="2002" patient_id="109" creator="1" date_created="2023-10-20 09:25:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="b9dd36c2-3bf7-4a20-8ddc-88ccf75ea4f0"/> | ||
<encounter_diagnosis diagnosis_id="8" encounter_id="2008" patient_id="109" creator="1" date_created="2023-10-20 09:27:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="28c583be-d9ae-4d0f-8d88-df7fcffb2d22"/> | ||
|
||
<!-- Second Visit for Scott Clark --> | ||
<visit visit_id="1015" patient_id="109" visit_type_id="1" date_started="2023-11-01 10:00:00.0" date_stopped="2023-11-01 10:30:00.0" location_id="1901" creator="1" date_created="2023-11-01 10:05:00.0" voided="0" uuid="1c72e1ac-9b18-11e0-93c3-18a905e044dc" /> | ||
|
||
<!-- Second Visit - Encounters for Scott Clark (2 encounters) --> | ||
<encounter encounter_id="2003" visit_id="1015" encounter_type="1003" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-11-01 10:10:00.0" creator="1" date_created="2023-11-01 10:12:00.0" voided="false" uuid="b1f847e9-444c-4a8c-bba5-cc8b8dc1b123"/> | ||
<encounter encounter_id="2004" visit_id="1015" encounter_type="1004" patient_id="109" location_id="1901" form_id="1" encounter_datetime="2023-11-01 10:20:00.0" creator="1" date_created="2023-11-01 10:25:00.0" voided="false" uuid="d58e3e83-dbd4-4ba8-a61f-b4ff5c1bc1e8"/> | ||
|
||
<!-- Diagnoses for Second Visit --> | ||
<encounter_diagnosis diagnosis_id="3" encounter_id="2003" patient_id="109" creator="1" date_created="2023-11-01 10:15:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="aef38d76-ffde-47a1-8717-5c74fddc2c35"/> | ||
<encounter_diagnosis diagnosis_id="4" encounter_id="2004" patient_id="109" creator="1" date_created="2023-11-01 10:25:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="b3241d9c-5d78-4d9c-bef6-0c264dfa3892"/> | ||
|
||
<!-- Third Visit for Scott Clark --> | ||
<visit visit_id="1017" patient_id="109" visit_type_id="1" date_started="2023-12-05 14:00:00.0" date_stopped="2023-12-05 14:30:00.0" location_id="1901" creator="1" date_created="2023-12-05 14:05:00.0" voided="0" uuid="3c72f2bc-9b18-11e0-93c3-18a905e044ec" /> | ||
|
||
<!-- Patient 2: John Doe --> | ||
<person person_id="110" gender="M" dead="false" creator="1" birthdate_estimated="0" date_created="2023-10-20 09:50:00.0" voided="false" void_reason="" uuid="c672b5ee-452d-4c97-b2d2-8b5ea84c8d0b"/> | ||
<person_name person_name_id="110" person_id="110" preferred="true" prefix="" given_name="John" middle_name="" family_name="Doe" family_name_suffix="" creator="1" date_created="2023-10-20 09:51:00.0" voided="false" void_reason="" uuid="5dc9b93f-4ebf-489e-98c6-fc52b362d12b"/> | ||
<patient patient_id="110" creator="1" date_created="2023-10-20 09:52:00.0" voided="false" /> | ||
|
||
<!-- Visit Information for John Doe --> | ||
<visit visit_id="1016" patient_id="110" visit_type_id="1" date_started="2023-10-25 08:30:00.0" date_stopped="2023-10-25 09:00:00.0" location_id="1901" creator="1" date_created="2023-10-25 08:35:00.0" voided="0" uuid="2a1c9274-791b-41d3-82f9-65482d22df65" /> | ||
|
||
<!-- Encounters for John Doe (3 encounters) --> | ||
<encounter encounter_id="2005" visit_id="1016" encounter_type="1003" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-10-25 08:40:00.0" creator="1" date_created="2023-10-25 08:42:00.0" voided="false" uuid="d14f3c6a-4cf6-48d4-97be-97f5f73d6c16"/> | ||
<encounter encounter_id="2006" visit_id="1016" encounter_type="1004" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-10-25 08:45:00.0" creator="1" date_created="2023-10-25 08:47:00.0" voided="false" uuid="e354f65a-8b36-4eaf-9a54-1342f84faabc"/> | ||
<encounter encounter_id="2007" visit_id="1016" encounter_type="1003" patient_id="110" location_id="1901" form_id="1" encounter_datetime="2023-11-05 09:00:00.0" creator="1" date_created="2023-11-05 09:05:00.0" voided="false" uuid="f6f8c39b-76f9-4879-8712-4cd178d2c2dc"/> | ||
|
||
<!-- Diagnoses for John Doe --> | ||
<encounter_diagnosis diagnosis_id="5" encounter_id="2005" patient_id="110" creator="1" date_created="2023-10-25 08:42:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="c0a9f874-2b47-4f58-93fc-d8a0d233b77f"/> | ||
<encounter_diagnosis diagnosis_id="6" encounter_id="2006" patient_id="110" creator="1" date_created="2023-10-25 08:47:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="0b75c6d4-45a5-477a-bf7a-f5a0476a47d8"/> | ||
<encounter_diagnosis diagnosis_id="7" encounter_id="2007" patient_id="110" creator="1" date_created="2023-11-05 09:05:00.0" rank="1" certainty="CONFIRMED" voided="false" uuid="33a9f72c-d911-42b9-950f-4f1a6201a20a"/> | ||
|
||
</dataset> |
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 have never seen this annotation actually used in this module before. Are we sure this works? I'd prefer we stick with existing conventions within the module for wiring things up, and then do a change to everything if we want to update to a new mechanism.