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

EA-198: Add support for configuring Mother Child relationships within… #237

Merged
merged 8 commits into from
Aug 8, 2024
Merged

EA-198: Add support for configuring Mother Child relationships within… #237

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a1367c7
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 5, 2024
c84a9ae
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 5, 2024
76327e3
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 5, 2024
26e9ff6
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 6, 2024
c4e92b7
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 6, 2024
00b2de2
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 7, 2024
fd9470d
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 7, 2024
77c0105
EA-198: Add support for configuring Mother Child relationships within…
mogoodrich Aug 8, 2024
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
2 changes: 2 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/EmrApiConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,6 @@ public class EmrApiConstants {
));

public static final String GP_USE_LEGACY_DIAGNOSIS_SERVICE = "emrapi.useLegacyDiagnosisService";

public static final String METADATA_MAPPING_MOTHER_CHILD_RELATIONSHIP_TYPE = "emrapi.motherChildRelationshipType";
mseaton marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 5 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/EmrApiProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.openmrs.PatientIdentifierType;
import org.openmrs.PersonAttributeType;
import org.openmrs.Provider;
import org.openmrs.RelationshipType;
import org.openmrs.Role;
import org.openmrs.VisitType;
import org.openmrs.module.emrapi.diagnosis.DiagnosisMetadata;
Expand Down Expand Up @@ -347,4 +348,8 @@ public List<Disposition> getDispositions() {
public DispositionDescriptor getDispositionDescriptor() {
return dispositionService.getDispositionDescriptor();
}

public RelationshipType getMotherChildRelationshipType() {
return getEmrApiMetadataByCode(RelationshipType.class, EmrApiConstants.METADATA_MAPPING_MOTHER_CHILD_RELATIONSHIP_TYPE, false);
}
}
12 changes: 12 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/Child.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.openmrs.module.emrapi.maternal;

import lombok.Data;
import org.openmrs.Patient;
import org.openmrs.module.emrapi.adt.InpatientAdmission;

@Data
public class Child {
private Patient child;
private Patient mother;
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts about making this to be private Mother mother;?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep the REST response as simple as possible. You mean have:

public class Mother {
    private Patient mother;
    private Child child;
    private InpatientAdmission motherAdmission;
}


public class Child {
    private Patient child;
    private Mother mother;
    private InpatientAdmission childAdmission;
}

Then on a child you've have to child.mother.mother to get the mother?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I think given what you have in place it would make sense to just have one object:

public class MotherAndChild {
    private Patient mother;
    private Child child;
    private InpatientAdmission motherAdmission;
    private InpatientAdmission childAdmission;
}

And then one service method to getMothersAndChildren(MothersAndChildrenSearchCriteria) that returns a List of these.

That list would contain all children of interest if you pass it a set of child uuids, and would contain all mothers of interest if you pass it was set of mother uuids, and everyone of interest if you just pass it criteria about the visit constraints expected...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking of that, though I thought there was a reason it wasn't going to work, will check again.

private InpatientAdmission childAdmission;
}
27 changes: 27 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/ChildSearchCriteria.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package org.openmrs.module.emrapi.maternal;

import java.util.List;

import lombok.Data;
import org.openmrs.Patient;

@Data
public class ChildSearchCriteria {
private List<String> motherUuids;
private Boolean motherHasActiveVisit = false;
private Boolean childHasActiveVisit = false;
private Boolean childBornDuringMothersActiveVisit = false;
Copy link
Member

Choose a reason for hiding this comment

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

The datatype and naming of these variables imply that you can have null, true, and false values, and that true and false both restrict, but in opposite ways. If you do it this way, I'd expect these to be null by default, and null would mean that there is no restriction, and true and false would both lead to filtering of results. eg. motherHasActiveVisi = true would do what I'd expect, but motherHasActiveVisit = false would enforce that the mother does not have an active visit, rather than ignoring the condition.

The alternative would be to make these boolean rather than Boolean, and name them something more clearly to indicate that false means no filtering, and true activates the filter. Something like requireMotherToHaveActiveVisit, requireChildToHaveActiveVisit, requireChildBornDuringMothersActiveVisit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't thing we need to support searching for patients that don't have active visits, but your commentary about the naming being confusion is fair, will fix.


public ChildSearchCriteria(List<String> motherUuids) {
this.motherUuids = motherUuids;
}

Copy link
Member

Choose a reason for hiding this comment

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

I believe there is a lombok annotation to add a full-argument constructor that you could use instead, if you really need this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Neat, done.

public ChildSearchCriteria(List<String> motherUuids, Boolean motherHasActiveVisit, Boolean childHasActiveVisit, Boolean childBornDuringMothersActiveVisit) {
this.motherUuids = motherUuids;
this.motherHasActiveVisit = motherHasActiveVisit;
this.childHasActiveVisit = childHasActiveVisit;
this.childBornDuringMothersActiveVisit = childBornDuringMothersActiveVisit;
}
}


