Skip to content

BED-28: Bed Management OWA – admissionLocations.html not working - #110

Open
jwnasambu wants to merge 2 commits into
openmrs:masterfrom
jwnasambu:BED-28
Open

jwnasambu wants to merge 2 commits into
openmrs:masterfrom
jwnasambu:BED-28

Conversation

@jwnasambu

@jwnasambu jwnasambu commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Description of what I changed
The work on this PR ensures that the admission locations on admin page loads as expected, lists wards as intended and gracefully handles server or data errors resolving issues where the page failed to render or showed no content.

Issues I worked on

https://openmrs.atlassian.net/browse/BED-28

@sonarqubecloud

Copy link
Copy Markdown

@jwnasambu

Copy link
Copy Markdown
Contributor Author

@chibongho , @ibacher , @wikumChamith , @dkayiwa kindly help me review this PR at your convenient time please.

@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

key={key}
bed={bed}
layoutRow={this.props.layoutColumn}
layoutRow={this.props.layoutRow}

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.

Nice catch. Could we lock this in with a test so it can't quietly regress? The one BedLayoutRow test renders with layoutRow={3} and layoutColumn={3}, so BedBlock gets 3 whether this line reads layoutRow or layoutColumn, and the snapshot is identical either way, which means nothing would fail if it regressed. Rendering with distinct values (say layoutRow={4} and layoutColumn={2}) and asserting the layoutRow prop passed to BedBlock would fail on master and pass on this branch. The other fixes here (the self and errorResponse.response guards, and the errorText fix in addEditBed) also aren't covered by a test, but this is the easiest to pin down. Not blocking.

const error = errorResponse.response && errorResponse.response.data
? errorResponse.response.data.error
: errorResponse;
self.props.admissionLocationFunctions.notify('error', error.message.replace(/\[|\]/g, ''));

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.

bedBlock.js now guards error.message before using it (error.message && ... and (error.message || '').replace(...)), but the four sibling catch handlers still call error.message.replace(...) unguarded: this line (index.js:139), index.js:95, addEditAdmissionLocation.js:127, and addEditBed.js:147. If an error reaches one of these without a .message (a server body that isn't in the {error: {message}} shape they assume, or a fallback errorResponse that isn't an Error), the .replace throws inside the .catch and the notification is swallowed. Unlikely for the standard OpenMRS REST error body, but since the safe form already exists in bedBlock.js, mirroring (error.message || '') in these four would keep all five consistent. Not blocking.

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