FM2-649: Implementation of _has parameter for patients #545
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #545 +/- ##
============================================
+ Coverage 77.84% 77.91% +0.06%
- Complexity 2683 2719 +36
============================================
Files 239 239
Lines 7452 7537 +85
Branches 901 922 +21
============================================
+ Hits 5801 5872 +71
Misses 1115 1115
- Partials 536 550 +14 ☔ View full report in Codecov by Sentry. |
|
@denniskigen @ibacher would it be possible for you to review this PR ? Thank you cc @icrc-jofrancisco @icrc-fdeniger @gracepotma |
There was a problem hiding this comment.
Couple of small things. Also, changes like this require:
- A WebTest showing that the parameters passed to REST are processed correctly
- An integration test showing the whole thing works
In addition to the above, we should have a data-level test where the cohort has more than one member (ideally something like 4).
| private List<Integer> getGroupMemberIds(String groupId) { | ||
| Cohort cohort = groupDao.get(groupId); | ||
| if (cohort != null) { | ||
| return cohort.getMemberships().stream().map(CohortMembership::getPatientId).collect(Collectors.toList()); | ||
| } else { | ||
| return Collections.emptyList(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we reformulate this as a subquery?
| switch (hasParam.getTargetResourceType()) { | ||
| case FhirConstants.GROUP: | ||
| switch (hasParam.getReferenceFieldName()) { | ||
| case FhirConstants.INCLUDE_MEMBER_PARAM: |
There was a problem hiding this comment.
Shouldn't we also have some sort of check for the :id part mentioned in the PR comments?
icrc-jofrancisco
changed the title
Description of what I changed
This feature extends the functionality of the
_hasparameter to support chained searches.Users can now perform complex queries to retrieve resources based on specific relationships.
For now, only support for the groups/members relationship has been added. In the future, new resource types may be supported.
For example, the following query can be used to search for Patients resources who are members of a specific group
GET /openmrs/ws/fhir2/R4/Patient?_has:Group:member:id=<GROUP_ID>several groups can be combined, not returning duplicate records:
GET /openmrs/ws/fhir2/R4/Patient?_has:Group:member:id=<GROUP_ID_1>,<GROUP_ID_2>Example:
GroupA
GroupB
GroupC
GET /openmrs/ws/fhir2/R4/Patient?_has:Group:member:id=<GROUP_A>_returns Patient1, Patient2
GET /openmrs/ws/fhir2/R4/Patient?_has:Group:member:id=<GROUP_A>,<GROUP_B>returns Patient1, Patient2, Patient3
GET /openmrs/ws/fhir2/R4/Patient?_has:Group:member:id=<GROUP_A>,<GROUP_C>returns Patient1, Patient2, Patient3
HL7 FHIR documentation: section 3.1.1.4.17 Reverse Chaining
Issue I worked on
N/A
Checklist: I completed these to help reviewers :)
My IDE is configured to follow the code style of this project.
No? Unsure? -> configure your IDE, format the code and add the changes with
git add . && git commit --amendI have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
No? -> write tests and add them to this commit
git add . && git commit --amendI ran
mvn clean packageright before creating this pull request andadded all formatting changes to my commit.
No? -> execute above command
All new and existing tests passed.
No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.
No? Unsure? -> execute command
git pull --rebase upstream masterThanks,
CC: @ibacher @denniskigen @icrc-psousa