Skip to content

(feat) O3-5265 implemented delete bed feature in Bed Management#2175

Open
sourav-jyoti wants to merge 10 commits intoopenmrs:mainfrom
sourav-jyoti:fix/O3-5265
Open

(feat) O3-5265 implemented delete bed feature in Bed Management#2175
sourav-jyoti wants to merge 10 commits intoopenmrs:mainfrom
sourav-jyoti:fix/O3-5265

Conversation

@sourav-jyoti
Copy link

@sourav-jyoti sourav-jyoti commented Dec 19, 2025

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.
  • My work includes tests or is validated by existing tests.

Summary

There was no delete bed feature pn frontend though an api was available for it -- (delete) /bed/[uuid]
this pr implements the delete feature on ui

Screenshots

Before

image

After

Screencast.From.2025-12-19.12-43-15.mp4

Related Issue

https://openmrs.atlassian.net/issues?filter=-1&selectedIssue=O3-5265

@gitcliff
Copy link
Contributor

thanks @sourav-jyoti for the work on this , have you seen the OpenMRS CI / build (pull_request) failure . its coming from test leakage and overly strict expectations, not by a bug in the component. mockMutateBeds is a shared Jest mock that is reused across multiple tests, and its call history is not reset between renders/tests. Because the DeleteBed component is rendered many times in the same test file—and React may re-render during async state updates—the mock accumulates calls. This makes assertions like toHaveBeenCalledTimes(1) fail even though the component logic correctly calls mutateBeds once per successful deletion.

@gitcliff gitcliff changed the title Feat/O3-5265 implemented delete bed feature in Bed Management featO3-5265 implemented delete bed feature in Bed Management Dec 22, 2025
@gitcliff gitcliff changed the title featO3-5265 implemented delete bed feature in Bed Management (feat) O3-5265 implemented delete bed feature in Bed Management Dec 22, 2025
@sourav-jyoti
Copy link
Author

sourav-jyoti commented Dec 22, 2025

Thanks @gitcliff I've identified and fixed the issues:

The mockMutateBeds is a shared Jest mock, and despite clearMocks: true in global jest config, it seems call history was persisting across tests.

Changes:

  1. Changed toHaveBeenCalledTimes(1) to toHaveBeenCalled() to handle React re-renders during async updates

  2. Split "closes the modal regardless of whether the deletion succeeds or fails" into two separate tests to avoid mock accumulation from double-rendering

  3. For "does not call mutateBeds when deletion fails" (L163) - I tried beforeEach(() => jest.resetAllMocks()) and afterEach(() => jest.clearAllMocks()) but neither worked. The only solution that worked was creating a fresh mock instance:

    Question:

    Is using a local mock instance the right approach here, or is there a better pattern I should follow?

@sourav-jyoti
Copy link
Author

@denniskigen could you pls review this pr

@UjjawalPrabhat
Copy link
Contributor

UjjawalPrabhat commented Feb 8, 2026

@sourav-jyoti have you confirmed with @VeronicaMuthee if we even want the delete functionality? Also if we do, consider adding them in an overflow menu. Would look much cleaner

@sourav-jyoti
Copy link
Author

@VeronicaMuthee could you please take a look at the functionality

@VeronicaMuthee
Copy link

Hey @sourav-jyoti, we need to clarify with PIH folks. And if that's the case, overflow menu makes sense as suggested by @UjjawalPrabhat. cc: @chibongho

@sourav-jyoti
Copy link
Author

sourav-jyoti commented Feb 9, 2026

Hey @sourav-jyoti, we need to clarify with PIH folks. And if that's the case, overflow menu makes sense as suggested by @UjjawalPrabhat. cc: @chibongho

Ok after the confirmation of the functionality i will update it

Comment on lines +79 to +80
<TextInput
{...field}
Copy link
Author

@sourav-jyoti sourav-jyoti Feb 13, 2026

Choose a reason for hiding this comment

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

Copy link
Member

@denniskigen denniskigen Feb 16, 2026

Choose a reason for hiding this comment

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

The Stack is used for consistent vertical spacing when we have more than one nested block-level element. In this case because we only have a TextInput as the sole nested element, we shouldn't need to use a Stack here.

@VeronicaMuthee
Copy link

Hey @sourav-jyoti, we need to clarify with PIH folks. And if that's the case, overflow menu makes sense as suggested by @UjjawalPrabhat. cc: @chibongho

@sourav-jyoti, here is the feedback from PIH (Fiona): "If it can be permissioned so not everyone can delete beds then it's a good feature to have."

@sourav-jyoti
Copy link
Author

sourav-jyoti commented Feb 25, 2026

@sourav-jyoti, here is the feedback from PIH (Fiona): "If it can be permissioned so not everyone can delete beds then it's a good feature to have."

awesome ! thnks for the follow up
@VeronicaMuthee
this is the updated guide for making components configurable right https://o3-docs.openmrs.org/docs/configure-o3 ?

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.

5 participants