fix(notifications): restore discharge notification navigation, delete, and filter fixes#21454
Merged
buddhika75 merged 4 commits intoJun 13, 2026
Conversation
fix: discharge notification navigation, removal, filters, real-time push, and UI display - Navigate to admission profile when clicking discharge notifications (PatientRoom/PatientEncounter) - Allow removal of PatientRoom/PatientEncounter-based notifications - Fix NPE in cancel filter for discharge notifications (clear + filter methods) - Fix today filter date comparison (use midnight truncation instead of equals) - Add WebSocket listener to user_notifications.xhtml for real-time notification list updates - Add Room Discharge and Patient Discharge type badges to notification list Closes #21389 Closes #21390 Closes #21391 Co-Authored-By: Claude <noreply@anthropic.com> @
…ork filters, add clear/restore - removeUserNotification: PatientEncounter-based (clinical/final discharge) notifications silently returned without removing; now removable. Null-safe department check (avoids NPE on unmatched bill types) and sets retiredAt/retirer. - Filters reworked: previously pruned the already-loaded 20-item list in memory (compounding, not reversible, contradictory checkboxes). Now a fresh JPQL query per filter: Today Only, Seen (All/Unseen/Seen), Status (All/Pending/Completed), Cancelled Bills Only. - Clear button now clears exactly what is listed (filter first, then clear), with explicit confirmation and cleared count; previously it only acted on checked criteria and did nothing when none were checked. - New Show Cleared filter with per-row Restore button so cleared notifications remain retrievable; Clear is guarded while viewing cleared items. - Push refresh and row removal refill via the filter-aware query so active filters are preserved; filters reset when navigating to the page. Refs #21389 #21390 #21391 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUserNotificationController now uses string/flag filter state and JPQL queries to load items, adds clear/restore operations, hardens removal and discharge navigation via BhtSummeryController; frontend adds Omnifaces socket-driven refresh, rebuilt filter UI, discharge badges, timezoneed timestamps, and retirement-aware row actions. ChangesNotification Filtering, Actions, and Display
Sequence DiagramsequenceDiagram
participant User
participant Frontend as user_notifications.xhtml
participant Controller as UserNotificationController
participant Database
User->>Frontend: Select filters / click notification / confirm clear
Frontend->>Controller: filterNotificationsByCriteria() / navigateToCurrentNotificationRequest() / clearNotificationsByCriteria()
Controller->>Database: JPQL query or retire/un-retire operations
Database->>Controller: results / acknowledge updates
Controller->>Frontend: updated items list
Frontend->>User: refreshed notification UI
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/divudi/bean/common/UserNotificationController.java (1)
385-407:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing null check on
un.getNotification()can causeNullPointerException.The method accesses
un.getNotification()at lines 390, 400, and 405 without validating that the notification is non-null. If aUserNotificationhas a nullNotification(e.g., due to orphaned data), this will throw an NPE.Proposed fix
public String navigateToCurrentNotificationRequest(UserNotification un) { + if (un == null || un.getNotification() == null) { + JsfUtil.addErrorMessage("Invalid notification"); + return ""; + } un.setSeen(true); getFacade().edit(un);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/divudi/bean/common/UserNotificationController.java` around lines 385 - 407, In navigateToCurrentNotificationRequest, un.getNotification() is accessed without a null check causing possible NPEs; after marking the notification seen and persisting (getFacade().edit(un)), immediately check if un.getNotification() == null and return an empty string (or appropriate fallback) to avoid further dereferencing; then proceed with the existing branches that reference un.getNotification(), PatientRoom, PatientEncounter and calls into bhtSummeryController.navigateToInpatientProfile().src/main/webapp/Notification/user_notifications.xhtml (1)
109-109:⚠️ Potential issue | 🟡 MinorGuard
un.notification.bill.departmentbefore using.namein bill-based notification badges.The XHTML renders
un.notification.bill.department.namewheneverun.notification.bill != null, butBill.departmentis a plain@ManyToOnewithout non-null constraints, and it’s treated as nullable in other code paths (if (this.getDepartment() != null) { ... }). Ifdepartmentis null, the badge will render empty text (or can risk EL property resolution issues). Add arendered="#{un.notification.bill.department != null}"wrapper (or an EL fallback) for theh6text.Also applies to: 118-118, 127-127, 136-136, 145-145.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/Notification/user_notifications.xhtml` at line 109, The badge currently accesses un.notification.bill.department.name without guarding against a null department; update the h6 elements that render department names (the <h6> that uses #{un.notification.bill.department.name} and the similar occurrences at the other noted locations) to guard the access by either adding rendered="#{un.notification.bill.department != null}" to the h6 or by using an EL fallback (e.g., #{un.notification.bill.department != null ? un.notification.bill.department.name : ''}) so the template won’t attempt to read .name when department is null.
🧹 Nitpick comments (2)
src/main/java/com/divudi/bean/common/UserNotificationController.java (1)
198-206: 💤 Low valueConsider specifying
TemporalType.TIMESTAMPfor the date parameter.The
fromDateparameter is ajava.util.Dateset to midnight. While default JPA binding typically works, explicitly usingTemporalType.TIMESTAMPensures consistent behavior across JPA providers and avoids potential truncation issues if the facade's default differs.This is a minor robustness concern given the existing date handling in this codebase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/divudi/bean/common/UserNotificationController.java` around lines 198 - 206, The fromDate is a java.util.Date set to midnight; when binding it to the JPQL parameter "fromDate" ensure you pass it with TemporalType.TIMESTAMP to avoid provider-specific truncation: locate where the JPQL built by UserNotificationController (jpql.append(...)) is executed and replace the plain setParameter("fromDate", m.get("fromDate")) with setParameter("fromDate", (Date) m.get("fromDate"), TemporalType.TIMESTAMP) (or the equivalent Query/TypedQuery call) so the parameter binding uses TIMESTAMP explicitly.src/main/webapp/Notification/user_notifications.xhtml (1)
186-186: ⚡ Quick winAdd explicit
timeZone="Asia/Colombo"tof:convertDateTimefor consistency.Notification creation timestamps are billing-related date-time values. Based on learnings, billing/patient date-time displays should include an explicit
timeZone="Asia/Colombo"attribute to guard against production JVM default timezone differences.🌐 Proposed fix to add explicit timezone
value="#{un.notification.createdAt}"> - <f:convertDateTime pattern="#{sessionController.applicationPreference.longDateFormat} HH:mm a" /> + <f:convertDateTime pattern="#{sessionController.applicationPreference.longDateFormat} HH:mm a" + timeZone="Asia/Colombo" /> </p:outputLabel>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/Notification/user_notifications.xhtml` at line 186, The f:convertDateTime used in user_notifications.xhtml (the tag rendering timestamps with pattern "#{sessionController.applicationPreference.longDateFormat} HH:mm a") lacks an explicit timezone; update that f:convertDateTime to include timeZone="Asia/Colombo" so billing/patient timestamps are rendered consistently regardless of JVM default timezone.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/webapp/Notification/user_notifications.xhtml`:
- Line 157: The room-discharge UI and backend assume
PatientRoom.getRoomFacilityCharge() is non-null; add null-guards so we never
call getName() on null: in the JSF template replace
un.notification.patientRoom.roomFacilityCharge.name with a guarded EL expression
(e.g. use the conditional/empty check to render an empty string or fallback
label when roomFacilityCharge is null) and in
NotificationController.createInwardRoomDischargeNotifications add a null check
around pr.getRoomFacilityCharge() before calling getName() (use empty string or
a sensible fallback value when null) to avoid NPEs.
---
Outside diff comments:
In `@src/main/java/com/divudi/bean/common/UserNotificationController.java`:
- Around line 385-407: In navigateToCurrentNotificationRequest,
un.getNotification() is accessed without a null check causing possible NPEs;
after marking the notification seen and persisting (getFacade().edit(un)),
immediately check if un.getNotification() == null and return an empty string (or
appropriate fallback) to avoid further dereferencing; then proceed with the
existing branches that reference un.getNotification(), PatientRoom,
PatientEncounter and calls into
bhtSummeryController.navigateToInpatientProfile().
In `@src/main/webapp/Notification/user_notifications.xhtml`:
- Line 109: The badge currently accesses un.notification.bill.department.name
without guarding against a null department; update the h6 elements that render
department names (the <h6> that uses #{un.notification.bill.department.name} and
the similar occurrences at the other noted locations) to guard the access by
either adding rendered="#{un.notification.bill.department != null}" to the h6 or
by using an EL fallback (e.g., #{un.notification.bill.department != null ?
un.notification.bill.department.name : ''}) so the template won’t attempt to
read .name when department is null.
---
Nitpick comments:
In `@src/main/java/com/divudi/bean/common/UserNotificationController.java`:
- Around line 198-206: The fromDate is a java.util.Date set to midnight; when
binding it to the JPQL parameter "fromDate" ensure you pass it with
TemporalType.TIMESTAMP to avoid provider-specific truncation: locate where the
JPQL built by UserNotificationController (jpql.append(...)) is executed and
replace the plain setParameter("fromDate", m.get("fromDate")) with
setParameter("fromDate", (Date) m.get("fromDate"), TemporalType.TIMESTAMP) (or
the equivalent Query/TypedQuery call) so the parameter binding uses TIMESTAMP
explicitly.
In `@src/main/webapp/Notification/user_notifications.xhtml`:
- Line 186: The f:convertDateTime used in user_notifications.xhtml (the tag
rendering timestamps with pattern
"#{sessionController.applicationPreference.longDateFormat} HH:mm a") lacks an
explicit timezone; update that f:convertDateTime to include
timeZone="Asia/Colombo" so billing/patient timestamps are rendered consistently
regardless of JVM default timezone.
🪄 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: 78d86cf6-cc08-40b2-b56e-8f7854bbe0b3
📒 Files selected for processing (2)
src/main/java/com/divudi/bean/common/UserNotificationController.javasrc/main/webapp/Notification/user_notifications.xhtml
- navigateToCurrentNotificationRequest: clicking Go now marks the notification retired (seen=true, retired=true, retireComments="Viewed") so it disappears from the active list and appears under Show Cleared. Also adds null guard on un.getNotification() to prevent NPE. - NotificationController: guard pr.getRoomFacilityCharge() before calling getName() to prevent NPE when a PatientRoom has no facility charge set. - user_notifications.xhtml: add timeZone="Asia/Colombo" to f:convertDateTime for consistent timestamp display regardless of JVM default timezone. Refs #21389 #21390 #21391 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
buddhika75
deleted the
restore-discharge-notification-fixes-21389-21390-21391
branch
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Restores the discharge-notification fixes that were lost when PR #21424 was closed. That PR was closed as "superseded by #21439 — notification changes already merged to development", but the notification commits were in fact never merged — #21439 kept only the pharmacy changes, silently dropping this work. This PR cherry-picks the three notification commits from
fix-discharge-notification-issues-21389-21390-21391onto currentdevelopment(clean cherry-pick; both files are byte-identical to that branch tip).Changes (cherry-picked)
140377d803— Navigate to admission profile when clicking discharge notifications (PatientRoom/PatientEncounter); allow removal of PatientRoom/PatientEncounter-based notifications; fix NPE in cancel filter; fix today-filter date comparison; WebSocket listener for real-time list updates; Room/Patient Discharge type badges.55390aa8e9— Replacef:websocketwith OmniFaceso:socketon the notification page.ff1babd16d— Make delete work for discharge notifications (PatientEncounter-based notifications previously returned silently); rework filters to fresh JPQL query per filter (Today Only, Seen, Status, Cancelled Bills Only); Clear button clears exactly what is listed with confirmation and count; new Show Cleared filter with per-row Restore; push refresh preserves active filters.Files
src/main/webapp/Notification/user_notifications.xhtmlsrc/main/java/com/divudi/bean/common/UserNotificationController.javaCloses #21389
Closes #21390
Closes #21391
🤖 Generated with Claude Code
@
Summary by CodeRabbit
New Features
Improvements