Lab reports APIs #17116
Draft
Lab reports APIs #17116
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
Contributor
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (4)
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.java (1)
45-119: Consider makingfromPatientInvestigationsnull‑safe for future callersRight now
fromPatientInvestigationsassumespatientInvestigationsis non-null and callsstream()directly. In this PR, the caller always passes a non-null list, but making the method tolerant ofnullwould align with patterns used in other DTO mappers and avoid fragile usage later.Example refactor:
- public static List<ViewPatientInvestigationResponseDTO> fromPatientInvestigations(final List<PatientInvestigation> patientInvestigations) { - return patientInvestigations.stream() - .map(ViewPatientInvestigationResponseDTO::fromPatientInvestigation) - .collect(java.util.stream.Collectors.toList()); - } + public static List<ViewPatientInvestigationResponseDTO> fromPatientInvestigations( + final List<PatientInvestigation> patientInvestigations) { + if (patientInvestigations == null) { + return java.util.Collections.emptyList(); + } + return patientInvestigations.stream() + .map(ViewPatientInvestigationResponseDTO::fromPatientInvestigation) + .collect(java.util.stream.Collectors.toList()); + }src/main/java/com/divudi/ws/report/ReportApi.java (1)
137-167: API key validation logic is clear; consider aligning error payload formatsThe
isValidKeychecks (null, user null, retired, not activated, expired) are straightforward and defensive. Note that invalid keys return a JSON error body, while validation failures for dates/mobile return plain text. If clients expect consistent response shapes, you may want to return structured JSON there too, but this is non-blocking.src/main/java/com/divudi/ws/report/ViewReportsResponseDTO.java (1)
21-34: Harden constructor against null/empty lists and missing nested associationsThe constructor currently:
- Assumes
patientInvestigationsis non-null.- For non-empty lists, dereferences
getPatient().getPerson().getTitle().name()andgetSex().name()without guarding those nested associations.Given this is a public DTO and earlier patterns in this codebase favor defensive DTO construction, consider centralizing the “first investigation” handling and adding null checks, e.g.:
- public ViewReportsResponseDTO(String mobile, String fromDate, String toDate, List<PatientInvestigation> patientInvestigations) { - this.nic = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPerson().getNic() : null; + public ViewReportsResponseDTO(String mobile, String fromDate, String toDate, + List<PatientInvestigation> patientInvestigations) { + PatientInvestigation first = (patientInvestigations == null || patientInvestigations.isEmpty()) + ? null + : patientInvestigations.get(0); + + if (first != null && first.getPatient() != null && first.getPatient().getPerson() != null) { + this.nic = first.getPatient().getPerson().getNic(); + this.name = first.getPatient().getPerson().getName(); + this.phn = first.getPatient().getPhn(); + this.dateOfBirth = first.getPatient().getPerson().getDob(); + this.title = first.getPatient().getPerson().getTitle() != null + ? first.getPatient().getPerson().getTitle().name() + : null; + this.sex = first.getPatient().getPerson().getSex() != null + ? first.getPatient().getPerson().getSex().name() + : null; + this.bloodGroup = first.getPatient().getPerson().getBloodGroup() != null + ? first.getPatient().getPerson().getBloodGroup().getName() + : null; + } this.mobile = mobile; - this.name = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPerson().getName() : null; - this.phn = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPhn() : null; - this.dateOfBirth = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPerson().getDob() : null; - this.title = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPerson().getTitle().name() : null; - this.sex = !patientInvestigations.isEmpty() ? patientInvestigations.get(0).getPatient().getPerson().getSex().name() : null; - this.bloodGroup = !patientInvestigations.isEmpty() && patientInvestigations.get(0).getPatient().getPerson().getBloodGroup() != null - ? patientInvestigations.get(0).getPatient().getPerson().getBloodGroup().getName() : null; this.fromDate = fromDate; this.toDate = toDate; - this.patientInvestigations = ViewPatientInvestigationResponseDTO.fromPatientInvestigations(patientInvestigations); + this.patientInvestigations = ViewPatientInvestigationResponseDTO.fromPatientInvestigations(patientInvestigations); }This keeps behavior the same for well-formed data but avoids NPEs if any nested association is missing and also tolerates a null
patientInvestigationsargument.src/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.java (1)
18-65: Make list mapper tolerant of nullpatientReportItemValues
fromPatientReportItemValuealready handles a null single value gracefully, butfromPatientReportItemValuesassumes the list argument is non-null and callsstream()directly, which would NPE if a caller passes null.To keep the API robust and consistent with other mappers:
- public static List<ViewReportItemValueResponseDTO> fromPatientReportItemValues(final List<PatientReportItemValue> patientReportItemValues) { - return patientReportItemValues.stream() - .map(ViewReportItemValueResponseDTO::fromPatientReportItemValue) - .collect(Collectors.toList()); - } + public static List<ViewReportItemValueResponseDTO> fromPatientReportItemValues( + final List<PatientReportItemValue> patientReportItemValues) { + if (patientReportItemValues == null) { + return java.util.Collections.emptyList(); + } + return patientReportItemValues.stream() + .map(ViewReportItemValueResponseDTO::fromPatientReportItemValue) + .collect(Collectors.toList()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java(3 hunks)src/main/java/com/divudi/service/PatientInvestigationService.java(1 hunks)src/main/java/com/divudi/ws/common/ApplicationConfig.java(1 hunks)src/main/java/com/divudi/ws/report/ReportApi.java(1 hunks)src/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.java(1 hunks)src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.java(1 hunks)src/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.java(1 hunks)src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java(1 hunks)src/main/java/com/divudi/ws/report/ViewReportsResponseDTO.java(1 hunks)
🧰 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/core/data/dataStructure/SearchKeyword.javasrc/main/java/com/divudi/ws/common/ApplicationConfig.javasrc/main/java/com/divudi/service/PatientInvestigationService.javasrc/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.javasrc/main/java/com/divudi/ws/report/ReportApi.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/core/data/dataStructure/SearchKeyword.javasrc/main/java/com/divudi/ws/common/ApplicationConfig.javasrc/main/java/com/divudi/service/PatientInvestigationService.javasrc/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.javasrc/main/java/com/divudi/ws/report/ReportApi.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Do not 'fix' intentional typos in model/controller property names used for database compatibility
Never 'fix' intentional typos in database column mappings (e.g., purcahseRate) - maintain backward compatibility with existing database
Files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.javasrc/main/java/com/divudi/ws/common/ApplicationConfig.javasrc/main/java/com/divudi/service/PatientInvestigationService.javasrc/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.javasrc/main/java/com/divudi/ws/report/ReportApi.java
**/*DTO.java
📄 CodeRabbit inference engine (AGENTS.md)
Never modify existing DTO constructors; add new ones if needed for non-breaking changes
**/*DTO.java: Never modify existing DTO constructors - only add new ones to maintain backward compatibility
Use direct DTO queries instead of entity-to-DTO conversion loops in data access code
Files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
🧠 Learnings (30)
📓 Common learnings
Learnt from: RaveeshaPeiris
Repo: hmislk/hmis PR: 16704
File: src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml:0-0
Timestamp: 2025-12-05T07:29:50.709Z
Learning: In PR #16704, RaveeshaPeiris confirmed that the Branch, Department, Consultant, Service Center, and Discharge Type dropdowns in src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml are intentional placeholder wireframe work with empty value="" bindings. Backend bindings and data integration will be added in a later PR.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 16488
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:3819-3826
Timestamp: 2025-11-11T23:14:31.322Z
Learning: For hmislk/hmis, when buddhika75 asks to create a separate issue for a review finding, they want validation of the expected behavior before starting any development, and prefer the issue to be assigned to themselves.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12502
File: src/main/java/com/divudi/ejb/SmsManagerEjb.java:0-0
Timestamp: 2025-05-25T14:04:39.505Z
Learning: SMS messages for lab reports are linked to PatientInvestigation entities, which contain the associated bill information for payment verification.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12553
File: src/main/java/com/divudi/bean/lab/PatientReportController.java:1617-1626
Timestamp: 2025-05-27T18:44:30.710Z
Learning: In the PatientReportController class, parameter validation is typically handled at the calling method level rather than within individual utility methods like emailSubject() and emailBody(). The approvePatientReport() method validates currentPatientReport early, so subsequent method calls don't need redundant null checks.
Learnt from: DhaneshKaushan
Repo: hmislk/hmis PR: 16790
File: src/main/webapp/reports/managementReports/surgery_count_type.xhtml:20-30
Timestamp: 2025-11-24T07:14:42.869Z
Learning: In src/main/webapp/reports/managementReports/surgery_count_type.xhtml for PR #16790, DhaneshKaushan confirmed this is wireframe-only work for Surgery Count (Type) report, intentionally deferring value bindings and backend integration to a later PR.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 13749
File: src/main/java/com/divudi/service/BillService.java:1101-1114
Timestamp: 2025-07-08T08:37:51.021Z
Learning: ImeshRanawellaSG prefers to refactor all calling places when modifying method signatures rather than maintaining backwards compatibility through method overloading. This approach is used for internal codebase changes where all callers can be controlled and updated.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12611
File: src/main/java/com/divudi/bean/common/CreditCompanyDueController.java:824-852
Timestamp: 2025-05-29T12:16:38.589Z
Learning: ImeshRanawellaSG prefers to keep commented-out code temporarily during testing phases when refactoring, removing it once the new implementation is validated.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12136
File: src/main/java/com/divudi/bean/common/CreditCompanyDueController.java:1996-1997
Timestamp: 2025-05-05T15:38:53.525Z
Learning: ImeshRanawellaSG prefers to be reminded about replacing printStackTrace() with proper logging (Logger.getLogger().log() and FacesContext.addMessage()) in future code reviews but doesn't want to change existing instances for now.
📚 Learning: 2025-09-05T17:23:11.351Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 15299
File: src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java:64-65
Timestamp: 2025-09-05T17:23:11.351Z
Learning: In the SearchKeyword class, String-based search fields like `fromInstitution` should be preferred over Institution entity fields for search functionality, as they enable free-text "LIKE" searches that are widely used across JSF pages. Entity-based fields would require significant UI and backend changes to accommodate the existing search patterns.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-05-13T09:29:44.305Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 12283
File: src/main/java/com/divudi/bean/lab/LaboratoryManagementController.java:172-174
Timestamp: 2025-05-13T09:29:44.305Z
Learning: In the LaboratoryManagementController, the variable named "patientInvestigationStatus" is intentionally used to filter bills by their status (pi.billItem.bill.status), not to filter PatientInvestigation entities by their status.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.javasrc/main/java/com/divudi/service/PatientInvestigationService.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/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-08-06T09:02:32.375Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 14735
File: src/main/java/com/divudi/bean/common/CollectingCentrePaymentController.java:224-224
Timestamp: 2025-08-06T09:02:32.375Z
Learning: In the HMIS system, the `deptId` field in the `Bill` entity is used to store the bill number, not the `billNo` field. This is confirmed for the CollectingCentrePaymentController.java where the JPQL query correctly uses `bill.deptId` to populate the BillLight constructor's billNo parameter.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-10-23T05:16:02.475Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 16051
File: src/main/webapp/resources/opd/sale_bill_five_five_custom_3.xhtml:84-90
Timestamp: 2025-10-23T05:16:02.475Z
Learning: In src/main/webapp/resources/opd/sale_bill_five_five_custom_3.xhtml and similar OPD bill templates, the Bill entity's paymentMethod property is guaranteed to be non-null, so null safety checks are not required when accessing cc.attrs.bill.paymentMethod or its nested properties like cc.attrs.bill.paymentMethod.label.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-10-15T08:09:34.143Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 15904
File: src/main/webapp/collecting_centre/bill_cancel.xhtml:82-97
Timestamp: 2025-10-15T08:09:34.143Z
Learning: In src/main/webapp/collecting_centre/bill_cancel.xhtml, the string comparison `billSearch.bill.paymentMethod eq 'Agent'` in the disabled condition works correctly as confirmed by development testing, so there's no need to use the enum constant T(com.divudi.core.data.PaymentMethod).Agent.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-05-27T17:08:50.105Z
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12552
File: src/main/webapp/reports/financialReports/opd_and_inward_due_report.xhtml:545-545
Timestamp: 2025-05-27T17:08:50.105Z
Learning: In the OPD and Inward Due report (opd_and_inward_due_report.xhtml), the OP table should continue using reportsController.bundle.groupedBillItemsByInstitution.entrySet() as its data source, while the IP table uses reportsController.billInstitutionEncounterMap.entrySet(). The different data sources are intentional and should not be unified.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-06-12T23:35:31.056Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 12992
File: src/main/java/com/divudi/bean/lab/PatientInvestigationController.java:345-357
Timestamp: 2025-06-12T23:35:31.056Z
Learning: In the `PatientSample` entity, the fields `institution` and `department` denote the institution/department that currently holds (owns) the sample, not the original ordering site.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-05-25T14:04:39.505Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12502
File: src/main/java/com/divudi/ejb/SmsManagerEjb.java:0-0
Timestamp: 2025-05-25T14:04:39.505Z
Learning: SMS messages for lab reports are linked to PatientInvestigation entities, which contain the associated bill information for payment verification.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-12-05T07:29:50.709Z
Learnt from: RaveeshaPeiris
Repo: hmislk/hmis PR: 16704
File: src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml:0-0
Timestamp: 2025-12-05T07:29:50.709Z
Learning: In PR #16704, RaveeshaPeiris confirmed that the Branch, Department, Consultant, Service Center, and Discharge Type dropdowns in src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml are intentional placeholder wireframe work with empty value="" bindings. Backend bindings and data integration will be added in a later PR.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-05-12T16:57:05.347Z
Learnt from: GSMgeeth
Repo: hmislk/hmis PR: 12271
File: src/main/java/com/divudi/core/entity/Category.java:17-20
Timestamp: 2025-05-12T16:57:05.347Z
Learning: In the hmis project, investigation-related classes such as InvestigationCategory and InvestigationTube are located in the com.divudi.core.entity.lab package, not in a separate investigation package.
Applied to files:
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.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/core/data/dataStructure/SearchKeyword.java
📚 Learning: 2025-07-23T13:35:44.014Z
Learnt from: DARKDRAGON-LK
Repo: hmislk/hmis PR: 14111
File: src/main/java/com/divudi/service/PaymentService.java:387-387
Timestamp: 2025-07-23T13:35:44.014Z
Learning: In src/main/java/com/divudi/service/PaymentService.java, the `site` parameter in the `fetchPayments()` method is intentionally included for future use, even though it's not currently used in the JPQL query implementation. This parameter should not be removed as it's part of planned functionality.
Applied to files:
src/main/java/com/divudi/service/PatientInvestigationService.java
📚 Learning: 2025-10-13T05:52:30.007Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 15840
File: src/main/java/com/divudi/bean/lab/LaboratoryManagementController.java:1205-1215
Timestamp: 2025-10-13T05:52:30.007Z
Learning: In LaboratoryManagementController.searchPatientInvestigationsWithSampleId(), using LIKE with wildcards on psc.patientSample.id (numeric field) is intentional. Users typically search samples from one day and benefit from partial ID matching. When exact matches are needed, users type the full sample ID. This pattern is acceptable because searches are already scoped by date range.
Applied to files:
src/main/java/com/divudi/service/PatientInvestigationService.java
📚 Learning: 2025-05-18T09:04:37.175Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 12361
File: src/main/java/com/divudi/core/data/PatientReportLight.java:32-39
Timestamp: 2025-05-18T09:04:37.175Z
Learning: When creating DTO constructors that take entity objects as parameters (like PatientReport → PatientReportLight), implement proper null checks for deeply nested property access and handle Boolean to boolean unboxing safely to prevent NullPointerExceptions.
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-11-24T21:54:09.952Z
Learnt from: CR
Repo: hmislk/hmis PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:54:09.952Z
Learning: Applies to **/*DTO.java : Never modify existing DTO constructors; add new ones if needed for non-breaking changes
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-11-30T03:27:27.137Z
Learnt from: CR
Repo: hmislk/hmis PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T03:27:27.137Z
Learning: Applies to **/*DTO.java : Never modify existing DTO constructors - only add new ones to maintain backward compatibility
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-10-12T05:32:34.145Z
Learnt from: CR
Repo: hmislk/hmis PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:32:34.145Z
Learning: Applies to **/*DTO.java : For DTOs, never modify existing constructors; add new ones to avoid breaking changes
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-08-27T18:25:23.940Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 15185
File: src/main/java/com/divudi/bean/lab/PatientInvestigationController.java:2202-2210
Timestamp: 2025-08-27T18:25:23.940Z
Learning: In PatientInvestigationController.java, the patientSamples field can be null despite developer assumptions. The class has multiple methods that explicitly set patientSamples = null (like clearReportData() and makeNull()), and the getter method getPatientSamples() includes null-checking with lazy initialization, proving the field can be null. Direct field access should be avoided in favor of the getter method to prevent NPEs.
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.java
📚 Learning: 2025-05-24T18:10:27.085Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12485
File: src/main/java/com/divudi/core/entity/lab/PatientReportItemValue.java:305-335
Timestamp: 2025-05-24T18:10:27.085Z
Learning: In PatientReportItemValue.java, the clone() method is designed to copy the patientReportGroup field because it will be used for other cases in the future where group copying is necessary. The ID field is already null after cloning by default, so explicit null setting is not required.
Applied to files:
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-05-18T07:19:23.677Z
Learnt from: damithdeshan98
Repo: hmislk/hmis PR: 12361
File: src/main/java/com/divudi/core/data/PatientReportLight.java:3-4
Timestamp: 2025-05-18T07:19:23.677Z
Learning: In the hmis project, ReportType is defined as an enum in the com.divudi.core.data package, so classes in the same package like PatientReportLight don't need to import it explicitly.
Applied to files:
src/main/java/com/divudi/ws/report/ViewReportsResponseDTO.javasrc/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-05-27T18:44:30.710Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 12553
File: src/main/java/com/divudi/bean/lab/PatientReportController.java:1617-1626
Timestamp: 2025-05-27T18:44:30.710Z
Learning: In the PatientReportController class, parameter validation is typically handled at the calling method level rather than within individual utility methods like emailSubject() and emailBody(). The approvePatientReport() method validates currentPatientReport early, so subsequent method calls don't need redundant null checks.
Applied to files:
src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
📚 Learning: 2025-11-30T03:27:27.137Z
Learnt from: CR
Repo: hmislk/hmis PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T03:27:27.137Z
Learning: Applies to **/*DTO.java : Use direct DTO queries instead of entity-to-DTO conversion loops in data access code
Applied to files:
src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
🧬 Code graph analysis (4)
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.java (1)
src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java (1)
ViewReportResponseDTO(10-305)
src/main/java/com/divudi/ws/report/ViewReportsResponseDTO.java (1)
src/main/java/com/divudi/ws/report/ViewPatientInvestigationResponseDTO.java (1)
ViewPatientInvestigationResponseDTO(8-336)
src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java (1)
src/main/java/com/divudi/ws/report/ViewReportItemValueResponseDTO.java (1)
ViewReportItemValueResponseDTO(10-114)
src/main/java/com/divudi/ws/report/ReportApi.java (2)
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java (1)
SearchKeyword(23-477)src/main/java/com/divudi/ws/common/ApplicationConfig.java (1)
javax(15-55)
🔇 Additional comments (7)
src/main/java/com/divudi/ws/report/ViewInvestigationResponseDTO.java (1)
13-38: Constructor mapping looks correct and null-safeThe constructor handles
investigation == nulland all nested associations defensively; field mapping aligns with typical Investigation structure. No issues from a DTO/backward-compat standpoint since this is a new class.src/main/java/com/divudi/service/PatientInvestigationService.java (1)
72-85: Status-based filter is correctly integrated into JPQLThe additional
pi.status=:pisclause and parameter binding follow the existing pattern and are gated onsearchKeyword.getPatientInvestigationStatus() != null, so existing callers remain unaffected while enabling the new filter.src/main/java/com/divudi/ws/common/ApplicationConfig.java (1)
39-53: ReportApi registration is consistent with existing resourcesAdding
com.divudi.ws.report.ReportApitoaddRestResourceClassesmatches the existing pattern and ensures the new endpoint is exposed under/api/report.src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java (1)
10-17: NewpatientInvestigationStatuscriterion is cleanly addedThe enum-typed field plus getter/setter integrates well with existing
SearchKeywordusage and supports the new JPQL status filter without impacting existing string-based criteria.Also applies to: 63-63, 455-461
src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java (3)
1-40: Package, imports, and field layout look consistentClass placement, imports, and the grouping/naming of fields (data entry / approve / printing / QR / items) are coherent and readable. No issues here.
104-112: Bulk factory is clear and null-safe; confirm semantics for null elementsThe collection factory handles a null
patientReportslist by returningCollections.emptyList(), and the stream mapping viafromPatientReportis concise and consistent.If callers might pass lists containing null elements, note that those will become “empty” DTOs (all fields null) due to the null-handling in
fromPatientReport. Please confirm that this behavior is intentional for your API contract.
114-304: Getters and setters correctly mirror fieldsAll accessors align with the declared fields and follow standard JavaBean naming, which should work well with serialization frameworks and REST mapping. No changes needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/divudi/ws/report/ReportApi.java (4)
41-42: Remove unused injection.
PatientInvestigationControlleris injected but never used in this class. Remove the unused dependency.- @Inject - private PatientInvestigationController patientInvestigationController;Also remove the corresponding import on line 4:
-import com.divudi.bean.lab.PatientInvestigationController;
84-92: Eliminate duplicate date parsing.The dates are parsed twice: first in
validateDates()(lines 113-116) and again here. Consider returning the parsed dates from validation to avoid redundant parsing and improve maintainability.+ // Add a simple holder class or use a Map/array to return parsed dates + private LocalDateTime[] validateAndParseDates(final String fromDate, final String toDate) { + // ... existing validation logic ... + return new LocalDateTime[]{fromLocal, toLocal}; + } // In getReportsByMobile: - try { - validateDates(fromDate, toDate); - validateMobileNumber(mobile); - } catch (WebApplicationException ex) { - // ... - } - - getSearchKeyword().setPatientPhone(mobile); - getSearchKeyword().setPatientInvestigationStatus(PatientInvestigationStatus.REPORT_APPROVED); - - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); - LocalDateTime fromLocal = LocalDateTime.parse(fromDate.trim(), formatter); - LocalDateTime toLocal = LocalDateTime.parse(toDate.trim(), formatter); + LocalDateTime[] parsedDates; + try { + parsedDates = validateAndParseDates(fromDate, toDate); + validateMobileNumber(mobile); + } catch (WebApplicationException ex) { + // ... + } + + getSearchKeyword().setPatientPhone(mobile); + getSearchKeyword().setPatientInvestigationStatus(PatientInvestigationStatus.REPORT_APPROVED); + + LocalDateTime fromLocal = parsedDates[0]; + LocalDateTime toLocal = parsedDates[1];
74-79: Standardize error response format.Error responses use inconsistent formats: 401 returns a structured JSON object (line 68), while validation errors return plain text strings (lines 77-78). This inconsistency complicates client-side error handling.
Consider wrapping validation errors in the same JSON structure:
} catch (WebApplicationException ex) { LOGGER.warning("Validation error: " + ex.getMessage()); - return Response.status(ex.getResponse().getStatus()) - .entity(ex.getMessage()) - .build(); + JSONObject errorResponse = new JSONObject(); + errorResponse.put("code", ex.getResponse().getStatus()); + errorResponse.put("type", "error"); + errorResponse.put("message", ex.getMessage()); + return Response.status(ex.getResponse().getStatus()) + .entity(errorResponse.toString()) + .build(); }
133-137: Consider adding basic mobile number sanitization.The validation only checks for null/empty. While strict format validation may not be appropriate for international numbers, consider trimming whitespace and optionally validating minimum length or allowed characters to prevent query issues with malformed input.
private void validateMobileNumber(final String mobile) { - if (mobile == null || mobile.isEmpty()) { + if (mobile == null || mobile.trim().isEmpty()) { throw new WebApplicationException("Mobile number must be provided", Response.Status.BAD_REQUEST); } + // Optional: validate only digits and common phone characters + if (!mobile.matches("[0-9+\\-\\s()]+")) { + throw new WebApplicationException("Invalid mobile number format", Response.Status.BAD_REQUEST); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/divudi/ws/report/ReportApi.java(1 hunks)src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/divudi/ws/report/ViewReportResponseDTO.java
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{xhtml,java}
📄 CodeRabbit inference engine (AGENTS.md)
Implement accessibility with proper tooltips and labeling
Files:
src/main/java/com/divudi/ws/report/ReportApi.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/ws/report/ReportApi.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Do not 'fix' intentional typos in model/controller property names used for database compatibility
Never 'fix' intentional typos in database column mappings (e.g., purcahseRate) - maintain backward compatibility with existing database
Files:
src/main/java/com/divudi/ws/report/ReportApi.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: RaveeshaPeiris
Repo: hmislk/hmis PR: 16704
File: src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml:0-0
Timestamp: 2025-12-05T07:29:50.709Z
Learning: In PR #16704, RaveeshaPeiris confirmed that the Branch, Department, Consultant, Service Center, and Discharge Type dropdowns in src/main/webapp/reports/inpatientReports/ip_unsettled_invoices.xhtml are intentional placeholder wireframe work with empty value="" bindings. Backend bindings and data integration will be added in a later PR.
Learnt from: buddhika75
Repo: hmislk/hmis PR: 16488
File: src/main/java/com/divudi/bean/report/PharmacyReportController.java:3819-3826
Timestamp: 2025-11-11T23:14:31.322Z
Learning: For hmislk/hmis, when buddhika75 asks to create a separate issue for a review finding, they want validation of the expected behavior before starting any development, and prefer the issue to be assigned to themselves.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 13749
File: src/main/java/com/divudi/service/BillService.java:1101-1114
Timestamp: 2025-07-08T08:37:51.021Z
Learning: ImeshRanawellaSG prefers to refactor all calling places when modifying method signatures rather than maintaining backwards compatibility through method overloading. This approach is used for internal codebase changes where all callers can be controlled and updated.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12611
File: src/main/java/com/divudi/bean/common/CreditCompanyDueController.java:824-852
Timestamp: 2025-05-29T12:16:38.589Z
Learning: ImeshRanawellaSG prefers to keep commented-out code temporarily during testing phases when refactoring, removing it once the new implementation is validated.
Learnt from: ImeshRanawellaSG
Repo: hmislk/hmis PR: 12136
File: src/main/java/com/divudi/bean/common/CreditCompanyDueController.java:1996-1997
Timestamp: 2025-05-05T15:38:53.525Z
Learning: ImeshRanawellaSG prefers to be reminded about replacing printStackTrace() with proper logging (Logger.getLogger().log() and FacesContext.addMessage()) in future code reviews but doesn't want to change existing instances for now.
📚 Learning: 2025-04-27T09:21:16.954Z
Learnt from: buddhika75
Repo: hmislk/hmis PR: 11950
File: src/main/java/com/divudi/bean/common/HistoricalRecordController.java:90-99
Timestamp: 2025-04-27T09:21:16.954Z
Learning: Error handling for date validation in HistoricalRecordController.processHistoricalRecordList() was deemed non-critical by the team for the initial implementation, as the consequence of invalid date ranges is simply that no data will be returned to the user.
Applied to files:
src/main/java/com/divudi/ws/report/ReportApi.java
🧬 Code graph analysis (1)
src/main/java/com/divudi/ws/report/ReportApi.java (1)
src/main/java/com/divudi/core/data/dataStructure/SearchKeyword.java (1)
SearchKeyword(23-477)
🔇 Additional comments (3)
src/main/java/com/divudi/ws/report/ReportApi.java (3)
106-131: LGTM!The date validation is comprehensive: checks for null/empty values, validates format with proper error handling, ensures chronological order, and enforces the 1-month maximum range. The
trim()usage addresses edge cases with whitespace.
139-160: LGTM!The API key validation is thorough, checking key existence, user status (not retired, activated), and expiry date. The early-return pattern keeps the logic readable. Note:
getDateOfExpiary()naming is preserved per coding guidelines for database compatibility.
171-180: LGTM!Lazy initialization is appropriate for the
@RequestScopedbean, ensuring thread safety per request.
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
✏️ Tip: You can customize this high-level summary in your review settings.