Skip to content

O3-5770: Report queue entry metrics per queue, and for open waits - #123

Merged
NethmiRodrigo merged 5 commits into
openmrs:mainfrom
UjjawalPrabhat:O3-5770-per-queue-metrics
Sep 7, 2026
Merged

NethmiRodrigo merged 5 commits into
openmrs:mainfrom
UjjawalPrabhat:O3-5770-per-queue-metrics

Conversation

@UjjawalPrabhat

@UjjawalPrabhat UjjawalPrabhat commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds groupBy=queue plus 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: count and averageWaitTime are still returned when no metric is named, the new ones only when asked for.

Worth noting:

  • longestOpenWait.queueEntry uses a custom representation rather than REF, 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 mocks ConversionUtil wholesale and the module has no web-context-sensitive test setup.
  • The queues array 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

@UjjawalPrabhat
UjjawalPrabhat force-pushed the O3-5770-per-queue-metrics branch from 1bfe65d to 963e047 Compare August 30, 2026 20:20
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.
Comment thread api/src/main/java/org/openmrs/module/queue/utils/QueueUtils.java Outdated
- 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.

@NethmiRodrigo NethmiRodrigo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Some minor things

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 NethmiRodrigo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @UjjawalPrabhat! One small follow-on from the blank ref fix, and a nit.

@NethmiRodrigo NethmiRodrigo left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, aside from the above few nits! Thanks @UjjawalPrabhat!

A blank ref resolves to a null concept, which matched no entry, so both
open wait metrics came back null. Also assert the entry lands on the row
that survives the blank queue ref filter.
@sonarqubecloud

sonarqubecloud Bot commented Sep 6, 2026

Copy link
Copy Markdown

@NethmiRodrigo NethmiRodrigo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @UjjawalPrabhat!

@NethmiRodrigo
NethmiRodrigo merged commit fb6de2a into openmrs:main Sep 7, 2026
9 checks passed
@UjjawalPrabhat
UjjawalPrabhat deleted the O3-5770-per-queue-metrics branch September 7, 2026 12:00
NethmiRodrigo added a commit to openmrs/openmrs-esm-patient-management that referenced this pull request Sep 10, 2026
…2681)

* (feat) O3-5770: Add a Clinic Administrator screen for Service Queues

* (feat) O3-5770: Read clinic metrics from the queue module

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.

* (fix) O3-5770: Address review feedback on the clinic administrator screen

- Show '--' in the totals cards when the roll-up request failed, not only
  while it is loading.
- Only mount the selected tab's content. Carbon keeps hidden panels mounted,
  so the tab nobody was looking at kept polling for queue entries.
- Show "No status configured" only for a queue that allows no status at all.
  One allowing just the in-service status has no table, but the Attending
  cards above it are its list.
- Gate the add-a-patient control on `defaultStatusConceptUuid`, the status a
  new entry actually starts in, so a status-scoped table only offers it where
  the patient would land. The unscoped waiting list keeps it either way: it is
  the dashboard's own add action.
- Drop `totalWaitMinutes` from the roll-up fixtures. It is not a field the
  hook returns; annotating rather than casting the fixture flags the next one.
- Cover what the tests were assuming: that `queueUuid` scopes the Attending
  cards and the status table to that queue, that `sortRow` compares longest
  waits numerically and keeps queues with no wait at the bottom either way,
  and that the unselected tab stays unmounted.
- Correct the comment on the allowed-status order: it comes from the concept
  set in the dictionary, not from the backend reversing it.

* (fix) O3-5770: Scope the clinic wait metrics to waiting entries

The queue module ends an entry and starts a new one at transition, so an
In Service entry's startedAt is when service began rather than when the
patient joined the queue. Asking for the open-wait metrics over every
unfinished entry therefore reported time in service as waiting time.

Split the roll-up into two requests: the counts still cover every
unfinished entry, so Waiting and Attending are unchanged, while the wait
metrics are scoped to the waiting status. The wait request leaves out
queues with nobody waiting, so the counts decide which rows exist.

Also notes the dependency on queue module #123 in the config schema,
drops the duplicate "Queues" heading from the overview's error state,
and gives the error and loading cards the table's inline spacing.

* (fix) O3-5770: Read the clinic metrics in a single request

The queue module grew a `waitStatus` parameter that scopes the open-wait
metrics server-side while leaving the counts over every unfinished entry,
so the counts and the two wait figures can come from one request rather
than two.

That halves the requests -- each loaded every unfinished entry at the
location on the server -- measures the counts and the waits at one
instant, and deletes the join by queue UUID.

It also drops the "leaves out queues with nobody waiting" premise: the
backend seeds a row for every queue at the location, so a queue nobody is
waiting in comes back with a null wait rather than being absent.

* (fix) O3-5770: Address review feedback on the empty state and imports

`EmptyState` now carries the border and padding every caller was passing it,
so the tile styles itself and a caller only names what differs. Attending
patients and the clinic overview drop their override entirely, the queue
screen keeps just its margin, and the admin page keeps the two deltas that
make it sit flush beneath a table.

Also rename the attending-patients test and move the clinic overview's hook
import below the components, both as suggested in review.

* (fix) O3-5770: Name the queue module version the clinic screen needs

Queue module #123 is merged, so the config description names 3.1.0 rather
than linking the pull request. routes.json already requires `queue
>=3.1.0-0 <4.0.0`, added for queue-entry-metrics, so the floor it declares
and the one #123 needs are the same version.

The admin page's two empty states also stop repeating the same className:
the inset that sits the tile flush beneath a table is set once, on the
section holding it.

* (fix) O3-5770: Report a retired queue rather than blanking the page

The queue list leaves out retired queues, but the clinic overview's rows
come from the queue entries, which do not, so a retired queue somebody is
still waiting in is reachable by link. `useQueue` resolves such a uuid to
nothing without erroring, so it slipped past the `errorFetchingQueue`
guard and the view read `.uuid` off it, blanking the page. The existing
"Invalid Queue" notification now covers that case too.

The loading branch becomes an early return, so the guard narrows
`selectedQueue` for the rest of the body rather than the component leaning
on reasoning the compiler cannot follow. That also retires a nested
ternary and the optional chaining on a value now known to be there.

Also drop `QueueTableTabConfig`, dead since the queue-table-by-status
component went, and trim a comment and a test that restated what they sit
next to.

* (fix) O3-5770: Gate the clinic overview on the privilege alone

* (test) O3-5770: Select the waiting list tab in the queue e2e specs

* (fix) O3-5770: Mark retired queues on the clinic overview and harden the privilege gate

A retired queue that still holds open entries now shows its name with a
Retired tag instead of a link, since the queue list leaves retired queues
out and the link landed on "Invalid Queue". The Queue type gains the
optional `retired` flag the grouped metric response carries.

An empty `clinicAdministratorScreen.privilege` now hides the tabs rather
than opening them to everyone, because userHasAccess grants access to every
user when the privilege is falsy.

The metric tiles show a skeleton while the clinic figures load, and the
config description drops the module version sentence that routes.json
already declares.

The e2e spec asserts the clinic overview tab and its queues table before
opening the waiting list. Tests cover the retired row, the empty privilege,
the privilege passed to userHasAccess, and the loading state.

---------

Co-authored-by: Nethmi Rodrigo <nethmi@openmrs.org>
Co-authored-by: NethmiRodrigo <nethmi.dilinika@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants