As part of openmrs-core 2.1.0, TRUNK-4906 enhances the core Cohort domain object so that instead of just being a Set of patientIds, it’s now a set of {patient, startDate, endDate}.
I want to update the REST module to reflect these changes, so help me figure out if I’m doing it right…
Also, do we know if anyone is using this API? (e.g. for android client, muzima, etc?)
Executive Summary
We have to make some breaking changes to the cohort resource, so let’s go further and redesign it more correctly:
- Get rid of the cohort.memberIds property
- when you create a cohort, you can set all its members by posting a “memberships” list of {patientUuid, startDate, endDate}s
- fetching a cohort won’t have this property
- replace the
cohort/(uuid)/member/(patientUuid)subresource with acohort/(uuid)/membership/(membershipUuid)subresource - its uuid will be the uuid of the membership, not the patient
- it should have startDate, endDate, and patientUuid properties
- it should give you a proper hyperlink to follow to get the patient’s details
- I need help deciding on what should happen when you DELETE a membership. (Should it void the membership, or just end it?)
Details
Currently I can create a cohort like this:
POST https://demo.openmrs.org/openmrs/ws/rest/v1/cohort
{"name":"Darius testing", "description": "REST", "memberIds": [515]}
Technically we could preserve this behavior. But if we’re going to break other things we should make it support UUIDs instead of integers, since REST isn’t supposed to know about patientIds.) So it would be better to support “memberships” that’s a list of patientUuid, startDate, and endDate.
The default representation of a cohort (and also what’s returned when you create one) contains a memberIds property, and I think we should remove this going forwards. It never fit well with the REST API anyway, since we’re not supposed to be exposing patientIds. Alternately we could preserve this property and have it return the current list of members.
Question: remove the memberIds property or preserve it?
Currently I can add a member to a cohort like this:
POST https://demo.openmrs.org/openmrs/ws/rest/v1/cohort/ca254345-429b-491c-b833-1933cdd7d828/member
{"patient":"c3bf8a00-52e7-4998-b18d-b1978adab4fa"}
We should extend this to support startDate and endDate fields also, but otherwise this behavior shouldn’t change. (The underlying openmrs-core API should do whatever validation about avoiding overlapping memberships.)
However the REST representation of cohort/member is currently exactly the REST rep of a patient (whether you get one, get all, or as the response to adding a member).
First, we need to change this to be an object with startDate, endDate, and patient properties; that will be a non-backwards-compatible change for the resource.
Second, it used to be that the sub-resource uuid was the patient’s uuid, e.g. the URI is like .../cohort/uuid-of-cohort/member/uuid-of-patient, but since now the same patient may be in a cohort more than once with different dates, and since CohortMembership in core now has its own UUID, we need to use that instead.
Third, a cohort is really supposed to just have references to its members, since the cohort may be large and fetching all the member details may be expensive. I propose that as long we’re breaking the REST API for this, we also correct things so that the cohort/member subresource has a patientUuid property (easy to use) and a link to the patient (proper hyperlink):
{
"startDate": "2017-01-01",
"endDate": null,
"patientUuid": "uuid-of-the-patient",
"links": [
{
"rel": "patient",
"uri": ".../patient/uuid-of-the-patient"
},
{
"rel": "self",
"uri": ".../cohort/uuid-of-the-cohort/member/uuid-of-the-membership"
}
]
}
Also, I propose we rename this subresource from cohort/member to cohort/membership, which conveys the meaning more correctly, and aligns with the new openmrs-core domain object CohortMembership.
Basically we have to make several significant backwards-incompatible changes to the cohort resource, and as long as we’re doing that I propose we just rewrite it to behave better.
The one thing I’m not sure about is what should happen when you do DELETE .../cohort/(uuid)/membership…
In the underlying Java API, Cohort.removeMembership() will set the membership’s endDate to be now(). This is my default thought for what the REST API should do when you send a DELETE. But if we do that there’s no way to void a cohort membership. Thoughts?
Default is to do:
- DELETE membership => set the membership’s endDate to now(), like Cohort.removeMembership()
- DELETE membership?purge=true => hard-delete the membership
- no way to void
The alternative would be for DELETE to void, and if you want to end the membership now you do a POST ... {endDate: "..."}.
As far as your last question, my opinion is that delete should void/purge. Setting an end date on a membership should be a post. The changes you suggest in terms of UUIDs and references to patient resources make sense to me.