O3-5770: Report queue entry metrics per queue, and for open waits - #123
Open
UjjawalPrabhat wants to merge 4 commits into
Open
O3-5770: Report queue entry metrics per queue, and for open waits#123UjjawalPrabhat wants to merge 4 commits into
UjjawalPrabhat wants to merge 4 commits into
Conversation
UjjawalPrabhat
force-pushed
the
O3-5770-per-queue-metrics
branch
from
August 30, 2026 13:58
39baccb to
1bfe65d
Compare
3 tasks
UjjawalPrabhat
force-pushed
the
O3-5770-per-queue-metrics
branch
from
August 30, 2026 20:20
1bfe65d to
963e047
Compare
UjjawalPrabhat
added a commit
to UjjawalPrabhat/openmrs-esm-patient-management
that referenced
this pull request
Aug 30, 2026
Rolls the clinic metrics up server-side via groupBy=queue, rather than fetching every unfinished queue entry at a location and reducing them in the browser. Requires openmrs/openmrs-module-queue#123; reverting this commit drops that dependency.
- Floor an open wait at zero, so a startedAt in the future reports as not yet waiting rather than as a negative duration - Report the queue under a custom representation: REF carries neither the location and service that label a row nor the retired flag, and DEFAULT also carries the allowed priorities and statuses - Sort the queue rows by name, so repeating a request returns them in the same order - Cover the branch that reports only the queues the request named, and the one where an unrecognised groupBy value keeps the flat response
An entry's startedAt is reset when it is called in to be seen, so a client asking for the open wait metrics with only isEnded=false has In Service entries measured on their time in service rather than on how long they waited. Adds a waitStatus parameter naming the statuses that count as waiting; averageOpenWaitTime and longestOpenWait are then computed over only those entries. count, averageWaitTime and countsByStatus keep the full list, and a request without the parameter behaves as before.
The wrapper resolves a blank queue ref to a null element, which would then reach the sort by name and fail there. Also covers the per-queue rows in the waitStatus test, and that the location and service filters reach the queue search.
|
NethmiRodrigo
left a comment
NethmiRodrigo
left a comment
There was a problem hiding this comment.
Thanks @UjjawalPrabhat! One small follow-on from the blank ref fix, and a nit.
| // One instant for every duration, so the per-queue figures and the totals cannot disagree | ||
| Date asOf = new Date(); | ||
| String[] waitStatusArray = parameters.get(WAIT_STATUS); | ||
| List<Concept> waitStatuses = (waitStatusArray == null ? null : services.getConcepts(waitStatusArray)); |
There was a problem hiding this comment.
Same for a blank waitStatus ref, otherwise filterByStatus measures over no entries and both open wait metrics come back null.
Suggested change
| List<Concept> waitStatuses = (waitStatusArray == null ? null : services.getConcepts(waitStatusArray)); | |
| List<Concept> waitStatuses = null; | |
| if (waitStatusArray != null) { | |
| waitStatuses = services.getConcepts(waitStatusArray); | |
| // A blank status ref resolves to a null element | |
| waitStatuses.removeIf(c -> c == null); | |
| } |
Lets also add a test for waitStatus="" that asserts the wait metrics match the unfiltered values.
|
|
||
| List<SimpleObject> queues = (List<SimpleObject>) result.get(QUEUES); | ||
| assertThat(queues, hasSize(1)); | ||
| assertThat(((Queue) queues.get(0).get(QUEUE)).getName(), equalTo("Triage")); |
There was a problem hiding this comment.
Nit: lets also assert that the entry landed on the surviving row
Suggested change
| assertThat(((Queue) queues.get(0).get(QUEUE)).getName(), equalTo("Triage")); | |
| assertThat(((Queue) queues.get(0).get(QUEUE)).getName(), equalTo("Triage")); | |
| assertThat(queues.get(0).get(COUNT), equalTo(1)); |
NethmiRodrigo
approved these changes
Sep 4, 2026
There was a problem hiding this comment.
LGTM, aside from the above few nits! Thanks @UjjawalPrabhat!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
groupBy=queueplus three open-wait metrics to/queue-entry-metric, so a client can get a per-queue breakdown in one request instead of pulling every unfinished entry and counting them in the browser. Raised from#2681.
Existing callers are unaffected:
countandaverageWaitTimeare still returned when nometricis named, the new ones only when asked for.Worth noting:
longestOpenWait.queueEntryuses a custom representation rather thanREF, which for a queue entry drags six lazy associations along for every queue reported on. The cost is that the rep string is not covered by a test, the controller test mocksConversionUtilwholesale and the module has no web-context-sensitive test setup.queuesarray is seeded from the queue search but extended from the entries, because the two do not select identically: the queue search excludes retired queues, the entry search has no such filter. Without that the totals could exceed the sum of the rows.Related Issue
O3-5770