PatientInvestigation Approval Audit Event Log#18501
PatientInvestigation Approval Audit Event Log#18501OsaVS wants to merge 3 commits intodevelopmentfrom
Conversation
Signed-off-by: OsaVS <41975253+OsaVS@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
buddhika75
left a comment
buddhika75
left a comment
There was a problem hiding this comment.
PR #18501 Review - PatientInvestigation Approval Audit Event Log
PR: #18501
Issue: #17232
Files Changed: 4
1. btnNewReport Change (Critical - Functionality Lost)
Before (current code):
<p:commandLink id="btnNewReport" ajax="false"
disabled="#{!pi.received}"
action="/lab/patient_report?faces-redirect=true"
actionListener="#{patientReportController.createNewReport(pi)}" >
<f:setPropertyActionListener value="#{pi}" target="#{patientReportController.currentPtIx}"/>After (PR):
<p:commandLink id="btnNewReport" ajax="false"
disabled="#{!pi.received}"
action="#{patientReportController.navigateToCreatedPatientReport(pi)}" >
<!-- <f:setPropertyActionListener ... commented out -->Functionality difference
The old code used createNewReport(pi) as an actionListener, which handles 3 report types:
Microbiology-> callscreateNewMicrobiologyReport()HtmlTemplate-> callscreateNewPatientTemplateReport()- Normal -> calls
createNewPatientReport()
The new code calls navigateToCreatedPatientReport(pi) which routes through navigateToNewlyCreatedPatientReport(pi), which only handles 2 report types:
Microbiology-> callscreateNewMicrobiologyReport()- Everything else -> calls
createNewPatientReport()
HtmlTemplate report type is lost. Any investigation configured with InvestigationReportType.HtmlTemplate will no longer call createNewPatientTemplateReport() - it will incorrectly fall into the normal createNewPatientReport() path.
Also, the new path adds extra validation (checkAlreadyGeneratedPatientReportsExists, alternative report checking) that the old path didn't have. This could be seen as an improvement, but it's a behavioral change that may prevent users from creating reports in some edge cases where they previously could.
2. enterNewReportFormat Link (Minor - OK)
Before: action="/lab/patient_report?faces-redirect=true" with actionListener="#{patientReportController.enterNewReportFormat(pi, ifi)}"
After: action="#{patientReportController.navigateToPatientReportPage}" with actionListener="#{patientReportController.enterNewReportFormat(pi, ifi)}"
The enterNewReportFormat method already returns "/lab/patient_report" (without ?faces-redirect=true). Since actionListener runs before action, the action used to be the hardcoded navigation. Now the action calls a new method navigateToPatientReportPage() that just sets initialInvestigation and returns the same path. This change is functionally okay.
3. cmdOldReport and cmdOldReportEdit Links (OK)
Before: action="/lab/patient_report?faces-redirect=true" with f:setPropertyActionListener setting currentPatientReport
After: action="#{patientReportController.navigateToPatientReportPage}" with the same f:setPropertyActionListener
The navigateToPatientReportPage() adds audit initialInvestigation capture. Since f:setPropertyActionListener runs before the action method in JSF lifecycle, currentPatientReport will be set before navigateToPatientReportPage() reads it. This is functionally OK.
4. Java Changes - Other Observations
Bug in navigateToViewPatientReport
In PatientReportController.navigateToViewPatientReport(), the audit map is populated using currentPatientReport:
patientInvestigationToAuditMap(initialInvestigation, currentPatientReport);But at that point, currentPatientReport hasn't been set yet - it's set on the next line via setCurrentPatientReport(patientReport). It should be using the patientReport parameter instead. This means the audit "before" snapshot will capture stale/wrong data.
Commented-out code in PatientReportUploadController
The uploadReport() method contains commented-out code that should be removed before merge:
// patientReportController.setInitialInvestigation(new HashMap<>(20));
// patientReportController.patientInvestigationToAuditMap(...)Summary Table
| Change | Impact | Verdict |
|---|---|---|
btnNewReport action change |
HtmlTemplate reports broken | Functionality lost |
btnNewReport removes setPropertyActionListener |
currentPtIx now set inside navigateToCreatedPatientReport |
OK |
enterNewReportFormat link |
Adds audit capture, navigation same | OK |
cmdOldReport / cmdOldReportEdit links |
Adds audit capture, navigation same | OK |
navigateToViewPatientReport audit uses wrong variable |
Audit "before" state will be wrong | Bug |
| Commented-out code in upload controller | Dead code in PR | Cleanup needed |
Recommendation
The PR should not be merged as-is because:
btnNewReportlosesHtmlTemplatereport type handling - investigations configured asHtmlTemplatewill be created incorrectly- Bug in
navigateToViewPatientReport- audit "before" state captures stalecurrentPatientReportinstead of thepatientReportparameter - Commented-out code in
PatientReportUploadControllershould be removed
2 participants
Not Ready for Merge
Issue: #17232
Files Changed:
New Method to map the
PatientReportvalues.UpdateInvestigation and initialInvestigation, two new variables to hold the
beforeandafterstates.Navigation methods are modified to set
initialInvestigation.Approval and ReverseApproval methods are modified to set
updateInvestigationandinitialInvestigation.patientReportController.navigateToViewPatientRepoort()method should be modified to setcurrentPatientReportwhen reportType isUpload.Same case with the
laboratoryManagementController.navigateToEditReport().search_for_reporting_ondemand.xhtml file modified to set
initialInvestigationwhen navigating topatient_reportpage.After:
Before: