(feat) Group medication orders by encounter by saidmurad · Pull Request #3272 · openmrs/openmrs-esm-patient-chart

saidmurad · 2026-04-29T13:36:15Z

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work is based on designs, which are linked or shown either in the Jira ticket or the description below. (See also: Styleguide)
My work includes tests or is validated by existing tests.

Summary

Updated medications table display to group orders by encounter instead of showing all orders in a flat list.
Added encounter-level headers with encounter date/time
Added encounter-level Renew all action that renews all eligible orders in that encounter in one click.
Preserved existing per-order actions (Modify, Renew, Discontinue) and existing pagination behavior.
Added component-level tests for:
- encounter header rendering
- Renew all visibility for valid encounter UUIDs
- Renew all basket enqueue + workspace launch behavior
- duplicate protection (skip already-in-basket orders)
- disabled-state behavior when all encounter orders are already in basket

Screenshots

group-medications-by-encounter.mp4

Related Issue

Other

…-all action

…ations details table

…gn tests

denniskigen

Welcome to OpenMRS @saidmurad, and thanks for taking on a non-trivial first PR. The encounter-grouping idea is a useful addition, and I appreciate that you covered the new behavior with tests. A few things I'd like to work through with you before this is mergeable:

Column sorting silently breaks once grouping is on. The sort callback still fires, but the rendered row order no longer reflects it. I confirmed this locally by clicking the Start date header on a two-row encounter; the rows didn't swap.
Encounter groups are computed from the paginated slice, so an encounter with more rows than pageSize (5) renders with duplicate headers across pages, and "Renew all" only acts on the visible page. Confirmed locally with a 6-row encounter: the basket got 5 orders, not 6.
A couple of type and shape concerns on the encounter datetime lookup, plus two new translation keys that don't really need translating. More on each in inline comments.
One coverage gap worth filling: there's no test for the no-encounter-datetime branch, or for the multi-encounter sort behavior. A test for the sort case in particular would help drive the fix.

One process note while I'm here: for features of this size we usually link a Jira ticket from https://issues.openmrs.org (project O3) in the PR description. It gives reviewers and future maintainers a place to capture design intent (things like what counts as "all" in Renew all? or how should this interact with column sort?). Would you mind filing one and adding the link? Happy to help if you're not sure where to start.

denniskigen · 2026-04-30T10:06:20Z

+  const encounterGroups = useMemo(() => {
+    const groups = new Map<string, { encounterUuid?: string; medications: Array<Order>; hasEncounterUuid: boolean }>();
+    const fallbackGroups: Array<{ encounterUuid?: string; medications: Array<Order>; hasEncounterUuid: boolean }> = [];
+
+    results?.forEach((medication, index) => {
+      const encounterUuid = medication.encounter?.uuid;
+      if (!encounterUuid) {
+        fallbackGroups.push({ encounterUuid: undefined, medications: [medication], hasEncounterUuid: false });
+        return;
+      }
+
+      const existingGroup = groups.get(encounterUuid);
+      if (existingGroup) {
+        existingGroup.medications.push(medication);
+      } else {
+        groups.set(encounterUuid, { encounterUuid, medications: [medication], hasEncounterUuid: true });
+      }
+    });
+
+    return [...groups.values(), ...fallbackGroups];
+  }, [results]);


This is built from results, which is the per-page slice from usePagination. So encounter grouping is also per-page. If an encounter has 6 orders and pageSize is 5, page 1 shows the encounter header with 5 rows and page 2 shows the same header with 1 row. More importantly, "Renew all" on that header only acts on the visible 5 orders, even though the label says "all". I'd build groups from medications (the full set), then for rendering filter each group against results for the visible rows while keeping the full list for the renew-all action.

Thanks for the detailed explanation, that matched what we were seeing.
i have Addressed this: encounter groups for Renew all are built from medications (full set); only visible rows come from results. Renew all now passes the complete encounter list. Rendering follows sorted table rows so sort works with grouping. Duplicate headers across page breaks may still appear; renew-all scope is fixed per your comment.

denniskigen · 2026-04-30T10:06:20Z

+  const getEncounterDateTime = useCallback((medication: Order) => {
+    const encounter = medication.encounter as Record<string, any> | undefined;
+    return encounter?.encounterDatetime ?? encounter?.date ?? encounter?.obsDatetime ?? medication.dateActivated;
+  }, []);


The cast to Record<string, any> is here so encounter.date and encounter.obsDatetime typecheck, but those fields don't exist on the Encounter shape from @openmrs/esm-framework (it only has encounterDatetime). I'd drop the cast and the dead fallbacks: return medication.encounter?.encounterDatetime;. Also worth dropping the fallback to medication.dateActivated: the order's activation date isn't the encounter date, and showing it under a header that represents an encounter is misleading. If encounterDatetime is missing, the no-date branch is more honest.

Updated per your note: medication.encounter?.encounterDatetime only, no cast or dateActivated fallback; missing datetime → unknownEncounterDate. Order fetch representation now requests encounterDatetime on encounter.

denniskigen · 2026-04-30T10:06:20Z

