RA-950: Add LastVisit and EditVisit require an EndDate#275
RA-950: Add LastVisit and EditVisit require an EndDate#275reagan-meant wants to merge 1 commit intoopenmrs:masterfrom
Conversation
b1d3fa3 to
dd320a9
Compare
e1c5339 to
32f0d66
Compare
sherrif10
left a comment
There was a problem hiding this comment.
@reagan-meant have you recognised the white spaces
| HttpServletRequest request, UiUtils ui) { | ||
|
|
||
| // if no stop date, set it to start date | ||
| if (stopDate == null) { |
There was a problem hiding this comment.
What was the motivation for removing this?
There was a problem hiding this comment.
This is because with this enhancement we can allow a user to pass a null stopDate which will make that retrospective visit an active visit....this code was instead forcing any such action to default the end date to startDate
| defaultDate: config.defaultStartDate | ||
| ])} | ||
| </p> | ||
| <% if(config.defaultEndDate != null) { %> |
There was a problem hiding this comment.
Why change this from defaultEndDate to visitEndDate?
There was a problem hiding this comment.
This was really a hack to allow the user to have a null defaultEndDate yet at the same time preventing the user setting an end date on an Active visit which was already in place....
| </label> | ||
|
|
||
| ${ ui.includeFragment("uicommons", "field/datetimepicker", [ | ||
| <% if(activeVisits){ %> |
There was a problem hiding this comment.
Any reason for changing the formatting here?
There was a problem hiding this comment.
Not an intended change...
|
|
||
| // should round to the time components to the start and end of the days, respectively | ||
| Date expectedStartDate = new DateTime(2012, 1, 1, 0, 0, 0, 0).toDate(); | ||
| Date expectedStopDate = new DateTime(2012, 1, 1, 23, 59, 59, 999).toDate(); |
There was a problem hiding this comment.
This test is to verify what I had changed in the sense that now we can allow the user to pass a null end Date and not default it to the startDate
|
|
||
| when(ui.format(any())).thenReturn("someDate"); | ||
|
|
||
| List<SimpleObject> result = (List<SimpleObject>) controller.create(adtService, patient, location, startDate, null, request, ui); |
There was a problem hiding this comment.
This method is to catch any overlaps which was automatically parsing any null endDate to startDate which this enhancement altered and thus for it to pass and still be logical it works only if there is an assigned endDate since i enclosed it in an If condition on line 39 in RetrospectiveVisitFragmentController
| startDate = new DateTime(startDate).withTime(0,0,0,0).toDate(); | ||
|
|
||
| // if stopDate is today, set stopDate to current datetime, otherwise set time component to end of date | ||
| if (stopDate != null){ |
There was a problem hiding this comment.
Why enclose these in if stopDate != null?
There was a problem hiding this comment.
the methods enclosed here are for formatting the endDate which is not neccessary when the stopDate is null....
| } | ||
|
|
||
| if ( (stopDate!=null) && !isSameDay(stopDate, visit.getStopDatetime())) { | ||
| if(stopDate == null){ |
There was a problem hiding this comment.
What is the motivation for this change?
There was a problem hiding this comment.
This same logic is mantained on line 48, which still is for analysing to see if the stopDate is now...And so i introduced this method to allow the retrospective stopdate to be set to null thus make it active and not default it to startDate as it was
| <% } else { %> | ||
|
|
||
| ${ ui.includeFragment("uicommons", "field/datetimepicker", [ | ||
| id: "noActiveVisitStopDate", |
There was a problem hiding this comment.
Why are we adding this field/fragment?
There was a problem hiding this comment.
From line 81. This is to force the user have a default stop date if they are entering a retrospective Visit yet we are currently having an active visit otherwise if there is no active visit then we can allow them create a retrospective visit that has a null end date thus making it Active(there is already a catch implemented if the restrospective visit is overlaping with another)
| startDateUpperLimit: wrapper.oldestEncounter == null && wrapper.stopDatetime == null ? editDateFormat.format(new Date()) : editDateFormat.format(wrapper.oldestEncounter == null ? wrapper.stopDatetime : wrapper.oldestEncounter.encounterDatetime), | ||
| defaultStartDate: wrapper.startDatetime, | ||
| defaultEndDate: wrapper.stopDatetime | ||
| defaultEndDate:idx == 0 ? null : wrapper.stopDatetime, |
There was a problem hiding this comment.
What is the motivation for this change?
There was a problem hiding this comment.
This is to allow user to make a most recent Visit endDate null so as to make it an active visit otherwise if the visit is behind another visit in time then you are forced to have a default end Date
https://issues.openmrs.org/browse/RA-950
I did the following
1.Enabled to add a past visit with a start date but no end date
2.But disabled adding a past visit with a start date but no end date if this would overlap with another existing visit
3.Enabled to edit a closed visit and remove its end date
4.But disabled editing a closed visit and remove its end date if this would overlap with another existing visit
Other changes are in EMR API openmrs/openmrs-module-emrapi#177