Skip to content

LUI-213: Support viewing archived observations in Legacy UI - #267

Open
shivvani-r wants to merge 3 commits into
openmrs:masterfrom
shivvani-r:LUI-213
Open

LUI-213: Support viewing archived observations in Legacy UI#267
shivvani-r wants to merge 3 commits into
openmrs:masterfrom
shivvani-r:LUI-213

Conversation

@shivvani-r

Copy link
Copy Markdown

Description of what I changed

This PR updates the Legacy UI to properly fetch and display archived observations, ensuring they are not lost to the user after being archived - implemented as part of https://openmrs.atlassian.net/browse/TRUNK-6648.

Specific changes include

  • DWRObsService.java: Merged archived observations into the search results so they can be viewed and managed on the "Manage Observations" page.
  • EncounterFormController.java: Switched to getAllObsIncludingArchived() to ensure archived observations are passed to the view, restoring the functionality of the "Toggle Voided" button on the Encounter form.
  • obs.js: Updated the DWR rowCreator to correctly apply the .voided CSS class so that voided observations dynamically render with the standard OpenMRS strikethrough styling while preserving alternating row colors.

Issue I worked on

see https://openmrs.atlassian.net/browse/LUI-213

@shivvani-r shivvani-r changed the title Support viewing archived observations in Legacy UI LUI-213: Support viewing archived observations in Legacy UI Jul 19, 2026
Comment thread omod/src/main/java/org/openmrs/web/dwr/DWRObsService.java
Comment thread omod/src/main/java/org/openmrs/web/dwr/DWRObsService.java Outdated
Comment thread omod/src/main/java/org/openmrs/web/dwr/DWRObsService.java
@shivvani-r
shivvani-r requested a review from dkayiwa July 27, 2026 11:36
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@shivvani-r
shivvani-r requested a review from dkayiwa August 5, 2026 16:33
</tr>
<c:set var="obsList" value="${obs.groupMembers}" scope="request"/>
<c:set var="field" value="${otherFormFields[groupMember]}" scope="request"/>
<c:set var="obsList" value="${groupMembersMap != null && groupMembersMap[obs] != null ? groupMembersMap[obs] : obs.groupMembers}" scope="request"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Deleting the old <c:set var="field" value="${otherFormFields[groupMember]}" scope="request"/> from below this line changes what group members render, was that intended? otherFormFields[groupMember] was always null (groupMember is not a defined variable anywhere in the page), and JSTL removes the scoped attribute when the value is null (SetSupport.doEndTag calls pageContext.removeAttribute(var, scope)), so that line was clearing request-scoped field before the recursion and member rows came out with an empty field-number column. With it gone, the recursion inherits the parent's FormField from encounterForm.jsp:551, so every member row now prints the group's field number, and a nested group header prints its ancestor's concept name in ${field.field.concept.name.name}.

I traced that through the JSTL 3.0.1 sources rather than rendering the page. Keeping the old blanking without the dead lookup:

Suggested change
<c:set var="obsList" value="${groupMembersMap != null && groupMembersMap[obs] != null ? groupMembersMap[obs] : obs.groupMembers}" scope="request"/>
<c:set var="obsList" value="${groupMembersMap != null && groupMembersMap[obs] != null ? groupMembersMap[obs] : obs.groupMembers}" scope="request"/>
<c:remove var="field" scope="request"/>

log.debug("Getting obss with patient and concept");
obss = Context.getObsService().getObservationsByPersonAndConcept(p, c);
obss = new ArrayList<Obs>(Context.getObsService().getObservations(
Collections.singletonList(p), null, Collections.singletonList(c), null, null, null, null, null, null, null, null, true));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Flipping includeVoidedObs to true here, and on the person-only branch at line 351, widens this past archived obs, is that part of LUI-213? The archived rows come from the helper two lines down, so the flag is not what brings them in; what it adds is every voided obs still sitting in the obs table. Built against your core branch, after voiding obs 7, getObsByPatientConceptEncounter("7", null, null) returns ... 15 9 7 on this branch against ... 15 9 on master, and encounter 3 returns 7 9 against 9, since getAllObs() was non-voided while getAllObsIncludingArchived() adds this.obs unfiltered. Manage Observations has no include-voided control, so those rows simply show up, struck through.

If that is wanted, getObsByPatientConceptEncounter_shouldIncludeArchivedObs pins only the archived half right now; putting a voided-but-live obs in the same assertion would stop someone quietly setting the flag back to false. If it is not wanted, false here and on line 351 still leaves the archive merge working.

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