Skip to content

O3-5174: Prevent duplicate billable services in bills #51

Draft
RajPrakash681 wants to merge 3 commits intoopenmrs:mainfrom
RajPrakash681:fix-duplicate-billable-services
Draft

O3-5174: Prevent duplicate billable services in bills #51
RajPrakash681 wants to merge 3 commits intoopenmrs:mainfrom
RajPrakash681:fix-duplicate-billable-services

Conversation

@RajPrakash681
Copy link
Contributor

@RajPrakash681 RajPrakash681 commented Nov 4, 2025

Fix: Prevent duplicate bill line items

Problem

When adding the same service/item to a bill multiple times, the system was creating duplicate line items instead of incrementing the quantity on the existing line item.

Solution

Added a Liquibase migration that:

  1. Cleans existing duplicates - Merges duplicate line items by summing their quantities
  2. Prevents future duplicates - Creates a unique index on (bill_id, service_id, item_id, price_id, price, voided)
  3. Handles NULL values - Uses COALESCE to properly handle NULL service_id, item_id, and price_id
  4. Idempotent execution - Uses MARK_RAN precondition to safely run multiple times

Testing Performed

  • Module builds successfully: mvn clean install
  • No compilation errors
  • Liquibase XML syntax validated
  • OMOD file generated: billing-1.3.3-SNAPSHOT.omod
  • Changeset structure verified with proper preconditions

Database Changes

  • New Index: unique_bill_line_item_idx
  • Columns: bill_id, service_id, item_id, price_id, price, voided

Migration Behavior

  1. Identifies all duplicate line items in cashier_bill_line_item
  2. Keeps the first occurrence of each duplicate set
  3. Updates its quantity to the sum of all duplicate quantities
  4. Soft-deletes other duplicates (sets voided=1)
  5. Creates unique index to prevent future duplicates

- Added mergeDuplicateLineItems() method to automatically merge line items with same billableService UUID
- Line items with identical billableService are now combined with quantities summed
- Merge occurs before saving new bills and after adding items to existing pending bills
- Resolves issue where same service could be added multiple times to a single bill
- Maintains first item's properties (price, order) while summing quantities

Fixes openmrs#123
@RajPrakash681
Copy link
Contributor Author

RajPrakash681 commented Nov 4, 2025

@denniskigen
sir I've completed the fix for issue O3-5174 (duplicate billable services) in the backend module as you suggested.

What I did:

Earlier, I had made changes in the billing app (frontend), but following your guidance, I've now implemented the fix in the correct location - the openmrs-module-billing backend module.

…mPrice + price)

- Updated mergeDuplicateLineItems() to check billableService UUID + itemPrice UUID + price amount
- Allows same service with different payment methods/prices as separate line items
- Only merges true duplicates (all three fields match)
- Addresses Dennis's feedback on legitimate use cases (e.g., part cash, part insurance)

Examples:
 ALLOWED: Same service with different payment methods (different itemPrice UUID)
 ALLOWED: Same service with different prices
 PREVENTED: Same service + same itemPrice + same price (true duplicate)

Fixes issue O3-5174 per updated requirements
@dkayiwa
Copy link
Member

dkayiwa commented Nov 4, 2025

Is it possible to have a unique constraint for this at the database level?

@RajPrakash681
Copy link
Contributor Author

RajPrakash681 commented Nov 4, 2025

Thanks for the suggestion about adding a database-level unique constraint that’s a good idea! I looked into it, but decided to keep the merge logic at the application layer for a few reasons:

User Experience: When users accidentally add the same service twice, the app automatically merges them instead of throwing a database error. It’s smoother and less frustrating.

Complex Fields: Some fields like item_price_id can be NULL, and handling that consistently across databases is tricky.

Different Item Types: A line item can be either a service or a stock item, which makes a single unique constraint hard to manage.

Precision & Flexibility: Decimal precision and custom merge rules (like keeping the latest price) are easier to handle in code.

Migration Effort: Adding a DB constraint would require cleaning up existing data and writing migration scripts.

@RajPrakash681
Copy link
Contributor Author

@denniskigen should i close that pr in which i made these changes in billing app?
also review this please!

This changeset addresses the issue where the same service/item could be added multiple times to a bill, creating duplicate line items instead of incrementing the quantity.

Changes:
- Added unique index on (bill_id, service_id, item_id, price_id, price, voided)
- Automatically merges existing duplicate line items by summing quantities
- Uses MARK_RAN precondition for safe idempotent execution
- Handles NULL values properly using COALESCE

Testing:
- Module builds successfully with mvn clean install
- Liquibase XML syntax validated
- OMOD file generated successfully

Resolves duplicate billable services issue
@RajPrakash681
Copy link
Contributor Author

@dkayiwa i have tried implementing please have a look !

@dkayiwa
Copy link
Member

dkayiwa commented Nov 10, 2025

Can you share an example or a screenshot of the duplicate line items that were created?

@RajPrakash681
Copy link
Contributor Author

RajPrakash681 commented Nov 11, 2025

image I tested the migration using MySQL commands.

What I verified:

  • The Liquibase changeset executed successfully
  • The unique index
    unique_bill_line_item_idx
    was created on the
    cashier_bill_line_item
    table
  • Index covers all 6 columns:
    bill_id, service_id, item_id, price_id, price, voided
  • Non_unique = 0
    ``` confirms it’s enforcing uniqueness  
  • Visible = YES
    ``` means the constraint is active  
    

The migration handles existing duplicates by merging them (summing quantities) and soft-deleting duplicate rows before creating the unique constraint.
This ensures duplicate bill line items can’t be created going forward.

@RajPrakash681
Copy link
Contributor Author

@dkayiwa sir can you have a look here 😊

@RajPrakash681
Copy link
Contributor Author

@dkayiwa sir any update on this pr?

@ibacher ibacher changed the title Fix(O3-5174): Prevent duplicate billable services in bills O3-5174: Prevent duplicate billable services in bills Nov 17, 2025
@denniskigen
Copy link
Member

Any ideas about how to move this forward, @ibacher, @wikumChamith, @dkayiwa?

@wikumChamith
Copy link
Member

wikumChamith commented Nov 19, 2025

Any ideas about how to move this forward, @ibacher, @wikumChamith, @dkayiwa?

As we are planning on a major release I think we could just add the unique constraint at the database level. This will cause issues if someone has duplicate entries if they try to update. However rather than we are deciding what to do I think it would be better to get handled by the maintainers of those implementations. This is what happens when a system is not well thought out at the start :)

@ibacher
Copy link
Member

ibacher commented Nov 19, 2025

So:

  1. I'm still not convinced this is something we want to move forward with. I raised some issues in the ticket, which is the right place to discuss this. I don't think this is well-specified enough yet for a PR.
  2. We do not generally use database-level uniqueness constraints for anything other than UUIDs, so I'm kind of confused why we're trying to do so here.

@RajPrakash681
Copy link
Contributor Author

RajPrakash681 commented Nov 25, 2025

@denniskigen @ibacher @dkayiwa any idea or comments now, how to proceed with this?

@wikumChamith wikumChamith marked this pull request as draft December 2, 2025 13:39
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