14 changes: 14 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/MaternalService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.openmrs.module.emrapi.maternal;

import java.util.List;

import org.openmrs.Patient;
import org.openmrs.api.OpenmrsService;

public interface MaternalService extends OpenmrsService {
mseaton marked this conversation as resolved.
Show resolved Hide resolved

// TODO update javadoc
public List<Child> getChildrenByMother(ChildSearchCriteria criteria);
Copy link
Member

Choose a reason for hiding this comment

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

This should be renamed getChildren I would think. There is nothing in the method signature any longer that implies that you must search by mother. I know the implementation of this currently requires setting mother uuids, but this may not always be the case, so I don't think we should name it like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I changed this to getChildren, but (almost) back to the original, making the mothers plural: "getChildrenByMothers". I did remove making setting mother uuids mandatroy, but I think the name should stay, reflecting the case that this returns child-mother pairs, (and, in particular, would return two rows for single child if they had two or mother mothers... admittedly, not a standard case, but certainly a standard case for the inverse, getMothersByChild).


public List<Mother> getMothersByChild(MotherSearchCriteria criteria);
Copy link
Member

Choose a reason for hiding this comment

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

This should be renamed getMothers. Same comments as above.

Copy link
Member Author

Choose a reason for hiding this comment

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

See above (and I did remove making setting child uuids mandatory)

}
134 changes: 134 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/MaternalServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package org.openmrs.module.emrapi.maternal;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.openmrs.Patient;
import org.openmrs.RelationshipType;
import org.openmrs.Visit;
import org.openmrs.api.APIException;
import org.openmrs.api.impl.BaseOpenmrsService;
import org.openmrs.module.emrapi.EmrApiProperties;
import org.openmrs.module.emrapi.adt.AdtService;
import org.openmrs.module.emrapi.adt.InpatientAdmission;
import org.openmrs.module.emrapi.adt.InpatientAdmissionSearchCriteria;
import org.openmrs.module.emrapi.db.EmrApiDAO;

