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

Jump to bottom

RA-950: Add LastVisit and EditVisit require an EndDate #275

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+59 −20
Open

RA-950: Add LastVisit and EditVisit require an EndDate #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions ...enmrs/module/coreapps/fragment/controller/visit/RetrospectiveVisitFragmentController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,30 @@ public Object create(@SpringBean("adtService") AdtService adtService,
@RequestParam(value = "stopDate", required = false) Date stopDate,
HttpServletRequest request, UiUtils ui) {

// if no stop date, set it to start date
if (stopDate == null) {
Copy link
Member

Choose a reason for hiding this comment

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

What was the motivation for removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

stopDate = startDate;
}

// set the startDate time component to the start of day
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){
Copy link
Member

Choose a reason for hiding this comment

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

Why enclose these in if stopDate != null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the methods enclosed here are for formatting the endDate which is not neccessary when the stopDate is null....

if (new DateTime().withTime(0,0,0,0).equals(new DateTime(stopDate).withTime(0,0,0,0))) {
stopDate = new Date();
}
else {
stopDate = new DateTime(stopDate).withTime(23, 59, 59, 999).toDate();
}
}

try {

VisitDomainWrapper createdVisit = adtService.createRetrospectiveVisit(patient, location, startDate, stopDate);

request.getSession().setAttribute(AppUiConstants.SESSION_ATTRIBUTE_INFO_MESSAGE,
ui.message("coreapps.retrospectiveVisit.addedVisitMessage"));
request.getSession().setAttribute(AppUiConstants.SESSION_ATTRIBUTE_TOAST_MESSAGE, "true");

return SimpleObject.create("success", true, "id", createdVisit.getVisit().getId().toString(), "uuid", createdVisit.getVisit().getUuid());
}

}
catch (ExistingVisitDuringTimePeriodException e) {

// if there are existing visit(s), return these existing visits
Expand All @@ -70,6 +69,7 @@ public Object create(@SpringBean("adtService") AdtService adtService,
}
}


