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 (and fetching) Mother Child relat… #239

Merged
merged 2 commits into from
Aug 12, 2024
Merged

EA-198: Add support for configuring (and fetching) Mother Child relat… #239

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
2e23aea
EA-198: Add support for configuring (and fetching) Mother Child relat…
mogoodrich Aug 9, 2024
7656c7c
EA-198: Add support for configuring (and fetching) Mother Child relat…
mogoodrich Aug 12, 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: 1 addition & 1 deletion 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 @@ -216,5 +216,5 @@ 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";
public static final String METADATA_MAPPING_MOTHER_CHILD_RELATIONSHIP_TYPE = "emr.motherChildRelationshipType";
Copy link
Member Author

Choose a reason for hiding this comment

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

All the other EMR API properties started with "emr", so I renamed this for consistency.

}
2 changes: 2 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
Expand Up @@ -47,7 +47,9 @@ public List<MotherAndChild> getMothersAndChildren(MothersAndChildrenSearchCriter

Map<String, Object> parameters = new HashMap<>();
parameters.put("motherUuids", criteria.getMotherUuids());
parameters.put("restrictByMothers", criteria.getMotherUuids() != null);
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 below

parameters.put("childUuids", criteria.getChildUuids());
parameters.put("restrictByChildren", criteria.getChildUuids() != null);
parameters.put("motherChildRelationshipType", motherChildRelationshipType);
parameters.put("motherRequiredToHaveActiveVisit", criteria.isMotherRequiredToHaveActiveVisit());
parameters.put("childRequiredToHaveActiveVisit", criteria.isChildRequiredToHaveActiveVisit());
Expand Down
4 changes: 2 additions & 2 deletions api/src/main/resources/hql/mother_child.hql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ from
inner join motherChildRelationship.personA as mother
inner join motherChildRelationship.personB as child
where
((:motherUuids) is null or mother.uuid in (:motherUuids))
and ((:childUuids) is null or child.uuid in (:childUuids))
(:restrictByMothers = false or mother.uuid in (:motherUuids))
and (:restrictByChildren = false or child.uuid in (:childUuids))
Copy link
Member Author

Choose a reason for hiding this comment

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

Was stumped by this for a while... the "(:motherUuids) is null" was throwing an error when motherUuids was an array, which in-and-of-itself isn't necessarily surprising, but it was working in the tests, it was only failing when trying to use it for "real". No idea what the difference was. I figured I was inadvertently passing in a slightly different datatype, but debugging they looked the same.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I ran into the same thing. I assume you saw how I ended up handling this in the other hql queries, but if not we arrived at the same solution :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I guess I missed that

and motherChildRelationship.relationshipType = :motherChildRelationshipType
and (:motherRequiredToHaveActiveVisit = false or (select count(motherVisit) from Visit as motherVisit where motherVisit.patient = mother and motherVisit.stopDatetime is null and motherVisit.voided = false) > 0)
and (:childRequiredToHaveActiveVisit = false or (select count(childVisit) from Visit as childVisit where childVisit.patient = child and childVisit.stopDatetime is null and childVisit.voided = false) > 0)
Expand Down
2 changes: 1 addition & 1 deletion api/src/test/resources/baseTestDataset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
<relationship_type relationship_type_id="1001" a_is_to_b="Mother" b_is_to_a="Child" preferred="0" weight="0" description="mother and child" creator="1" date_created="2008-08-15 15:55:18.0" retired="false" uuid="39f1bd38-2056-434c-874a-368b13c7265f"/>
<metadatamapping_metadata_term_mapping metadata_term_mapping_id = "4001" metadata_source_id = "2134"
creator = "1" date_created="2008-08-18 12:38:58.0" uuid="7126c900-85dc-4756-98a2-1f0595824600"
retired = "false" code = "emrapi.motherChildRelationshipType" name = "emr.motherChildRelationshipType"
retired = "false" code = "emr.motherChildRelationshipType" name = "emr.motherChildRelationshipType"
metadata_uuid = "39f1bd38-2056-434c-874a-368b13c7265f" metadata_class = "org.openmrs.RelationshipType"/>

</dataset>
54 changes: 54 additions & 0 deletions .../src/main/java/org/openmrs/module/emrapi/web/controller/MothersAndChildrenController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.openmrs.module.emrapi.web.controller;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import java.util.Arrays;
import java.util.List;

import org.apache.commons.lang.StringUtils;
import org.openmrs.module.emrapi.maternal.MaternalService;
import org.openmrs.module.emrapi.maternal.MotherAndChild;
import org.openmrs.module.emrapi.maternal.MothersAndChildrenSearchCriteria;
import org.openmrs.module.emrapi.rest.converter.SimpleBeanConverter;
import org.openmrs.module.webservices.rest.SimpleObject;
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.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;

@Controller
public class MothersAndChildrenController {

@Autowired
private MaternalService maternalService;

@RequestMapping(method = RequestMethod.GET, value = "/rest/**/emrapi/maternal/mothersAndChildren")
@ResponseBody
public SimpleObject getMothersAndChildren(
HttpServletRequest request,
HttpServletResponse response,
@RequestParam(required = false, value = "motherUuids") String motherUuids,
@RequestParam(required = false, value = "childUuids") String childUuids,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call these properties mothers and children? Or limitToMothers and limitToChildren?

Copy link
Member Author

Choose a reason for hiding this comment

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

If best practice is not to use the comments, I would change this probably to "mother" and "child" like to specified below, let me fiddle around with this.

@RequestParam(required = false, value = "requireMotherHasActiveVisit") boolean requireMotherHasActiveVisit,
@RequestParam(required = false, value = "requireChildHasActiveVisit") boolean requireChildHasActiveVisit,
@RequestParam(required = false, value = "requireChildBornDuringMothersActiveVisit") boolean requireChildBornDuringMothersActiveVisit
) {
RequestContext context = RestUtil.getRequestContext(request, response, Representation.DEFAULT);
MothersAndChildrenSearchCriteria criteria = new MothersAndChildrenSearchCriteria();
criteria.setMotherUuids(StringUtils.isNotBlank(motherUuids) ? Arrays.asList(motherUuids.split(",")) : null);
criteria.setChildUuids(StringUtils.isNotBlank(childUuids) ? Arrays.asList(childUuids.split(",")) : null);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if there is a built-in way to map a parameter to an array, but the few ways I tried didn't seem to work.

Copy link
Member

Choose a reason for hiding this comment

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

If it isn't supported out of the box, I'd assume it isn't considered best practice. The way to accomplish this is typically to just pass in the request parameter multiple times. This is one of the reasons I was thinking the singular tense maybe: mother=xxx&mother=yyy&mother=zzz. I agree that mothers=xxx,yyy,zzz is more concise, I just don't know if it is considered typical practice...

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, makes sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to use:

            @RequestParam(required = false, value = "mother") List<String> motherUuids,
            @RequestParam(required = false, value = "child") List<String> childUuids,

criteria.setMotherRequiredToHaveActiveVisit(requireMotherHasActiveVisit);
criteria.setChildRequiredToHaveActiveVisit(requireChildHasActiveVisit);
criteria.setChildRequiredToBeBornDuringMothersActiveVisit(requireChildBornDuringMothersActiveVisit);
List<MotherAndChild> motherAndChildList = maternalService.getMothersAndChildren(criteria);
return new NeedsPaging<>(motherAndChildList, context).toSimpleObject(new SimpleBeanConverter<MotherAndChild>());
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 started to create a custom converter, but at the end-of-the-day the buildt in converter seemed to give reps with reasonable defaults. I tested full, default, rep, and a custom rep.

}

}
Loading