-
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
EA-208: Change Inpatient Admission and Inaptient Request endpoints to… #251
Conversation
… support searching by patient and visits
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'm not really sure about this @chibongho . The patientIds and visitIds is there because we were supporting existing downstream functionality to stay backwards compatible I believe. We can't change this without doing another major release potentially. We might have to deprecate and add additional methods to support uuids in parallel.
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.
Yeah, I agree with @mseaton ... and I think it should just be as simple as mapping the incoming patients and visits to their underlying ids (available via getId()) and passing them into the existing criteria?
Thanks @chibongho !
) { | ||
RequestContext context = RestUtil.getRequestContext(request, response, Representation.DEFAULT); | ||
InpatientRequestSearchCriteria criteria = new InpatientRequestSearchCriteria(); | ||
criteria.setVisitLocation(visitLocation); | ||
criteria.setDispositionLocations(dispositionLocations); | ||
criteria.setDispositionTypes(dispositionTypes); | ||
criteria.setPatients(patients); |
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.
Can't we just map patients (and visits) to their ids here and keep the criteria the same?
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.
LGTM, thanks @chibongho !
… support searching by patient and visits
Ticket: https://openmrs.atlassian.net/browse/EA-208