Fixes: Display manual case classification.#13880
Conversation
Summary: Displaying the manual classification, which has configured in disease level Changes: Verifying the caseClassificationMode, if it's DISABLED, does nothing. For AUTOMATIC, display the automatic case classification in the table. For MANUAL, display the plain text box with the configured details.
📝 WalkthroughWalkthroughCaseDataForm now shows classification UI conditionally based on disease configuration and calculation mode: if automatic classification is enabled and data exists it shows the rules/info button; otherwise it exposes a manual case-definition button that opens a popup with the disease's case definition HTML. Backend null-check prevents NPE when fetching case definition text. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java (1)
1367-1373: Reuse the fetched classification mode instead of querying config twice.Line [1372] re-calls
getCaseClassificationCalculationMode(disease)although Line [1367] already has the value. This is avoidable facade churn during form init.♻️ Proposed refactor
- if (FacadeProvider.getConfigFacade().getCaseClassificationCalculationMode(disease).isAutomaticEnabled() + if (caseClassificationCalculationMode.isAutomaticEnabled() && diseaseClassificationExists()) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java` around lines 1367 - 1373, The code fetches the case classification mode once into caseClassificationCalculationMode but then calls FacadeProvider.getConfigFacade().getCaseClassificationCalculationMode(disease) again; replace the second call with the already-fetched variable (caseClassificationCalculationMode) so the if uses caseClassificationCalculationMode.isAutomaticEnabled() && diseaseClassificationExists(), avoiding the redundant config fetch and facade churn in CaseDataForm.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java`:
- Around line 1545-1549: The backend method
DiseaseConfigurationFacadeEjb.getCaseDefinitionText currently calls
service.getDiseaseConfiguration(disease).getCaseDefinitionText() without
guarding for a null configuration; change it to first retrieve the configuration
into a local variable (or use Optional.map) and null-check it before calling
getCaseDefinitionText, or use service.getDiseaseConfiguration(disease).map(cfg
-> cfg.getCaseDefinitionText()).orElse(null) so the method returns null safely
when no configuration exists instead of throwing a NullPointerException.
- Around line 1555-1559: Replace the HTML rendering mode for the label showing
caseDefinitionText to avoid rendering raw HTML: locate the Label instance
suspectContent in CaseDataForm and change the call
suspectContent.setContentMode(ContentMode.HTML) to
suspectContent.setContentMode(ContentMode.TEXT) so the value from
caseDefinitionText is rendered as plain text before adding it to
classificationRulesLayout.
---
Nitpick comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java`:
- Around line 1367-1373: The code fetches the case classification mode once into
caseClassificationCalculationMode but then calls
FacadeProvider.getConfigFacade().getCaseClassificationCalculationMode(disease)
again; replace the second call with the already-fetched variable
(caseClassificationCalculationMode) so the if uses
caseClassificationCalculationMode.isAutomaticEnabled() &&
diseaseClassificationExists(), avoiding the redundant config fetch and facade
churn in CaseDataForm.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91f95597-9a8c-4c4e-99ee-d0887f0df9e4
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java
| Label suspectContent = new Label(); | ||
| suspectContent.setContentMode(ContentMode.HTML); | ||
| suspectContent.setWidth(100, Unit.PERCENTAGE); | ||
| suspectContent.setValue(caseDefinitionText); | ||
| classificationRulesLayout.addComponent(suspectContent); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the lines in question
head -1559 "sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java" | tail -20 | cat -nRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 1181
🏁 Script executed:
# Get more context around the flagged lines
sed -n '1540,1570p' "sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java" | cat -nRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 1730
🏁 Script executed:
# Search for where caseDefinitionText is defined/assigned
rg "caseDefinitionText" "sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java" -B 5 -A 2Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 1014
🏁 Script executed:
# Check for any sanitization or escaping functions in the file
rg "sanitize|escape|HTML\.escape" "sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java"Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 58
🏁 Script executed:
# Search for other ContentMode.HTML usages in this file to understand patterns
rg "ContentMode\.HTML" "sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java" -B 3 -A 3Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 2311
🏁 Script executed:
# Find the DiseaseConfigurationFacade implementation
find . -name "*DiseaseConfigurationFacade*" -type f | head -20Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 459
🏁 Script executed:
# Search for getCaseDefinitionText method definition
rg "getCaseDefinitionText" --type java -B 2 -A 10Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 14621
🏁 Script executed:
# Check if caseDefinitionText is stored in database and could be user-controlled
rg "caseDefinitionText" --type java -B 3 -A 3 | head -100Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 11482
🏁 Script executed:
# Look for any HTML sanitization utilities in the codebase
rg "sanitize|escapeHtml|HtmlUtils" --type java | head -30Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 2852
Change ContentMode.HTML to ContentMode.TEXT to prevent stored XSS.
The caseDefinitionText is editable by administrators through a RichTextArea in the disease configuration form and stored in the database without sanitization. Rendering it with ContentMode.HTML allows arbitrary HTML/JavaScript injection, creating a stored XSS vulnerability. Using ContentMode.TEXT will safely display the content as plain text.
Proposed fix
- Label suspectContent = new Label();
- suspectContent.setContentMode(ContentMode.HTML);
+ Label suspectContent = new Label();
+ suspectContent.setContentMode(ContentMode.TEXT);
suspectContent.setWidth(100, Unit.PERCENTAGE);
suspectContent.setValue(caseDefinitionText);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Label suspectContent = new Label(); | |
| suspectContent.setContentMode(ContentMode.HTML); | |
| suspectContent.setWidth(100, Unit.PERCENTAGE); | |
| suspectContent.setValue(caseDefinitionText); | |
| classificationRulesLayout.addComponent(suspectContent); | |
| Label suspectContent = new Label(); | |
| suspectContent.setContentMode(ContentMode.TEXT); | |
| suspectContent.setWidth(100, Unit.PERCENTAGE); | |
| suspectContent.setValue(caseDefinitionText); | |
| classificationRulesLayout.addComponent(suspectContent); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java` around
lines 1555 - 1559, Replace the HTML rendering mode for the label showing
caseDefinitionText to avoid rendering raw HTML: locate the Label instance
suspectContent in CaseDataForm and change the call
suspectContent.setContentMode(ContentMode.HTML) to
suspectContent.setContentMode(ContentMode.TEXT) so the value from
caseDefinitionText is rendered as plain text before adding it to
classificationRulesLayout.
Summary: Displaying the manual classification, which has configured in disease level Changes: Verifying the caseClassificationMode, if it's DISABLED, does nothing. For AUTOMATIC, display the automatic case classification in the table. For MANUAL, display the plain text box with the configured details.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13880 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/disease/DiseaseConfigurationFacadeEjb.java (1)
446-448: Avoid double invocation ofservice.getDiseaseConfiguration(disease).The null-guard correctly prevents the NPE, but the service method is called twice. Store the result in a local variable to avoid redundant lookups and potential TOCTOU issues.
♻️ Proposed refactor
public String getCaseDefinitionText(Disease disease) { - return service.getDiseaseConfiguration(disease) == null ? null : service.getDiseaseConfiguration(disease).getCaseDefinitionText(); + DiseaseConfiguration configuration = service.getDiseaseConfiguration(disease); + return configuration == null ? null : configuration.getCaseDefinitionText(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/disease/DiseaseConfigurationFacadeEjb.java` around lines 446 - 448, In getCaseDefinitionText, avoid calling service.getDiseaseConfiguration(disease) twice: call service.getDiseaseConfiguration(disease) once, store the result in a local variable (e.g., diseaseConfig), check it for null, and then return diseaseConfig.getCaseDefinitionText() or null; this removes the redundant lookup and prevents a potential TOCTOU window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/disease/DiseaseConfigurationFacadeEjb.java`:
- Around line 446-448: In getCaseDefinitionText, avoid calling
service.getDiseaseConfiguration(disease) twice: call
service.getDiseaseConfiguration(disease) once, store the result in a local
variable (e.g., diseaseConfig), check it for null, and then return
diseaseConfig.getCaseDefinitionText() or null; this removes the redundant lookup
and prevents a potential TOCTOU window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 356bc39a-dcf3-46ce-a2e4-4692fbe99420
📒 Files selected for processing (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/disease/DiseaseConfigurationFacadeEjb.java
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13880 |
3 participants
Summary:
Displaying the manual classification, which has configured in disease level Changes:
Verifying the caseClassificationMode, if it's DISABLED, does nothing. For AUTOMATIC, display the automatic case classification in the table. For MANUAL, display the plain text box with the configured details.
Fixes # Display manual case classification.
Summary:
Displaying the manual classification, which has configured in disease level
Changes:
Verifying the caseClassificationMode, if it's DISABLED, does nothing.
For AUTOMATIC, display the automatic case classification in the table.
For MANUAL, display the plain text box with the configured details.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes