Skip to content

Registration page label fixes - #2657

Open
ShresthaKat wants to merge 5 commits into
openmrs:mainfrom
PHCC-Openmrs:registration-page-label-fixes
Open

Registration page label fixes#2657
ShresthaKat wants to merge 5 commits into
openmrs:mainfrom
PHCC-Openmrs:registration-page-label-fixes

Conversation

@ShresthaKat

Copy link
Copy Markdown

No description provided.

anmolpawar121 and others added 5 commits August 3, 2026 00:10
…abel

The config framework backfills label with "" (not undefined) for gender
options that don't specify one, since the elements schema defines
label: {_default: ""}. gender-field.component.tsx used ?? when falling
back to option.value, which only catches null/undefined, so labelText
resolved to t("", "") and Carbon's RadioButton rendered no label text at
all. Switched to || so an empty string also falls back to option.value.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Restore work that only existed as uncommitted changes in a local clone and
was lost when that clone was reset, using the copy preserved in
openmrs-distro-referenceapplication's frontend/esm-patient-registration-app-patched:

- config-schema.ts: add selectFields config for rendering an address field
  as a dropdown (used for the Governorate/State-Province field)
- address-field.component.tsx: render a SelectInput when a field has
  selectFields options configured, instead of always rendering free text
- patient-registration.component.tsx: wire through the address select
  field options
- validation/luhn.ts: Luhn checksum validator
- validation/patient-registration-validation.ts: validate the National ID
  address field against its configured Luhn/regex rules

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a fifth tab alongside Starred/System/My/All lists that shows every
registered patient, not just ones added to a list, so staff can find and
navigate to any patient even if they were never added to a cohort.

- useAllPatients hook queries the FHIR2 Patient search endpoint (the core
  REST /patient resource doesn't support listing without a search term),
  with pagination and an optional name filter.
- AllPatientsTable renders a searchable, paginated table; clicking a
  patient's name navigates to their chart, same pattern as the existing
  list-details table.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown

Greptile Summary

This change expands registration configuration and address validation, adds a default patient-status field, and introduces an all-patients dashboard view backed by FHIR search.

Two failures were reproduced. The Luhn validator accepts values containing letters and punctuation when the remaining digits satisfy the checksum, and the all-patients table presents failed patient requests as an empty result set.

The default patient-status metadata concern was disproved by an executed render-and-submit check: when the optional person-attribute type is unavailable, the field shows its fetch error but registration can still be submitted without a patient-status attribute.

Merge is not safe until the validation and patient-list error handling issues are resolved.

Confidence Score: 4/5

The change should not merge until malformed Luhn values are rejected and failed patient-list requests have a visible error state.

Direct execution reproduced independent failures in validation and patient-list rendering. The default patient-status field was also exercised through its missing-metadata submission path and does not prevent an optional empty attribute from being omitted.

Files Needing Attention: packages/esm-patient-registration-app/src/patient-registration/validation/luhn.ts and packages/esm-patient-list-management-app/src/all-patients-table/all-patients-table.component.tsx

T-Rex T-Rex Logs

What T-Rex did

  • Ran the focused Vitest harness for patient-status validation; it passed 1 test and showed that a missing optional metadata renders an inline error but does not block submission.
  • Produced a P1 finding proof for Luhn contamination, including baseline and contaminated validation outputs that illustrate the contamination effect.
  • Produced a P1 finding proof for the FHIR Patient request-error UI harness, including the normal empty state and the failed request state observed in Chromium.
  • Validated the overall harness run: the focused harness completed with one test passed, no product files were modified, and outputs were captured as validation artifacts.
  • Validated that the hook exposes error but the component does not render it, with no product code changes beyond harness artifacts.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 Luhn validator accepts identifiers containing non-digit characters

    • Bug
      • The executed harness called isValidLuhn('799-ABC273!98713') and observed true, despite the input containing letters and punctuation. The valid digits-only baseline 79927398713 also returned true, so the contaminated value is accepted solely because its non-digits are removed.
    • Cause
      • At packages/esm-patient-registration-app/src/patient-registration/validation/luhn.ts:2, value.replace(/\D/g, '') removes every non-digit before the empty check on lines 3-5 and before the checksum calculation. The function therefore validates the filtered value rather than requiring the supplied value to be all digits.
    • Fix
      • Reject an input containing any non-digit before calculating Luhn—for example, return false unless the original value matches a digits-only pattern—then perform the existing checksum calculation.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 FHIR Patient request failures are presented as an empty patient list

    • Bug
      • When useAllPatients reports a request error after loading completes with no patient data, the component displays “There are no patients to display” and no error or alert.
    • Cause
      • The component ignores the hook's error return value at lines 41-45 and bases its non-loading UI solely on tableRows.length at lines 105 and 152.
    • Fix
      • Destructure error from useAllPatients and render a dedicated request-error state before the empty-state fallback.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Added Patient status dropdown" | Re-trigger Greptile

Comment on lines +2 to +5
const digits = value.replace(/\D/g, '');
if (!digits) {
return false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Checksum accepts non-numeric characters

isValidLuhn removes all non-digits before calculating the checksum, so a configured address value such as 799-ABC273!98713 is accepted because its sanitized digits form a valid Luhn number. Reject values containing non-digit characters before performing the checksum so malformed identifiers cannot be saved.

Artifacts

Executable Luhn contamination harness source

  • Vitest harness imports and calls the production Luhn validator with the baseline and contaminated inputs; it is the executable test used to obtain the observed results.

Baseline Luhn validation output

  • Executed Vitest baseline run from `/home/user/repo` shows `79927398713` returned true and exited 0, establishing the valid-digit control.

Contaminated Luhn validation output

  • Executed Vitest contaminated-input run from `/home/user/repo` shows `799-ABC273!98713` returned true and exited 0, confirming the validator accepts letters and punctuation.

View artifacts

T-Rex Ran code and verified through T-Rex

Comment on lines +41 to +45
const { patients, totalPatients, isLoading, isValidating } = useAllPatients(
(currentPage - 1) * currentPageSize,
currentPageSize,
debouncedSearchTerm,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Patient request failures render as an empty list

When useAllPatients completes with an error and no rows, this component ignores the hook's error value and falls through to “There are no patients to display.” This hides server and connectivity failures from users. Render a dedicated error state before the empty-list fallback.

Artifacts

FHIR Patient request-error UI harness source

  • Authored Vitest harness mocks a completed FHIR Patient HTTP-500-style error and asserts the rendered empty-state output, confirming the exact path.

FHIR Patient request-error UI harness output

  • Executed `yarn vitest run trex-artifacts/all-patients-request-error.test.tsx --config packages/esm-patient-list-management-app/vitest.config.ts` from `/home/user/repo` and recorded one passing assertion, confirming the path.

▶ Normal empty patient-list state in Chromium

  • Chromium rendered the no-data state for the component and showed “There are no patients to display,” establishing the normal empty-state baseline.

▶ Failed FHIR Patient request state in Chromium

  • Chromium rendered the completed FHIR request-error output and showed the same “There are no patients to display” empty state, confirming the defect.

Chromium capture command output

  • Executed the Playwright Chromium capture harness from `/home/user/repo` with exit code 0 and produced the comparison videos and poster frames, confirming captured UI evidence.

View artifacts

T-Rex Ran code and verified through T-Rex

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