-
Notifications
You must be signed in to change notification settings - Fork 143
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.
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
O3-3193: REST API for EMR API ADT functionality #231
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So after rabbit-holing a bit I pulled back and did the bare-bones simplest stuff to get the two major REST queries we will need for the Ward app as a proof-of-concept, see attached.
Certainly not ready for merging, and we may go another way, but wanted to get this out in draft form since I'm out tomorrw... @chibongho @mseaton
omod/src/main/java/org/openmrs/module/emrapi/web/controller/InpatientVisitsController.java
Show resolved
Hide resolved
inpatientVisit.put("patient", ConversionUtil.convertToRepresentation(visit.getVisit().getPatient(), Representation.DEFAULT)); | ||
inpatientVisit.put("currentLocation", ConversionUtil.convertToRepresentation(currentLocation, Representation.DEFAULT)); | ||
inpatientVisit.put("timeSinceAdmissionInMinutes", Minutes.minutesBetween(new DateTime(visit.getAdmissionEncounter().getEncounterDatetime()), new DateTime()).getMinutes()); | ||
inpatientVisit.put("timeOnWardInMinutes", Minutes.minutesBetween(new DateTime(visit.getLatestAdtEncounter().getEncounterDatetime()), new DateTime()).getMinutes()); // TODO: assumption, an ADT ecounter always results in change of ward? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this wouldn't cover the (probably rare) case of a bed swap within the same ward, assuming a TRANSFER encounter is used for this. Though if a different encounter type were used, then this would work. I tried to describe this situation in the ticket, but it wasn't very clear, and it's something we can discuss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that would be a limitation. I just dropped in those "timeSince" and "timeWard" values during my last few minutes of work on Monday to has some examples of parameter, (and there some satisfaction in seeing them work without much extra work!). I think we are going to need to rewrite the underlying service method using a query, and we may want to tackle calculating these values as part of that, but if we don't, it should should be very straightforward to move these two time calculations out of the controller and into the VDW and handle the edge case you describe there by iterating over encounters and comparing locations. In fact, easy enough that it probably makes sense for me to move them now.
omod/src/main/java/org/openmrs/module/emrapi/web/controller/InpatientVisitsController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mseaton makes sense, see my added thoughts!
inpatientVisit.put("patient", ConversionUtil.convertToRepresentation(visit.getVisit().getPatient(), Representation.DEFAULT)); | ||
inpatientVisit.put("currentLocation", ConversionUtil.convertToRepresentation(currentLocation, Representation.DEFAULT)); | ||
inpatientVisit.put("timeSinceAdmissionInMinutes", Minutes.minutesBetween(new DateTime(visit.getAdmissionEncounter().getEncounterDatetime()), new DateTime()).getMinutes()); | ||
inpatientVisit.put("timeOnWardInMinutes", Minutes.minutesBetween(new DateTime(visit.getLatestAdtEncounter().getEncounterDatetime()), new DateTime()).getMinutes()); // TODO: assumption, an ADT ecounter always results in change of ward? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that would be a limitation. I just dropped in those "timeSince" and "timeWard" values during my last few minutes of work on Monday to has some examples of parameter, (and there some satisfaction in seeing them work without much extra work!). I think we are going to need to rewrite the underlying service method using a query, and we may want to tackle calculating these values as part of that, but if we don't, it should should be very straightforward to move these two time calculations out of the controller and into the VDW and handle the edge case you describe there by iterating over encounters and comparing locations. In fact, easy enough that it probably makes sense for me to move them now.
omod/src/main/java/org/openmrs/module/emrapi/web/controller/InpatientVisitsController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mseaton made a few additions, namely:
- added an (unimplemented) getVisitsAwaitingTransfer service method
- added new endpoints to fetch transfers, and combined admission and transfers, setting a "type" property
- Moved the time calculations into the VDW (and support the case where a patient is transferred without changing a ward) (they should get some test methods, thought)
If we think this is a reasonable approach, I could potentially ticket the follow-on tasks. (and start working on them, though likely not until July)
I did look a bit into support for parsing custom refs, but haven't come to a conclusion on that yet. At minimum I believe we could take in a "rep" and parse it into the visit and patietn components and then rely on existing methods from there.
} | ||
} | ||
|
||
public Integer timeAtLocationInMinutes() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rename this "timeAtInpatientLocationInMinutes". This basically correspondes to the Location returned by the getInpatientLocation
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, make sense!
|
||
for (Encounter encounter : getSortedEncounters(SortOrder.MOST_RECENT_FIRST)) { | ||
if (encounter.getEncounterType().equals(admissionEncounterType) || encounter.getEncounterType().equals(transferEncounterType)) { | ||
time = Minutes.minutesBetween(new DateTime(encounter.getEncounterDatetime()), new DateTime()).getMinutes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting time here doesn't seem right, as you are setting the time before you are comparing the ward to the encounter location. So this will actually end up with the time since the encounter right before the encounter at the current location. You should only set the time if ward == null || ward.equals(encounter.getLocation())
(eg. after the if /else statement, not before it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did it like that to make sure I had something to test and fail against... :) no, duh, will add the tests and fix...
omod/src/main/java/org/openmrs/module/emrapi/web/controller/InpatientVisitsController.java
Show resolved
Hide resolved
O3-3467: REST API - Create EmrApiConfig endpoint
No description provided.