Feat: add specialist name to the eChart Consultations box#2487
Open
LiamStanziani wants to merge 4 commits into
Open
Feat: add specialist name to the eChart Consultations box#2487LiamStanziani wants to merge 4 commits into
LiamStanziani wants to merge 4 commits into
Conversation
… the hover format to follow the new format in the echart box, guard for missing values to format correctly in potential use cases
📝 WalkthroughWalkthrough
ChangesReferral Label N/A Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Collaborator
Author
|
@SourceryAI review |
Collaborator
Author
|
@coderabbitai review |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
✅ Action performedReview finished.
|
Reviewer's GuideUpdate eChart Consultations list items to show a combined service/specialist label before the date, with HTML-encoded titles/tooltips and shared handling of the "N/A" sentinel. Sequence diagram for building consultation row label and tooltipsequenceDiagram
participant EctDisplayConsult2Action
participant ConsultationListDTO
participant NavBarItem
EctDisplayConsult2Action->EctDisplayConsult2Action: hasService = !service.trim().isEmpty()
EctDisplayConsult2Action->EctDisplayConsult2Action: trimmedSpecialist = specialist.trim()
EctDisplayConsult2Action->ConsultationListDTO: check NOT_APPLICABLE against trimmedSpecialist
EctDisplayConsult2Action->EctDisplayConsult2Action: build referralLabel (service, specialist)
EctDisplayConsult2Action->EctDisplayConsult2Action: linkTitle = referralLabel + " " + date or date alone
EctDisplayConsult2Action->NavBarItem: setLinkTitle(Encode.forHtmlAttribute(linkTitle))
EctDisplayConsult2Action->NavBarItem: setTitle(Encode.forHtml(StringUtils.maxLenString(referralLabel)))
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the referral label construction in EctDisplayConsult2Action to dynamically format "Service - Specialist" while omitting missing parts or "N/A" placeholders. To support this, NOT_APPLICABLE in ConsultationListDTO was made public. The review feedback suggests conditionally formatting the link title to prevent a leading space when both the service and specialist are empty.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- If both service and specialist are empty,
referralLabelbecomes an empty string; consider falling back to showing at least the date or a minimal label so the row isn’t visually blank. - The
referralLabelconstruction logic is embedded inEctDisplayConsult2Action; consider extracting it into a reusable helper or method (potentially nearConsultationListDTO) so other views that show consultations can use the same formatting consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If both service and specialist are empty, `referralLabel` becomes an empty string; consider falling back to showing at least the date or a minimal label so the row isn’t visually blank.
- The `referralLabel` construction logic is embedded in `EctDisplayConsult2Action`; consider extracting it into a reusable helper or method (potentially near `ConsultationListDTO`) so other views that show consultations can use the same formatting consistently.
## Individual Comments
### Comment 1
<location path="src/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java" line_range="145-154" />
<code_context>
+ String referralLabel;
</code_context>
<issue_to_address>
**issue:** Handle the case where both service and specialist are missing to avoid odd-looking labels.
If both `service` and `specialist` are empty/blank/`N/A`, `referralLabel` becomes `""`. This makes `setLinkTitle(referralLabel + " " + serviceDateStr)` start with a space when only the date is present, and `setTitle(...)` become an empty title. Please add a fallback (e.g., `serviceDateStr` alone or a minimal description) so the navbar label remains meaningful and avoids leading whitespace.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java`:
- Around line 142-157: The referral label built in EctDisplayConsult2Action is
passed into two HTML sinks without encoding, which can allow stored XSS. Update
the values sent to item.setTitle() and item.setLinkTitle() so the visible text
is encoded with Encode.forHtml() and the tooltip/attribute value is encoded with
Encode.forHtmlAttribute(), using the OWASP Encoder imports from
org.owasp.encoder. Keep the sanitization at the point where referralLabel and
serviceDateStr are combined so LeftNavBarDisplay.jsp receives already-safe
output.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4fdfb90d-bb1f-4dd2-b5d4-7a7284e51355
📒 Files selected for processing (2)
src/main/java/ca/openosp/openo/consultation/dto/ConsultationListDTO.javasrc/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java
…l and conditional path for hasService boolean, catch potential formatting issue with LinkTitle if referralLabel is empty (fall back to only service date in that case), and encode LinkTitle and Title
Collaborator
Author
|
@SourceryAI review |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The referral label construction in
getInfois getting a bit complex; consider extracting it into a reusable helper (e.g., onConsultationListDTOor a formatter class) so the formatting rules are defined and updated in a single place. - Now that
NOT_APPLICABLEis public and used in equality checks, it may be worth normalizing specialist values (trim/case) before comparison or centralizing similar sentinel values to avoid subtle mismatches in placeholder filtering.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The referral label construction in `getInfo` is getting a bit complex; consider extracting it into a reusable helper (e.g., on `ConsultationListDTO` or a formatter class) so the formatting rules are defined and updated in a single place.
- Now that `NOT_APPLICABLE` is public and used in equality checks, it may be worth normalizing specialist values (trim/case) before comparison or centralizing similar sentinel values to avoid subtle mismatches in placeholder filtering.
## Individual Comments
### Comment 1
<location path="src/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java" line_range="140-144" />
<code_context>
- item.setTitle(service);
+ //label as "Service - Specialist", omitting any missing part or the "N/A" placeholder
+ boolean hasService = service != null && !service.trim().isEmpty();
+ boolean hasSpecialist = specialist != null && !specialist.trim().isEmpty()
+ && !ConsultationListDTO.NOT_APPLICABLE.equals(specialist);
+ String referralLabel = "";
+ if (hasService && hasSpecialist) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align the NOT_APPLICABLE comparison with the trimmed specialist value.
You already trim `specialist` to check emptiness, but the NOT_APPLICABLE comparison uses the untrimmed value. This means values like " N/A" or "N/A " will still be treated as valid and displayed. Compare against the trimmed (and possibly case-insensitive) value instead, e.g.:
```java
String trimmedSpecialist = specialist.trim();
boolean hasSpecialist = specialist != null
&& !trimmedSpecialist.isEmpty()
&& !ConsultationListDTO.NOT_APPLICABLE.equalsIgnoreCase(trimmedSpecialist);
```
This keeps the display logic consistent even when the source data has extra whitespace.
```suggestion
//label as "Service - Specialist", omitting any missing part or the "N/A" placeholder
boolean hasService = service != null && !service.trim().isEmpty();
String trimmedSpecialist = specialist != null ? specialist.trim() : "";
boolean hasSpecialist = specialist != null
&& !trimmedSpecialist.isEmpty()
&& !ConsultationListDTO.NOT_APPLICABLE.equalsIgnoreCase(trimmedSpecialist);
String referralLabel = "";
```
</issue_to_address>
### Comment 2
<location path="src/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java" line_range="153-157" />
<code_context>
+
+ //date alone when no label, to avoid a leading space in the tooltip
+ String linkTitle = referralLabel.isEmpty() ? serviceDateStr : referralLabel + " " + serviceDateStr;
+ //encode both sinks (JSP writes them unescaped); truncate before encoding
+ item.setLinkTitle(Encode.forHtmlAttribute(linkTitle));
+ item.setTitle(Encode.forHtml(StringUtils.maxLenString(referralLabel, MAX_LEN_TITLE, CROP_LEN_TITLE, ELLIPSES)));
item.setURL(url);
item.setDate(date);
</code_context>
<issue_to_address>
**suggestion:** Consider truncating the linkTitle string before encoding, or update the comment to match behavior.
Only `referralLabel` (used for `setTitle`) is truncated; `linkTitle` (used for `setLinkTitle`) is encoded without truncation, so the current behavior doesn’t match the "truncate before encoding" comment. Either also apply `StringUtils.maxLenString` to `linkTitle` before encoding, or update the comment to reflect that only the title is truncated.
```suggestion
//date alone when no label, to avoid a leading space in the tooltip
String linkTitle = referralLabel.isEmpty() ? serviceDateStr : referralLabel + " " + serviceDateStr;
//encode both sinks (JSP writes them unescaped); truncate before encoding
String truncatedLinkTitle = StringUtils.maxLenString(linkTitle, MAX_LEN_TITLE, CROP_LEN_TITLE, ELLIPSES);
item.setLinkTitle(Encode.forHtmlAttribute(truncatedLinkTitle));
item.setTitle(Encode.forHtml(StringUtils.maxLenString(referralLabel, MAX_LEN_TITLE, CROP_LEN_TITLE, ELLIPSES)));
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… edge case issues
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When building
referralLabel, you use the untrimmedspecialistrather thantrimmedSpecialist, which can reintroduce leading/trailing spaces; consider using the trimmed value consistently. - The tooltip falls back to
serviceDateStrwhenreferralLabelis empty; ifserviceDateStrcan be null or blank, it may be worth guarding against that to avoid an entirely empty tooltip.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When building `referralLabel`, you use the untrimmed `specialist` rather than `trimmedSpecialist`, which can reintroduce leading/trailing spaces; consider using the trimmed value consistently.
- The tooltip falls back to `serviceDateStr` when `referralLabel` is empty; if `serviceDateStr` can be null or blank, it may be worth guarding against that to avoid an entirely empty tooltip.
## Individual Comments
### Comment 1
<location path="src/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java" line_range="145-152" />
<code_context>
+ boolean hasSpecialist = !trimmedSpecialist.isEmpty()
+ && !ConsultationListDTO.NOT_APPLICABLE.equals(trimmedSpecialist);
+ String referralLabel = "";
+ if (hasService && hasSpecialist) {
+ referralLabel = service + " - " + specialist;
+ } else if (hasSpecialist) {
+ referralLabel = specialist;
</code_context>
<issue_to_address>
**suggestion:** Use the trimmed specialist value when building the referral label to avoid stray whitespace or inconsistent display.
`trimmedSpecialist` is used for the presence check, but `referralLabel` still uses `specialist`, which can reintroduce leading/trailing spaces in the UI. Please use `trimmedSpecialist` when constructing `referralLabel` (and in other branches using `specialist`) so the displayed label is consistently normalized.
```suggestion
String referralLabel = "";
if (hasService && hasSpecialist) {
referralLabel = service + " - " + trimmedSpecialist;
} else if (hasSpecialist) {
referralLabel = trimmedSpecialist;
} else if (hasService) {
referralLabel = service;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 5/5
- In
src/main/java/ca/openosp/openo/encounter/pageUtil/EctDisplayConsult2Action.java, specialist-only labels still use the untrimmed value, so leading/trailing whitespace can leak into rendered text and cause minor formatting inconsistencies after merge; switch those label assignments totrimmedSpecialistbefore merging.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
D3V41
approved these changes
Jun 29, 2026
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.
Summary
Adds the referred-to specialist's name into each row of the Consultations box in the eChart
left navbar, so referrals can be identified at a glance without opening them. Rows now read
Service - Specialistbefore the date (e.g.OBGYN - Smith, John ... 2023-09-15), and the hovertooltip is aligned to the same format.
Closes: Add the specialist name into the consultation box in the chart
Problem
In the eChart Consultations box, each referral row showed only the service type and the referral
date — with no indication of who the referral was to:
This made it slow to identify the right referral when processing appointment notifications or
locating a referral referenced in a tickler, and contributed to duplicate-referral errors (a new
referral created when one already existed).
Solution
Surface the specialist inline as
Service - Specialist, immediately before the existing date. Thespecialist name was already loaded by the same query that builds the list (and already shown in the
row's hover tooltip), so this surfaces existing data rather than adding any new query.
Changes:
EctDisplayConsult2Action.java— builds the row label asService - Specialist, omittingwhichever part is missing so there is never a stray separator or an
"N/A"placeholder(both →
Service - Specialist; specialist only →Specialist; service only →Service). Thevisible title and the hover tooltip now use the same format; the tooltip also appends the date,
replacing the legacy
Service(Specialist) Dateparenthesis format (which removes the oldService(N/A) Dateartifact on no-specialist rows).ConsultationListDTO.java—NOT_APPLICABLE("N/A") promoted topublicso theno-specialist sentinel has a single source of truth that the action references.
Behavior (visible row / hover tooltip):
OBGYN - Smith, JohnOBGYN - Smith, John 2023-09-15OBGYNOBGYN 2023-09-15Cardiology - Dr. JonesCardiology - Dr. Jones 2023-09-15Smith, JohnSmith, John 2023-09-15UI Before:
UI After
Summary by Sourcery
Show referral service and specialist inline in eChart Consultations rows and tooltips to make referrals identifiable at a glance.
New Features:
Bug Fixes:
Enhancements:
Summary by cubic
Show the specialist’s name in each row of the eChart Consultations box so referrals are identifiable at a glance. Rows now read “Service - Specialist” before the date, and the hover tooltip matches this format.
New Features
NOT_APPLICABLEas public for consistent checks.Bug Fixes
NOT_APPLICABLE) to avoid edge-case formatting issues.Written for commit e5c915b. Summary will update on new commits.
Summary by CodeRabbit