Conversation
Reviewer's GuideRestores the drug profile legend UI (tabs and change-view link) in SearchDrug3.jsp and re-enables server-side filtering in ListDrugs.jsp so that the profile tabs once again return correctly filtered drug lists. Sequence diagram for restored drug profile tab filteringsequenceDiagram
actor Clinician
participant Browser
participant SearchDrug3_jsp
participant ListDrugs_jsp
Clinician->>Browser: Clicks Active_tab
Browser->>SearchDrug3_jsp: onclick callReplacementWebService(ListDrugs.jsp?status=active, drugProfile)
activate SearchDrug3_jsp
SearchDrug3_jsp->>ListDrugs_jsp: HTTP GET ListDrugs.jsp?status=active
activate ListDrugs_jsp
ListDrugs_jsp->>ListDrugs_jsp: Apply status and longTermOnly and drugLocation filters
ListDrugs_jsp-->>SearchDrug3_jsp: Filtered HTML fragment for drugProfile
deactivate ListDrugs_jsp
SearchDrug3_jsp-->>Browser: Updated drugProfile HTML
deactivate SearchDrug3_jsp
Browser->>Browser: Replace drugProfile div contents
Clinician->>Browser: Clicks LongTermAcuteInactiveExternal_tab
Browser->>SearchDrug3_jsp: Multiple calls to callReplacementWebService and callAdditionWebService
activate SearchDrug3_jsp
SearchDrug3_jsp->>ListDrugs_jsp: GET ListDrugs.jsp?longTermOnly=true&heading=Long_Term_Meds
SearchDrug3_jsp->>ListDrugs_jsp: GET ListDrugs.jsp?longTermOnly=acute&heading=Acute&status=active
SearchDrug3_jsp->>ListDrugs_jsp: GET ListDrugs.jsp?longTermOnly=acute&heading=Inactive&status=inactive
SearchDrug3_jsp->>ListDrugs_jsp: GET ListDrugs.jsp?heading=External&drugLocation=external
ListDrugs_jsp-->>SearchDrug3_jsp: Filtered segments for each request
deactivate SearchDrug3_jsp
Browser->>Browser: Render combined filtered profile view
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Review Summary by QodoRestore drug profile legend and filter tabs functionality
WalkthroughsDescription• Uncomments server-side drug filtering logic in ListDrugs.jsp - Restores Active, Inactive, Long Term/Acute, External filter functionality • Restores drug profile legend table in SearchDrug3.jsp - Re-adds filter tabs and "Change View" link that were removed Diagramflowchart LR
A["DataTables Upgrade<br/>Broke Filtering"] --> B["Commented Code<br/>in ListDrugs.jsp"]
C["Missing Legend<br/>in SearchDrug3.jsp"] --> D["No Filter Tabs<br/>or Change View Link"]
B --> E["Uncomment<br/>Filter Logic"]
D --> F["Restore Legend<br/>Table HTML"]
E --> G["Active/Inactive<br/>Filters Work"]
F --> H["Filter Tabs &<br/>Change View Restored"]
File Changes1. src/main/webapp/oscarRx/ListDrugs.jsp
|
Code Review by Qodo
|
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughListDrugs.jsp now supports multiple independently-initialized drug tables with dynamic DOM ids and per-row data attributes; SearchDrug3.jsp replaces Prototype AJAX with jQuery, adds a legend UI, queues/serializes profile updates, caches DataTable rows, and rebuilds client-side views (filters/sections). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Legend as "SearchDrug3.jsp Legend UI"
participant AJAX as "jQuery AJAX"
participant Server as "ListDrugs.jsp"
participant Browser as "Browser DOM/JS"
User->>Legend: Click legend/filter control
Legend->>AJAX: Send GET/POST (heading / table id)
AJAX->>Server: Request ListDrugs.jsp fragment
Server->>Server: Render table HTML (dynamic id, data-* attrs)
Server-->>AJAX: Return HTML fragment
AJAX-->>Browser: Inject fragment into `#drugProfile`
Browser->>Browser: Cache row nodes, destroy DataTables
Browser->>Browser: Rebuild tables (filter/showSections) using cached rows and window.drugListTableConfig
Browser-->>User: Updated drug profile view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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 restores filtering logic in ListDrugs.jsp and introduces a new legend UI in SearchDrug3.jsp for filtering drug profiles. The feedback suggests optimizing the restored logic by reducing redundant request parameter calls and using more idiomatic string comparisons to improve code quality and maintainability.
![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:
- In ListDrugs.jsp, the repeated
request.getParameter(...)lookups and string equality checks forstatus,longTermOnly, anddrugLocationwould be easier to maintain if you cached the parameters into local variables and normalized them once before the loop. - The legend markup in SearchDrug3.jsp repeats
<fmt:setBundle basename="oscarResources"/>multiple times inside anchors; consider setting the bundle once at a higher scope to reduce noise and potential mistakes. - The long
onclickhandlers in SearchDrug3.jsp that concatenate multiplecallReplacementWebService/callAdditionWebServicecalls are hard to read and error-prone; consider extracting them into named JavaScript functions to make the behavior clearer and easier to modify.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ListDrugs.jsp, the repeated `request.getParameter(...)` lookups and string equality checks for `status`, `longTermOnly`, and `drugLocation` would be easier to maintain if you cached the parameters into local variables and normalized them once before the loop.
- The legend markup in SearchDrug3.jsp repeats `<fmt:setBundle basename="oscarResources"/>` multiple times inside anchors; consider setting the bundle once at a higher scope to reduce noise and potential mistakes.
- The long `onclick` handlers in SearchDrug3.jsp that concatenate multiple `callReplacementWebService`/`callAdditionWebService` calls are hard to read and error-prone; consider extracting them into named JavaScript functions to make the behavior clearer and easier to modify.
## Individual Comments
### Comment 1
<location path="src/main/webapp/oscarRx/SearchDrug3.jsp" line_range="1223-1224" />
<code_context>
+ <table class="legend">
+ <tr>
+ <td class="legend-change-view">
+ <a href="#"
+ title="<fmt:setBundle basename="oscarResources"/><fmt:message key="provider.rxChangeProfileViewMessage"/>"
+ onclick="popupPage(230,860,'../setProviderStaleDate.do?method=viewRxProfileView');"
+ class="link-red-no-decoration">
</code_context>
<issue_to_address>
**issue (bug_risk):** The nested double quotes in the `title` attribute will break JSP/HTML parsing; adjust quoting for the JSTL tags.
Because the `title` attribute is in double quotes, the inner `<fmt:setBundle basename="oscarResources"/>` also using double quotes will prematurely close the `title` value and break JSP/HTML parsing. Use a consistent quoting strategy, e.g. single quotes for the JSTL attributes (`basename='oscarResources'`) or single quotes around the HTML attribute and keep double quotes inside. Apply the same fix to any other attributes embedding JSTL tags.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![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: 4/5
- There’s a minor but concrete UI risk: the nested double quotes in the
titleattribute could break HTML attribute parsing insrc/main/webapp/oscarRx/SearchDrug3.jsp. - Severity is moderate (6/10) and localized, so overall this looks safe to merge with minimal risk if the markup is corrected.
- Pay close attention to
src/main/webapp/oscarRx/SearchDrug3.jsp- fix thetitleattribute quoting to avoid malformed HTML.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/main/webapp/oscarRx/SearchDrug3.jsp">
<violation number="1" location="src/main/webapp/oscarRx/SearchDrug3.jsp:1224">
P2: Nested double quotes in the `title` attribute will break HTML attribute parsing. `basename="oscarResources"` uses double quotes inside a double-quoted `title`, which can cause the attribute value to be prematurely terminated. Other title attributes in this same block correctly use single quotes for JSTL tag attributes (e.g., `basename='oscarResources'`). Use single quotes consistently for JSTL attributes inside double-quoted HTML attributes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
… DataTables upgrade
D3V41
force-pushed
the
fix/restore-drug-profile-view-legend-searchdrug3
branch
from
0211d37 to
e192e60
Compare
2 tasks
…ering - Fetch all drugs once on load (show=all) instead of a separate server request per legend button click - Cache all <tr> rows via DataTables fnGetNodes() after each load so pagination does not silently truncate the row set - Filter the cached rows in JS on every legend click — no AJAX round-trips - Track activeViewMode / activeSectionDefs so non-legend refreshes (changeLt, re-rx) re-apply the correct view automatically after recaching - Extract showLongTermAcuteView() and showFullProfileView() named functions to replace unreadable inline showSections blobs in onclick attributes - Add row predicate helpers (rowIsArchived, rowIsLongTerm, rowIsCurrent, rowIsExternal) to eliminate repeated data-* string comparisons - Move ListDrugs.jsp legend styles to SearchDrug3.jsp <head> so they survive the rebuildDrugProfile() container wipe on every legend switch - Fix legend selected-state highlight: clear link-default-selected class from Current button when another legend item is activated
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/webapp/oscarRx/SearchDrug3.jsp (1)
1327-1397: Prefer JSTL for the new legend tab gating.This new legend block adds a large conditional scriptlet section to the view. Since these flags are already computed server-side, passing them as request attributes and rendering each tab with
<c:if>would keep the JSP aligned with repo conventions and make future security/template work easier.As per coding guidelines:
JSP views should use JSTL tags and avoid Java scriptlets - use request.getAttribute() for passing data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/oscarRx/SearchDrug3.jsp` around lines 1327 - 1397, Replace the scriptlet-heavy legend gating with JSTL by reading the already-computed flags from request attributes (show_current, show_all, active, inactive, longterm_acute, longterm_acute_inactive_external) instead of using inline Java scriptlets and OscarProperties.getInstance() calls; wrap each legend <td> block in a <c:if test="${...}"> (e.g. <c:if test="${show_current}">) and keep the same anchors and onclick handlers (filterDrugView, CngClass, showLongTermAcuteView, showFullProfileView) and the same i18n <fmt:message/> usages, and move the rx.profile_legend.hide check to a JSTL condition like <c:if test="${not eq paramOrAttrForRxProfileLegendHide 'true'}"> using the request attribute you set on the server.
🤖 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/webapp/oscarRx/SearchDrug3.jsp`:
- Around line 760-764: The CngClass function and the initial activeViewMode
assume the "selected_default" element exists; guard accesses and initial state
against its absence. In CngClass (and the other similar blocks at the referenced
locations), check document.getElementById("selected_default") !== null before
reading or setting .className and only clear/set it when present; also ensure
the initial activeViewMode is set based on the presence of the
"selected_default" element or the show_current flag rather than unconditionally
defaulting to "current". Update references to Lst and any legend click handlers
to tolerate a missing selected_default element so tab switching won't throw.
- Around line 2512-2525: The section filter predicates are not mutually
exclusive causing inactive and external meds to appear in multiple sections;
update the filters in showLongTermAcuteView and showFullProfileView so each
section excludes rows meant for other sections: ensure "Acute" excludes
long-term, external, and inactive rows (e.g., && !rowIsExternal(r) &&
rowIsCurrent(r)); ensure "Long Term Meds" excludes external rows (e.g., &&
!rowIsExternal(r)); ensure "Inactive" excludes external and long-term rows
(e.g., && !rowIsExternal(r) && !rowIsLongTerm(r) && !rowIsCurrent(r)); and keep
"External" as the only place that includes rowIsExternal(r) so externals are not
rendered twice (apply these changes to functions showLongTermAcuteView and
showFullProfileView).
---
Nitpick comments:
In `@src/main/webapp/oscarRx/SearchDrug3.jsp`:
- Around line 1327-1397: Replace the scriptlet-heavy legend gating with JSTL by
reading the already-computed flags from request attributes (show_current,
show_all, active, inactive, longterm_acute, longterm_acute_inactive_external)
instead of using inline Java scriptlets and OscarProperties.getInstance() calls;
wrap each legend <td> block in a <c:if test="${...}"> (e.g. <c:if
test="${show_current}">) and keep the same anchors and onclick handlers
(filterDrugView, CngClass, showLongTermAcuteView, showFullProfileView) and the
same i18n <fmt:message/> usages, and move the rx.profile_legend.hide check to a
JSTL condition like <c:if test="${not eq paramOrAttrForRxProfileLegendHide
'true'}"> using the request attribute you set on the server.
🪄 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: 110e9032-a84d-4b77-b979-c7aab6d362c4
📒 Files selected for processing (2)
src/main/webapp/oscarRx/ListDrugs.jspsrc/main/webapp/oscarRx/SearchDrug3.jsp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/oscarRx/ListDrugs.jsp
There was a problem hiding this comment.
4 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/main/webapp/oscarRx/ListDrugs.jsp">
<violation number="1" location="src/main/webapp/oscarRx/ListDrugs.jsp:148">
P1: Filter-tab request parameters are no longer applied, so tab-specific loads can return unfiltered drug lists.</violation>
</file>
<file name="src/main/webapp/oscarRx/SearchDrug3.jsp">
<violation number="1" location="src/main/webapp/oscarRx/SearchDrug3.jsp:761">
P1: `document.getElementById("selected_default")` returns `null` when `show_current` is disabled (the element is conditionally rendered inside `<%if (show_current) {%>`). This will throw a `TypeError` and break all legend tab switching. Add a null guard before accessing `.className`.</violation>
<violation number="2" location="src/main/webapp/oscarRx/SearchDrug3.jsp:2514">
P2: The Long Term, Acute, and Inactive section filters in `showFullProfileView()` don't exclude external rows, but the External section independently matches on `rowIsExternal`. This causes external medications to appear in two sections simultaneously (e.g., an external long-term med appears in both "Long Term Meds" and "External"). Add `!rowIsExternal(r)` to the first three predicates.</violation>
<violation number="3" location="src/main/webapp/oscarRx/SearchDrug3.jsp:2515">
P1: The Acute filter `r => !rowIsArchived(r) && !rowIsLongTerm(r)` will include inactive and external medications in the "Acute" section. Since the server now returns all drugs (including archived/inactive/external), this predicate needs to also exclude external rows and non-current rows to match the intended semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Collaborator
|
@D3V41 Can we also make these headings look more like headings (i.e. larger font, and maybe bold?) while we're here? 
|
LiamStanziani
approved these changes
Apr 14, 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
3 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
SearchDrug3.jspthat was removed in edac1715d550de8062d066badc82eab62d833908Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes