From 2ac5dde448b9fa2a8e3f7a98f5072f813358f3e9 Mon Sep 17 00:00:00 2001 From: Raj-bytecommandor Date: Tue, 4 Nov 2025 15:00:15 +0530 Subject: [PATCH 1/3] Fix: Prevent duplicate billable services in bills by merging line items - 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 --- .../billing/api/impl/BillServiceImpl.java | 72 ++++++++++++++++--- 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java index 5100c612..466c5d96 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java @@ -22,8 +22,11 @@ import java.net.URL; import java.security.AccessControlException; import java.text.DecimalFormat; +import java.util.ArrayList; import java.util.Date; +import java.util.HashMap; import java.util.List; +import java.util.Map; import com.itextpdf.io.font.constants.StandardFonts; import com.itextpdf.io.image.ImageDataFactory; @@ -100,6 +103,58 @@ protected IEntityAuthorizationPrivileges getPrivileges() { protected void validate(Bill bill) { } + /** + * Merges duplicate line items in a bill based on billableService UUID. Line items with the same + * billableService are combined into a single line item, with quantities summed up and the first + * item's other properties preserved. + * + * @param bill The bill whose line items should be merged. + */ + private void mergeDuplicateLineItems(Bill bill) { + if (bill == null || bill.getLineItems() == null || bill.getLineItems().isEmpty()) { + return; + } + + List lineItems = bill.getLineItems(); + List mergedItems = new ArrayList<>(); + + // Track processed billableService UUIDs + Map serviceMap = new HashMap<>(); + + for (BillLineItem item : lineItems) { + if (item == null) { + continue; // Skip null items + } + + // Only merge items with billableService (not regular stock items) + if (item.getBillableService() != null) { + String serviceUuid = item.getBillableService().getUuid(); + + if (serviceMap.containsKey(serviceUuid)) { + // Duplicate found - merge quantities + BillLineItem existingItem = serviceMap.get(serviceUuid); + int newQuantity = existingItem.getQuantity() + item.getQuantity(); + existingItem.setQuantity(newQuantity); + + // Don't add the duplicate item to mergedItems + LOG.debug("Merged duplicate line item for billableService UUID: " + serviceUuid + ", new quantity: " + + newQuantity); + } else { + // First occurrence of this service + serviceMap.put(serviceUuid, item); + mergedItems.add(item); + } + } else { + // Item without billableService - keep as is (could be a regular stock item) + mergedItems.add(item); + } + } + + // Replace the bill's line items with the merged list + bill.getLineItems().clear(); + bill.getLineItems().addAll(mergedItems); + } + /** * Saves the bill to the database, creating a new bill or updating an existing one. * @@ -117,6 +172,9 @@ public Bill save(Bill bill) { throw new NullPointerException("The bill must be defined."); } + // Merge duplicate line items before any other processing + mergeDuplicateLineItems(bill); + // Check for refund. // A refund is given when the total of the bill's line items is negative. if (bill.getTotal().compareTo(BigDecimal.ZERO) < 0 && !Context.hasPrivilege(PrivilegeConstants.REFUND_MONEY)) { @@ -133,7 +191,6 @@ public Bill save(Bill bill) { bill.setReceiptNumber(generator.generateNumber(bill)); } } - // Check if there is an existing pending bill for the patient List bills = searchBill(bill.getPatient()); if (!bills.isEmpty()) { Bill billToUpdate = bills.get(0); @@ -143,18 +200,17 @@ public Bill save(Bill bill) { billToUpdate.getLineItems().add(item); } - // Calculate the total payments made on the bill + mergeDuplicateLineItems(billToUpdate); + BigDecimal totalPaid = billToUpdate.getPayments().stream().map(Payment::getAmountTendered) .reduce(BigDecimal.ZERO, BigDecimal::add); - // Check if the bill is fully paid if (totalPaid.compareTo(billToUpdate.getTotal()) >= 0) { billToUpdate.setStatus(BillStatus.PAID); } else { billToUpdate.setStatus(BillStatus.PENDING); } - // Save the updated bill return super.save(billToUpdate); } @@ -228,10 +284,7 @@ public void apply(Criteria criteria) { }); } - /* - These methods are overridden to ensure that any null line items (created as part of a bug in 1.7.0) are removed - from the results before being returned to the caller. - */ + @Override public List getAll(boolean includeVoided, PagingInfo pagingInfo) { List results = super.getAll(includeVoided, pagingInfo); @@ -267,8 +320,7 @@ public File downloadBillReceipt(Bill bill) { .concat(patient.getFamilyName() != null ? bill.getPatient().getFamilyName() : "").concat(" ") .concat(patient.getMiddleName() != null ? bill.getPatient().getMiddleName() : ""); String gender = patient.getGender() != null ? patient.getGender() : ""; - String dob = patient.getBirthdate() != null - ? Utils.getSimpleDateFormat("dd-MMM-yyyy").format(patient.getBirthdate()) + String dob = patient.getBirthdate() != null ? Utils.getSimpleDateFormat("dd-MMM-yyyy").format(patient.getBirthdate()) : ""; File returnFile; From b0050c02ddb0350ad6b77419f0c69d7e3780131b Mon Sep 17 00:00:00 2001 From: Raj-bytecommandor Date: Tue, 4 Nov 2025 15:55:50 +0530 Subject: [PATCH 2/3] Fix: Use composite key for duplicate detection (billableService + itemPrice + 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 --- .../billing/api/impl/BillServiceImpl.java | 61 +++++++++---------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java index 466c5d96..cc472b3c 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java @@ -103,13 +103,6 @@ protected IEntityAuthorizationPrivileges getPrivileges() { protected void validate(Bill bill) { } - /** - * Merges duplicate line items in a bill based on billableService UUID. Line items with the same - * billableService are combined into a single line item, with quantities summed up and the first - * item's other properties preserved. - * - * @param bill The bill whose line items should be merged. - */ private void mergeDuplicateLineItems(Bill bill) { if (bill == null || bill.getLineItems() == null || bill.getLineItems().isEmpty()) { return; @@ -117,44 +110,51 @@ private void mergeDuplicateLineItems(Bill bill) { List lineItems = bill.getLineItems(); List mergedItems = new ArrayList<>(); - - // Track processed billableService UUIDs - Map serviceMap = new HashMap<>(); + Map itemMap = new HashMap<>(); for (BillLineItem item : lineItems) { if (item == null) { - continue; // Skip null items + continue; } - // Only merge items with billableService (not regular stock items) - if (item.getBillableService() != null) { - String serviceUuid = item.getBillableService().getUuid(); + String compositeKey = buildCompositeKey(item); + + if (itemMap.containsKey(compositeKey)) { + BillLineItem existingItem = itemMap.get(compositeKey); + int newQuantity = existingItem.getQuantity() + item.getQuantity(); + existingItem.setQuantity(newQuantity); - if (serviceMap.containsKey(serviceUuid)) { - // Duplicate found - merge quantities - BillLineItem existingItem = serviceMap.get(serviceUuid); - int newQuantity = existingItem.getQuantity() + item.getQuantity(); - existingItem.setQuantity(newQuantity); - - // Don't add the duplicate item to mergedItems - LOG.debug("Merged duplicate line item for billableService UUID: " + serviceUuid + ", new quantity: " - + newQuantity); - } else { - // First occurrence of this service - serviceMap.put(serviceUuid, item); - mergedItems.add(item); - } + LOG.debug("Merged duplicate line item with key: " + compositeKey + ", new quantity: " + newQuantity); } else { - // Item without billableService - keep as is (could be a regular stock item) + itemMap.put(compositeKey, item); mergedItems.add(item); } } - // Replace the bill's line items with the merged list bill.getLineItems().clear(); bill.getLineItems().addAll(mergedItems); } + private String buildCompositeKey(BillLineItem item) { + StringBuilder key = new StringBuilder(); + + if (item.getBillableService() != null) { + key.append("service:").append(item.getBillableService().getUuid()); + } else if (item.getItem() != null) { + key.append("stock:").append(item.getItem().getUuid()); + } + + if (item.getItemPrice() != null) { + key.append("|price:").append(item.getItemPrice().getUuid()); + } + + if (item.getPrice() != null) { + key.append("|amount:").append(item.getPrice().toPlainString()); + } + + return key.toString(); + } + /** * Saves the bill to the database, creating a new bill or updating an existing one. * @@ -284,7 +284,6 @@ public void apply(Criteria criteria) { }); } - @Override public List getAll(boolean includeVoided, PagingInfo pagingInfo) { List results = super.getAll(includeVoided, pagingInfo); From eab079f4e9826a733e1c865850c08943baec6026 Mon Sep 17 00:00:00 2001 From: Raj-bytecommandor Date: Mon, 10 Nov 2025 23:14:12 +0530 Subject: [PATCH 3/3] Add Liquibase migration to prevent duplicate bill line items 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 --- omod/src/main/resources/liquibase.xml | 65 +++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/omod/src/main/resources/liquibase.xml b/omod/src/main/resources/liquibase.xml index ca038697..f22dd3b3 100644 --- a/omod/src/main/resources/liquibase.xml +++ b/omod/src/main/resources/liquibase.xml @@ -869,6 +869,71 @@ + + + + + + + Add unique constraint to prevent duplicate line items with same service, price, and item price + + + + CREATE TEMPORARY TABLE IF NOT EXISTS temp_duplicates AS + SELECT + bill_id, + COALESCE(service_id, -1) as service_id_val, + COALESCE(item_id, -1) as item_id_val, + COALESCE(price_id, -1) as price_id_val, + price, + MIN(bill_line_item_id) as keep_id, + SUM(quantity) as total_quantity + FROM cashier_bill_line_item + WHERE voided = 0 + GROUP BY bill_id, COALESCE(service_id, -1), COALESCE(item_id, -1), COALESCE(price_id, -1), price + HAVING COUNT(*) > 1; + + + + + UPDATE cashier_bill_line_item bli + INNER JOIN temp_duplicates td ON bli.bill_line_item_id = td.keep_id + SET bli.quantity = td.total_quantity; + + + + + UPDATE cashier_bill_line_item bli + INNER JOIN temp_duplicates td ON + bli.bill_id = td.bill_id + AND COALESCE(bli.service_id, -1) = td.service_id_val + AND COALESCE(bli.item_id, -1) = td.item_id_val + AND COALESCE(bli.price_id, -1) = td.price_id_val + AND bli.price = td.price + AND bli.bill_line_item_id != td.keep_id + SET bli.voided = 1, + bli.void_reason = 'Duplicate line item merged during database migration', + bli.date_voided = NOW(), + bli.voided_by = 1 + WHERE bli.voided = 0; + + + + + DROP TEMPORARY TABLE IF EXISTS temp_duplicates; + + + + + + + + + + + + + Migrate global properties from 'cashier.*' prefix to 'billing.*'