return simpleVisits;
}
catch (Exception e) {
Expand Down
13 changes: 9 additions & 4 deletions ...a/org/openmrs/module/coreapps/fragment/controller/visit/VisitDatesFragmentController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,16 @@ public SimpleObject setDuration(@SpringBean("visitService") VisitService visitSe
@RequestParam(value="stopDate", required = false) Date stopDate,
HttpServletRequest request, UiUtils ui) {


if (!isSameDay(startDate, visit.getStartDatetime())) {
visit.setStartDatetime(new DateTime(startDate).toDateMidnight().toDate());
}

if ( (stopDate!=null) && !isSameDay(stopDate, visit.getStopDatetime())) {
if(stopDate == null){
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


visit.setStopDatetime(null);

} else if (!isSameDay(stopDate, visit.getStopDatetime())) {

Date now = new DateTime().toDate();
if (isSameDay(stopDate, now)) {
visit.setStopDatetime(now);
Expand All @@ -54,15 +58,16 @@ public SimpleObject setDuration(@SpringBean("visitService") VisitService visitSe
.withMillisOfSecond(999)
.toDate());
}

}

visitService.saveVisit(visit);

request.getSession().setAttribute(AppUiConstants.SESSION_ATTRIBUTE_INFO_MESSAGE, ui.message("coreapps.editVisitDate.visitSavedMessage"));
request.getSession().setAttribute(AppUiConstants.SESSION_ATTRIBUTE_TOAST_MESSAGE, "true");

return SimpleObject.create("success", true, "search", "?patientId=" + visit.getPatient().getId() + "&visitId=" + visit.getId());

}

}
2 changes: 1 addition & 1 deletion omod/src/main/webapp/fragments/patientdashboard/editVisitDatesDialog.gsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
defaultDate: config.defaultStartDate
])}
</p>
<% if(config.defaultEndDate != null) { %>
Copy link
Member

Choose a reason for hiding this comment

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

Why change this from defaultEndDate to visitEndDate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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....

<% if(config.visitEndDate != null) { %>
<p>
<label for="stopDate" class="required">
${ ui.message("coreapps.stopDate.label") }
Expand Down
18 changes: 16 additions & 2 deletions omod/src/main/webapp/fragments/patientdashboard/visitIncludes.gsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,28 @@
${ ui.message("coreapps.stopDate.label") }
</label>
${ ui.includeFragment("uicommons", "field/datetimepicker", [
<% if(activeVisits){ %>
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for changing the formatting here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not an intended change...

${ ui.includeFragment("uicommons", "field/datetimepicker", [
id: "retrospectiveVisitStopDate",
formFieldName: "retrospectiveVisitStopDate",
label:"",
defaultDate: visitEndTime,
endDate: editDateFormat.format(visitEndTime),
useTime: false,
])}
])}
<% } else { %>
${ ui.includeFragment("uicommons", "field/datetimepicker", [
id: "noActiveVisitStopDate",
Copy link
Member

Choose a reason for hiding this comment

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

Why are we adding this field/fragment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

formFieldName: "retrospectiveVisitStopDate",
label:"",
defaultDate: null,
endDate: editDateFormat.format(visitEndTime),
useTime: false,
])}
<% } %>
</p>
<br><br>
Expand Down
3 changes: 2 additions & 1 deletion omod/src/main/webapp/fragments/patientdashboard/visits.gsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@
startDateLowerLimit: (idx + 1 == visits.size || visits[idx + 1].stopDatetime == null) ? null : editDateFormat.format(org.apache.commons.lang.time.DateUtils.addDays(visits[idx + 1].stopDatetime, 1)),
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,
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

visitEndDate: wrapper.stopDatetime
]) }
${ ui.includeFragment("coreapps", "patientdashboard/editVisit", [
visit: wrapper.visit,
Expand Down
3 changes: 2 additions & 1 deletion omod/src/main/webapp/resources/scripts/custom/visits.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ visit.showEditVisitDateDialog = function(visitId) {
if (data.success) {
jq('#edit-visit-dates-dialog-form-' + visitId + ' .icon-spin').css('display', 'inline-block').parent().addClass('disabled');
// TODO Do we need to update this to specify return url, or is this link only going to ever be used from the old visits view?
data.patientId = visit.patientId;
window.location.search = data.search;
}
}
}
);
return false;
Expand Down
7 changes: 3 additions & 4 deletions ...s/module/coreapps/fragment/controller/visit/RetrospectiveVisitFragmentControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,10 @@ public void test_shouldCreateNewRetrospectiveVisit_whenNoStopDateSpecified() thr

// 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();
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the above line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


Visit visit = createVisit();

when(adtService.createRetrospectiveVisit(patient, location, expectedStartDate, expectedStopDate)).thenReturn(new VisitDomainWrapper(visit));
when(adtService.createRetrospectiveVisit(patient, location, expectedStartDate, null)).thenReturn(new VisitDomainWrapper(visit));

SimpleObject result = (SimpleObject) controller.create(adtService, patient, location, startDate, null, request, ui);

Expand All @@ -118,7 +117,7 @@ public void test_shouldReturnConflictingVisits() throws Exception {
Patient patient = new Patient();
Location location = new Location();
Date startDate = new DateTime(2012, 1, 1, 12, 12, 12).toDate();

Date stopDate = startDate;
// 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();
Expand All @@ -135,7 +134,7 @@ public void test_shouldReturnConflictingVisits() throws Exception {

when(ui.format(any())).thenReturn("someDate");

List<SimpleObject> result = (List<SimpleObject>) controller.create(adtService, patient, location, startDate, null, request, ui);
Copy link
Member

Choose a reason for hiding this comment

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

Why replace null with stopDate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

List<SimpleObject> result = (List<SimpleObject>) controller.create(adtService, patient, location, startDate, stopDate, request, ui);

assertThat(result.size(), is(1));
assertThat(result.get(0).toJson(), is("{\"startDate\":\"someDate\",\"stopDate\":\"someDate\",\"id\":null,\"uuid\":\"" + conflictingVisit.getUuid() + "\"}"));
Expand Down
21 changes: 20 additions & 1 deletion ...g/openmrs/module/coreapps/fragment/controller/visit/VisitDatesFragmentControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void setUp() throws Exception {

controller = new VisitDatesFragmentController();
visitService = mock(VisitService.class);

request = mock(HttpServletRequest.class);
session = mock(HttpSession.class);
when(request.getSession()).thenReturn(session);
Expand Down Expand Up @@ -76,6 +76,25 @@ public void shouldSetVisitStartAndStopDates() throws Exception {
assertThat(actualVisit.getStopDatetime(), is(expectedStopDate));
}

@Test
public void shouldSetVisitStopDateAsNullIfStopDateIsNotSpecified() throws Exception {
Date startDate = (new DateTime(2013, 6, 24, 13, 1, 7)).toDate();
Date stopDate = null;

Date expectedStartDate = (new DateTime(2013, 6, 24, 0, 0, 0)).toDate();
Date expectedStopDate = null;

Visit visit = new Visit(1);
visit.setStartDatetime(new Date());
visit.setStopDatetime(new Date());
visit.setPatient(new Patient(1));

controller.setDuration(visitService, visit, startDate, stopDate, request, mock(UiUtils.class));

Visit actualVisit = savedVisit();
assertThat(actualVisit.getStartDatetime(), is(expectedStartDate));
assertThat(actualVisit.getStopDatetime(), is(expectedStopDate));
}
@Test
public void shouldNotChangeStartOrStopDatetimeIfSettingToSameDay() throws Exception {
Visit visit = new Visit(1);
Expand Down