Skip to content

O3-5145: Add uniqueness constraints for billable service fields#43

Open
UjjawalPrabhat wants to merge 2 commits intoopenmrs:mainfrom
UjjawalPrabhat:fix/o3-5145-billable-service-uniqueness
Open

O3-5145: Add uniqueness constraints for billable service fields#43
UjjawalPrabhat wants to merge 2 commits intoopenmrs:mainfrom
UjjawalPrabhat:fix/o3-5145-billable-service-uniqueness

Conversation

@UjjawalPrabhat
Copy link

@UjjawalPrabhat UjjawalPrabhat commented Oct 24, 2025

Summary

This PR implements server-side validation to ensure the uniqueness of BillableService names and short names. Previously, it was possible to create duplicate services, leading to data integrity issues.

Changes

  • Validator: Added BillableServiceValidator to reject duplicate names and short names (case-insensitive).
  • Model Fix: Changed BillableService.billableServiceId from int to Integer to correctly distinguish between new (null ID) and existing (ID 0) services during validation.
  • DAO: Enhanced HibernateBillableServiceDAO and BillableServiceSearch to support filtering by shortName for efficient validation.
  • Tests: Added regression tests to BillableServiceServiceImplTest to verify the new constraints.

Related Issue

O3-5145

Copy link
Contributor

@NethmiRodrigo NethmiRodrigo left a comment

Choose a reason for hiding this comment

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

This mentions that "Administrators should clean duplicate data before deploying update", but I think we should have a better way of having a migration so that this change wouldn't affect existing implementations, that possible doesn't require administrators having to mess with existing data. Is that possible? @wikumChamith @dkayiwa tagging you for advice

@wikumChamith
Copy link
Member

@UjjawalPrabhat have you added the constraints in the Hibernate mappings for Billable Services?

@wikumChamith
Copy link
Member

This mentions that "Administrators should clean duplicate data before deploying update", but I think we should have a better way of having a migration so that this change wouldn't affect existing implementations, that possible doesn't require administrators having to mess with existing data. Is that possible? @wikumChamith @dkayiwa tagging you for advice

This might be a bit tricky since handling duplicates really depends on the user. Some may prefer to delete them, while others might want to rename them. If the next release is going to be a major one, I think this approach should be fine.

@dkayiwa @ibacher what do you both think?

@dkayiwa
Copy link
Member

dkayiwa commented Oct 27, 2025

Is this pull request about adding a unique constraint for billable service fields? Or is it adding adding new fetch methods?

@denniskigen
Copy link
Member

@wikumChamith can we move this forward?

@ibacher ibacher changed the title (fix) O3-5145: Add uniqueness constraints for billable service fields O3-5145: Add uniqueness constraints for billable service fields Nov 19, 2025
@ibacher
Copy link
Member

ibacher commented Nov 19, 2025

We don't generally enforce uniqueness constraints through Hibernate or DB level indices. Usually we have a validator that rejects the ability to create duplicates. I'm not sure why were doing something different here?

@UjjawalPrabhat UjjawalPrabhat marked this pull request as draft January 15, 2026 21:21
@UjjawalPrabhat UjjawalPrabhat force-pushed the fix/o3-5145-billable-service-uniqueness branch from f7ccd63 to 572db17 Compare January 16, 2026 14:26
@UjjawalPrabhat UjjawalPrabhat marked this pull request as ready for review January 16, 2026 14:39
@UjjawalPrabhat
Copy link
Author

We don't generally enforce uniqueness constraints through Hibernate or DB level indices. Usually we have a validator that rejects the ability to create duplicates. I'm not sure why were doing something different here?

@ibacher @dkayiwa I’ve updated my approach based on the discussion. Could you please take a look and let me know if there are any further enhancements or changes you’d recommend?

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.

6 participants