+        ? t('encounterGroupHeader', '{{dateTime}}', {
+            dateTime: formatDate(new Date(encounterDateTime), { time: true }),
+          })
+        : t('encounterGroupHeaderNoDate', '--');


encounterGroupHeader: '{{dateTime}}' is a no-op for translators since the value is just the placeholder. You can render {formatDate(...)} inline and skip the key. Same for encounterGroupHeaderNoDate: '--': a literal -- doesn't carry meaning that needs localizing. If we keep it, prefer a real label like t('unknownEncounterDate', 'Unknown date') so it actually translates.

Done: formatDate(...) is rendered inline in the header (no encounterGroupHeader key). If encounterDatetime is missing, we use t('unknownEncounterDate', 'Unknown date') instead of -- / encounterGroupHeaderNoDate.

denniskigen · 2026-04-30T10:06:20Z

+                  {encounterGroups.flatMap(
+                    ({ encounterUuid, medications: encounterMedications, hasEncounterUuid }, index) => {
+                      const groupReferenceMedication = encounterMedications[0];
+                      const allOrdersAlreadyInBasket = encounterMedications.every((medication) =>
+                        orders.some((existingOrder) => existingOrder.uuid === medication.uuid),
+                      );
+
+                      const groupRows = encounterMedications
+                        .map((medication) => {
+                          const row = rows.find((currentRow) => currentRow.id === medication.uuid);


The rows arg here is sorted by Carbon based on sortRow, but the iteration is over encounterGroups (in results insertion order), and we just look up each row by uuid. Clicking the Start date or Details column header to sort no longer reorders the list. If you want to keep grouping and sorting together, you'll need to apply the active sort state to encounters and to the rows within each group. If sorting isn't in scope here, I'd drop isSortable: true on the headers since they currently look interactive but do nothing.

Addressed: rendering no longer iterates encounterGroups in results order. The table body walks Carbon’s sorted rows (from sortRow) and injects encounter header rows when the encounter changes along that sequence, so Start date / Details sort does reorder visible medication rows. isSortable stays on; a unit test covers the sort + grouping case.

denniskigen · 2026-04-30T10:06:20Z

+                                  size="sm"
+                                  className={styles.renewAllButton}
+                                  disabled={allOrdersAlreadyInBasket}
+                                  onClick={() => handleRenewAllClick(encounterUuid!, encounterMedications)}


Same root cause as the grouping comment above. Once groups are computed from the full medications set, pass the encounter's complete order list here so "Renew all" really does renew all of them rather than just the visible page.

Same fix as the grouping thread: encounterMedications here is the full encounter list from medications (via encounterMedicationsByUuid), not the paginated slice, Renew all adds every order in that encounter to the basket, not only the current page.

Render grouped rows using Carbon’s sorted row order. Inject encounter headers during row traversal so sorting by "Start date" and "Details" remains visible.

…unter renew all - Add encounterDatetime to order API representation so group headers use real encounter time - Simplify group header: encounter.encounterDatetime only, Unknown date when missing - Build renew-all from full medications list (not current page only) - i18n: add unknownEncounterDate, remove no-op encounterGroupHeader keys

…r groups

saidmurad · 2026-05-02T07:00:29Z

Thank you @denniskigen for the welcome and for the thorough, concrete review, it made it much easier to align the implementation with how the table should behave. I appreciate you taking the time on a first contribution; I’d like to keep learning from the team and contributing where I can.
I’ve replied on the inline threads with what changed.
O3 / Jira: I wasn’t able to create a ticket in O3 after signing in with my OpenMRS ID. when i click Projects in dashboard, it takes me to empty page. Could you point me to the right steps, e.g. how to access project , which issue type to use, and whether I need a Jira role or permission to create issues there? Happy to fill in the summary once I can raise it.
Thanks again.

saidmurad · 2026-05-06T04:30:41Z

@denniskigen , I’ve pushed updates addressing the review feedback.Ready for another look. Thank you.

saidmurad added 3 commits April 27, 2026 12:01

feat(medications): group medication orders by encounter and add renew…

9432e67

…-all action

test(medications): add encounter-grouped renew-all coverage for medic…

2ba3f0a

…ations details table

fix(medications): localize encounter group datetime in header and ali…

b6216ce

…gn tests

saidmurad changed the title ~~Group medication order by encounter~~ (feat) Group medication orders by encounter Apr 29, 2026

denniskigen requested review from chibongho and ibacher April 30, 2026 10:00

denniskigen reviewed Apr 30, 2026

View reviewed changes

saidmurad added 3 commits May 1, 2026 20:04

fix(medications): preserve row sorting in encounter-grouped table

88ae1ed

Render grouped rows using Carbon’s sorted row order. Inject encounter headers during row traversal so sorting by "Start date" and "Details" remains visible.

test(medications): assert Start date sort reorders rows with encounte…

7fb8b5f

…r groups

fix(e2e): target medication row in drug order edit assertion

b402745

Conversation

saidmurad commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Requirements

Summary

Screenshots

Related Issue

Other

Uh oh!

denniskigen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saidmurad commented May 2, 2026

Uh oh!

saidmurad commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saidmurad commented Apr 29, 2026 •

edited

Loading