Summary of Changes #17506
Open
Summary of Changes #17506
+155 −1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
1. BillService.java (lines 3128-3278) Added new method createBillFinancialDetailsForInpatientDirectIssueBill(Bill bill) that: - Loops through all bill items - Creates BillItemFinanceDetails (BIFD) for each item with: - Rate fields from pharmaItem.getXxxRate() (includes margins) - Costing value fields from itemBatch rates (for accurate stock valuation) - Quantities (negative - stock going out) - Values at cost/purchase/retail/wholesale rates (negative) - Updates PharmaceuticalBillItem with cost, retail, and purchase values - Creates BillFinanceDetails (BFD) with aggregated totals (negative for stock out) - Persists all entities 2. PharmacySaleBhtController.java - Updated 3 settlement methods: | Method | Line | Change | |-------------------------------|------|---------------------------------------| | settleBhtIssue() | 1183 | Added call to new service method | | settleBhtIssueRequest() | 1226 | Added call to new service method | | settleBhtIssueRequestAccept() | 1443 | Replaced old method call with new one | 3. Methods automatically covered (call settleBhtIssue()): - settlePharmacyBhtIssue() → calls settleBhtIssue() - settleSurgeryBhtIssue() → calls settleBhtIssue() - settleStoreBhtIssue() → calls settleBhtIssue() Data Flow When an inpatient direct issue bill is settled: settleBhtIssue() / settleBhtIssueRequest() / settleBhtIssueRequestAccept() ↓ savePreBillFinally() - Creates Bill ↓ savePreBillItemsFinally() - Creates BillItems, deducts stock ↓ billService.createBillFinancialDetailsForInpatientDirectIssueBill() ↓ For each BillItem: ├── Create/populate BillItemFinanceDetails │ ├── Rates (from pharmaItem) │ ├── Costing values (from itemBatch, negative) │ └── Quantities (negative) ├── Update PharmaceuticalBillItem values └── Persist BillItem ↓ Create/populate BillFinanceDetails (aggregated totals, negative) ↓ Persist Bill The implementation is now complete and ready for testing. Closes #17500 Signed-off-by: Dr M H B Ariyaratne <buddhika.ari@gmail.com>
Contributor
WalkthroughThe changes implement financial details creation for inpatient direct issue bills in the pharmacy module. A new service method populates BillItemFinanceDetails and BillFinanceDetails with cost, purchase, and retail rates from pharmaceutical batch data, applying negated quantities to reflect inventory outflow. Three BHT settlement methods in the controller are modified to invoke this method after saving bill items. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/divudi/service/BillService.java (1)
3197-3207: Minor cleanups: rely on lazy-init for finance details and (optionally) guard by bill typeTwo small, non-blocking observations:
BillItemFinanceDetailsis obtained viabillItem.getBillItemFinanceDetails()and used directly, which matches the lazy-initialization pattern from the entities (no extra null checks needed). ForBillFinanceDetails, you still have:BillFinanceDetails bfd = bill.getBillFinanceDetails(); if (bfd == null) { bfd = new BillFinanceDetails(); bfd.setBill(bill); bill.setBillFinanceDetails(bfd); }Given
Bill.getBillFinanceDetails()is also lazily initializing and never returns null (per existing patterns), this block should be redundant and can be removed in a future cleanup to rely solely on the getter.
- The method name suggests it’s specific to inpatient direct issues, but it doesn’t assert anything about
bill.getBillTypeAtomic(). If you’re worried about accidental reuse from other contexts later, an optional guard like:if (bill.getBillTypeAtomic() == null || !bill.getBillTypeAtomic().name().contains("DIRECT_ISSUE")) { return; }would make the intent explicit.
Both are minor and can be deferred or moved to a separate tech-debt issue if you prefer to keep this PR focused.
Also applies to: 3257-3267, 3276-3277
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{xhtml,java}
📄 CodeRabbit inference engine (AGENTS.md)
Implement accessibility with proper tooltips and labeling
Files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
**/*.{java,xhtml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{java,xhtml}: UsewebUserController.hasPrivilege()for UI privilege integration
UseconfigOptionApplicationControllerfor dynamic configuration values
Files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
**/*Controller.java
📄 CodeRabbit inference engine (AGENTS.md)
Reuse existing controller properties and methods; avoid introducing new filtered values, global filters, or backend logic for UI-only tasks
Files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Do not 'fix' intentional typos in model/controller property names used for database compatibility
Files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
🧠 Learnings (22)
📓 Common learnings
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15012 File: src/main/webapp/resources/pharmacy/pharmacy_donation_receipt.xhtml:126-159 Timestamp: 2025-08-19T04:39:11.840Z Learning: In src/main/webapp/resources/pharmacy/pharmacy_donation_receipt.xhtml and PharmacyDonationController.java, buddhika75 confirmed that the BillFinanceDetails population issue (empty summary values in donation receipts due to unpopulated totalCostValue, totalExpense, totalTaxValue fields) will be handled separately in a donation-related issue, preferring to keep the current PR focused on the main donation feature implementation. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15012 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDonationController.java:170-195 Timestamp: 2025-08-19T04:37:07.435Z Learning: In PharmacyDonationController.java, buddhika75 confirmed that the BillFinanceDetails population issue in the calculateBillTotalsFromItems() method will be handled separately in a donation-related issue, preferring to keep the current PR focused on the main donation feature implementation. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15628 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1056-1069 Timestamp: 2025-09-27T23:19:04.896Z Learning: In PharmacyDirectPurchaseController.java bill number generation logic, buddhika75 chose to defer the legacy behavior preservation fix (avoiding double counter consumption when configuration flags are false) to a separate issue rather than addressing it in PR #15628, following their pattern of keeping PRs focused on primary objectives. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12425 File: src/main/java/com/divudi/core/entity/BillItemFinanceDetails.java:0-0 Timestamp: 2025-05-23T01:36:07.199Z Learning: In BillItemFinanceDetails entity, some fields like percentage-related and payment method fields have been commented out in PR #12425, and further refactoring will be done in a separate issue to prevent "Row size too large" errors in MySQL/InnoDB by moving additional fields to separate entities. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15012 File: src/main/java/com/divudi/core/data/BillTypeAtomic.java:152-154 Timestamp: 2025-08-19T04:38:27.422Z Learning: In src/main/java/com/divudi/core/data/BillTypeAtomic.java, buddhika75 confirmed that the PaymentCategory issue for PHARMACY_DONATION_BILL (using NON_CREDIT_SPEND instead of NON_CREDIT_COLLECTION for a CASH_IN transaction) will be handled in a separate donation-related issue, preferring to keep the current PR focused on the main donation feature implementation. Learnt from: buddhika75 Repo: hmislk/hmis PR: 13628 File: src/main/java/com/divudi/service/pharmacy/PharmacyCostingService.java:39-40 Timestamp: 2025-07-03T02:50:06.769Z Learning: In src/main/java/com/divudi/service/pharmacy/PharmacyCostingService.java, buddhika75 considers the Double vs double precision issue for monetary calculations to be unrelated to PR #13628 which focuses on bill department ID bulk editor implementation, confirming their preference to keep PRs focused on specific objectives. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12640 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1072-1075 Timestamp: 2025-05-31T09:22:42.659Z Learning: In PharmacyDirectPurchaseController.java, the zero-quantity validation that was previously checked at the controller level (billItemsTotalQty == 0.0) has been moved to the BillItemFinanceDetails entity level as part of the financial calculation refactoring. The validation is now handled by the bill item finance details entity rather than explicitly in the controller. Learnt from: buddhika75 Repo: hmislk/hmis PR: 16572 File: src/main/java/com/divudi/service/BillService.java:608-618 Timestamp: 2025-11-14T18:13:59.969Z Learning: In BillService.java, fetchBillTypeAtomicsForOpdFinance() returns atomics for ORIGINAL BILLS (e.g., PHARMACY_RETAIL_SALE, PHARMACY_WHOLESALE) used by OPD Due Search/Due Age queries that check outstanding balances with (abs(b.netTotal)-abs(b.paidAmount))> :val. It should NOT include settlement record atomics (PHARMACY_CREDIT_COMPANY_PAYMENT_RECEIVED, OPD_CREDIT_COMPANY_PAYMENT_RECEIVED) because settlement records have paidAmount = netTotal (no outstanding balance) and would not match the query. Settlement atomics are handled separately in listBillsOpdCreditCompanySettle() for "OPD Done Search" functionality. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12389 File: src/main/java/com/divudi/core/entity/BillFinanceDetails.java:33-45 Timestamp: 2025-05-22T22:22:28.337Z Learning: In BillFinanceDetails, BillItemFinanceDetails, and similar entity classes, the team has agreed to gradually remove eager initialization of BigDecimal fields with BigDecimal.ZERO, allowing them to be null by default. This change will be implemented incrementally with careful addition of null checks wherever these fields are used, as it affects multiple places in the codebase. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14779 File: src/main/java/com/divudi/bean/pharmacy/PharmacyIssueController.java:464-480 Timestamp: 2025-08-07T16:47:52.912Z Learning: In the HMIS PharmacyIssueController, validation for from/to departments in bill number generation is already handled earlier in the workflow (in settleDisposalIssueBill() and other entry points), so additional validation checks in savePreBillFinally() are considered redundant by buddhika75. The system follows a pattern of validating once at entry points rather than multiple defensive checks throughout the flow. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14191 File: src/main/java/com/divudi/bean/pharmacy/PharmacyAdjustmentController.java:922-922 Timestamp: 2025-07-26T13:58:28.251Z Learning: In PharmacyAdjustmentController.java cost rate adjustment methods, the pharmaceutical bill item data (like setPurchaseRate calls) is maintained only for backward compatibility and is not actually used in adjustment bills. This data is only utilized in sale bills, GRN, purchase bills, and transfer bills, so field assignment correctness in adjustment contexts should be ignored. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15280 File: src/main/java/com/divudi/bean/pharmacy/PharmacyBillSearch.java:2591-2654 Timestamp: 2025-09-04T01:21:19.151Z Learning: In PharmacyBillSearch.java, the reversePharmacyDirectIssueToBht() method is designed as a generic admin utility for reversing any bill type in error scenarios, not just direct inpatient issues. It preserves the original bill's BillTypeAtomic to maintain proper typing for the reversal, and is intended for rare use by administrators when correcting system errors. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12635 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1349-1349 Timestamp: 2025-05-31T05:42:46.053Z Learning: In PharmacyDirectPurchaseController.java, the financial calculation follows a multi-level approach: line-level calculations (gross → discount → net), then bill-level calculations where grossTotal at bill level equals the sum of lineNetTotal values (not lineGrossTotal), preventing duplication of line-level discounts. Bill-level adjustments (billDiscount, billTax, billExpense) are then applied to this grossTotal to calculate the final netTotal, and these bill-level values are redistributed proportionally back to items. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12600 File: src/main/java/com/divudi/core/data/IncomeBundle.java:0-0 Timestamp: 2025-05-29T22:31:52.551Z Learning: In the HMIS pharmacy system, Bill Items and Items serve different purposes: Bill Items are transaction entities generated frequently by cashiers for patient visits, while Items are master data entities configured by system administrators. Although methods like generateRetailAndCostDetailsForPharmaceuticalBillItems() and generateRetailAndCostDetailsForByItems() may appear similar, they implement different grouping logics for these distinct entity types. Learnt from: DARKDRAGON-LK Repo: hmislk/hmis PR: 12693 File: src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java:0-0 Timestamp: 2025-06-04T01:24:39.430Z Learning: In PharmacySaleBhtController.java, an updateBillTotals method was temporarily added for testing Bill Totals functionality but was accidentally left in the code. The method duplicated functionality from the existing deprecated updateMargin method and was never called. This was later removed in commit 88013af88688c528397c777c2ab61dce884321c4. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15510 File: src/main/java/com/divudi/service/pharmacy/PharmacyCostingService.java:254-264 Timestamp: 2025-09-20T16:49:06.368Z Learning: In PharmacyCostingService.java, both recalculateFinancialsBeforeAddingBillItem() and recalculateFinancialsForBillItemForGrnReturn() methods intentionally use the same pattern for retail value calculations: both setRetailPackValue() and setRetailValue() use the retailValue variable. This consistency is deliberate and should not be changed without proper justification and verification of financial impact. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14183 File: src/main/java/com/divudi/core/data/BillTypeAtomic.java:159-163 Timestamp: 2025-07-26T06:20:28.022Z Learning: In the HMIS pharmacy system, the new granular adjustment BillTypeAtomic constants (PHARMACY_PURCHASE_RATE_ADJUSTMENT, PHARMACY_RETAIL_RATE_ADJUSTMENT, etc.) intentionally do not have corresponding cancellation or refund variants. As confirmed by buddhika75, cancellation functionality is not required for these adjustment types at the moment and will be addressed when developed in the future. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14070 File: src/main/webapp/resources/ezcomp/menu.xhtml:757-762 Timestamp: 2025-07-22T03:26:56.672Z Learning: buddhika75 prefers to test new functionality before adding privilege checks, indicating a preference for ensuring features work correctly before implementing security restrictions. Learnt from: buddhika75 Repo: hmislk/hmis PR: 11765 File: src/main/java/com/divudi/core/data/IncomeBundle.java:48-51 Timestamp: 2025-04-17T21:03:00.643Z Learning: buddhika75 is receptive to detailed code reviews and suggestions for improvement, even for subtle issues that others on the project might not prioritize. Code quality and thorough error handling are important to them. Learnt from: buddhika75 Repo: hmislk/hmis PR: 11765 File: src/main/java/com/divudi/core/data/IncomeBundle.java:48-51 Timestamp: 2025-04-17T21:03:00.643Z Learning: buddhika75 values detailed code reviews and is receptive to suggestions for improvement, even for subtle issues that others on the project might not prioritize. They specifically requested continued attention to these types of details in future reviews. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12560 File: src/main/java/com/divudi/bean/lab/PatientReportController.java:2067-2094 Timestamp: 2025-05-28T00:15:28.751Z Learning: The user buddhika75 prefers to handle refactoring suggestions as separate issues rather than including them in the current PR, showing a preference for keeping PRs focused on their primary objectives. Learnt from: buddhika75 Repo: hmislk/hmis PR: 13620 File: src/main/java/com/divudi/bean/common/PatientController.java:3061-3072 Timestamp: 2025-07-03T00:35:49.303Z Learning: When buddhika75 requests to create a separate GitHub issue for refactoring suggestions that are out of scope for the current PR, they prefer to have the issue assigned to them and want it tracked separately rather than included in the current PR changes. Learnt from: buddhika75 Repo: hmislk/hmis PR: 15094 File: src/main/java/com/divudi/bean/common/ItemFeeManager.java:166-168 Timestamp: 2025-08-24T05:22:26.932Z Learning: buddhika75 prefers to track suggested improvements as separate GitHub issues when they are outside the scope of the current PR, showing good practice of keeping PRs focused while ensuring improvements aren't forgotten. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14483 File: src/main/java/com/divudi/bean/pharmacy/TransferReceiveController.java:0-0 Timestamp: 2025-08-03T06:54:45.735Z Learning: The user buddhika75 has confirmed they prefer to remove debug System.out.println statements entirely rather than converting them to proper logging when they are temporary debugging additions in TransferReceiveController.java, showing preference for clean code without unnecessary logging overhead. Learnt from: buddhika75 Repo: hmislk/hmis PR: 14484 File: src/main/java/com/divudi/core/entity/WebUserPasswordHistory.java:31-32 Timestamp: 2025-08-03T06:37:24.031Z Learning: buddhika75 considers timezone handling and audit timestamp standardization work as substantial and prefers to defer it when the team is overloaded with essential and high priority tasks, showing preference to focus on current PR objectives rather than comprehensive timezone refactoring. Learnt from: buddhika75 Repo: hmislk/hmis PR: 12560 File: src/main/java/com/divudi/ejb/SmsManagerEjb.java:70-70 Timestamp: 2025-05-28T00:04:01.772Z Learning: User buddhika75 prefers code reviews to focus on PR-specific changes rather than flagging unrelated existing code issues. They work under deadlines and want reviews scoped to the actual changes being made for the feature, not comprehensive audits of untouched existing code. Learnt from: buddhika75 Repo: hmislk/hmis PR: 11765 File: src/main/java/com/divudi/core/data/IncomeBundle.java:48-51 Timestamp: 2025-04-17T21:06:42.948Z Learning: buddhika75 appreciates thorough code reviews that identify subtle issues like missing fields in utility methods (resetTotals, aggregation methods). The user specifically requested continued detailed code reviews in the future. 📚 Learning: 2025-05-31T09:22:42.659Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 12640 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1072-1075 Timestamp: 2025-05-31T09:22:42.659Z Learning: In PharmacyDirectPurchaseController.java, the zero-quantity validation that was previously checked at the controller level (billItemsTotalQty == 0.0) has been moved to the BillItemFinanceDetails entity level as part of the financial calculation refactoring. The validation is now handled by the bill item finance details entity rather than explicitly in the controller. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-06-04T01:24:39.430Z
Learnt from: DARKDRAGON-LK Repo: hmislk/hmis PR: 12693 File: src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java:0-0 Timestamp: 2025-06-04T01:24:39.430Z Learning: In PharmacySaleBhtController.java, an updateBillTotals method was temporarily added for testing Bill Totals functionality but was accidentally left in the code. The method duplicated functionality from the existing deprecated updateMargin method and was never called. This was later removed in commit 88013af88688c528397c777c2ab61dce884321c4. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-09-04T01:21:19.151Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15280 File: src/main/java/com/divudi/bean/pharmacy/PharmacyBillSearch.java:2591-2654 Timestamp: 2025-09-04T01:21:19.151Z Learning: In PharmacyBillSearch.java, the reversePharmacyDirectIssueToBht() method is designed as a generic admin utility for reversing any bill type in error scenarios, not just direct inpatient issues. It preserves the original bill's BillTypeAtomic to maintain proper typing for the reversal, and is intended for rare use by administrators when correcting system errors. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-08-19T04:37:07.435Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15012 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDonationController.java:170-195 Timestamp: 2025-08-19T04:37:07.435Z Learning: In PharmacyDonationController.java, buddhika75 confirmed that the BillFinanceDetails population issue in the calculateBillTotalsFromItems() method will be handled separately in a donation-related issue, preferring to keep the current PR focused on the main donation feature implementation. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-09-27T23:19:04.896Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15628 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1056-1069 Timestamp: 2025-09-27T23:19:04.896Z Learning: In PharmacyDirectPurchaseController.java bill number generation logic, buddhika75 chose to defer the legacy behavior preservation fix (avoiding double counter consumption when configuration flags are false) to a separate issue rather than addressing it in PR #15628, following their pattern of keeping PRs focused on primary objectives. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-05-31T23:44:43.825Z
Learnt from: DARKDRAGON-LK Repo: hmislk/hmis PR: 12631 File: src/main/java/com/divudi/bean/pharmacy/PharmacyRequestForBhtController.java:918-918 Timestamp: 2025-05-31T23:44:43.825Z Learning: In the PharmacyRequestForBhtController class, comment handling (bill.setComments(comment) and comment = "") is only required in the settleBhtIssueRequest(...) method. The other settlement methods settleEditedBhtIssueRequest(...) and settleBhtIssueRequestAccept(...) intentionally do not require comment functionality as per business requirements. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-08-19T04:39:11.840Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15012 File: src/main/webapp/resources/pharmacy/pharmacy_donation_receipt.xhtml:126-159 Timestamp: 2025-08-19T04:39:11.840Z Learning: In src/main/webapp/resources/pharmacy/pharmacy_donation_receipt.xhtml and PharmacyDonationController.java, buddhika75 confirmed that the BillFinanceDetails population issue (empty summary values in donation receipts due to unpopulated totalCostValue, totalExpense, totalTaxValue fields) will be handled separately in a donation-related issue, preferring to keep the current PR focused on the main donation feature implementation. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-05-31T05:42:46.053Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 12635 File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:1349-1349 Timestamp: 2025-05-31T05:42:46.053Z Learning: In PharmacyDirectPurchaseController.java, the financial calculation follows a multi-level approach: line-level calculations (gross → discount → net), then bill-level calculations where grossTotal at bill level equals the sum of lineNetTotal values (not lineGrossTotal), preventing duplication of line-level discounts. Bill-level adjustments (billDiscount, billTax, billExpense) are then applied to this grossTotal to calculate the final netTotal, and these bill-level values are redistributed proportionally back to items. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-08-07T16:47:52.912Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 14779 File: src/main/java/com/divudi/bean/pharmacy/PharmacyIssueController.java:464-480 Timestamp: 2025-08-07T16:47:52.912Z Learning: In the HMIS PharmacyIssueController, validation for from/to departments in bill number generation is already handled earlier in the workflow (in settleDisposalIssueBill() and other entry points), so additional validation checks in savePreBillFinally() are considered redundant by buddhika75. The system follows a pattern of validating once at entry points rather than multiple defensive checks throughout the flow. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-08-25T21:27:52.085Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15113 File: src/main/java/com/divudi/bean/pharmacy/PurchaseOrderController.java:496-541 Timestamp: 2025-08-25T21:27:52.085Z Learning: In PurchaseOrderController.java, buddhika75 removed manual BillItemFinanceDetails creation that was implemented based on AI suggestions, preferring to rely on the getter's built-in lazy initialization pattern. This demonstrates the importance of verifying AI suggestions against existing codebase patterns rather than implementing defensive code that duplicates existing functionality. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-08-28T06:32:37.427Z
Learnt from: damithdeshan98 Repo: hmislk/hmis PR: 15190 File: src/main/java/com/divudi/bean/lab/PatientReportController.java:1605-1620 Timestamp: 2025-08-28T06:32:37.427Z Learning: In the HMIS system, patient reports cannot be created without a bill, so Bill objects are guaranteed to be non-null when accessed from PatientReport contexts due to business rule constraints. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-05-29T22:31:52.551Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 12600 File: src/main/java/com/divudi/core/data/IncomeBundle.java:0-0 Timestamp: 2025-05-29T22:31:52.551Z Learning: In the HMIS pharmacy system, Bill Items and Items serve different purposes: Bill Items are transaction entities generated frequently by cashiers for patient visits, while Items are master data entities configured by system administrators. Although methods like generateRetailAndCostDetailsForPharmaceuticalBillItems() and generateRetailAndCostDetailsForByItems() may appear similar, they implement different grouping logics for these distinct entity types. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-05-23T05:52:57.369Z
Learnt from: KithminiOKD Repo: hmislk/hmis PR: 12432 File: src/main/java/com/divudi/bean/common/ExcelController.java:1331-1331 Timestamp: 2025-05-23T05:52:57.369Z Learning: In the ExcelController class, the addDataToExcelForpharmacyServiceCancellations method uses getBill().getInsId() instead of getDeptId() to ensure consistency with the UI which displays insId in the corresponding column. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-08-31T02:08:36.806Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15225 File: src/main/java/com/divudi/bean/pharmacy/GrnReturnApprovalController.java:67-96 Timestamp: 2025-08-31T02:08:36.806Z Learning: In the HMIS project, PharmaceuticalBillItem entities are persisted automatically through cascade persistence when their parent BillItem is persisted, so manual PharmaceuticalBillItem facade operations (create/edit) are unnecessary and should be avoided in favor of letting the cascade handle the persistence. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.javasrc/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-08-07T17:51:43.976Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 14779 File: src/main/java/com/divudi/bean/pharmacy/PharmacyIssueController.java:1548-1552 Timestamp: 2025-08-07T17:51:43.976Z Learning: In PharmacyIssueController.java, the formatBillNumber() method doesn't need null checks for departments or department codes because all calling methods (generateBillNumberForFromDepartment, generateBillNumberForToDepartment, generateBillNumberForFromAndToDepartmentsCorrected) already validate that departments are not null before calling the downstream methods. This follows the HMIS pattern of handling validation upstream rather than adding redundant checks. Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java
📚 Learning: 2025-10-05T02:22:51.361Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15729 File: src/main/java/com/divudi/bean/pharmacy/PharmacyController.java:2160-2170 Timestamp: 2025-10-05T02:22:51.361Z Learning: In the HMIS Java codebase, com.divudi.core.entity.Bill implements lazy initialization in getBillFinanceDetails(), so this getter never returns null. Code reviews should not suggest adding null guards around b.getBillFinanceDetails() usages (e.g., in src/main/java/com/divudi/bean/pharmacy/PharmacyController.java and similar controllers). Applied to files:
src/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-07-03T02:50:06.769Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 13628 File: src/main/java/com/divudi/service/pharmacy/PharmacyCostingService.java:39-40 Timestamp: 2025-07-03T02:50:06.769Z Learning: In src/main/java/com/divudi/service/pharmacy/PharmacyCostingService.java, buddhika75 considers the Double vs double precision issue for monetary calculations to be unrelated to PR #13628 which focuses on bill department ID bulk editor implementation, confirming their preference to keep PRs focused on specific objectives. Applied to files:
src/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-05-22T22:22:28.337Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 12389 File: src/main/java/com/divudi/core/entity/BillFinanceDetails.java:33-45 Timestamp: 2025-05-22T22:22:28.337Z Learning: In BillFinanceDetails, BillItemFinanceDetails, and similar entity classes, the team has agreed to gradually remove eager initialization of BigDecimal fields with BigDecimal.ZERO, allowing them to be null by default. This change will be implemented incrementally with careful addition of null checks wherever these fields are used, as it affects multiple places in the codebase. Applied to files:
src/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-10-05T02:22:51.361Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 15729 File: src/main/java/com/divudi/bean/pharmacy/PharmacyController.java:2160-2170 Timestamp: 2025-10-05T02:22:51.361Z Learning: The same lazy-initialization pattern applies to finance/detail getters such as BillItem.getBillItemFinanceDetails(). Review feedback should avoid recommending null checks on these getters, while it remains acceptable to guard their inner BigDecimal fields when necessary per the team’s policy of allowing null BigDecimals. Applied to files:
src/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-04-18T10:12:48.474Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 11779 File: src/main/java/com/divudi/bean/pharmacy/PharmacyBillSearch.java:1699-1751 Timestamp: 2025-04-18T10:12:48.474Z Learning: When cancelling pharmacy bills, new BillItem and PharmaceuticalBillItem entities should be created with proper references to the cancellation bill, rather than modifying existing entities, to maintain data integrity and proper audit trails. Applied to files:
src/main/java/com/divudi/service/BillService.java
📚 Learning: 2025-06-27T02:04:07.826Z
Learnt from: buddhika75 Repo: hmislk/hmis PR: 13408 File: src/main/java/com/divudi/bean/pharmacy/TransferIssueController.java:0-0 Timestamp: 2025-06-27T02:04:07.826Z Learning: In the HMIS project, BillItem's getBillItemFinanceDetails() getter method handles null initialization internally using lazy initialization pattern, so explicit null checks before calling this getter are not necessary. Applied to files:
src/main/java/com/divudi/service/BillService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: validate-compilation
🔇 Additional comments (1)
src/main/java/com/divudi/bean/pharmacy/PharmacySaleBhtController.java (1)
1180-1185: Wiring of inpatient direct-issue finance generation looks correctCalling
billService.createBillFinancialDetailsForInpatientDirectIssueBill(getPreBill())immediately aftersavePreBillItemsFinally*()(and after reusingsettleBhtIssueRequestAccept’s existing hook) keeps the finance-detail generation aligned with the point where bill items and stock deductions are finalized. I don’t see sequencing or lifecycle issues here.Also applies to: 1224-1228, 1441-1444
Comment on lines +3171 to +3251
| // Get rates from PharmaceuticalBillItem (includes margins for net values) | ||
| BigDecimal retailRate = BigDecimal.valueOf(pharmaItem.getRetailRate()); | ||
| BigDecimal purchaseRate = BigDecimal.valueOf(pharmaItem.getPurchaseRate()); | ||
| BigDecimal wholesaleRate = BigDecimal.valueOf(pharmaItem.getWholesaleRate()); | ||
| | ||
| // Get costing rates from ItemBatch (for accurate stock valuation) | ||
| BigDecimal batchRetailRate = BigDecimal.valueOf( | ||
| pharmaItem.getStock().getItemBatch().getRetailsaleRate() != null | ||
| ? pharmaItem.getStock().getItemBatch().getRetailsaleRate() : 0.0); | ||
| BigDecimal batchPurchaseRate = BigDecimal.valueOf( | ||
| pharmaItem.getStock().getItemBatch().getPurcahseRate() != null | ||
| ? pharmaItem.getStock().getItemBatch().getPurcahseRate() : 0.0); | ||
| BigDecimal batchWholesaleRate = BigDecimal.valueOf( | ||
| pharmaItem.getStock().getItemBatch().getWholesaleRate() != null | ||
| ? pharmaItem.getStock().getItemBatch().getWholesaleRate() : 0.0); | ||
| | ||
| // Get cost rate from ItemBatch with fallback to purchase rate | ||
| BigDecimal costRate = batchPurchaseRate; | ||
| if (pharmaItem.getStock().getItemBatch().getCostRate() != null | ||
| && pharmaItem.getStock().getItemBatch().getCostRate() > 0) { | ||
| costRate = BigDecimal.valueOf(pharmaItem.getStock().getItemBatch().getCostRate()); | ||
| } | ||
| | ||
| // Get or create BillItemFinanceDetails (auto-created via getter if null) | ||
| BillItemFinanceDetails bifd = billItem.getBillItemFinanceDetails(); | ||
| | ||
| // SET RATE FIELDS in BillItemFinanceDetails (from pharmaItem - includes margins) | ||
| bifd.setLineNetRate(BigDecimal.valueOf(billItem.getNetRate())); | ||
| bifd.setGrossRate(BigDecimal.valueOf(billItem.getRate())); | ||
| bifd.setLineGrossRate(BigDecimal.valueOf(billItem.getRate())); | ||
| bifd.setBillCostRate(BigDecimal.ZERO); | ||
| bifd.setTotalCostRate(costRate); | ||
| bifd.setLineCostRate(costRate); | ||
| bifd.setCostRate(costRate); | ||
| bifd.setPurchaseRate(purchaseRate); | ||
| bifd.setRetailSaleRate(retailRate); | ||
| bifd.setWholesaleRate(wholesaleRate); | ||
| | ||
| // SET TOTAL FIELDS in BillItemFinanceDetails | ||
| bifd.setLineGrossTotal(BigDecimal.valueOf(billItem.getGrossValue())); | ||
| bifd.setGrossTotal(BigDecimal.valueOf(billItem.getGrossValue())); | ||
| bifd.setLineNetTotal(BigDecimal.valueOf(billItem.getNetValue())); | ||
| bifd.setNetTotal(BigDecimal.valueOf(billItem.getNetValue())); | ||
| | ||
| // Calculate item values using batch rates for accurate costing | ||
| BigDecimal itemCostValue = costRate.multiply(qty); | ||
| BigDecimal itemRetailValue = batchRetailRate.multiply(totalQty); | ||
| BigDecimal itemPurchaseValue = batchPurchaseRate.multiply(totalQty); | ||
| BigDecimal itemWholesaleValue = batchWholesaleRate.multiply(totalQty); | ||
| | ||
| // SET COST VALUES in BillItemFinanceDetails | ||
| bifd.setLineCost(itemCostValue); | ||
| bifd.setBillCost(BigDecimal.ZERO); | ||
| bifd.setTotalCost(itemCostValue); | ||
| | ||
| // SET VALUE FIELDS (NEGATIVE for stock going out - issue to patient) | ||
| bifd.setValueAtCostRate(costRate.multiply(totalQty).negate()); | ||
| bifd.setValueAtPurchaseRate(batchPurchaseRate.multiply(totalQty).negate()); | ||
| bifd.setValueAtRetailRate(batchRetailRate.multiply(totalQty).negate()); | ||
| bifd.setValueAtWholesaleRate(batchWholesaleRate.multiply(totalQty).negate()); | ||
| | ||
| // SET QUANTITIES (NEGATIVE for stock going out) | ||
| bifd.setQuantity(qty.negate()); | ||
| bifd.setQuantityByUnits(qty.negate()); | ||
| bifd.setTotalQuantity(totalQty.negate()); | ||
| bifd.setFreeQuantity(freeQty.negate()); | ||
| | ||
| // UPDATE PHARMACEUTICAL BILL ITEM VALUES | ||
| pharmaItem.setCostRate(costRate.doubleValue()); | ||
| pharmaItem.setCostValue(itemCostValue.doubleValue()); | ||
| pharmaItem.setRetailValue(itemRetailValue.doubleValue()); | ||
| pharmaItem.setPurchaseValue(itemPurchaseValue.doubleValue()); | ||
| | ||
| // Accumulate bill-level totals | ||
| totalCostValue = totalCostValue.add(itemCostValue); | ||
| totalPurchaseValue = totalPurchaseValue.add(itemPurchaseValue); | ||
| totalRetailSaleValue = totalRetailSaleValue.add(itemRetailValue); | ||
| totalWholesaleValue = totalWholesaleValue.add(itemWholesaleValue); | ||
| totalQuantity = totalQuantity.add(qty); | ||
| totalFreeQuantity = totalFreeQuantity.add(freeQty); | ||
| |
Contributor
There was a problem hiding this comment.
Cost value aggregation ignores free quantity at bill level (BFD vs BIFD inconsistency)
Within createBillFinancialDetailsForInpatientDirectIssueBill(...):
- Item-level cost in
BillItemFinanceDetailsuses totalQty (qty + freeQty):
bifd.setValueAtCostRate(costRate.multiply(totalQty).negate()); - Bill-level
totalCostValueinBillFinanceDetailsaggregatesitemCostValuecomputed as costRate * qty only:
BigDecimal itemCostValue = costRate.multiply(qty);→ accumulated intototalCostValue→bfd.setTotalCostValue(totalCostValue.negate());
So when freeQty > 0, the sum of bifd.getValueAtCostRate() over items will differ from bfd.getTotalCostValue() by costRate * freeQty per item. That breaks the expectation that bill-level cost totals equal the aggregation of item-level values and undercounts stock-out cost whenever free units are issued.
Unless you deliberately want bill-level cost to exclude free quantities, you probably want itemCostValue to use totalQty (as you already do for purchase/retail/wholesale values).
Proposed fix: include freeQty in bill-level cost totals
- BigDecimal itemCostValue = costRate.multiply(qty); + BigDecimal itemCostValue = costRate.multiply(totalQty); @@ - totalCostValue = totalCostValue.add(itemCostValue); + totalCostValue = totalCostValue.add(itemCostValue);Also applies to: 3269-3274
🤖 Prompt for AI Agents
In src/main/java/com/divudi/service/BillService.java around lines 3171-3251 (and similarly 3269-3274), the bill-level cost aggregation uses itemCostValue = costRate.multiply(qty) which excludes freeQty while item-level BIFD values use totalQty (qty + freeQty), causing a mismatch; change the cost computations and accumulations to use totalQty (or the existing totalQty variable) for itemCostValue, for setting pharmaItem.setCostValue, and when adding into totalCostValue so bill-level totals include free quantities and match the sum of BIFD values. Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Data Flow
When an inpatient direct issue bill is settled:
settleBhtIssue() / settleBhtIssueRequest() / settleBhtIssueRequestAccept()
↓
savePreBillFinally() - Creates Bill
↓
savePreBillItemsFinally() - Creates BillItems, deducts stock
↓
billService.createBillFinancialDetailsForInpatientDirectIssueBill()
↓
For each BillItem:
├── Create/populate BillItemFinanceDetails
│ ├── Rates (from pharmaItem)
│ ├── Costing values (from itemBatch, negative)
│ └── Quantities (negative)
├── Update PharmaceuticalBillItem values
└── Persist BillItem
↓
Create/populate BillFinanceDetails (aggregated totals, negative)
↓
Persist Bill
The implementation is now complete and ready for testing.
Closes #17500
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.