Shigellosis lab message processing + minor defects.#13988
Open
KarnaiahPesula wants to merge 2 commits into
Open
Shigellosis lab message processing + minor defects.#13988KarnaiahPesula wants to merge 2 commits into
KarnaiahPesula wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis PR implements Shigellosis lab message support ( ChangesShigellosis Lab Messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java`:
- Around line 470-472: The sexualContact field in the ExposureDto class is
missing the `@SensitiveData` annotation, which is required for proper
sensitive-data access control. Add the `@SensitiveData` annotation above the
`@Diseases` annotation on the sexualContact field to ensure it is properly
protected as sensitive data.
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java`:
- Line 206: The enum constant SHISPP in PathogenSpecie.java was renamed from
SHIGELLA_SPP, but this breaking change requires three synchronization updates to
prevent deserialization failures and UI display issues. Update all 46 language
enum.properties files (enum.properties, enum_en-KE.properties, etc.) to replace
Pathogen.SHIGELLA_SPP with Pathogen.SHISPP. Update sormas_schema.sql to replace
the seed value 'SHIGELLA_SPP' with 'SHISPP' in the customizableenumvalue table
insert statement. Create a database migration script to update any existing
persisted records in the customizableenumvalue table that contain the old value
'SHIGELLA_SPP' to 'SHISPP' to ensure existing data can be properly deserialized
when JPA attempts to map them to the renamed enum constant.
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 16305-16312: The migration logic contains an impossible condition
where both the EXISTS and NOT EXISTS checks look for the same column
'undermedication', making the condition always false and preventing the
migration from running. Fix this by changing the NOT EXISTS clause to check for
'onmedication' instead of 'undermedication', so the condition properly verifies
that the old column exists and the new column does not. Additionally, remove the
duplicate IF block that appears immediately after (the second identical IF
statement in each pair), since only one migration check is needed for each
column rename operation. Apply these fixes to both occurrences of this pattern
in the file.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/ShigellosisSectionComponent.java`:
- Around line 149-171: The updateFieldVisibility method clears field values in
the setVisibleClear calls before subsequently making those same fields visible
again, causing data loss. The negative condition guards (if (!isSerogrouping),
if (!isSerotype), if (!showDrugSusceptibilityForm)) cause overlapping field
clearances. Restructure the logic so that setVisibleClear only executes when the
field will definitely remain hidden, by either: (1) inverting the guard
conditions to check that the field won't be shown in the subsequent visibility
assignments, or (2) reorganizing to clear fields only when they should remain
invisible across all visibility-setting paths. Ensure specie is only cleared
when both isSerogrouping and isSerotype are false, serotypeTF is only cleared
when isSerotype is false, and serotypingMethodCBF is only cleared when both
isSerotype and showDrugSusceptibilityForm are false.
🪄 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: 7e9bb40f-d88d-4f01-9bf5-8d3137dd0940
📒 Files selected for processing (25)
sormas-api/src/main/java/de/symeda/sormas/api/clinicalcourse/HealthConditionsDto.javasormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.javasormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureSubSetting.javasormas-api/src/main/java/de/symeda/sormas/api/exposure/SexualContact.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/labmessage/TestReportDto.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SerotypingMethod.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditions.javasormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditionsMapper.javasormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/exposure/Exposure.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/labmessage/TestReport.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/labmessage/TestReportFacadeEjb.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/person/PersonEditForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/components/TestMethodComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/AbstractDiseaseSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/ShigellosisSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/sidecomponent/SideComponentField.java
Some minor issues fixed.
KarnaiahPesula
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@sormas-backend/src/main/java/de/symeda/sormas/backend/therapy/DrugSusceptibilityMapper.java`:
- Around line 133-137: The hasData method in DrugSusceptibilityMapper is missing
validation for the five newly introduced *Method fields that were added to the
mapping logic. Update the hasData method to include checks for
AzithromycinMethod, AmpicillinMethod, CeftazidimeMethod, CefotaximeMethod, and
TrimethoprimSulfamethoxazoleMethod so that records containing only these method
values are not incorrectly treated as empty data. Apply the same fix to all
locations where hasData performs similar validation checks for
DrugSusceptibility fields.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/therapy/DrugSusceptibilityForm.java`:
- Around line 96-97: Remove the empty string field-location placeholders from
all fluidRowLocsCss method calls in the DrugSusceptibilityForm class. These
empty strings ("") are being passed as arguments for zone-diameter and
surveillance fields, but they generate invalid location='' HTML attributes that
break Vaadin template binding. For each affected fluidRowLocsCss call, identify
the empty strings and either omit those parameters entirely from the argument
list or restructure the call to pass only the fields that have actual location
values, ensuring no empty strings are passed as location placeholders.
🪄 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: 421de8d0-a1bf-4321-8918-cb6f91f286fd
📒 Files selected for processing (10)
sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SerotypingMethod.javasormas-api/src/main/java/de/symeda/sormas/api/therapy/DrugSusceptibilityDto.javasormas-api/src/main/resources/enum.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/therapy/DrugSusceptibility.javasormas-backend/src/main/java/de/symeda/sormas/backend/therapy/DrugSusceptibilityMapper.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/ShigellosisSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/therapy/DrugSusceptibilityForm.java
🚧 Files skipped from review as they are similar to previous changes (2)
- sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java
- sormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/ShigellosisSectionComponent.java
Comment on lines
+133
to
+137
| target.setAzithromycinMethod(source.getAzithromycinMethod()); | ||
| target.setAmpicillinMethod(source.getAmpicillinMethod()); | ||
| target.setCeftazidimeMethod(source.getCeftazidimeMethod()); | ||
| target.setCefotaximeMethod(source.getCefotaximeMethod()); | ||
| target.setTrimethoprimSulfamethoxazoleMethod(source.getTrimethoprimSulfamethoxazoleMethod()); |
There was a problem hiding this comment.
hasData is missing the newly introduced method fields.
You mapped the five new *Method fields in both directions, but hasData still ignores them. If a record only has one of these method values, hasData returns false and that data can be treated as empty.
Suggested fix
@@
|| dto.getAzithromycinMic() != null
|| dto.getAzithromycinSusceptibility() != null
+ || dto.getAzithromycinMethod() != null
|| dto.getCeftazidimeMic() != null
|| dto.getCeftazidimeSusceptibility() != null
+ || dto.getCeftazidimeMethod() != null
|| dto.getCefotaximeMic() != null
|| dto.getCefotaximeSusceptibility() != null
+ || dto.getCefotaximeMethod() != null
|| dto.getAmpicillinMic() != null
|| dto.getAmpicillinSusceptibility() != null
+ || dto.getAmpicillinMethod() != null
|| dto.getTrimethoprimSulfamethoxazoleMic() != null
- || dto.getTrimethoprimSulfamethoxazoleSusceptibility() != null;
+ || dto.getTrimethoprimSulfamethoxazoleSusceptibility() != null
+ || dto.getTrimethoprimSulfamethoxazoleMethod() != null;Also applies to: 244-248
🤖 Prompt for 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.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/therapy/DrugSusceptibilityMapper.java`
around lines 133 - 137, The hasData method in DrugSusceptibilityMapper is
missing validation for the five newly introduced *Method fields that were added
to the mapping logic. Update the hasData method to include checks for
AzithromycinMethod, AmpicillinMethod, CeftazidimeMethod, CefotaximeMethod, and
TrimethoprimSulfamethoxazoleMethod so that records containing only these method
values are not incorrectly treated as empty data. Apply the same fix to all
locations where hasData performs similar validation checks for
DrugSusceptibility fields.
Comment on lines
+96
to
+97
| + fluidRowLocsCss(CssStyles.GRID_ROW_GAP_1, AZITHROMYCIN_LABEL_LOC, DrugSusceptibilityDto.AZITHROMYCIN_MIC, "", DrugSusceptibilityDto.AZITHROMYCIN_METHOD, DrugSusceptibilityDto.AZITHROMYCIN_SUSCEPTIBILITY, "") | ||
| + fluidRowLocsCss(CssStyles.GRID_ROW_GAP_1, AMPICILLIN_LABEL_LOC, DrugSusceptibilityDto.AMPICILLIN_MIC, "", DrugSusceptibilityDto.AMPICILLIN_METHOD, DrugSusceptibilityDto.AMPICILLIN_SUSCEPTIBILITY, "") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate LayoutUtil and inspect fluidRowLocsCss implementation:"
fd -i 'LayoutUtil.java' | sed 's/^/ - /'
echo
echo "Show fluidRowLocsCss definition and nearby logic:"
rg -n -C4 'fluidRowLocsCss\s*\(' $(fd -i 'LayoutUtil.java')
echo
echo "Find all calls that pass empty-string placeholders:"
rg -n -C2 'fluidRowLocsCss\s*\(.*""' sormas-ui/src/main/java/de/symeda/sormas/ui/therapy/DrugSusceptibilityForm.javaRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 3960
🏁 Script executed:
cat -n sormas-ui/src/main/java/de/symeda/sormas/ui/utils/LayoutUtil.java | sed -n '225,260p'Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 1778
🏁 Script executed:
rg -n 'public static.*fluidColumnLoc' sormas-ui/src/main/java/de/symeda/sormas/ui/utils/LayoutUtil.java -A 20Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 1126
🏁 Script executed:
fd -i 'FluidColumn.java' && echo "---" && rg -n 'class FluidColumn' $(fd -i 'FluidColumn.java') -A 30Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 3140
🏁 Script executed:
rg -n 'fluidRowLocsCss.*"".*""' sormas-ui/src/main/java/ | head -20Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 1622
Remove empty string field-location placeholders from fluidRowLocsCss calls.
The fluidRowLocsCss method does not ignore empty strings—it generates invalid location='' HTML attributes that could break Vaadin template binding. Lines 96, 97, 107, 108, and 110 pass "" for zone-diameter and surveillance fields, but these should be omitted entirely instead of left as placeholders. Either exclude these fields from the row definition or pass the full list only when all fields are present.
Example of the issue
Lines 96-97 and 107-108, 110:
fluidRowLocsCss(CssStyles.GRID_ROW_GAP_1, AZITHROMYCIN_LABEL_LOC, DrugSusceptibilityDto.AZITHROMYCIN_MIC, "", DrugSusceptibilityDto.AZITHROMYCIN_METHOD, DrugSusceptibilityDto.AZITHROMYCIN_SUSCEPTIBILITY, "")
generates:
<div ... location=''> ... </div>
🤖 Prompt for 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.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/therapy/DrugSusceptibilityForm.java`
around lines 96 - 97, Remove the empty string field-location placeholders from
all fluidRowLocsCss method calls in the DrugSusceptibilityForm class. These
empty strings ("") are being passed as arguments for zone-diameter and
surveillance fields, but they generate invalid location='' HTML attributes that
break Vaadin template binding. For each affected fluidRowLocsCss call, identify
the empty strings and either omit those parameters entirely from the argument
list or restructure the call to pass only the fields that have actual location
values, ensuring no empty strings are passed as location placeholders.
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
1 participant
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.
Fixes #
Summary by CodeRabbit
Release Notes
New Features
Improvements