19690 - Improve Lab Report Patient Details Panel with Privilege-Controlled Editing#19691
Conversation
Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
…olled Editing Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds UI controls and a controller action to reload patient name/age/gender into a patient report, optionally re-evaluate dynamic-label item values, and record two new lab-test history types for patient-detail edits and dynamic-label recalculation. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as JSF View
participant PRC as PatientReportController
participant PRB as PatientReportBean
participant LTHS as LabTestHistoryService
participant DB as Database
User->>UI: Click "Reload Patient Details"
UI->>PRC: reloadPatientDetailsInReport()
PRC->>PRB: load patient & data
PRB->>DB: Query Patient / Flags
DB-->>PRB: Return Patient / Flags
PRC->>PRC: Update report (name, age, gender)
PRC->>PRB: edit(currentPatientReport)
PRB->>DB: Persist report
DB-->>PRB: Confirm
PRC->>LTHS: addParientDetailsEditHistory(...)
LTHS->>DB: Insert history (PATIENT_DETAILS_CHANGE)
DB-->>LTHS: Confirm
alt Dynamic-label items present
PRC->>PRB: getPatientDynamicLabel(...) per item
PRB->>DB: Query InvestigationItemValueFlag (inclusive age bounds)
DB-->>PRB: Return matching flags
PRC->>PRC: Update report item values
PRC->>PRB: savePatientReportItemValues()
PRB->>DB: Persist item values
DB-->>PRB: Confirm
PRC->>LTHS: addReCalculateDynamicLabelHistory(...)
LTHS->>DB: Insert history (RECALCULATE_DYNAMICLABEL)
DB-->>LTHS: Confirm
end
PRC-->>UI: Show success message
UI-->>User: Reload complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/divudi/bean/lab/LabTestHistoryController.java`:
- Around line 181-187: The two controller wrappers (addParientDetailsEditHistory
and addReCalculateDynamicLabelHistory) currently call
labTestHistoryService.addApprovalHistory(...) and thus log events as
REPORT_APPROVED; change each to call the dedicated history methods on
labTestHistoryService instead (e.g., replace
labTestHistoryService.addApprovalHistory(...) in addParientDetailsEditHistory
with labTestHistoryService.addPatientDetailsEditHistory(...), and replace the
call in addReCalculateDynamicLabelHistory with
labTestHistoryService.addDynamicLabelRecalculateHistory(...)), passing the same
parameters (patientInvestigation, patientReport,
sessionController.getInstitution(), sessionController.getDepartment(),
sessionController.getLoggedUser()) so the proper history types are recorded.
In `@src/main/java/com/divudi/bean/lab/PatientReportController.java`:
- Line 268: The null/empty guard in PatientReportController is using || which
calls isEmpty() when getPatientReportItemValues() is null; change the condition
to use && so it reads that patientReportItemValues is not null AND not empty
(i.e., replace the currentPatientReport.getPatientReportItemValues() != null ||
!currentPatientReport.getPatientReportItemValues().isEmpty() check with a
null-safe AND check). This ensures getPatientReportItemValues() is only
dereferenced when non-null and prevents the NPE during save/reload.
- Around line 255-262: The code unconditionally updates currentPatientReport and
logs history even when values didn't change; modify the logic in
PatientReportController so you compute the new values (patientName, patientAge,
patientGender — and any DynamicLabel-derived values) and compare them against
the existing fields on currentPatientReport, and only call
getFacade().edit(currentPatientReport) and
labTestHistoryController.addParientDetailsEditHistory(...) when at least one of
those comparisons shows a real change; apply the same change-gating pattern to
the other similar block (the code block around lines 270-281) to avoid no-op
writes and false audit entries.
In `@src/main/java/com/divudi/ejb/PatientReportBean.java`:
- Line 513: The age-bound query in PatientReportBean (the SQL string building
the InvestigationItemValueFlag lookup using "f.retired=false" and inclusive
fromAge/toAge) is now inconsistent with the exclusive lookup in
LimsMiddlewareController (lines ~1704-1717); fix by unifying the logic: extract
the lookup into a single shared helper (e.g., a new method like
findInvestigationItemValueFlagByInvestigationItemAndAge in a common EJB/utility)
and update both PatientReportBean and LimsMiddlewareController to call that
helper so they use the same retired check and inclusive-from/toAge semantics for
consistent dynamic-label matching.
In `@src/main/webapp/lab/patient_report_without_sample_sending_process.xhtml`:
- Around line 880-913: The new p:inputText components with ids PatientName,
PatientAge, PatientGender conflict with existing components of the same ids in
the Developers-only section and must be made unique; rename these input ids (and
their matching p:outputLabel for attributes) to distinct names (for example
PatientName_readonly, PatientAge_readonly, PatientGender_readonly) and ensure
the value bindings remain
#{patientReportController.currentPatientReport.patientName|patientAge|patientGender}
and the readonly logic remains unchanged so labels and AJAX behavior target the
updated ids.
In `@src/main/webapp/lab/patient_report.xhtml`:
- Around line 937-944: Add server-side privilege enforcement inside
PatientReportController.reloadPatientDetailsInReport(): before performing any
reload logic call webUserController.hasPrivilege('LabDeAutherizing') and if it
returns false use JsfUtil.addErrorMessage("Unauthorized action") and return to
stop execution; ensure this check is the first thing in
reloadPatientDetailsInReport() so crafted requests cannot bypass the
frontend-disabled button.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0c9c13e-b39d-4289-993e-45300e92c20d
📒 Files selected for processing (7)
src/main/java/com/divudi/bean/lab/LabTestHistoryController.javasrc/main/java/com/divudi/bean/lab/PatientReportController.javasrc/main/java/com/divudi/core/data/lab/TestHistoryType.javasrc/main/java/com/divudi/ejb/LabTestHistoryService.javasrc/main/java/com/divudi/ejb/PatientReportBean.javasrc/main/webapp/lab/patient_report.xhtmlsrc/main/webapp/lab/patient_report_without_sample_sending_process.xhtml
Signed-off-by: damithdeshan98 <hkddrajapaksha@gmail.com>
Summary:
report header fields (col-md-7).
is not yet approved).
patientReportController.reloadPatientDetailsInReport() to sync report header from the current patient record.
patient age/gender.
Summary by CodeRabbit
New Features
Bug Fixes