259 medical history returning 500 error#309
Closed
AlexSilveira1 wants to merge 30 commits intodevelop/coyotefrom
Closed
259 medical history returning 500 error#309AlexSilveira1 wants to merge 30 commits intodevelop/coyotefrom
AlexSilveira1 wants to merge 30 commits intodevelop/coyotefrom
Conversation
…ome key observations and recommendations for addressing remaining Struts 1 to Struts 2 migration issues:
1. Action Package Configuration
The added `actionPackages` parameter in web.xml is a good start. This helps Struts 2 discover action classes in the specified packages automatically.
2. Relative Path Fixes
Common patterns for fixing relative paths include:
```java
// Replace
href="../some/path"
// With
href="<%= request.getContextPath() %>/some/path"
```
3. Parameter Dispatching
Look for actions still using `request.getParameter("method")` and replace with more specific parameter names like `action` or `dispatch`.
Example refactoring:
```java
// Old style
String method = request.getParameter("method");
if ("someAction".equals(method)) {
// do something
}
// New style
String action = request.getParameter("action");
if ("someAction".equals(action)) {
// do something
}
```
4. Struts 2 Mapping Recommendations
In struts.xml, ensure actions are mapped correctly:
```xml
<action name="actionName" class="fully.qualified.ActionClass">
<result name="success">/path/to/success.jsp</result>
<result name="error">/path/to/error.jsp</result>
</action>
```
5. Static Resource Handling
The previous commit added static resource exclusions in struts.xml:
```xml
<constant name="struts.serve.static" value="true" />
<constant name="struts.serve.static.browserCache" value="true" />
<constant name="struts.action.excludePattern" value=".*\.(css|js|png|jpg|gif)$" />
```
To proceed, I recommend:
1. Reviewing all Action classes for old-style method dispatching
2. Checking JSP files for relative path issues
3. Verifying struts.xml mappings
4. Ensuring all static resources are properly handled
Would you like me to help you systematically review these areas?
…d parameter and fix social history note saving
…t number of bullets
- Updated JavaScript to fall back to main hidden encType field when specific encTypeSelect element doesn't exist for issue sections (Social History, Medical History, etc.) - Added JSTL expression to populate hidden encType field from form bean in ChartNotes.jsp - This fixes the issue where clicking + button on issue sections wasn't saving properly due to missing encounter type parameter Issue: Struts 1 automatically populated form fields, but Struts 2 requires explicit JSTL injection
…tion' into 259-medical-history-returning-500-error
Reviewer's GuideThis PR addresses a 500 error in the medical history tab by injecting the logged-in provider number into client requests and extensively refactors the note-editing JavaScript, JSP form markup, AJAX flows, and action classes to add logging, improve parameter handling, and strengthen error handling across the application. Sequence diagram for editing and saving a CPP note (Medical History)sequenceDiagram
actor User
participant Browser
participant JS as JavaScript (newCaseManagementView.js.jsp)
participant Server as CaseManagementEntry2Action
participant DB as Database
User->>Browser: Clicks edit on Medical History note
Browser->>JS: Calls showEdit(...)
JS->>Browser: Populates and displays edit form
User->>Browser: Edits note and clicks Save
Browser->>JS: Submits form (onsubmit=updateCPPNote)
JS->>Server: AJAX POST to /CaseManagementEntry.do (with providerNo, demographicNo, etc.)
Server->>Server: Handles method param (method, action, dispatch, parameterValue)
Server->>DB: Updates note and issues
DB-->>Server: DB update result
Server-->>JS: Returns updated note HTML
JS->>Browser: Updates note display
JS->>Server: (If Medical History) Reloads only Medical History tab via AJAX
Server-->>JS: Returns refreshed Medical History notes
JS->>Browser: Updates Medical History tab
Class diagram for CaseManagementEntry2Action parameter handling and note savingclassDiagram
class CaseManagementEntry2Action {
+execute() : String
+issueNoteSave() : String
-request : HttpServletRequest
-sessionFrm : CaseManagementEntryForm
}
class CaseManagementEntryForm {
+getIssueCheckList() : CheckBoxBean[]
}
class CheckBoxBean {
+issue : CaseManagementIssue
+checked : String
}
class CaseManagementIssue {
+getIssue() : Issue
}
class Issue {
+getCode() : String
}
CaseManagementEntry2Action --> CaseManagementEntryForm : uses
CaseManagementEntryForm --> CheckBoxBean : contains
CheckBoxBean --> CaseManagementIssue : has
CaseManagementIssue --> Issue : has
Class diagram for JavaScript note editing and AJAX update flowclassDiagram
class showEdit {
+showEdit(e, title, noteId, editors, date, revision, note, url, containerDiv, reloadUrl, noteIssues, noteExts, demoNo)
}
class updateCPPNote {
+updateCPPNote()
}
class prepareExtraFields {
+prepareExtraFields(cpp, exts)
}
class openAnnotation {
+openAnnotation()
}
showEdit ..> updateCPPNote : triggers on form submit
showEdit ..> prepareExtraFields : calls
showEdit ..> openAnnotation : sets up event
updateCPPNote ..> AjaxRequest : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
yingbull
suggested changes
Jul 3, 2025
| // copy existing issues for sessionfrm | ||
| for (int idx = 0; idx < existingCaseIssueList.length; ++idx) { | ||
| caseIssueList.add(existingCaseIssueList[idx]); | ||
| CheckBoxBean[] existingCaseIssueList = null; |
There was a problem hiding this comment.
not anything I will send for a request for change, but do look to up the inline comment bar etc at least in the areas being worked on where possible.
| <filter> | ||
| <filter-name>struts2</filter-name> | ||
| <filter-class>org.apache.struts2.dispatcher.filter.StrutsPrepareAndExecuteFilter</filter-class> | ||
| <init-param> |
There was a problem hiding this comment.
OK, going to ask for some comments here that help explain what the additional filter settings do/why they are needed.
| <location>/errorpage.jsp</location> | ||
| </error-page> | ||
| <error-page> | ||
| <exception-type>java.lang.Exception</exception-type> |
There was a problem hiding this comment.
I like this, but should we have a different setup for development we should have an alternative file we someone use etc?
| title="${remoteNote.location} by ${remoteNote.providerName} on ${remoteNote.observationDate}" | ||
| href="javascript:void(0)" | ||
| onclick="showIntegratedNote('${titleMsg}', '${remoteNote.note}', '${remoteNote.location}', '${remoteNote.providerName}', '${remoteNote.observationDate}');"> | ||
| onclick="showIntegratedNote('<%=StringEscapeUtils.escapeJavaScript(titleMsg)%>', '${remoteNote.note}', '${remoteNote.location}', '${remoteNote.providerName}', '${remoteNote.observationDate}');"> |
There was a problem hiding this comment.
I'll take the c:out's for now (as I think down the road the right way to use them is to get a custom tag that wraps the owasp sanitizer), but the owasp one is better that stringescape tools, particularly as it has context for different things ie attributes as this case. I see some other non sanitized data; let's go with using the owasp tool to do it. I know some of it is just refactoring what was not clean before, but let's leave it better if we are working on it.
Collaborator
|
Closing this PR since issue was fixed in another branch by Deval that was merged. PR: #372 |
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 by Sourcery
Fix 500 error when loading medical history by properly injecting provider number into the notes API call and refactor related JavaScript and server code to improve reliability, debugging, and UI behavior.
Bug Fixes:
Enhancements: