#14984 stock transfer #16271
Merged
#14984 stock transfer #16271
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
… #14984_stock_transfer
Contributor
WalkthroughRefactors stock-transfer reporting to accept multiple BillTypeAtomic values, adds Good‑In‑Transit calculations, restructures aggregation into department-centric nested summaries, extends PharmacySummery with purchase/cost/retail fields and department links, and replaces the stock transfer UI with an expandable, componentized report view. Changes
Sequence DiagramsequenceDiagram
participant UI as Stock Transfer UI
participant Ctrl as PharmacyController
participant Repo as Bill Repository
participant Map as Dept Summary Map
participant GIT as GIT Calculator
UI->>Ctrl: createStockTransferReport(transferType)
Ctrl->>Ctrl: build List<BillTypeAtomic> btAtomics
Ctrl->>Ctrl: generateReportAsSummary(btAtomics)
Ctrl->>Repo: query bills WHERE billTypeAtomic IN :btAtomics
Repo-->>Ctrl: bill records
Ctrl->>Map: aggregate by department and direction\n(build nested detail maps)
Ctrl->>GIT: calculateGoodInTransitAmounts(btAtomics)
GIT->>Repo: query GIT amounts across departments
Repo-->>GIT: GIT results
GIT->>Map: update PharmacySummery entries with GIT
Ctrl->>Ctrl: compute final totals row
Ctrl-->>UI: render summary (expandable details + totals + GIT)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.39.6)src/main/java/com/divudi/bean/pharmacy/PharmacyController.javaThanks 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 |
Closed
3 tasks
![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 (6)
src/main/java/com/divudi/core/data/dataStructure/PharmacySummery.java (2)
55-75: Consider defensive initialization for BigDecimal parameters.The new constructors accept BigDecimal parameters but don't guard against null values. If null values are passed, subsequent arithmetic operations will fail with NPEs. Consider either:
- Initializing null BigDecimal parameters to
BigDecimal.ZEROin constructors, or- Adding explicit validation to reject null values
Example defensive pattern:
public PharmacySummery(String departmentName, BigDecimal totalPurchaseValue, BigDecimal totalCostValue, BigDecimal totalRetailSaleValue) { this.departmentName = departmentName; - this.totalPurchaseValue = totalPurchaseValue; - this.totalCostValue = totalCostValue; - this.totalRetailSaleValue = totalRetailSaleValue; + this.totalPurchaseValue = totalPurchaseValue != null ? totalPurchaseValue : BigDecimal.ZERO; + this.totalCostValue = totalCostValue != null ? totalCostValue : BigDecimal.ZERO; + this.totalRetailSaleValue = totalRetailSaleValue != null ? totalRetailSaleValue : BigDecimal.ZERO; }
167-213: Accessor methods look correct.The getter and setter implementations follow standard patterns. However, consider adding JavaDoc to document:
- The purpose of
summeriesMapand what the String key represents (e.g., department ID, category name)- The structure and expected content of the nested
List<PharmacySummery>- The difference between
issuedDeptNameandreceivedDeptNamein the context of stock transfersThis would help developers understand how to populate and consume these fields correctly.
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java (3)
4182-4337: Summary query: guard against department-name collisions; consider completed filter
- Grouping by b.department.name / b.fromDepartment.name / b.toDepartment.name can merge different departments that share a name across institutions and hinders indexing. Prefer grouping by IDs and selecting names via MIN/MAX or by post-processing, e.g., GROUP BY b.department.id, …, then map to names.
- Many reporting queries use completed=true; consider aligning here if business rules require excluding incomplete bills.
Would you like me to draft a version grouping by IDs and verify whether completed=true should apply to this report? Based on learnings (BillTypeAtomic appropriate for specific filters).
4343-4395: GIT calculation clarifications and minor JPQL refinements
- Readability: prefer collection emptiness over SIZE(...)=0 in the CASE predicate:
- Use b.forwardReferenceBills IS EMPTY.
- Semantics: GIT is currently summed using totalRetailSaleValue. Confirm that retail is the intended basis (vs purchase or cost) for “in transit”.
- Totals: consider also populating a GIT grand total row, mirroring department totals.
I can update the JPQL and add a small accumulation for a GIT total row if retail is confirmed as the correct measure.
6657-6666: Large result ORDER BY createdAt — ensure supporting indexesYou added ORDER BY bi.bill.createdAt DESC with a 1000-row cap. On large datasets, ensure an index exists on Bill.createdAt (and Bill.department if filtered) to avoid heavy sorts.
I can list candidate indexes (DDL) for Bill(createdAt), Bill(department, createdAt) if helpful.
src/main/webapp/reports/inventoryReports/stock_transfer_report.xhtml (1)
355-482: Follow JSF heading guidelinesPer the repository’s XHTML guidelines we should avoid raw heading tags. Please swap the
<h5>blocks forh:panelGroup/h:outputText(or similar JSF components) so we stay consistent with JSF rendering and localization.- <h5> - <h:outputText value="..."/> - </h5> + <h:panelGroup styleClass="report-heading"> + <h:outputText value="..."/> + </h:panelGroup>Repeat for the other
<h5>instances. As per coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java (1)
3680-3690: Receive atomics now populated — resolves empty summary.Including PHARMACY_RECEIVE and PHARMACY_RECEIVE_CANCELLED fixes the prior receive-summary early-return gap. LGTM.
Also applies to: 3693-3693
🧹 Nitpick comments (2)
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java (2)
4184-4216: Avoid grouping by names to prevent cross-institution collisions.Group by department entities or IDs (plus names if ONLY_FULL_GROUP_BY requires) to avoid merging departments with identical names across institutions. Example: GROUP BY b.department, b.fromDepartment, b.toDepartment (or their IDs).
4339-4342: Prefer logger over printStackTrace for server code.Replace printStackTrace with structured logging for consistency and production hygiene.
- } catch (Exception e) { - e.printStackTrace(); - JsfUtil.addErrorMessage("Error generating summary report: " + e.getMessage()); - } + } catch (Exception e) { + Logger.getLogger(PharmacyController.class.getName()).log(Level.SEVERE, "Error generating summary report", e); + JsfUtil.addErrorMessage("Error generating summary report: " + e.getMessage()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java(4 hunks)
🧰 Additional context used
🧠 Learnings (46)
📓 Common learnings
Learnt from: buddhika75
Repo: hmislk/hmis PR: 0
File: :0-0
Timestamp: 2025-09-01T22:35:24.496Z
Learning: buddhika75 in PR #15249 implemented comprehensive privilege security fixes for pharmacy stock adjustment functionality, including dual privilege requirements (PharmacyAdjustmentDepartmentStockBySingleItemQTY AND PharmacyStockAdjustmentSingleItem), UI access control with unauthorized messages, and proper privilege tree structure in UserPrivilageController.java. Demonstrates systematic security implementation and thorough cross-validation.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 0
File: :0-0
Timestamp: 2025-09-01T22:35:24.496Z
Learning: buddhika75 successfully implemented comprehensive dual-privilege security for pharmacy stock adjustment functionality in PR #15249. The implementation includes proper privilege tree structure, dual privilege requirements (PharmacyAdjustmentDepartmentStockBySingleItemQTY AND PharmacyStockAdjustmentSingleItem), unauthorized access prevention with clear messages, complete UI gating, and deep-link protection across all affected XHTML files.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14791
File: pr_description_substitute_functionality.md:69-74
Timestamp: 2025-08-08T13:42:59.349Z
Learning: In PR hmislk/hmis#14791, buddhika75 decided to defer adding a transactional boundary for replaceSelectedSubstitute() (and related flows) to a future change. If a follow-up issue is created, assign it to buddhika75 and keep this PR focused on substitute UX/validation improvements.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 0
File: :0-0
Timestamp: 2025-08-31T18:33:43.772Z
Learning: buddhika75 in PharmacyStockTakeController.java demonstrates excellent systematic fix implementation, including JavaScript error resolution in PrimeFaces.monitorDownload() calls, UI enhancement with meaningful tooltips, Bootstrap class cleanup with inline CSS, and comprehensive security privilege checks for pharmacy operations. Shows strong attention to review feedback and production readiness.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14093
File: src/main/java/com/divudi/service/pharmacy/StockSearchService.java:25-59
Timestamp: 2025-07-22T22:39:10.763Z
Learning: buddhika75 considers StockSearchService functionality as very critical since issues with it can affect stocks, showing their preference to handle refactoring suggestions for critical business functionality as separate issues rather than in the current PR.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15798
File: src/main/java/com/divudi/bean/clinic/ClinicController.java:5583-5584
Timestamp: 2025-10-10T14:10:03.618Z
Learning: For hmislk/hmis, when a fix is deferred, buddhika75 prefers a separate GitHub issue to track it and to be assigned to themselves; e.g., create an issue for ClinicController single-payment Staff_Welfare handling gap and link back to the PR/comment.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12343
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:2406-2407
Timestamp: 2025-05-16T08:24:49.604Z
Learning: The processClosingStockForItemReport() method in PharmacyReportController now groups by `sh.department, sh.itemBatch` instead of `sh.department, sh.itemBatch.item` to provide more granular batch-level detail in the report.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12343
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:2089-2092
Timestamp: 2025-05-16T08:24:15.589Z
Learning: The PharmacyReportController's closing stock reports should be able to retrieve data from all departments when needed, not just from pharmacy departments. That's why filtering by departmentType.Pharmacy was removed from the processClosingStockForBatchReport() and processClosingStockForItemReport() methods.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15012
File: src/main/java/com/divudi/bean/pharmacy/PharmacyProcurementReportController.java:278-296
Timestamp: 2025-08-19T04:39:09.055Z
Learning: In PharmacyProcurementReportController.java, buddhika75 confirmed that the date defaults usage, department filter alignment, and BillLight constructor verification issues in the generateDonationBillList() 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: 13503
File: src/main/java/com/divudi/core/data/BillTypeAtomic.java:144-147
Timestamp: 2025-06-29T18:28:55.699Z
Learning: The user buddhika75 considers updating the getBillTypeAtomic method in BillTypeAtomic.java to use PHARMACY_GRN_RETURN instead of deprecated PHARMACY_GRN_REFUND as "too complicated" for the current PR context and prefers to handle it separately through discussion.
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: ImeshRanawellaSG
Repo: hmislk/hmis PR: 13229
File: src/main/webapp/reports/inventoryReports/stock_transfer_report.xhtml:374-379
Timestamp: 2025-06-19T09:28:06.217Z
Learning: In the HMIS codebase stock transfer reports, the term "Good In Transist Amount" is intentionally used as-is, and should not be changed to "Good In Transit Amount" as it aligns with the backend property naming convention.
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: 13549
File: src/main/java/com/divudi/bean/pharmacy/PharmacyController.java:4104-4112
Timestamp: 2025-06-30T23:14:49.027Z
Learning: In the HMIS pharmacy system, BillType and BillTypeAtomic serve different purposes: BillType (like BillType.PharmacyOrderApprove) is useful when working with bill categories such as billed bills, cancelled bills, or refund bills, while BillTypeAtomic (like BillTypeAtomic.PHARMACY_ORDER_APPROVAL) is more appropriate for specific filtering contexts. Both have valid use cases and are not necessarily legacy.
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: 14459
File: src/main/webapp/resources/sql/FixTrnasferBills.sql:51-67
Timestamp: 2025-08-02T04:29:19.341Z
Learning: The FixTrnasferBills.sql script in src/main/webapp/resources/sql/ is a one-time migration function intended for temporary use to fix pharmacy transfer bill finance details, so suggestions for improvements like error handling, data integrity enhancements, or other refinements should be ignored for this script.
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.
📚 Learning: 2025-06-07T00:31:33.358Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12749
File: src/main/java/com/divudi/bean/common/SearchController.java:17253-17254
Timestamp: 2025-06-07T00:31:33.358Z
Learning: In src/main/java/com/divudi/bean/common/SearchController.java, PHARMACY_GRN_RETURN should not be included in the pharmacyBillTypesAtomics list for generatePharmacyCollection(); its inclusion is not required for this context.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-05-16T08:24:49.604Z
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12343
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:2406-2407
Timestamp: 2025-05-16T08:24:49.604Z
Learning: The processClosingStockForItemReport() method in PharmacyReportController now groups by `sh.department, sh.itemBatch` instead of `sh.department, sh.itemBatch.item` to provide more granular batch-level detail in the report.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-19T04:39:09.055Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15012
File: src/main/java/com/divudi/bean/pharmacy/PharmacyProcurementReportController.java:278-296
Timestamp: 2025-08-19T04:39:09.055Z
Learning: In PharmacyProcurementReportController.java, buddhika75 confirmed that the date defaults usage, department filter alignment, and BillLight constructor verification issues in the generateDonationBillList() 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/PharmacyController.java
📚 Learning: 2025-05-16T08:24:15.589Z
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12343
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:2089-2092
Timestamp: 2025-05-16T08:24:15.589Z
Learning: The PharmacyReportController's closing stock reports should be able to retrieve data from all departments when needed, not just from pharmacy departments. That's why filtering by departmentType.Pharmacy was removed from the processClosingStockForBatchReport() and processClosingStockForItemReport() methods.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-06-29T18:28:55.699Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 13503
File: src/main/java/com/divudi/core/data/BillTypeAtomic.java:144-147
Timestamp: 2025-06-29T18:28:55.699Z
Learning: The user buddhika75 considers updating the getBillTypeAtomic method in BillTypeAtomic.java to use PHARMACY_GRN_RETURN instead of deprecated PHARMACY_GRN_REFUND as "too complicated" for the current PR context and prefers to handle it separately through discussion.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-19T04:38:27.422Z
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.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 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/PharmacyController.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/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-10-09T10:16:17.416Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15793
File: src/main/java/com/divudi/bean/pharmacy/TransferIssueCancellationController.java:381-439
Timestamp: 2025-10-09T10:16:17.416Z
Learning: In the HMIS Java codebase, BillItem.getPharmaceuticalBillItem() implements lazy initialization and never returns null. The getter creates a new PharmaceuticalBillItem instance if one doesn't exist. Code reviews should not suggest adding null guards around pharmaceuticalBillItem obtained via this getter (e.g., in src/main/java/com/divudi/bean/pharmacy/TransferIssueCancellationController.java and similar controllers).
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-09T15:23:34.848Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14805
File: src/main/java/com/divudi/bean/pharmacy/GrnController.java:807-817
Timestamp: 2025-08-09T15:23:34.848Z
Learning: In GrnController's settleWholesale() method, the approveBill is guaranteed to be valid (non-null with proper ID and type) by the business flow and navigation logic, so additional defensive checks for getApproveBill().getId() != null and getBillTypeAtomic() == BillTypeAtomic.PHARMACY_ORDER are unnecessary according to buddhika75.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-06-02T11:30:02.679Z
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12668
File: src/main/java/com/divudi/bean/common/ReportsController.java:3586-3598
Timestamp: 2025-06-02T11:30:02.679Z
Learning: In `src/main/java/com/divudi/bean/common/ReportsController.java`, within the `removeNonDues()` method, the filtering logic for cancelled and refund bills in the OP visit section is intentionally different from the IP logic. For OP visits, removing rows when the balance is greater than or equal to zero is correct for this specific location, as per business requirements. This exception should be remembered for future reviews.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-06-30T23:14:49.027Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 13549
File: src/main/java/com/divudi/bean/pharmacy/PharmacyController.java:4104-4112
Timestamp: 2025-06-30T23:14:49.027Z
Learning: In the HMIS pharmacy system, BillType and BillTypeAtomic serve different purposes: BillType (like BillType.PharmacyOrderApprove) is useful when working with bill categories such as billed bills, cancelled bills, or refund bills, while BillTypeAtomic (like BillTypeAtomic.PHARMACY_ORDER_APPROVAL) is more appropriate for specific filtering contexts. Both have valid use cases and are not necessarily legacy.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-07-27T16:17:25.233Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14264
File: src/main/java/com/divudi/service/BillService.java:1186-1201
Timestamp: 2025-07-27T16:17:25.233Z
Learning: In the HMIS project, the fetchOpdIncomeReportDTOs method in BillService allows billTypeAtomics parameter to be null or empty according to business requirements, meaning the method should return all bill types when no specific bill types are specified.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-07-26T06:20:28.022Z
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.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-06-29T20:11:51.401Z
Learnt from: DARKDRAGON-LK
Repo: hmislk/hmis PR: 13506
File: src/main/java/com/divudi/bean/opd/OpdReportController.java:823-823
Timestamp: 2025-06-29T20:11:51.401Z
Learning: In the HMIS system, FundTransfer is not a PaymentMethod enum value. Fund transfers are handled as specific bill types (BillTypeAtomic.FUND_TRANSFER_RECEIVED_BILL, BillTypeAtomic.FUND_TRANSFER_BILL, etc.) and are processed separately from payment methods. They are processed at the bill level in methods like genarateRowBundleOther rather than through individual bill item payment methods like Cash, Card, and Credit.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-07-01T14:53:13.582Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 13577
File: src/main/java/com/divudi/bean/opd/OpdBillController.java:2218-2219
Timestamp: 2025-07-01T14:53:13.582Z
Learning: In the HMIS system, OPD Bills handle both investigations and services, and Inward Service Bills are also used for billing both investigations and services (despite the "Service" name being somewhat misleading). Therefore, the method `findOpdAndInpatientServiceAndInvestigationBillTypes()` in BillTypeAtomic is correctly named as it returns bill types that collectively handle both services and investigations for OPD and inpatient contexts.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-10-07T14:16:50.620Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15748
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:2090-2134
Timestamp: 2025-10-07T14:16:50.620Z
Learning: In the HMIS pharmacy domain, bills created with MultiplePaymentMethods never mix credit-type tenders with non-credit tenders. Credit and non-credit payment methods are not allowed to be combined during creation. Consequently, in PharmacyReportController.retrieveBillItems(List<BillTypeAtomic> btas, List<PaymentMethod> paymentMethods), the EXISTS check for MultiplePaymentMethods does not lead to double counting between GRN cash and GRN credit flows.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-06-29T20:10:41.659Z
Learnt from: DARKDRAGON-LK
Repo: hmislk/hmis PR: 13506
File: src/main/java/com/divudi/bean/opd/OpdReportController.java:658-658
Timestamp: 2025-06-29T20:10:41.659Z
Learning: The generateDailyLabSummaryByDepartment method in OpdReportController.java is designed specifically for daily reports, so the dataset is naturally constrained to a single day's transactions. Pagination is not necessary for such daily reporting methods due to the inherently limited data volume.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-07-25T22:22:21.503Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 14178
File: src/main/java/com/divudi/service/BillService.java:1587-1662
Timestamp: 2025-07-25T22:22:21.503Z
Learning: In the laboratory income summary report implementation (PR #14178), the fetchBills method in BillService.java intentionally hardcodes DepartmentType.Lab filter because it's specifically designed to fetch laboratory bills only, as confirmed by damithdeshan98. The toDepartment.departmentType filter should remain hardcoded to Lab for this use case.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-08-02T04:28:55.328Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14459
File: src/main/java/com/divudi/bean/common/DataAdministrationController.java:589-593
Timestamp: 2025-08-02T04:28:55.328Z
Learning: The migrateTransferBillItemFinanceDetails() method in DataAdministrationController.java is a one-time migration function intended for temporary use, so suggestions for improvements like correcting summary messages, error handling enhancements, or other refinements should be ignored for this method.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-30T14:59:38.647Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15211
File: src/main/java/com/divudi/bean/pharmacy/GrnReturnWorkflowController.java:320-352
Timestamp: 2025-08-30T14:59:38.647Z
Learning: In GrnReturnWorkflowController.java, BillItem's getPharmaceuticalBillItem() method uses lazy initialization pattern and will never return null, so explicit null checks for this getter are not necessary. This follows the same pattern as other HMIS getters like getBillFinanceDetails().
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-10-05T15:09:43.927Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15737
File: src/main/webapp/pharmacy/pharmacy_grn_return_request.xhtml:143-151
Timestamp: 2025-10-05T15:09:43.927Z
Learning: In `pharmacy_grn_return_request.xhtml`, when displaying GRNs in the search results table, `p.referenceBill` is always non-null by business logic, as every GRN has an associated Purchase Order (PO). Additional null-safety checks for `p.referenceBill` are unnecessary in this context.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-06-29T18:29:45.757Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 13503
File: src/main/java/com/divudi/core/data/dto/PharmacyGrnReturnItemDTO.java:13-18
Timestamp: 2025-06-29T18:29:45.757Z
Learning: In PharmacyGrnReturnItemDTO, the purchaseRate field is intentionally declared as Double (not BigDecimal) because this DTO constructor is used with JPQL queries where the target database field is double type. Using BigDecimal would cause JPQL errors, so the Double type is required despite the general preference for BigDecimal in monetary calculations.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-06-27T02:05:01.330Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 13408
File: src/main/java/com/divudi/bean/pharmacy/TransferIssueController.java:773-773
Timestamp: 2025-06-27T02:05:01.330Z
Learning: In the HMIS project, BillItem's getPharmaceuticalBillItem() 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/bean/pharmacy/PharmacyController.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/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-16T18:30:49.089Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14943
File: src/main/java/com/divudi/bean/pharmacy/PharmacySaleController.java:1679-1710
Timestamp: 2025-08-16T18:30:49.089Z
Learning: In PharmaceuticalBillItem (src/main/java/com/divudi/core/entity/pharmacy/PharmaceuticalBillItem.java), qtyInUnit/freeQtyInUnit getters/setters are deprecated aliases that directly read/write the same underlying fields as qty/freeQty. Using either pair yields the same quantity values in stock calculations.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-06-13T03:27:16.112Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12998
File: src/main/webapp/pharmacy/pharmacy_grn_costing.xhtml:143-150
Timestamp: 2025-06-13T03:27:16.112Z
Learning: In `pharmacy_grn_costing.xhtml`, `referanceBillItem` and its nested `pharmaceuticalBillItem` are always non-null by business logic, so additional null-safety checks in EL expressions for quantities are unnecessary.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-05-29T23:34:10.629Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12600
File: src/main/java/com/divudi/bean/pharmacy/StockController.java:407-426
Timestamp: 2025-05-29T23:34:10.629Z
Learning: In the HMIS codebase, billItemFacade.findDoubleByJpql() returns a primitive double value, not a nullable Double object. Some legacy null checks exist in the code but are not necessary for this method's implementation.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-17T19:45:08.792Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 14959
File: src/main/java/com/divudi/bean/pharmacy/PharmacyDirectPurchaseController.java:285-289
Timestamp: 2025-08-17T19:45:08.792Z
Learning: In PharmacyDirectPurchaseController.java, BillItem.qty should store quantities as entered by the user (not converted to quantityByUnits). This is the correct business logic as confirmed by buddhika75, who has verified the entered data is accurate.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/bean/pharmacy/PharmacyController.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/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-05-23T01:36:07.199Z
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.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-05-22T21:57:58.063Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12389
File: src/main/java/com/divudi/core/entity/BillItemFinanceDetails.java:0-0
Timestamp: 2025-05-22T21:57:58.063Z
Learning: In the BillItemFinanceDetails class and similar entities, BigDecimal fields should not be eagerly initialized with BigDecimal.ZERO. Instead, they should be left as null by default to distinguish between unset values and explicit zeros. This improves code clarity and reduces unnecessary storage usage.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-10-22T04:44:36.402Z
Learnt from: ChinthakaPrasad
Repo: hmislk/hmis PR: 16033
File: src/main/webapp/channel/income_for_scanning_bookings.xhtml:188-216
Timestamp: 2025-10-22T04:44:36.402Z
Learning: In `src/main/webapp/channel/income_for_scanning_bookings.xhtml`, within the scanning bookings income report table, `row.bill.singleBillItem` and `row.bill.singleBillSession` are guaranteed to be non-null by business logic for the rows displayed, so null-safety checks are not required when accessing nested properties like `singleBillItem.item.staff.person.nameWithTitle` or `singleBillSession.sessionInstance.name` in the Doctor, Session, and Item columns.
<!--
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
📚 Learning: 2025-08-20T12:05:22.703Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 15055
File: src/main/java/com/divudi/bean/common/CollectingCentrePaymentController.java:291-295
Timestamp: 2025-08-20T12:05:22.703Z
Learning: In CollectingCentrePaymentController.java, when BillLight objects are created via JPQL queries using bill.totalCenterFee and bill.totalHospitalFee, these values are guaranteed to be non-null because they originate from primitive double fields in the Bill entity, so null checking is not needed when negating getCcTotal() and getHospitalTotal() values from these specific BillLight objects.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.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/PharmacyController.java
📚 Learning: 2025-08-20T07:42:05.640Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 15049
File: src/main/java/com/divudi/bean/common/CollectingCentrePaymentController.java:500-509
Timestamp: 2025-08-20T07:42:05.640Z
Learning: In CollectingCentrePaymentController.java, damithdeshan98 confirmed that BillLight objects in the selectedCCpaymentBills list are created from JPQL queries using bill.createdAt which is guaranteed to be non-null due to database constraints, so null checking is not needed when formatting dates from these specific BillLight objects in the Excel export functionality. This is different from other parts of the codebase that may deal with different data sources where null checks are necessary.
Applied to files:
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java
🔇 Additional comments (2)
src/main/java/com/divudi/bean/pharmacy/PharmacyController.java (2)
6659-6659: Ordering pending GRNs by createdAt — OK.Improves UX without affecting logic.
6416-6417: Type mismatch between COALESCE result and constructor parameter.The JPQL query at line 6416 uses
COALESCE(b.billItemFinanceDetails.netTotal, 0), which returns BigDecimal (since netTotal is BigDecimal and gets promoted per Hibernate type rules). However, the matching 12-parameter constructor expects Double for billNetTotal (parameter 11). Although a BigDecimal variant of this constructor exists in the DTO, this type mismatch can cause Hibernate to fail constructor resolution or apply unexpected casting. Either:
- Update the query to use the BigDecimal constructor (which already exists in the DTO), or
- Explicitly cast
COALESCE(b.billItemFinanceDetails.netTotal, 0)to Double
buddhika75
approved these changes
Nov 3, 2025
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.
Summary by CodeRabbit
New Features
UI Improvements