Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
RajPrakash681 wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ac5dde
Fix: Prevent duplicate billable services in bills by merging line items
RajPrakash681 Nov 4, 2025
b0050c0
Fix: Use composite key for duplicate detection (billableService + ite…
RajPrakash681 Nov 4, 2025
eab079f
Add Liquibase migration to prevent duplicate bill line items
RajPrakash681 Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 61 additions & 10 deletions api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,6 +103,58 @@ protected IEntityAuthorizationPrivileges getPrivileges() {
protected void validate(Bill bill) {
}

private void mergeDuplicateLineItems(Bill bill) {
if (bill == null || bill.getLineItems() == null || bill.getLineItems().isEmpty()) {
return;
}

List<BillLineItem> lineItems = bill.getLineItems();
List<BillLineItem> mergedItems = new ArrayList<>();
Map<String, BillLineItem> itemMap = new HashMap<>();

for (BillLineItem item : lineItems) {
if (item == null) {
continue;
}

String compositeKey = buildCompositeKey(item);

if (itemMap.containsKey(compositeKey)) {
BillLineItem existingItem = itemMap.get(compositeKey);
int newQuantity = existingItem.getQuantity() + item.getQuantity();
existingItem.setQuantity(newQuantity);

LOG.debug("Merged duplicate line item with key: " + compositeKey + ", new quantity: " + newQuantity);
} else {
itemMap.put(compositeKey, item);
mergedItems.add(item);
}
}

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.
*
Expand All @@ -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)) {
Expand All @@ -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<Bill> bills = searchBill(bill.getPatient());
if (!bills.isEmpty()) {
Bill billToUpdate = bills.get(0);
Expand All @@ -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);
}

Expand Down Expand Up @@ -228,10 +284,6 @@ 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<Bill> getAll(boolean includeVoided, PagingInfo pagingInfo) {
List<Bill> results = super.getAll(includeVoided, pagingInfo);
Expand Down Expand Up @@ -267,8 +319,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;
Expand Down
65 changes: 65 additions & 0 deletions omod/src/main/resources/liquibase.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,71 @@
</createIndex>
</changeSet>

<changeSet id="openmrs.billing-001-v3.0.0-20251110-01" author="Raj Prakash">
<preConditions onFail="MARK_RAN">
<not>
<indexExists indexName="unique_bill_line_item_idx"/>
</not>
</preConditions>
<comment>Add unique constraint to prevent duplicate line items with same service, price, and item price</comment>

<!-- First, clean up any existing duplicates in the database -->
<sql>
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;
</sql>

<!-- Update quantities for the line items we're keeping -->
<sql>
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;
</sql>

<!-- Soft delete the duplicate line items (keep the first occurrence) -->
<sql>
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;
</sql>

<!-- Drop temporary table -->
<sql>
DROP TEMPORARY TABLE IF EXISTS temp_duplicates;
</sql>

<!-- Create the unique index that handles NULL values properly -->
<createIndex indexName="unique_bill_line_item_idx" tableName="cashier_bill_line_item" unique="true">
<column name="bill_id"/>
<column name="service_id"/>
<column name="item_id"/>
<column name="price_id"/>
<column name="price"/>
<column name="voided"/>
</createIndex>
</changeSet>

<changeSet id="openmrs.billing-001-v3.0.0-20251007" author="Wikum Weerakutti">
<comment>Migrate global properties from 'cashier.*' prefix to 'billing.*'</comment>

Expand Down
Loading