public class MaternalServiceImpl extends BaseOpenmrsService implements MaternalService {

private EmrApiProperties emrApiProperties;

private AdtService adtService;

private EmrApiDAO emrApiDAO;

public void setEmrApiProperties(EmrApiProperties emrApiProperties) {
this.emrApiProperties = emrApiProperties;
}

public void setEmrApiDAO(EmrApiDAO emrApiDAO) {
this.emrApiDAO = emrApiDAO;
}

public void setAdtService(AdtService adtService) {
this.adtService = adtService;
}

public List<Child> getChildrenByMother(ChildSearchCriteria criteria) {

RelationshipType motherChildRelationshipType = emrApiProperties.getMotherChildRelationshipType();

if (motherChildRelationshipType == null) {
throw new APIException("Mother-Child relationship type has not been configured");
}

if (criteria.getMotherUuids() == null || criteria.getMotherUuids().isEmpty()) {
throw new APIException("No mothers provided");
}
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be required? I think it likely does to perform and not crash the system if none of the other properties are set. If some of the other properties are set, it might not really be strictly required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, looks possible remove this restriction, will do.


Map<String, Object> parameters = new HashMap<>();
parameters.put("motherUuids", criteria.getMotherUuids());
parameters.put("motherChildRelationshipType", motherChildRelationshipType);
parameters.put("motherHasActiveVisit", criteria.getMotherHasActiveVisit());
parameters.put("childHasActiveVisit", criteria.getChildHasActiveVisit());
parameters.put("childBornDuringMothersActiveVisit", criteria.getChildBornDuringMothersActiveVisit());

List<?> l = emrApiDAO.executeHqlFromResource("hql/children_by_mother.hql", parameters, List.class);

List<Child> ret = new ArrayList<>();

for (Object req : l) {
Object[] row = (Object[]) req;
Child child = new Child();
child.setChild((Patient) row[0]);
child.setMother((Patient) row[1]);
ret.add(child);
}

// now fetch all the admissions for children in the result set
InpatientAdmissionSearchCriteria inpatientAdmissionSearchCriteria = new InpatientAdmissionSearchCriteria();
inpatientAdmissionSearchCriteria.setPatientIds(new ArrayList<>(ret.stream().map(Child::getChild).map(Patient::getId).collect(Collectors.toSet())));
List<InpatientAdmission> admissions = adtService.getInpatientAdmissions(inpatientAdmissionSearchCriteria);
Map<Patient, InpatientAdmission> admissionsByPatient = new HashMap<>();
if (admissions != null) {
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 believe this can ever be null, so you should be able to remove this null check.

for (InpatientAdmission admission : admissions) {
admissionsByPatient.put(admission.getVisit().getPatient(), admission);
}
}
for (Child child : ret) {
child.setChildAdmission(admissionsByPatient.get(child.getChild()));
}

return ret;
}

public List<Mother> getMothersByChild(MotherSearchCriteria criteria) {
RelationshipType motherChildRelationshipType = emrApiProperties.getMotherChildRelationshipType();

if (motherChildRelationshipType == null) {
throw new APIException("Mother-Child relationship type has not been configured");
}

if (criteria.getChildUuids() == null || criteria.getChildUuids().isEmpty()) {
throw new APIException("No children provided");
Copy link
Member

Choose a reason for hiding this comment

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

Same comment

}

Map<String, Object> parameters = new HashMap<>();
parameters.put("childUuids", criteria.getChildUuids());
parameters.put("motherChildRelationshipType", motherChildRelationshipType);
parameters.put("motherHasActiveVisit", criteria.getMotherHasActiveVisit());
parameters.put("childHasActiveVisit", criteria.getChildHasActiveVisit());

List<?> l = emrApiDAO.executeHqlFromResource("hql/mothers_by_child.hql", parameters, List.class);

List<Mother> ret = new ArrayList<>();

for (Object req : l) {
Object[] row = (Object[]) req;
Mother mother = new Mother();
mother.setMother((Patient) row[0]);
mother.setChild((Patient) row[1]);
ret.add(mother);
}

// now fetch all the admissions for mothers in the result set
InpatientAdmissionSearchCriteria inpatientAdmissionSearchCriteria = new InpatientAdmissionSearchCriteria();
inpatientAdmissionSearchCriteria.setPatientIds(new ArrayList<>(ret.stream().map(Mother::getMother).map(Patient::getId).collect(Collectors.toSet())));
List<InpatientAdmission> admissions = adtService.getInpatientAdmissions(inpatientAdmissionSearchCriteria);
Map<Patient, InpatientAdmission> admissionsByPatient = new HashMap<>();
if (admissions != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto on above.

for (InpatientAdmission admission : admissions) {
admissionsByPatient.put(admission.getVisit().getPatient(), admission);
}
}
for (Mother mother : ret) {
mother.setMotherAdmission(admissionsByPatient.get(mother.getMother()));
}


return ret;
}
}
13 changes: 13 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/Mother.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.openmrs.module.emrapi.maternal;

import lombok.Data;
import org.openmrs.Patient;
import org.openmrs.module.emrapi.adt.InpatientAdmission;


@Data
public class Mother {
private Patient mother;
private Patient child;
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts about making this private Child child;?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

Copy link
Member

Choose a reason for hiding this comment

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

See above again.

private InpatientAdmission motherAdmission;
mseaton marked this conversation as resolved.
Show resolved Hide resolved
}
22 changes: 22 additions & 0 deletions api/src/main/java/org/openmrs/module/emrapi/maternal/MotherSearchCriteria.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.openmrs.module.emrapi.maternal;

import java.util.List;

import lombok.Data;

@Data
Copy link
Member

