Skip to content

(fix) Scope concept-only fallback in findObsByFormField to respect obsGroup boundaries#722

Open
oisujeh wants to merge 3 commits into
openmrs:mainfrom
oisujeh:fix/obsgroup-concept-fallback-bleed
Open

(fix) Scope concept-only fallback in findObsByFormField to respect obsGroup boundaries#722
oisujeh wants to merge 3 commits into
openmrs:mainfrom
oisujeh:fix/obsgroup-concept-fallback-bleed

Conversation

@oisujeh

@oisujeh oisujeh commented Apr 13, 2026

Copy link
Copy Markdown

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

Fixes a data contamination bug in findObsByFormField where the concept-only fallback mechanism does not respect obsGroup boundaries. When editing an encounter whose form contains multiple obsGroup sections with child fields that share the same concept UUID, observations from one group bleed into the fields of a different group.

The problem in detail

findObsByFormField resolves which saved observation belongs to which form field using a two-stage strategy:

  1. Primary match — match by formFieldPath and concept UUID.
    This works when the saved obs has a formFieldPath that exactly matches the field's current ID.

  2. Concept-only fallback — if the primary match returns nothing, search the entire flattened obs list for any obs whose concept UUID matches the field's concept.
    The claimedObsIds set is used to avoid double-assignment.

The fallback is necessary because form field IDs can change across schema revisions (a field may be renamed or re-ordered), so the engine cannot always rely on path-based matching.

However, the fallback is currently unscoped. It searches every obs in the encounter regardless of which obsGroup that obs belongs to. This causes cross-group contamination when:

  • A form has two or more obsGroup sections (e.g., "Blood Tests" and "Urine Tests", or in real-world pharmacy forms: "ARV Medication" and "PrEP Drugs").
  • Child fields in those groups share the same concept (e.g., both have a "Result Value" or "Drug Strength" field mapped to the same concept UUID).
  • The user edits an existing encounter.

During the edit-mode hydration the engine flattens all obs (including groupMembers) into one list. When a field in Group A fails its primary path match, the fallback grabs the first unclaimed obs with a matching concept — which may belong to Group B. Group A's fields are now populated with Group B's data.

The fix

When a field belongs to an obsGroup (field.meta.groupId is set), the concept-only fallback now checks whether each candidate obs is a member of the correct parent group before accepting it. For each candidate obs the fix determines its owner group by scanning the flat obs list for a parent whose groupMembers array contains the candidate. Four cases are handled:

Case Condition Action
1 Obs is standalone (not inside any group) ✅ Allow — normal fallback behavior
2 Obs is inside a group whose formFieldPath matches the expected parent ✅ Allow — correct group
3 Obs is inside a group whose formFieldPath differs from the expected parent ❌ Exclude — prevents cross-group bleeding
4 Obs is inside a group that has no formFieldPath (old encounter data) ✅ Allow — backward compatibility

The scoping is implemented as an additional .filter() step applied to obsByConcept only when field.meta?.groupId is truthy. Fields that do not belong to an obsGroup are completely unaffected.

Code change

if (field.meta?.groupId) {
  const parentPath = `rfe-forms-${field.meta.groupId}`;
  obsByConcept = obsByConcept.filter((candidate) => {
    const ownerGroup = obsList.find(
      (o) => o.groupMembers?.some((m) => m.uuid === candidate.uuid),
    );
    if (!ownerGroup) return true;                   // case 1: standalone
    if (!ownerGroup.formFieldPath) return true;     // case 4: old encounter
    return ownerGroup.formFieldPath == parentPath;  // case 2 or 3
  });
}

Tests

Added 3 new test cases to obs-adapter.test.ts inside the existing findObsByFormField describe block. The tests use a generic "Lab Results" analogy (Blood Tests vs. Urine Tests sharing a "Result Value" concept) to validate the fix without domain-specific dependencies:

  1. Should scope concept fallback to the parent obsGroup when field has a groupId
    A Blood Test field must NOT steal a Urine Test obs when they share the same concept.

  2. Should allow concept fallback within the correct parent obsGroup
    A Urine Test field with a changed ID should still match obs in its own group via concept fallback.

  3. Should preserve backward compatibility when parent obsGroup has no formFieldPath
    Encounters saved before formFieldPath was recorded on groups must continue to hydrate correctly.

All existing tests continue to pass.

Screenshots

No UI changes — this is a logic-only fix in the obs adapter.

Related Issue

No existing Jira ticket. This was discovered while building pharmacy forms with multiple repeating obsGroups that share child concepts (Drug Strength, Unit, Frequency, Duration, Quantity Prescribed, Quantity Dispensed, Duration Dispensed).

Other

  • Backward compatible: encounters saved without formFieldPath on parent obsGroups continue to work via the existing concept-only fallback (case 4 above).
  • No performance concern: the additional .filter() and inner .find() only execute during the concept fallback path for fields that belong to an obsGroup. The primary path-based match is not touched.
  • Scope: 2 files changed — obs-adapter.ts (fix) and obs-adapter.test.ts (tests).

@oisujeh oisujeh marked this pull request as ready for review April 13, 2026 20:15
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.

1 participant