COH-54: Update cohort member endDate for POST requests #30
Conversation
| public CohortMember save(CohortMember cohortMember) throws ResponseException { | ||
| CohortM cohort = cohortMember.getCohort(); | ||
| Patient newPatient = cohortMember.getPatient(); | ||
| public CohortMember save(CohortMember newMember) throws ResponseException { |
There was a problem hiding this comment.
Will this parameter always be a new member? Don't we have cases where this method is called for existing members?
There was a problem hiding this comment.
Yes good point, calling it newMember could be confusing. I was trying to distinguish it from the member variable we get from the cohort, so I could change it to "newOrUpdatedMember" or do you have another preference?
There was a problem hiding this comment.
How about leaving the name as it was before your changes?
There was a problem hiding this comment.
How about leaving the name as it was before your changes?
Done - reverted the name change
| } | ||
| currentMember.setEndDate(cohortMember.getEndDate()); | ||
| currentMember.setVoided(false); | ||
| cohortMember = currentMember; |
There was a problem hiding this comment.
Shouldn't we break out of the loop?
There was a problem hiding this comment.
Yes makes sense. Updated to add break.
|
It would be awesome if you can also add a |
Added these tests |
Do you mind adding at least one test for a POST which updates the cohort member endDate? |
Yes this test I added updates the endDate from a null endDate to a non-null endDate - https://github.com/openmrs/openmrs-module-cohort/pull/30/files#diff-934798dffa2af2f1b6a9e9e66060d932389e5c4d3d0cc5c6d63d5ddfdb7d8168R83 Unless you mean to update two different non-null endDates? |
I mean the equivalent of this below: Where the only value in the post data is the endDate. |
Added the additional test to update only the endDate for the member with its UUID in the url |
|
Do you mind looking into this build failure? |
Seems my test was senstive to timezones, I made a change that will hopefully fix it if you can approve the pipeline please. If it fails will investigate further. |
|
Jumping to the ticket, by the reviewer, is always easier if you include the ticket url as advised here: https://wiki.openmrs.org/display/docs/Pull+Request+Tips |
Added ticket url to description |
Issue
Fixes the bug where endDate is ignored during POST requests to save cohort members.
Ticket: https://issues.openmrs.org/browse/COH-54
Changes
Allows for endDate to be updated correctly during POST requests.
Enables reinstatement of cohort membership if an existing cohort member with a non-null endDate is updated to have a null endDate.
Impact
These changes ensure that the endDate is handled correctly, improving the accuracy of cohort memberships.
Testing
Added tests for the following scenarios:
Below is a manual POST test to show the endDate in the result being updated:
{ "patient": { "uuid": "a4508cb5-89ff-41b2-9a0c-a65ab17fcbf0", "display": "", "links": [ { "rel": "self", "uri": "http://localhost:8080/openmrs/ws/rest/v1/patient/a4508cb5-89ff-41b2-9a0c-a65ab17fcbf0", "resourceAlias": "patient" } ] }, "startDate": "2023-08-22T01:00:00.000+0100", "endDate": "2023-12-31T00:00:00.000+0000", "uuid": "2d50fa4d-1a44-4c2a-92d2-f20a2182b620", "voided": false, "attributes": [], "cohort": { "uuid": "4eece76a-111e-40cb-be1c-e717801876f6", "display": "My Sample Cohort", "links": [ { "rel": "self", "uri": "http://localhost:8080/openmrs/ws/rest/v1/cohortm/cohort/4eece76a-111e-40cb-be1c-e717801876f6", "resourceAlias": "cohort" } ] }, "links": [ { "rel": "self", "uri": "http://localhost:8080/openmrs/ws/rest/v1/cohortm/cohortmember/2d50fa4d-1a44-4c2a-92d2-f20a2182b620", "resourceAlias": "cohortmember" }, { "rel": "full", "uri": "http://localhost:8080/openmrs/ws/rest/v1/cohortm/cohortmember/2d50fa4d-1a44-4c2a-92d2-f20a2182b620?v=full", "resourceAlias": "cohortmember" } ], "resourceVersion": "1.8" }