Conversation
…Drug3, ShowAllergies2, StaticScript, SideLinks, RxSearchAllergy2Form)
Reviewer's GuideFixes multiple JavaScript runtime issues and HTML/script tag malformations in oscarRx JSPs, and hardens JSON deserialization for allergy search, to restore favorites and allergy search behavior without console or server errors. Sequence diagram for allergy search JSON handling with RxSearchAllergy2FormsequenceDiagram
actor User
participant Browser
participant ShowAllergies2_jsp
participant RxSearchAllergy2Action
participant JacksonObjectMapper
participant RxSearchAllergy2Form
User->>Browser: Fill and submit allergy search form
Browser->>ShowAllergies2_jsp: POST serialized form data
ShowAllergies2_jsp->>RxSearchAllergy2Action: Forward request with JSON payload
RxSearchAllergy2Action->>JacksonObjectMapper: Map JSON to RxSearchAllergy2Form
JacksonObjectMapper->>RxSearchAllergy2Form: Populate known fields (searchString, etc.)
JacksonObjectMapper-->>RxSearchAllergy2Action: Ignore unknown fields due to JsonIgnoreProperties_ignoreUnknown_true
RxSearchAllergy2Action->>RxSearchAllergy2Action: Execute allergy search
RxSearchAllergy2Action-->>ShowAllergies2_jsp: Return search results model
ShowAllergies2_jsp-->>Browser: Render allergy search results
Sequence diagram for global toggleSection in allergy search resultssequenceDiagram
actor User
participant Browser
participant ShowAllergies2_jsp
participant ChooseAllergy2_jsp
participant toggleSection
User->>Browser: Click expander icon in allergy results
Browser->>ChooseAllergy2_jsp: Trigger onclick handler on anchor
ChooseAllergy2_jsp->>toggleSection: Call toggleSection(typecode)
toggleSection->>toggleSection: Lookup typecode_img and typecode_content elements
alt Expander_icon_shows_expander
toggleSection->>Browser: Set image src to collapser and display content block
else Expander_icon_shows_collapser
toggleSection->>Browser: Set image src to expander and hide content block
end
Sequence diagram for useFav2 favorites navigation from allergies pagesequenceDiagram
actor User
participant Browser
participant ShowAllergies2_jsp
participant SideLinksEditFavorites2_jsp
participant SearchDrug3_jsp
User->>Browser: Click favorite in allergies sidebar
Browser->>SideLinksEditFavorites2_jsp: Execute onclick handler
SideLinksEditFavorites2_jsp->>Browser: Call useFav2(favoriteId)
Browser->>Browser: useFav2 sets location href to SearchDrug3_jsp with usefav and favid
Browser->>SearchDrug3_jsp: Navigate to SearchDrug3.jsp?usefav=true&favid=favoriteId
SearchDrug3_jsp->>SearchDrug3_jsp: Load page and stage selected favorite
SearchDrug3_jsp-->>Browser: Render medications page with staged favorite
Updated class diagram for RxSearchAllergy2Form JSON handlingclassDiagram
class RxSearchAllergy2Form {
<<JsonIgnoreProperties_ignoreUnknown_true>>
-searchString : String
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis pull request introduces null-safety checks, HTTP response metadata configuration, and JavaScript enhancements across backend Java classes and JSP frontend files. Changes include Jackson annotation support for JSON deserialization, explicit HTTP response headers in action classes, defensive programming patterns in drug search and allergy pages, and corrections to script tag syntax and event handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 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
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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:
- The new global
toggleSectionanduseFav2helpers are now defined inline in individual JSPs; consider centralizing these in a shared JS asset (e.g., Oscar/global scripts) to avoid duplication and keep behavior consistent across pages. - The
toggleSectionlogic still relies on checkingimg.src.indexOf('expander')to determine state; if you touch this again, it may be more robust to use a data attribute or a CSS class toggle rather than depending on filename substrings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new global `toggleSection` and `useFav2` helpers are now defined inline in individual JSPs; consider centralizing these in a shared JS asset (e.g., Oscar/global scripts) to avoid duplication and keep behavior consistent across pages.
- The `toggleSection` logic still relies on checking `img.src.indexOf('expander')` to determine state; if you touch this again, it may be more robust to use a data attribute or a CSS class toggle rather than depending on filename substrings.
## Individual Comments
### Comment 1
<location> `src/main/webapp/oscarRx/SideLinksEditFavorites2.jsp:158-160` </location>
<code_context>
+<script type="text/javascript">
+ // Pulled from function in SearchDrug3.jsp - function needs to be defined here
+ // for ShowAllergies2.jsp for favorite staging in allergies page
+ function useFav2(favoriteId) {
+ location.href = "<%= request.getContextPath() %>/oscarRx/SearchDrug3.jsp?usefav=true&favid=" + favoriteId;
+ }
+</script>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** URL-encode `favoriteId` when building the navigation URL.
If `favoriteId` ever includes non-numeric or special characters, concatenating it directly into the query string can produce malformed URLs or unexpected behavior. Use `encodeURIComponent(favoriteId)` when building the URL to ensure the value is safely encoded.
```suggestion
function useFav2(favoriteId) {
location.href = "<%= request.getContextPath() %>/oscarRx/SearchDrug3.jsp?usefav=true&favid=" + encodeURIComponent(favoriteId);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Code Review - CLAUDE.md Compliance IssuesI found 2 issues that need to be addressed: 1. Missing
|
LiamStanziani
changed the title
|
@claude review |
|
Claude finished @LiamStanziani's task in 2m 21s —— View job Code Review Complete ✅Todo List:
SummaryThis PR successfully fixes multiple JavaScript runtime errors and JSON parsing issues in the oscarRx prescription module. The changes are well-documented and address real bugs that would impact user experience. I found 3 critical issues and 2 minor suggestions that need attention before merging. ✅ What's Working Well
❌ Critical Issues1. Missing
|
|
Those two compliance issue I don't think matter currently, but could be addressed if needed (especially targeting develop since develop is behind on changes and targeting it will flood the diff changes with everything in staging currently, on top of the changes in this branch) |
|
@coderabbitai create separate follow up issue tickets for the code recommendations as coverd in this. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 674 🏁 Script executed: Length of output: 1356 🏁 Script executed: Length of output: 27380
🏁 Script executed: Length of output: 230
Created Issues:
All issues have been assigned to you and include:
🧠 Learnings used |
…ute/Inactive/External, fixed undefined error when clicking Longterm/Acute/Inactive/External due to the element being checked before its checked to exist
|
Small additional updates for null checking and possible 404 error with sorting, will add to the PR description |
…ew Exact behaviour to match production and moved the new Exact as the first radio button option
|
Now "Exact" is flipped to be the old "Any" logic, (searches left to right) and "Any" is now flipped to be the "Exact" logic (searches anywhere), "Exact" is now the functionality of "Any" so it is set as the default checked value and is moved infront of "Any" 
Image 1: default radio box checked when opening the medications page 
Image 3: Searching for "METFORMIN" with the new "Any" radio button selected 
|
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Review Summary:
This PR effectively fixes multiple console errors and workflow issues in the prescription module. |
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/webapp/oscarRx/SearchDrug3.jsp (1)
786-846: Add CSRF token todrugForm.
The POST form at Line 786 is missing the required CSRF hidden field.As per coding guidelines, include CSRF token in all HTML forms.🔒 Proposed fix
- <form action="${pageContext.request.contextPath}/oscarRx/searchDrug.do" onsubmit="return checkEnterSendRx();" style="display: inline; margin-bottom:0;" id="drugForm" name="drugForm" method="post"> + <form action="${pageContext.request.contextPath}/oscarRx/searchDrug.do" onsubmit="return checkEnterSendRx();" style="display: inline; margin-bottom:0;" id="drugForm" name="drugForm" method="post"> + <input type="hidden" name="${_csrf.parameterName}" value="${_csrf.token}"/>
🤖 Fix all issues with AI agents
In `@src/main/java/ca/openosp/openo/prescript/pageUtil/RxSearchAllergy2Form.java`:
- Around line 31-40: The class JavaDoc for RxSearchAllergy2Form is missing the
required `@since` tag; update the class-level comment (above class
RxSearchAllergy2Form) to include an `@since` entry using the file’s original
introduction date from the git history (find the commit that added this file and
use that date), ensuring the tag is formatted like other public classes in the
codebase.
In `@src/main/webapp/oscarRx/SearchDrug3.jsp`:
- Around line 1799-1803: Replace the HTML-only <c:out value="${ctx}"/> inside
callAdditionWebService with the OWASP Java Encoder call for JS context so the
ctx is safely encoded for a JavaScript string (e.g. use <%=
org.owasp.encoder.Encode.forJavaScript(ctx) %>); update the JSP to import or
reference org.owasp.encoder.Encode if needed and keep the rest of the
callAdditionWebService function and its url handling unchanged.
src/main/java/ca/openosp/openo/prescript/pageUtil/RxSearchAllergy2Form.java
Show resolved
Hide resolved
2 participants
In this PR, I have fixed:
File: src/main/webapp/oscarRx/SearchDrug3.jsp
Issue: Extra } at line 1237 broke JS parsing, causing iterateStash is not defined
Fix: Removed the orphan brace
File: src/main/webapp/oscarRx/SideLinksNoEditFavorites.jsp (line 81)
Issue: <script ... /> is invalid HTML5
Fix: Changed to <script ...></script>
File: src/main/webapp/oscarRx/SideLinksNoEditFavorites2.jsp (line 82)
Issue: <script ... /> is invalid HTML5
Fix: Changed to <script ...></script> (didn't break the pages, but could cause issues in the future if the surrounding scripts are modified)
File: src/main/webapp/oscarRx/StaticScript2.jsp (lines 131-134)
Issue: Broken script tag syntax
Fix: Corrected to proper <script ...></script> format
Files: Multiple files with addFavorite function
Issue: window.prompt() returns null when cancelled, causing .length error, doesn't break the page but will output the error
Fix: Added null check: if (favoriteName !== null && favoriteName.length > 0)
File: src/main/webapp/oscarRx/SideLinksEditFavorites2.jsp
Issue: useFav2 is not defined when clicking favorites in ShowAllergies2.jsp
Fix: Added the useFav2() function with comment explaining its purpose
File: src/main/webapp/oscarRx/ShowAllergies2.jsp
Issue: Function was defined as jQuery plugin inside $(document).ready(), not accessible to AJAX-loaded content
Fix:
Java Fixes
File: src/main/java/ca/openosp/openo/prescript/pageUtil/RxSearchAllergy2Form.java
Issue: Failed to parse JSON to POJO - unrecognized field reactionDescription due to having different JSON structures between the add allergies and search allergies functionality (having both open at the same time will fail)
Fix: Added @JsonIgnoreProperties(ignoreUnknown = true) annotation and import
Additional fixes after first PR state:
some entries may not be present
I have tested this by:
Summary by Sourcery
Fix JavaScript and JSON parsing issues in the oscarRx prescription module to restore allergy search behavior and favorites handling without runtime errors.
Bug Fixes:
Enhancements:
Summary by cubic
Fixes multiple JavaScript errors and malformed script tags in the oscarRx prescription module, restoring allergy search toggling and favorites without console errors. Also hardens JSON parsing, prevents an NPE in ChooseDrug.jsp when drugref returns null, and removes 404s/undefined errors in Rx filters.
Written for commit 69299d5. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.