From c768722b1f9cb3258a03bc84c7c7ea7544d0d07d Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Wed, 10 Jun 2026 09:43:05 +0530 Subject: [PATCH 1/6] fix(pharmacy): fix NPE in settle() when stock is below requested quantity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a bill item's available stock was less than the requested quantity, the insufficient-stock error message navigated through a minimal ItemBatch object (built from DTO data without the Item relationship populated) to retrieve the item name, causing a NullPointerException: bi.getPharmaceuticalBillItem().getItemBatch().getItem().getName() ^^^^^^^^^^^ null Root cause: createBillItemFromStockDTO() constructs a minimal ItemBatch from bulk-query DTO data (id, batchNo, rates, expiry) but never calls batch.setItem(...), leaving the item field null. Fix: use bi.getItem().getName() instead — BillItem.item is always set correctly via newBillItem.setItem(referenceItem.getItem()) during bill item creation and is updated by replaceSelectedSubstitute() when a substitute is chosen. Diagnosed from server log: NullPointerException at TransferIssueForRequestsController.settle(TransferIssueForRequestsController.java:422) Closes #20972 Co-Authored-By: Claude Sonnet 4.6 --- .../bean/pharmacy/TransferIssueForRequestsController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java index f839cd524c..12077f36a0 100644 --- a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java +++ b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java @@ -787,7 +787,7 @@ public synchronized void settle() { for (BillItem bi : getBillItems()) { if (bi.getPharmaceuticalBillItem().getItemBatch() != null) { if (bi.getPharmaceuticalBillItem().getStock().getStock() < bi.getPharmaceuticalBillItem().getQty()) { - JsfUtil.addErrorMessage("Available quantity is less than issued quantity in " + bi.getPharmaceuticalBillItem().getItemBatch().getItem().getName()); + JsfUtil.addErrorMessage("Available quantity is less than issued quantity in " + bi.getItem().getName()); return; } } else if (bi.getPharmaceuticalBillItem().getItemBatch() == null) { From 1dec49b4a9cb0caec2cae161a37a69cd722f7bd0 Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Wed, 10 Jun 2026 09:47:38 +0530 Subject: [PATCH 2/6] fix(pharmacy): remove item name from insufficient-stock error to prevent NPE The previous fix replaced getItemBatch().getItem().getName() with bi.getItem().getName(), but bi.getItem() or getName() can also be null (e.g. legacy data, Vmpp/Vmp items). Remove the item name reference entirely and use a safe generic message instead. Co-Authored-By: Claude Sonnet 4.6 --- .../bean/pharmacy/TransferIssueForRequestsController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java index 12077f36a0..28404eeaab 100644 --- a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java +++ b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java @@ -787,7 +787,7 @@ public synchronized void settle() { for (BillItem bi : getBillItems()) { if (bi.getPharmaceuticalBillItem().getItemBatch() != null) { if (bi.getPharmaceuticalBillItem().getStock().getStock() < bi.getPharmaceuticalBillItem().getQty()) { - JsfUtil.addErrorMessage("Available quantity is less than issued quantity in " + bi.getItem().getName()); + JsfUtil.addErrorMessage("One or more items have insufficient stock to complete the transfer."); return; } } else if (bi.getPharmaceuticalBillItem().getItemBatch() == null) { From b83a2611c08be30975c64e528ff0fde40c08887e Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Wed, 10 Jun 2026 10:01:29 +0530 Subject: [PATCH 3/6] fix(pharmacy): prevent over-issuing when stock changes after page load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent bugs allowed a transfer issue to proceed even when actual DB stock was insufficient: 1. Stale in-memory stock check The validation loop compared bi.getPharmaceuticalBillItem().getStock() .getStock() against qty, but that Stock object is constructed at page-load time by createBillItemFromStockDTO() with the available qty captured from the bulk DTO query. Any stock movement that occurs after the page loads (retail sale, concurrent issue) is invisible to this check. Fix: replace with a live DB lookup via StockFacade.find() for every item immediately before saveBill() is called, so the check always reflects the true current stock level. 2. Negated qty passed to isStockAvailable() In the post-save processing loop, qty is negated at line 831 before isStockAvailable() is called at line 856. The internal check (qty > fetchedStock.getStock()) evaluates as e.g. -10 > 2 → false, causing the guard to always return true regardless of actual stock. Fix: pass Math.abs(tmpPh.getQty()) so the comparison uses the magnitude of the quantity. Also made the error messages null-safe (bi.getItem() null guard) and added a clearer message telling the user to refresh when live stock check fails, since the cause is stale page state. Closes #20972 Co-Authored-By: Claude Sonnet 4.6 --- .../TransferIssueForRequestsController.java | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java index 28404eeaab..84169e452c 100644 --- a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java +++ b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java @@ -34,6 +34,7 @@ import com.divudi.core.facade.BillFacade; import com.divudi.core.facade.BillItemFacade; import com.divudi.core.facade.PharmaceuticalBillItemFacade; +import com.divudi.core.facade.StockFacade; import com.divudi.bean.common.ConfigOptionApplicationController; import com.divudi.core.entity.BillFinanceDetails; import com.divudi.core.entity.BillItemFinanceDetails; @@ -72,6 +73,8 @@ public class TransferIssueForRequestsController implements Serializable { @EJB private BillItemFacade billItemFacade; @EJB + private StockFacade stockFacade; + @EJB private PharmacyBean pharmacyBean; @Inject private PharmacyCalculation pharmacyCalculation; @@ -784,23 +787,36 @@ public synchronized void settle() { JsfUtil.addErrorMessage("No Bill Items are added to Transfer"); return; } + // Live stock check: fetch current DB values for every item before committing anything. + // The in-memory Stock object was captured at page-load time and may be stale — a retail + // sale or concurrent issue after page load will not be reflected in it. for (BillItem bi : getBillItems()) { - if (bi.getPharmaceuticalBillItem().getItemBatch() != null) { - if (bi.getPharmaceuticalBillItem().getStock().getStock() < bi.getPharmaceuticalBillItem().getQty()) { - JsfUtil.addErrorMessage("One or more items have insufficient stock to complete the transfer."); + PharmaceuticalBillItem pbi = bi.getPharmaceuticalBillItem(); + if (pbi.getItemBatch() == null) { + if (pbi.getQty() > 0) { + String name = bi.getItem() != null ? bi.getItem().getName() : "An item"; + JsfUtil.addErrorMessage(name + " is not available in the stock."); return; } - } else if (bi.getPharmaceuticalBillItem().getItemBatch() == null) { - if (bi.getPharmaceuticalBillItem().getQty() > 0) { - JsfUtil.addErrorMessage(bi.getItem().getName() + " is not available in the stock"); - return; + } else { + if (pbi.getStock() != null && pbi.getStock().getId() != null) { + Stock liveStock = stockFacade.find(pbi.getStock().getId()); + if (liveStock == null || liveStock.getStock() < pbi.getQty()) { + JsfUtil.addErrorMessage("Insufficient stock for one or more items. " + + "Stock levels may have changed since this page was loaded. " + + "Please refresh and try again."); + return; + } } } double remainingQty = getRemainingQuantityForItem(bi.getReferanceBillItem()); - double issuingQty = bi.getBillItemFinanceDetails().getQuantity() != null ? bi.getBillItemFinanceDetails().getQuantity().doubleValue() : 0.0; + double issuingQty = bi.getBillItemFinanceDetails().getQuantity() != null + ? bi.getBillItemFinanceDetails().getQuantity().doubleValue() : 0.0; if (issuingQty > remainingQty) { - JsfUtil.addErrorMessage("Issued quantity (" + issuingQty + ") is higher than remaining requested quantity (" + remainingQty + ") for " + bi.getItem().getName()); + String name = bi.getItem() != null ? bi.getItem().getName() : "one of the items"; + JsfUtil.addErrorMessage("Issued quantity (" + issuingQty + ") is higher than remaining " + + "requested quantity (" + remainingQty + ") for " + name + "."); return; } } @@ -853,7 +869,7 @@ public synchronized void settle() { getBillItemFacade().edit(billItemsInIssue); //Checking User Stock Entity - if (!userStockController.isStockAvailable(tmpPh.getStock(), tmpPh.getQty(), getSessionController().getLoggedUser())) { + if (!userStockController.isStockAvailable(tmpPh.getStock(), Math.abs(tmpPh.getQty()), getSessionController().getLoggedUser())) { billItemsInIssue.setTmpQty(0); getBillItemFacade().edit(billItemsInIssue); getIssuedBill().getBillItems().add(billItemsInIssue); From 3277fb71b5c71a10337e9732d101657dae8257d1 Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Wed, 10 Jun 2026 14:41:03 +0530 Subject: [PATCH 4/6] fix(pharmacy): load full ItemBatch before populating staff Stock metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a transfer issue is settled, the PharmaceuticalBillItem carries a minimal ItemBatch built from a DTO — only id, batchNo, and rates are set; the item relationship is null. Both addToStock(pbi, qty, Staff) and deductFromStock(pbi, qty, Staff) create a new Staff Stock row when none exists, and they attempt to populate that row's metadata from the batch: ItemBatch ib = pharmaceuticalBillItem.getItemBatch(); Item i = ib != null ? ib.getItem() : null; if (i != null) { /* set itemName, barcode, longCode, dateOfExpire, retailsaleRate */ } else { s.setItemName("UNKNOWN"); s.setBarcode(""); s.setLongCode(0L); } Because the DTO batch has no item loaded, i is always null, so every Staff Stock row created through a transfer issue is stored with: - ITEM_NAME = "UNKNOWN" - BARCODE = "" - LONGCODE = 0 - DATEOFEXPIRE = null - RETAILSALE_RATE = 0.0 (skipped in the else branch) This does not break stock quantity movement — the JPQL lookup key is (itemBatch, staff) by ID — but it corrupts the metadata fields used by stock valuation and expiry reports for in-transit staff stock. Fix: before accessing ib.getItem(), check whether the item relation is unloaded (null) while the batch has a valid DB id. If so, call itemBatchFacade.find(ib.getId()) to obtain the full entity. The loaded entity is used only for metadata population; the FK already stored on the Stock row is unchanged. The guard (getItem() == null && getId() != null) ensures no extra query is issued when the full entity is already loaded (normal receive-side path where ItemBatch is eagerly fetched from DB). Applied to both: - addToStock(PharmaceuticalBillItem, double qty, Staff) - deductFromStock(PharmaceuticalBillItem, double qty, Staff) Closes #20972 Co-Authored-By: Claude Sonnet 4.6 --- src/main/java/com/divudi/ejb/PharmacyBean.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/main/java/com/divudi/ejb/PharmacyBean.java b/src/main/java/com/divudi/ejb/PharmacyBean.java index 1ebbd4f8dd..ab3077ec5c 100644 --- a/src/main/java/com/divudi/ejb/PharmacyBean.java +++ b/src/main/java/com/divudi/ejb/PharmacyBean.java @@ -924,6 +924,14 @@ public Stock addToStock(PharmaceuticalBillItem pharmaceuticalBillItem, double qt s.setItemBatch(pharmaceuticalBillItem.getItemBatch()); s.setStock(qty); ItemBatch ib = pharmaceuticalBillItem.getItemBatch(); + // The ItemBatch may be a minimal DTO-constructed object (id set, but no item/dateOfExpire). + // Load the full entity so metadata fields on the Staff Stock row are populated correctly. + if (ib != null && ib.getItem() == null && ib.getId() != null) { + ItemBatch full = itemBatchFacade.find(ib.getId()); + if (full != null) { + ib = full; + } + } Item i = null; if (ib != null) { i = ib.getItem(); @@ -1061,6 +1069,12 @@ public boolean deductFromStock(PharmaceuticalBillItem pharmaceuticalBillItem, do s.setStaff(staff); s.setItemBatch(pharmaceuticalBillItem.getItemBatch()); ItemBatch ib = pharmaceuticalBillItem.getItemBatch(); + if (ib != null && ib.getItem() == null && ib.getId() != null) { + ItemBatch full = itemBatchFacade.find(ib.getId()); + if (full != null) { + ib = full; + } + } Item i = null; if (ib != null) { i = ib.getItem(); From 181376696f01f5407be00ad1abe805c26adaa41f Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Fri, 12 Jun 2026 20:48:52 +0530 Subject: [PATCH 5/6] fix(pharmacy): remove redundant UserStock check that left orphan issue lines deductFromStock() already does an atomic UPDATE...WHERE s.stock >= :qty check (the #20138 TOCTOU-safe guard) and returns false if insufficient, handled by the existing else branch. The userStockController.isStockAvailable() check ran after the BillItem/PharmaceuticalBillItem were already persisted, and on failure only zeroed tmpQty and continued - leaving a saved transfer issue line with no corresponding stock movement. UserStockContainer reservations are not populated in this flow, so the check added no real protection beyond the live stock check already done earlier in settle(). Co-Authored-By: Claude Sonnet 4.6 --- .../bean/pharmacy/TransferIssueForRequestsController.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java index 992201a403..4a4d3a3b38 100644 --- a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java +++ b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java @@ -878,14 +878,6 @@ public synchronized void settle() { billItemsInIssue.setPharmaceuticalBillItem(tmpPh); getBillItemFacade().edit(billItemsInIssue); - //Checking User Stock Entity - if (!userStockController.isStockAvailable(tmpPh.getStock(), Math.abs(tmpPh.getQty()), getSessionController().getLoggedUser())) { - billItemsInIssue.setTmpQty(0); - getBillItemFacade().edit(billItemsInIssue); - getIssuedBill().getBillItems().add(billItemsInIssue); - continue; - } - //Remove Department Stock boolean returnFlag = pharmacyBean.deductFromStock(billItemsInIssue.getPharmaceuticalBillItem().getStock(), Math.abs(billItemsInIssue.getPharmaceuticalBillItem().getQty()), From 9e26712d60550fbfc2375a44061e43bda73931ee Mon Sep 17 00:00:00 2001 From: Dr M H B Ariyaratne Date: Fri, 12 Jun 2026 20:53:46 +0530 Subject: [PATCH 6/6] fix(pharmacy): guard settle() against empty bill items with no pharmaceuticalBillItem createEmptyBillItem() (used in generateBillComponent when an item has no available stock) adds a BillItem with pharmaceuticalBillItem left unset. Both the live stock validation loop and the zero-qty filter in settle() dereferenced getPharmaceuticalBillItem() unconditionally, NPEing whenever such an item was present in the transfer issue. Co-Authored-By: Claude Sonnet 4.6 --- .../bean/pharmacy/TransferIssueForRequestsController.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java index 4a4d3a3b38..1870fe82cd 100644 --- a/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java +++ b/src/main/java/com/divudi/bean/pharmacy/TransferIssueForRequestsController.java @@ -802,6 +802,11 @@ public synchronized void settle() { // moving any stock, keeping the positive pre-settle sign convention. syncStockQtyFromUserEnteredQty(bi); PharmaceuticalBillItem pbi = bi.getPharmaceuticalBillItem(); + if (pbi == null) { + // createEmptyBillItem() leaves pharmaceuticalBillItem unset when no stock + // was available at generation time - nothing to validate or settle for it. + continue; + } if (pbi.getItemBatch() == null) { if (pbi.getQty() > 0) { String name = bi.getItem() != null ? bi.getItem().getName() : "An item"; @@ -834,7 +839,7 @@ public synchronized void settle() { //Remove Zero Qty Item List billItemList = new ArrayList<>(); for (BillItem bi : getBillItems()) { - if (bi.getPharmaceuticalBillItem().getQty() != 0.0) { + if (bi.getPharmaceuticalBillItem() != null && bi.getPharmaceuticalBillItem().getQty() != 0.0) { billItemList.add(bi); } }