Session Instance Management additional item tab UI improvement#19679
Merged
buddhika75 merged 2 commits intodevelopmentfrom Apr 4, 2026
Merged
Conversation
Signed-off-by: OsaVS <41975253+OsaVS@users.noreply.github.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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded a null-safe guard in SMS recipient lookup and introduced a public handler that loads session instance and fee totals. Updated JSF view layout and controls for session selection, session details, and restructured the Additional Items area with adjusted styling and event wiring. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
![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 (2)
src/main/webapp/channel/session_instance_management.xhtml (1)
380-425: Unconventional usage ofp:columnwithinp:panelGridwithcolumnsattribute.When using
p:panelGridwith thecolumnsattribute, direct children are automatically distributed across columns. Thep:columnwrapper elements are designed for use withp:row(manual layout mode), not with thecolumnsattribute.The current structure will render but is unconventional. Consider removing the
p:columnwrappers for cleaner code:♻️ Suggested simplification
-<p:panelGrid columns="3"> - <p:column > - <p:outputLabel value="Item to add"/> - </p:column> - <p:column > - <p:autoComplete - ... - </p:autoComplete> - </p:column> - <p:column > - <p:commandButton - ... - </p:commandButton> - </p:column> -</p:panelGrid> +<p:panelGrid columns="3"> + <p:outputLabel value="Item to add"/> + <p:autoComplete + ... + </p:autoComplete> + <p:commandButton + ... + </p:commandButton> +</p:panelGrid>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/channel/session_instance_management.xhtml` around lines 380 - 425, The p:column tags are being used inside a p:panelGrid that already uses the columns attribute (unconventional); remove the p:column wrappers so the direct children (the outputLabel, p:autoComplete with widgetVar/acAdditionalItem and its inner columns, and the p:commandButton btnAddAdditionalItem) are immediate children of p:panelGrid, preserving the same order so they distribute across the three columns, and keep existing ids/attributes (acAdditionalItem, btnAddAdditionalItem, and action #{channelScheduleController.addAdditionalItems()}) and AJAX/process/update references unchanged.src/main/java/com/divudi/bean/channel/ChannelScheduleController.java (1)
385-387: Good defensive null check, but inconsistent with similar method.The added null guards for
getPatient()andgetPerson()correctly prevent potential NPEs. However, the similar methodsendSmsOnChannelDoctorArrival()at line 355 still uses the less defensive check:if (bs.getBill().getPatient().getPerson().getSmsNumber() == null) {Consider applying the same fix there for consistency.
🛡️ Suggested fix for sendSmsOnChannelDoctorArrival()
- if (bs.getBill().getPatient().getPerson().getSmsNumber() == null) { + if (bs.getBill().getPatient() == null || bs.getBill().getPatient().getPerson() == null || bs.getBill().getPatient().getPerson().getSmsNumber() == null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/divudi/bean/channel/ChannelScheduleController.java` around lines 385 - 387, In ChannelScheduleController, update sendSmsOnChannelDoctorArrival() to match the defensive null checks used elsewhere: when inspecting bs, replace the current bs.getBill().getPatient().getPerson().getSmsNumber() == null check with a guarded chain that verifies bs.getBill() != null, bs.getBill().getPatient() != null, bs.getBill().getPatient().getPerson() != null, and finally bs.getBill().getPatient().getPerson().getSmsNumber() != null before proceeding; ensure you reference the same bs variable and methods (getBill(), getPatient(), getPerson(), getSmsNumber()) as in the other block to avoid NPEs and keep behavior consistent.
🤖 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/channel/ChannelScheduleController.java`:
- Around line 1204-1207: The method fillSessionInstance() currently calls
fillFees(), causing duplicate DB writes and overriding callers' intent (notably
BookingControllerViewScopeMonth, BookingControllerViewScope, and
ClinicController which call fillFees() themselves or intentionally skip it);
revert this by removing the embedded fillFees() call from fillSessionInstance()
so callers retain control, or alternatively change the signature to
fillSessionInstance(boolean includeFees) and have callers pass true/false
(update all usages in BookingControllerViewScopeMonth,
BookingControllerViewScope, ClinicController accordingly) so the behavior is
explicit and no extra getFacade().edit(current) is performed implicitly.
---
Nitpick comments:
In `@src/main/java/com/divudi/bean/channel/ChannelScheduleController.java`:
- Around line 385-387: In ChannelScheduleController, update
sendSmsOnChannelDoctorArrival() to match the defensive null checks used
elsewhere: when inspecting bs, replace the current
bs.getBill().getPatient().getPerson().getSmsNumber() == null check with a
guarded chain that verifies bs.getBill() != null, bs.getBill().getPatient() !=
null, bs.getBill().getPatient().getPerson() != null, and finally
bs.getBill().getPatient().getPerson().getSmsNumber() != null before proceeding;
ensure you reference the same bs variable and methods (getBill(), getPatient(),
getPerson(), getSmsNumber()) as in the other block to avoid NPEs and keep
behavior consistent.
In `@src/main/webapp/channel/session_instance_management.xhtml`:
- Around line 380-425: The p:column tags are being used inside a p:panelGrid
that already uses the columns attribute (unconventional); remove the p:column
wrappers so the direct children (the outputLabel, p:autoComplete with
widgetVar/acAdditionalItem and its inner columns, and the p:commandButton
btnAddAdditionalItem) are immediate children of p:panelGrid, preserving the same
order so they distribute across the three columns, and keep existing
ids/attributes (acAdditionalItem, btnAddAdditionalItem, and action
#{channelScheduleController.addAdditionalItems()}) and AJAX/process/update
references unchanged.
🪄 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: d84a806e-6aeb-4545-a5f8-3f5882995c92
📒 Files selected for processing (2)
src/main/java/com/divudi/bean/channel/ChannelScheduleController.javasrc/main/webapp/channel/session_instance_management.xhtml
buddhika75
approved these changes
Apr 4, 2026
buddhika75
deleted the
19661-session-details-additional-items-section-ui-alignment-issue
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
2 participants
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.
Issue: #19661
Files changed:
fillSessionInstance() method changed to load the relevant fee and additional item details for selected service session
sendSmsOnChannelAppointmentTimeChange method null checks improved
Additional Items and Details tabs UI improved
Summary by CodeRabbit
Bug Fixes
Improvements