Choose a reason for hiding this comment

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

Same comments apply here that I made in the ChildSearchCriteria

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

public class MotherSearchCriteria {
private List<String> childUuids;
private Boolean motherHasActiveVisit = false;
private Boolean childHasActiveVisit = false;

public MotherSearchCriteria(List<String> childUuids) {
this.childUuids = childUuids;
}

public MotherSearchCriteria(List<String> childUuids, Boolean motherHasActiveVisit, Boolean childHasActiveVisit) {
this.childUuids = childUuids;
this.motherHasActiveVisit = motherHasActiveVisit;
this.childHasActiveVisit = childHasActiveVisit;
}
}
20 changes: 20 additions & 0 deletions api/src/main/resources/hql/children_by_mother.hql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
select
child,
mother
from
Relationship as motherChildRelationship
inner join motherChildRelationship.personA as mother
inner join motherChildRelationship.personB as child
where
mother.uuid in (:motherUuids)
and motherChildRelationship.relationshipType = :motherChildRelationshipType
and (:motherHasActiveVisit = false or (select count(motherVisit) from Visit as motherVisit where motherVisit.patient = mother and motherVisit.stopDatetime is null and motherVisit.voided = false) > 0)
and (:childHasActiveVisit = false or (select count(childVisit) from Visit as childVisit where childVisit.patient = child and childVisit.stopDatetime is null and childVisit.voided = false) > 0)
and (:childBornDuringMothersActiveVisit = false or (select count(motherVisit) from Visit as motherVisit where motherVisit.patient = mother and motherVisit.stopDatetime is null and motherVisit.voided = false
and year(child.birthdate) >= year(motherVisit.startDatetime)
and month(child.birthdate) >= month(motherVisit.startDatetime)
and day(child.birthdate) >= day(motherVisit.startDatetime)) > 0)
and mother.voided = false and child.voided = false and motherChildRelationship.voided = false



16 changes: 16 additions & 0 deletions api/src/main/resources/hql/mothers_by_child.hql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
select
mother,
child
from
Relationship as motherChildRelationship
inner join motherChildRelationship.personA as mother
inner join motherChildRelationship.personB as child
where
child.uuid in (:childUuids)
and motherChildRelationship.relationshipType = :motherChildRelationshipType
and (:motherHasActiveVisit = false or (select count(motherVisit) from Visit as motherVisit where motherVisit.patient = mother and motherVisit.stopDatetime is null and motherVisit.voided = false) > 0)
and (:childHasActiveVisit = false or (select count(childVisit) from Visit as childVisit where childVisit.patient = child and childVisit.stopDatetime is null and childVisit.voided = false) > 0)
and mother.voided = false and child.voided = false and motherChildRelationship.voided = false


Copy link
Member

Choose a reason for hiding this comment

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

There is so much similarity between the two queries. Can we accomplish this with only one query that we have to maintain which is used to both getMothers and getChildren? I could see both supporting either motherUuids and/or childUuids, for example. And I could see both supporting childBornDuringMothersActiveVisit. Would seem to have value in consolidating.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. And now both support childBornDuringMothersActiveVisit


28 changes: 28 additions & 0 deletions api/src/main/resources/moduleApplicationContext.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,34 @@
</property>
</bean>

<bean id="maternalService" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
<property name="transactionManager">
<ref bean="transactionManager"/>
</property>
<property name="target">
<bean class="org.openmrs.module.emrapi.maternal.MaternalServiceImpl">
<property name="adtService" ref="adtService"/>
<property name="emrApiProperties" ref="emrApiProperties"/>
<property name="emrApiDAO" ref="emrApiDAOImpl"/>
</bean>
</property>
<property name="preInterceptors">
<ref bean="serviceInterceptors"/>
</property>
<property name="transactionAttributeSource">
<ref bean="transactionAttributeSource"/>
</property>
</bean>

<bean parent="serviceContext">
<property name="moduleService">
<list merge="true">
<value>org.openmrs.module.emrapi.maternal.MaternalService</value>
<ref bean="maternalService"/>
</list>
</property>
</bean>

<bean id="exitFromCareService" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
<property name="transactionManager">
<ref bean="transactionManager"/>
Expand Down
Loading
Loading