Summary: Malaria and Dengue Sample and Pathogentestform changes.#13889
Merged
KarnaiahPesula merged 5 commits intoApr 1, 2026
Conversation
33 tasks
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded malaria and dengue sample/test support, serotype and genotype refactors, a DiseaseTransmission enum, exposure-date population using disease incubation config, DTO/entity/mapping/UI updates, i18n key additions/renames, and DB schema migrations. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (EpiDataForm / CaseController)
participant API as DiseaseConfigurationFacade (remote)
participant EJB as DiseaseConfigurationFacadeEjb
participant Service as DiseaseConfigurationService
UI->>API: getDiseaseConfiguration(disease)
API->>EJB: remote call getDiseaseConfiguration(disease)
EJB->>Service: service.getDiseaseConfiguration(disease)
Service-->>EJB: DiseaseConfiguration
EJB-->>API: DiseaseConfigurationDto
API-->>UI: DiseaseConfigurationDto
UI->>UI: compute exposureStartDate/endDate using symptomOnsetDate and incubation bounds
UI-->>EpiDataDto: setExposureStartDate / setExposureEndDate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 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)
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: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (1)
1135-1162:⚠️ Potential issue | 🟠 MajorMissing visibility update calls in disease change listener.
When the disease changes,
updateSerotypeFieldandupdateSpecieFieldBiConsumers are not invoked. This means if a user changes the disease, the serotype and specie field visibility won't update until they also change the test type. The items are updated (lines 1151-1152), but the visibility logic is missing.🐛 Proposed fix
FieldHelper.updateItems(disease, seroTypeField, Serotype.class); FieldHelper.updateItems(disease, specieField, PathogenSpecie.class); + // Update visibility based on disease and current test type + PathogenTestType currentTestType = (PathogenTestType) testTypeField.getValue(); + updateSerotypeField.accept(disease, currentTestType); + updateSpecieField.accept(disease, currentTestType); if (FacadeProvider.getConfigFacade().isConfiguredCountry(CountryHelper.COUNTRY_CODE_LUXEMBOURG)) {🤖 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/samples/PathogenTestForm.java` around lines 1135 - 1162, The disease change listener on diseaseField doesn't invoke the visibility BiConsumers updateSerotypeField and updateSpecieField, so update their visibility when the disease changes: inside the ValueChangeListener (the lambda registered on diseaseField) after setting disease and after calling FieldHelper.updateItems for seroTypeField and specieField, call updateSerotypeField.accept(disease) and updateSpecieField.accept(disease) (or the appropriate BiConsumer signatures) so serotype and specie visibility updates immediately on disease change; ensure the calls are placed alongside the existing FieldHelper.updateItems calls and before any early returns.
🟡 Minor comments (6)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/sampleLink/SampleListEntry.java-163-170 (1)
163-170:⚠️ Potential issue | 🟡 MinorGate measles variants by
VARIANT_MAPtoo.Unlike the malaria and dengue branches, this one ignores the allowed test-type list. If the latest measles test isn't a genotyping assay but still has
genoTypepopulated, the side panel will show a variant that doesn't belong to that test.💡 Proposed fix
- } else if (latestTest.getTestedDisease() == Disease.MEASLES) { + } else if (latestTest.getTestedDisease() == Disease.MEASLES + && PathogenTestListEntry.VARIANT_MAP.get(latestTest.getTestedDisease()).stream().anyMatch(latestTest.getTestType()::equals)) { variant = StringUtils.abbreviate((latestTest.getGenoType() != null ? latestTest.getGenoType().toString() : ""), 125);🤖 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/samples/sampleLink/SampleListEntry.java` around lines 163 - 170, The measles branch currently ignores the allowed test-type list and always uses latestTest.getGenoType(); modify the Disease.MEASLES case to first check PathogenTestListEntry.VARIANT_MAP.get(latestTest.getTestedDisease()).stream().anyMatch(latestTest.getTestType()::equals) (same pattern as the MALARIA and DENGUE branches) and only then set variant = StringUtils.abbreviate((latestTest.getGenoType() != null ? latestTest.getGenoType().toString() : ""), 125); otherwise leave variant unset/null.sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java-84-85 (1)
84-85:⚠️ Potential issue | 🟡 MinorRemove the extra
CASE_IMPORTED_STATUSslot.
EpiDataDto.CASE_IMPORTED_STATUSis already rendered in the conclusion row below. Reusing the same layout location here can leave the field attached to the wrong section or one of the rows blank.💡 Proposed fix
- fluidRowLocs(6, EpiDataDto.EXPOSURE_START_DATE, 6, EpiDataDto.EXPOSURE_END_DATE)+ - fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"") + + fluidRowLocs(6, EpiDataDto.EXPOSURE_START_DATE, 6, EpiDataDto.EXPOSURE_END_DATE) +🤖 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/epidata/EpiDataForm.java` around lines 84 - 85, The second fluidRowLocs invocation currently passes EpiDataDto.CASE_IMPORTED_STATUS (fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"")), but that field is already rendered in the conclusion row below; remove the duplicate by replacing EpiDataDto.CASE_IMPORTED_STATUS with an empty slot (e.g. "") or otherwise dropping that parameter so the call does not render CASE_IMPORTED_STATUS here; update the fluidRowLocs call in EpiDataForm to no longer reference EpiDataDto.CASE_IMPORTED_STATUS.sormas-backend/src/main/resources/sql/sormas_schema.sql-15496-15496 (1)
15496-15496:⚠️ Potential issue | 🟡 MinorTypo in schema version comment: "sampel" should be "sample".
-INSERT INTO schema_version (version_number, comment) VALUES (615, '#13801, `#13814` - Malaria and Dengue sampel changes'); +INSERT INTO schema_version (version_number, comment) VALUES (615, '#13801, `#13814` - Malaria and Dengue sample changes');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` at line 15496, Fix the typo in the schema version comment for the INSERT INTO schema_version row (the VALUES entry for version_number 615): change the string '#13801, `#13814` - Malaria and Dengue sampel changes' to '#13801, `#13814` - Malaria and Dengue sample changes' so the comment text for that INSERT reflects the correct spelling.sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java-328-329 (1)
328-329:⚠️ Potential issue | 🟡 MinorAvoid nullable state for
retestRequestedwhen default is false.Line 328 initializes
retestRequestedto false, but Line 948-950 allows settingnull, which reintroduces tri-state behavior.🔧 Suggested fix
- private Boolean retestRequested = false; + private Boolean retestRequested = Boolean.FALSE; @@ public void setRetestRequested(Boolean retestRequested) { - this.retestRequested = retestRequested; + this.retestRequested = Boolean.TRUE.equals(retestRequested); }Also applies to: 944-950
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java` around lines 328 - 329, The field retestRequested is declared as a boxed Boolean with default false but elsewhere (setter/getRetestRequested methods) allow null, reintroducing tri-state; change the field to primitive boolean (retestRequested) and update its accessor methods (getRetestRequested / setRetestRequested or isRetestRequested) and any places that accept/assign null to use primitive boolean (or treat null as false) so the property can never be null; ensure serialization/deserialization and any callers are adjusted to pass a boolean instead of null.sormas-api/src/main/resources/enum.properties-1199-1203 (1)
1199-1203:⚠️ Potential issue | 🟡 MinorUse acronym casing for
LAMPcaption.Line 1203 currently uses
Lamp; this should be uppercase (LAMP) for consistency with lab-test acronym style.✏️ Suggested fix
-PathogenTestType.LAMP = Lamp +PathogenTestType.LAMP = LAMP🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/resources/enum.properties` around lines 1199 - 1203, The caption for the enum key PathogenTestType.LAMP uses "Lamp" but should be the uppercase acronym "LAMP"; update the value for PathogenTestType.LAMP in enum.properties from "Lamp" to "LAMP" to match the lab-test acronym style and maintain consistency with other test-type captions.sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java-176-176 (1)
176-176:⚠️ Potential issue | 🟡 MinorIncorrect formatter annotation.
Line 176 should be
//@formatter:onto close the formatter-off section that started at line 147. Currently it says//@formatter:offwhich leaves the formatter disabled.🔧 Proposed fix
- //@formatter:off + //@formatter:on🤖 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/samples/PathogenTestForm.java` at line 176, In PathogenTestForm (class PathogenTestForm) the inline formatter region was never closed: replace the second occurrence of the comment marker currently written as //@formatter:off with //@formatter:on to close the formatter-off started earlier (the pair around the block beginning at line 147); update the comment so formatter directives are matched (ensure only the trailing comment near the end of the hand-formatted block uses //@formatter:on).
🧹 Nitpick comments (7)
sormas-backend/src/main/resources/sql/sormas_schema.sql (1)
15475-15479: Inconsistent column naming conventions: mixed camelCase and lowercase.PostgreSQL lowercases unquoted identifiers, so
antibodyTitrebecomesantibodytitrein the catalog. However, the inconsistency between explicit lowercase (serotypetext,resultdetails,specietext) and camelCase (antibodyTitre,performedByReferenceLaboratory,retestRequested) can cause confusion when the Java entity field names don't match.Consider using consistent lowercase with underscores (e.g.,
antibody_titre,performed_by_reference_laboratory) or consistent lowercase without separators to match the existing pattern in this migration.Also applies to: 15488-15492
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` around lines 15475 - 15479, The ALTER TABLE statements on table pathogentest introduce mixed camelCase and lowercase column names; rename them to a consistent lowercase_snake_case convention (or match the existing lowercase pattern) so Java entity mappings remain predictable — replace antibodyTitre, performedByReferenceLaboratory, retestRequested, resultdetails, specietext with antibody_titre, performed_by_reference_laboratory, retest_requested, result_details, species_text (and apply the same renaming in the subsequent related statements at the other block referenced: 15488-15492), ensuring defaults and boolean types are preserved and update any corresponding Java entity/ORM mappings to use the new column names.sormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.java (2)
154-159: Inconsistent membership check style.Lines 155 and 158 use
.stream().anyMatch(testType::equals)while line 141 uses.contains(testType). For consistency and simplicity, prefer.contains()throughout.Suggested simplification
- } else if (pathogenTest.getTestedDisease() == Disease.MALARIA - && VARIANT_MAP.get(pathogenTest.getTestedDisease()).stream().anyMatch(testType::equals)) { + } else if (pathogenTest.getTestedDisease() == Disease.MALARIA + && VARIANT_MAP.get(Disease.MALARIA).contains(testType)) { resultText = StringUtils.abbreviate((pathogenTest.getSpecie() != null ? pathogenTest.getSpecie().toString() : ""), 125); - } else if (pathogenTest.getTestedDisease() == Disease.DENGUE - && VARIANT_MAP.get(pathogenTest.getTestedDisease()).stream().anyMatch(testType::equals)) { + } else if (pathogenTest.getTestedDisease() == Disease.DENGUE + && VARIANT_MAP.get(Disease.DENGUE).contains(testType)) { resultText = StringUtils.abbreviate((pathogenTest.getSerotype() != null ? pathogenTest.getSerotype().toString() : ""), 125);🤖 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/samples/pathogentestlink/PathogenTestListEntry.java` around lines 154 - 159, In PathogenTestListEntry, the membership checks against VARIANT_MAP currently use stream().anyMatch(testType::equals) for MALARIA and DENGUE; replace those with the simpler VARIANT_MAP.get(pathogenTest.getTestedDisease()).contains(testType) to match the style used elsewhere (e.g., the check on line 141) so all variant membership checks are consistent and clearer.
56-68: Avoid double-brace initialization for static map.Double-brace initialization (
new HashMap<>() {{ ... }}) creates an anonymous inner class, which can cause subtle issues (e.g., serialization problems, retaining references). For apublic static finalconstant, preferMap.of()andList.of()(available in Java 9+, supported by this project's Java 11+ target).Suggested refactor using Map.of()
- public static final Map<Disease, List<PathogenTestType>> VARIANT_MAP = Collections.unmodifiableMap(new HashMap<>() { - { - put(Disease.MALARIA, Collections.unmodifiableList(Arrays.asList(PathogenTestType.THIN_BLOOD_SMEAR, PathogenTestType.RAPID_TEST, PathogenTestType.PCR_RT_PCR, - PathogenTestType.Q_PCR, PathogenTestType.LAMP, PathogenTestType.IFAT))); - put(Disease.DENGUE, Collections.unmodifiableList(Arrays.asList(PathogenTestType.NAAT, PathogenTestType.NEUTRALIZING_ANTIBODIES, PathogenTestType.PCR_RT_PCR))); - put(Disease.MEASLES, Collections.unmodifiableList(Arrays.asList(PathogenTestType.GENOTYPING))); - put(Disease.INVASIVE_PNEUMOCOCCAL_INFECTION, Collections.unmodifiableList(Arrays.asList(PathogenTestType.SEROGROUPING, PathogenTestType.MULTILOCUS_SEQUENCE_TYPING, - PathogenTestType.SLIDE_AGGLUTINATION,PathogenTestType.WHOLE_GENOME_SEQUENCING, PathogenTestType.SEQUENCING))); - put(Disease.TUBERCULOSIS, Collections.unmodifiableList(Arrays.asList(PathogenTestType.MICROSCOPY, PathogenTestType.BEIJINGGENOTYPING, - PathogenTestType.SPOLIGOTYPING, PathogenTestType.MIRU_PATTERN_CODE))); - } - }); + public static final Map<Disease, List<PathogenTestType>> VARIANT_MAP = Map.of( + Disease.MALARIA, List.of(PathogenTestType.THIN_BLOOD_SMEAR, PathogenTestType.RAPID_TEST, PathogenTestType.PCR_RT_PCR, + PathogenTestType.Q_PCR, PathogenTestType.LAMP, PathogenTestType.IFAT), + Disease.DENGUE, List.of(PathogenTestType.NAAT, PathogenTestType.NEUTRALIZING_ANTIBODIES, PathogenTestType.PCR_RT_PCR), + Disease.MEASLES, List.of(PathogenTestType.GENOTYPING), + Disease.INVASIVE_PNEUMOCOCCAL_INFECTION, List.of(PathogenTestType.SEROGROUPING, PathogenTestType.MULTILOCUS_SEQUENCE_TYPING, + PathogenTestType.SLIDE_AGGLUTINATION, PathogenTestType.WHOLE_GENOME_SEQUENCING, PathogenTestType.SEQUENCING), + Disease.TUBERCULOSIS, List.of(PathogenTestType.MICROSCOPY, PathogenTestType.BEIJINGGENOTYPING, + PathogenTestType.SPOLIGOTYPING, PathogenTestType.MIRU_PATTERN_CODE) + );🤖 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/samples/pathogentestlink/PathogenTestListEntry.java` around lines 56 - 68, Replace the double-brace anonymous-map initialization for VARIANT_MAP with an explicit immutable construction using Map.of(...) and List.of(...) to avoid creating an anonymous inner class; specifically, in PathogenTestListEntry change the public static final Map<Disease, List<PathogenTestType>> VARIANT_MAP declaration to use Map.of(...) with each Disease mapped to a List.of(...) of PathogenTestType (keeping the same Disease keys: MALARIA, DENGUE, MEASLES, INVASIVE_PNEUMOCOCCAL_INFECTION, TUBERCULOSIS) so the map and lists are truly unmodifiable and no anonymous inner class is created.sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.java (1)
294-300: Redundant null check insideisNotBlankblock.Line 296 contains
source.getSerotypeText() == null ? null : source.getSerotypeText().toString(), but this is inside a block whereStringUtils.isNotBlank(source.getSerotypeText())already ensures the value is non-null and non-empty. The null check is unnecessary.Additionally, calling
.toString()on aStringis redundant.Suggested simplification
- // If serotypetext is not null, then serotype is Other, otherwise it'd be normal serotype - if (StringUtils.isNotBlank(source.getSerotypeText())) { - target.setSerotype(Serotype.fromString(source.getSerotypeText() == null ? null : source.getSerotypeText().toString())); - } else { - target.setSerotype(source.getSerotype()); - } + // If serotypeText is not blank, derive serotype from text; otherwise use existing serotype + if (StringUtils.isNotBlank(source.getSerotypeText())) { + target.setSerotype(Serotype.fromString(source.getSerotypeText())); + } else { + target.setSerotype(source.getSerotype()); + }🤖 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/sample/PathogenTestFacadeEjb.java` around lines 294 - 300, In PathogenTestFacadeEjb where you set the serotype (inside the block guarded by StringUtils.isNotBlank(source.getSerotypeText())), remove the redundant null check and the unnecessary .toString() call; call Serotype.fromString(source.getSerotypeText()) directly when setting target.setSerotype(...) (leave the else branch target.setSerotype(source.getSerotype()) and the subsequent target.setSerotypeText(source.getSerotypeText()) unchanged).sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (3)
520-523: Initial visibility may cause premature field display.Both
genoTypingCBandgenoTypingResultTextTFare set tosetVisible(true)immediately after creation. However, the visibility should depend on disease/test type combinations (as handled bycryptoGenoTypingDependenciesat line 1090). Settingtruehere may cause these fields to briefly appear before the conditional visibility logic is applied.Consider initializing with
setVisible(false)to match the typical pattern used for other conditional fields in this form.♻️ Suggested change
genoTypingCB = addField(PathogenTestDto.GENOTYPE, ComboBox.class); - genoTypingCB.setVisible(true); + genoTypingCB.setVisible(false); genoTypingResultTextTF = addField(PathogenTestDto.GENOTYPE_TEXT, TextField.class); - genoTypingResultTextTF.setVisible(true); + genoTypingResultTextTF.setVisible(false);🤖 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/samples/PathogenTestForm.java` around lines 520 - 523, In PathogenTestForm change the initial visibility of the genotype fields so they don't flash before dependency logic runs: set genoTypingCB and genoTypingResultTextTF to not visible on creation (replace setVisible(true) with setVisible(false)) so the conditional visibility controlled by cryptoGenoTypingDependencies (and any similar dependency methods) determines when they become visible.
611-611: Remove commented-out code if no longer needed.If the old
SPECIE_VISIBILITY_CONDITIONSapproach has been replaced by the newSPECIE_VISIBILITY_MAPwith BiConsumer, this commented line should be removed.🤖 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/samples/PathogenTestForm.java` at line 611, Remove the stale commented-out line that references the old SPECIE_VISIBILITY_CONDITIONS to avoid confusion: delete the comment "// FieldHelper.setVisibleWhen(getFieldGroup(), PathogenTestDto.SPECIE, SPECIE_VISIBILITY_CONDITIONS, true);" from PathogenTestForm.java since the code now uses SPECIE_VISIBILITY_MAP with a BiConsumer-based approach; ensure no other references to SPECIE_VISIBILITY_CONDITIONS remain and confirm behavior relies on SPECIE_VISIBILITY_MAP and its consumer logic for PathogenTestDto.SPECIE visibility.
1293-1300: Consider removing commented-out code.Line 1295 contains commented-out code. If this is no longer needed, it should be removed to improve code cleanliness.
♻️ Suggested cleanup
// displaying the serotype text field only if the serotype is "other" and it has the visibility if (isVisibleAllowed(PathogenTestDto.SEROTYPE)) { - // FieldHelper.setVisibleWhen(getFieldGroup(), PathogenTestDto.SEROTYPE, SEROTYPE_VISIBILITY_MAP, true); FieldHelper.setVisibleWhen(getFieldGroup(), PathogenTestDto.SEROTYPE_TEXT, PathogenTestDto.SEROTYPE, Serotype.OTHER, true); }🤖 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/samples/PathogenTestForm.java` around lines 1293 - 1300, Remove the leftover commented-out call to FieldHelper.setVisibleWhen inside PathogenTestForm (the commented line referencing PathogenTestDto.SEROTYPE) to clean up the code; keep the active visibility rules for PathogenTestDto.SEROTYPE_TEXT and PathogenTestDto.SPECIE_TEXT intact and ensure no other logic depended on that commented invocation (search for FieldHelper.setVisibleWhen and the symbols PathogenTestDto.SEROTYPE, SEROTYPE_VISIBILITY_MAP to confirm).
🤖 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-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java`:
- Around line 55-65: In PathogenSpecie add the missing malaria disease marker to
FALCIPARUM: update the enum constant FALCIPARUM to include the annotation
`@Diseases`(Disease.MALARIA) alongside its existing `@ApplicableToPathogenTests` so
disease-filtered lookups include this primary malaria species; locate the
FALCIPARUM enum entry in class PathogenSpecie and annotate it with
`@Diseases`(Disease.MALARIA).
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java`:
- Line 232: The new free-text fields in PathogenTestDto (specieText,
antibodyTitre, resultDetails) lack `@Size` validation like neighboring string
fields; add appropriate `@Size`(max = N) annotations (matching nearby fields'
limits) to each of these fields in the PathogenTestDto class so input length is
constrained and consistent with existing DTO validations, and update any
existing imports to include javax.validation.constraints.Size if needed.
- Around line 116-117: PathogenTestDto renamed the fields GENOTYPE/GENOTYPE_TEXT
from the old genoTypeResult/gen oTypeResultText keys which breaks
deserialization for clients using the old names; add backward-compatible aliases
by annotating the getters getGenoType and getGenoTypeText (or the corresponding
fields) with `@JsonAlias`("genoTypeResult") and `@JsonAlias`("genoTypeResultText")
respectively and keep the existing GENOTYPE/GENOTYPE_TEXT constants (optionally
add GENOTYPE_RESULT/GENOTYPE_RESULT_TEXT constants for clarity) so incoming JSON
with the old keys will map to the current genoType and genoTypeText properties.
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java`:
- Around line 298-301: Remove the duplicate IFAT enum constant from the
PathogenTestType enum and replace all usages of IFAT with
INDIRECT_FLUORESCENT_ANTIBODY so the API and UI agree; specifically, delete the
IFAT entry in PathogenTestType (preserving the Diseases({Disease.MALARIA})
semantics by applying it to INDIRECT_FLUORESCENT_ANTIBODY if needed) and update
references in UI classes PathogenTestListEntry and PathogenTestForm (and any
other callers) to use PathogenTestType.INDIRECT_FLUORESCENT_ANTIBODY; run a
quick compile/search to ensure no lingering references to IFAT remain.
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/Serotype.java`:
- Around line 50-56: The method Serotype.fromString currently builds
serotypeEnumVal from the raw input which can include surrounding whitespace,
causing valid inputs like " DENV-1 " to map to OTHER; fix by trimming and
normalizing the input first. In Serotype.fromString, after the initial
null/blank check, create a normalized string from serotype.trim() then call
toUpperCase().replaceAll("[-_]", "_") (use this normalized value instead of the
original serotype when computing serotypeEnumVal and when calling
Serotype.valueOf), and use that normalized serotypeEnumVal in the noneMatch(...)
check and the valueOf(...) call so trimmed inputs are resolved correctly.
In `@sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java`:
- Around line 2864-2875: The `@Complication`() annotation on fields reoccurrence
and overnightStayRequired is empty so getComplicatedSymptomsWithDiseases (which
checks Arrays.asList(complicationAnnotation.value()).contains(disease)) will
never match; fix by either adding explicit disease values to the annotations
(e.g., change `@Complication`() to `@Complication`({GIARDIASIS, CRYPTOSPORIDIOSIS})
on reoccurrence and overnightStayRequired) or modify
getComplicatedSymptomsWithDiseases to treat an empty
complicationAnnotation.value() as a wildcard by falling back to the field's
`@Diseases` values when the complication array is empty.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditions.java`:
- Around line 341-347: The getter getMalaria() is missing the JPA enum mapping
annotation and will default to ordinal storage; add `@Enumerated`(EnumType.STRING)
to the getMalaria() method so the YesNoUnknown enum is persisted as a VARCHAR
(matching existing fields and the DB schema), leaving setMalaria(...) unchanged.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/disease/DiseaseConfigurationFacadeEjb.java`:
- Around line 182-184: getDiseaseConfiguration currently returns the raw DTO
from toDto(service.getDiseaseConfiguration(disease)) without applying default
normalization; change it to store the DTO in a local variable, call
updateDtoNullPropertiesWithDefaultDiseaseEnumProperties(dto) to fill null config
values, then return the normalized dto so behavior matches existing retrieval
logic (use the existing toDto, service.getDiseaseConfiguration(disease) and
updateDtoNullPropertiesWithDefaultDiseaseEnumProperties(...) methods).
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/labmessage/TestReport.java`:
- Line 127: The genoType field in class TestReport is mapped implicitly to
column "genotype" but the DB column is "genotyperesult"; fix by adding an
explicit column mapping—annotate the getter (or the field) for genoType with
`@Column`(name = "genotyperesult") in TestReport so Hibernate uses the correct
column name, or alternatively run an SQL migration to rename the testreport
table column to "genotype" to match the implicit mapping; ensure the change
targets the genoType property in TestReport.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.java`:
- Around line 614-615: The serotype conversion in fillOrBuildEntity() is
asymmetric with toDto(): update the logic in
PathogenTestFacadeEjb.fillOrBuildEntity (where target.setSerotype(...) and
target.setSerotypeText(...) are set) so that it mirrors toDto(): if
source.getSerotypeText() is not blank, derive the entity serotype from that text
(and set serotypeText accordingly); otherwise fall back to
Serotype.fromString(source.getSerotype() == null ? null :
source.getSerotype().toString()). Keep setting
target.setSerotypeText(source.getSerotypeText()) as before to preserve the
custom text.
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15486-15487: The two unconditional ALTER TABLE statements renaming
columns in pathogentest_history (genotyperesult → genotype and
genotyperesulttext → genotypetext) must be made idempotent; update them to first
check for the existence of the old columns (and absence of the new names) and
only perform the ALTER TABLE ... RENAME COLUMN if needed. Locate the ALTER TABLE
pathogentest_history statements referring to genotyperesult and
genotyperesulttext and wrap each rename in a conditional that queries
information_schema.columns for the column names before executing the rename so
repeated runs won’t error.
- Around line 15473-15474: The plain RENAME COLUMN statements (ALTER TABLE
pathogentest rename column genotyperesult to genotype; and rename column
genotyperesulttext to genotypetext;) are not idempotent and will fail on re-run;
replace each with a small conditional DO block that checks for the existence of
the source column (e.g., "genotyperesult" / "genotyperesulttext") and the
non-existence of the target column ("genotype" / "genotypetext") using
pg_catalog or information_schema queries, and only then issues the ALTER TABLE
... RENAME COLUMN command via EXECUTE; do this for both renames (reference the
exact identifiers pathogentest, genotyperesult->genotype and
genotyperesulttext->genotypetext) so the migration becomes safe to re-run.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.java`:
- Around line 1397-1409: The code mutates the live DTO by calling
includeExposureDates(symptomOnsetDate, epiDataDto, caze.getDisease()) on
epiDataDto which is caze.getEpiData(), causing UI-only derived exposure dates to
be persisted; to fix, create a detached copy of the epi data (e.g., new
EpiDataDto(copy constructor or clone) or build a new EpiDataDto from
caze.getEpiData()), call includeExposureDates(...) on that copy, and pass the
copy to EpiDataForm.setValue(...) so the persisted caze.getEpiData() is not
mutated by the UI; alternatively move the exposure-date derivation into the
backend read/save path used by the CaseController.
- Around line 1443-1444: The null check for the boxed Integer returned by
diseaseConfigurationDto.getMaxIncubationPeriod() must be performed before any
numeric comparison to avoid auto-unboxing NPE; update the conditional in
CaseController (where getMaxIncubationPeriod() is used) to check for null first
(e.g., if (diseaseConfigurationDto.getMaxIncubationPeriod() == null ||
diseaseConfigurationDto.getMaxIncubationPeriod() == 0) return;) or use
Objects.equals(Integer.valueOf(0),
diseaseConfigurationDto.getMaxIncubationPeriod()) to safely compare to zero.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseSymptomSideViewComponent.java`:
- Around line 100-105: The branch in CaseSymptomSideViewComponent that handles
String sourceFieldObj should not skip updates when componentMap already contains
the key; remove the !componentMap.containsKey(...) gate and instead, for
sourceField.getId() in complicatedSymptoms, check
componentMap.get(sourceField.getId())—if it's a Label update its text to
sourceField.getValue().toString(), otherwise create a new Label and put it into
componentMap; ensure you still only act when sourceFieldObj is a non-blank
String to avoid overwriting with empty values.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java`:
- Around line 136-137: The fieldsList used to build the disabled/unbound set in
setInaccessible() is missing the MALARIA_INFECTED_YEAR constant, so add
MALARIA_INFECTED_YEAR to the fieldsList declaration alongside
RECURRENT_BRONCHIOLITIS and MALARIA (i.e., include MALARIA_INFECTED_YEAR in the
same array/collection initialization) so that setInaccessible() will properly
disable and unbind that field.
- Around line 307-330: The visibility/access initialization calls
initializeVisibilitiesAndAllowedVisibilities() and
initializeAccessAndAllowedAccesses() are executed before the
MALARIA_INFECTED_YEAR field is created, which can skip enforcing field-level
rules; move those two initialization calls to after the code that creates and
configures the malaria infected year components (the addField call that returns
malInfectedYearCB, the malariaInfectedYearLayout setup, and the
FieldHelper.setVisibleWhen call) so that the new field is present when
visibilities/accesses are initialized; ensure
initializeVisibilitiesAndAllowedVisibilities() and
initializeAccessAndAllowedAccesses() run once after creating
MALARIA_INFECTED_YEAR (and any similar dynamic fields) to enforce correct
visibility/access.
---
Outside diff comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 1135-1162: The disease change listener on diseaseField doesn't
invoke the visibility BiConsumers updateSerotypeField and updateSpecieField, so
update their visibility when the disease changes: inside the ValueChangeListener
(the lambda registered on diseaseField) after setting disease and after calling
FieldHelper.updateItems for seroTypeField and specieField, call
updateSerotypeField.accept(disease) and updateSpecieField.accept(disease) (or
the appropriate BiConsumer signatures) so serotype and specie visibility updates
immediately on disease change; ensure the calls are placed alongside the
existing FieldHelper.updateItems calls and before any early returns.
---
Minor comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java`:
- Around line 328-329: The field retestRequested is declared as a boxed Boolean
with default false but elsewhere (setter/getRetestRequested methods) allow null,
reintroducing tri-state; change the field to primitive boolean (retestRequested)
and update its accessor methods (getRetestRequested / setRetestRequested or
isRetestRequested) and any places that accept/assign null to use primitive
boolean (or treat null as false) so the property can never be null; ensure
serialization/deserialization and any callers are adjusted to pass a boolean
instead of null.
In `@sormas-api/src/main/resources/enum.properties`:
- Around line 1199-1203: The caption for the enum key PathogenTestType.LAMP uses
"Lamp" but should be the uppercase acronym "LAMP"; update the value for
PathogenTestType.LAMP in enum.properties from "Lamp" to "LAMP" to match the
lab-test acronym style and maintain consistency with other test-type captions.
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Line 15496: Fix the typo in the schema version comment for the INSERT INTO
schema_version row (the VALUES entry for version_number 615): change the string
'#13801, `#13814` - Malaria and Dengue sampel changes' to '#13801, `#13814` -
Malaria and Dengue sample changes' so the comment text for that INSERT reflects
the correct spelling.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java`:
- Around line 84-85: The second fluidRowLocs invocation currently passes
EpiDataDto.CASE_IMPORTED_STATUS
(fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"")), but that field is
already rendered in the conclusion row below; remove the duplicate by replacing
EpiDataDto.CASE_IMPORTED_STATUS with an empty slot (e.g. "") or otherwise
dropping that parameter so the call does not render CASE_IMPORTED_STATUS here;
update the fluidRowLocs call in EpiDataForm to no longer reference
EpiDataDto.CASE_IMPORTED_STATUS.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Line 176: In PathogenTestForm (class PathogenTestForm) the inline formatter
region was never closed: replace the second occurrence of the comment marker
currently written as //@formatter:off with //@formatter:on to close the
formatter-off started earlier (the pair around the block beginning at line 147);
update the comment so formatter directives are matched (ensure only the trailing
comment near the end of the hand-formatted block uses //@formatter:on).
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/sampleLink/SampleListEntry.java`:
- Around line 163-170: The measles branch currently ignores the allowed
test-type list and always uses latestTest.getGenoType(); modify the
Disease.MEASLES case to first check
PathogenTestListEntry.VARIANT_MAP.get(latestTest.getTestedDisease()).stream().anyMatch(latestTest.getTestType()::equals)
(same pattern as the MALARIA and DENGUE branches) and only then set variant =
StringUtils.abbreviate((latestTest.getGenoType() != null ?
latestTest.getGenoType().toString() : ""), 125); otherwise leave variant
unset/null.
---
Nitpick comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.java`:
- Around line 294-300: In PathogenTestFacadeEjb where you set the serotype
(inside the block guarded by StringUtils.isNotBlank(source.getSerotypeText())),
remove the redundant null check and the unnecessary .toString() call; call
Serotype.fromString(source.getSerotypeText()) directly when setting
target.setSerotype(...) (leave the else branch
target.setSerotype(source.getSerotype()) and the subsequent
target.setSerotypeText(source.getSerotypeText()) unchanged).
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15475-15479: The ALTER TABLE statements on table pathogentest
introduce mixed camelCase and lowercase column names; rename them to a
consistent lowercase_snake_case convention (or match the existing lowercase
pattern) so Java entity mappings remain predictable — replace antibodyTitre,
performedByReferenceLaboratory, retestRequested, resultdetails, specietext with
antibody_titre, performed_by_reference_laboratory, retest_requested,
result_details, species_text (and apply the same renaming in the subsequent
related statements at the other block referenced: 15488-15492), ensuring
defaults and boolean types are preserved and update any corresponding Java
entity/ORM mappings to use the new column names.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 520-523: In PathogenTestForm change the initial visibility of the
genotype fields so they don't flash before dependency logic runs: set
genoTypingCB and genoTypingResultTextTF to not visible on creation (replace
setVisible(true) with setVisible(false)) so the conditional visibility
controlled by cryptoGenoTypingDependencies (and any similar dependency methods)
determines when they become visible.
- Line 611: Remove the stale commented-out line that references the old
SPECIE_VISIBILITY_CONDITIONS to avoid confusion: delete the comment "//
FieldHelper.setVisibleWhen(getFieldGroup(), PathogenTestDto.SPECIE,
SPECIE_VISIBILITY_CONDITIONS, true);" from PathogenTestForm.java since the code
now uses SPECIE_VISIBILITY_MAP with a BiConsumer-based approach; ensure no other
references to SPECIE_VISIBILITY_CONDITIONS remain and confirm behavior relies on
SPECIE_VISIBILITY_MAP and its consumer logic for PathogenTestDto.SPECIE
visibility.
- Around line 1293-1300: Remove the leftover commented-out call to
FieldHelper.setVisibleWhen inside PathogenTestForm (the commented line
referencing PathogenTestDto.SEROTYPE) to clean up the code; keep the active
visibility rules for PathogenTestDto.SEROTYPE_TEXT and
PathogenTestDto.SPECIE_TEXT intact and ensure no other logic depended on that
commented invocation (search for FieldHelper.setVisibleWhen and the symbols
PathogenTestDto.SEROTYPE, SEROTYPE_VISIBILITY_MAP to confirm).
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.java`:
- Around line 154-159: In PathogenTestListEntry, the membership checks against
VARIANT_MAP currently use stream().anyMatch(testType::equals) for MALARIA and
DENGUE; replace those with the simpler
VARIANT_MAP.get(pathogenTest.getTestedDisease()).contains(testType) to match the
style used elsewhere (e.g., the check on line 141) so all variant membership
checks are consistent and clearer.
- Around line 56-68: Replace the double-brace anonymous-map initialization for
VARIANT_MAP with an explicit immutable construction using Map.of(...) and
List.of(...) to avoid creating an anonymous inner class; specifically, in
PathogenTestListEntry change the public static final Map<Disease,
List<PathogenTestType>> VARIANT_MAP declaration to use Map.of(...) with each
Disease mapped to a List.of(...) of PathogenTestType (keeping the same Disease
keys: MALARIA, DENGUE, MEASLES, INVASIVE_PNEUMOCOCCAL_INFECTION, TUBERCULOSIS)
so the map and lists are truly unmodifiable and no anonymous inner class is
created.
🪄 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: 6000b50a-8495-46ae-92ae-334eee6d733d
📒 Files selected for processing (39)
sormas-api/src/main/java/de/symeda/sormas/api/clinicalcourse/HealthConditionsDto.javasormas-api/src/main/java/de/symeda/sormas/api/disease/DiseaseConfigurationFacade.javasormas-api/src/main/java/de/symeda/sormas/api/epidata/DiseaseTransmission.javasormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.javasormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseDiseaseExportEntryDto.javasormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseLaboratoryMapper.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/labmessage/TestReportDto.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Strings.javasormas-api/src/main/java/de/symeda/sormas/api/sample/GenoType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SampleListEntryDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.javasormas-api/src/main/java/de/symeda/sormas/api/sample/Serotype.javasormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-api/src/main/resources/strings.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/disease/DiseaseConfigurationFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/MeaslesExportStrategy.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/java/de/symeda/sormas/backend/sample/PathogenTest.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseSymptomSideViewComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/sampleLink/SampleListEntry.javasormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java
Comment on lines
+136
to
+137
| RECURRENT_BRONCHIOLITIS, | ||
| MALARIA); |
There was a problem hiding this comment.
MALARIA_INFECTED_YEAR is missing from fieldsList, affecting inaccessible mode cleanup.
setInaccessible() builds its disabled/unbound field set from fieldsList (Line 392). Since MALARIA_INFECTED_YEAR isn’t included at Lines 136-137, that field may remain visible/bound when the form is set inaccessible.
Suggested fix
VACCINATED_AGAINST_MOSQUITO_BORNE_VIRUSES,
RECURRENT_BRONCHIOLITIS,
- MALARIA);
+ MALARIA,
+ MALARIA_INFECTED_YEAR);📝 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.
Suggested change
| RECURRENT_BRONCHIOLITIS, | |
| MALARIA); | |
| VACCINATED_AGAINST_MOSQUITO_BORNE_VIRUSES, | |
| RECURRENT_BRONCHIOLITIS, | |
| MALARIA, | |
| MALARIA_INFECTED_YEAR); |
🤖 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/clinicalcourse/HealthConditionsForm.java`
around lines 136 - 137, The fieldsList used to build the disabled/unbound set in
setInaccessible() is missing the MALARIA_INFECTED_YEAR constant, so add
MALARIA_INFECTED_YEAR to the fieldsList declaration alongside
RECURRENT_BRONCHIOLITIS and MALARIA (i.e., include MALARIA_INFECTED_YEAR in the
same array/collection initialization) so that setInaccessible() will properly
disable and unbind that field.
Comment on lines
307
to
+330
| initializeVisibilitiesAndAllowedVisibilities(); | ||
| initializeAccessAndAllowedAccesses(); | ||
|
|
||
| if (isVisibleAllowed(MALARIA)) { | ||
| // Malaria infected year visibility. | ||
| CustomLayout malariaInfectedYearLayout = new CustomLayout(); | ||
| malariaInfectedYearLayout.setTemplateContents(MALARIA_YEAR_LAYOUT); | ||
|
|
||
| // infected year label | ||
| Label lblInfectedYear = new Label(I18nProperties.getCaption(Captions.HealthConditions_malariaInfectedYear)); | ||
| malariaInfectedYearLayout.addComponent(lblInfectedYear, "LBL_MALARIA_INFECTED_YEAR"); | ||
| getContent().addComponent(malariaInfectedYearLayout, "MALARIA_INFECTED_YEAR_LAYOUT"); | ||
|
|
||
| // infection year combobox | ||
| ComboBox malInfectedYearCB = addField(malariaInfectedYearLayout, MALARIA_INFECTED_YEAR, ComboBox.class); | ||
| malInfectedYearCB.setInputPrompt(I18nProperties.getString(Strings.year)); | ||
| malInfectedYearCB.setNullSelectionAllowed(true); | ||
| malInfectedYearCB.setCaption(null); | ||
| malInfectedYearCB.addItems(DateHelper.getYearsToNow()); | ||
| malInfectedYearCB.setItemCaptionMode(AbstractSelect.ItemCaptionMode.ID_TOSTRING); | ||
| malariaInfectedYearLayout.addComponent(malInfectedYearCB, MALARIA_INFECTED_YEAR); | ||
|
|
||
| FieldHelper.setVisibleWhen(getFieldGroup(), Arrays.asList(MALARIA_INFECTED_YEAR), MALARIA, Arrays.asList(YesNoUnknown.YES), true); | ||
| } |
There was a problem hiding this comment.
Initialize visibilities/access after adding MALARIA_INFECTED_YEAR.
At Line 307 and Line 308, visibility/access initialization runs before the field is created at Line 321. This can bypass normal field-level visibility/access enforcement for the new malaria year field.
Suggested fix
- initializeVisibilitiesAndAllowedVisibilities();
- initializeAccessAndAllowedAccesses();
-
- if (isVisibleAllowed(MALARIA)) {
+ if (isVisibleAllowed(MALARIA) && isVisibleAllowed(MALARIA_INFECTED_YEAR)) {
// Malaria infected year visibility.
CustomLayout malariaInfectedYearLayout = new CustomLayout();
malariaInfectedYearLayout.setTemplateContents(MALARIA_YEAR_LAYOUT);
@@
FieldHelper.setVisibleWhen(getFieldGroup(), Arrays.asList(MALARIA_INFECTED_YEAR), MALARIA, Arrays.asList(YesNoUnknown.YES), true);
}
+
+ initializeVisibilitiesAndAllowedVisibilities();
+ initializeAccessAndAllowedAccesses();🤖 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/clinicalcourse/HealthConditionsForm.java`
around lines 307 - 330, The visibility/access initialization calls
initializeVisibilitiesAndAllowedVisibilities() and
initializeAccessAndAllowedAccesses() are executed before the
MALARIA_INFECTED_YEAR field is created, which can skip enforcing field-level
rules; move those two initialization calls to after the code that creates and
configures the malaria infected year components (the addField call that returns
malInfectedYearCB, the malariaInfectedYearLayout setup, and the
FieldHelper.setVisibleWhen call) so that the new field is present when
visibilities/accesses are initialized; ensure
initializeVisibilitiesAndAllowedVisibilities() and
initializeAccessAndAllowedAccesses() run once after creating
MALARIA_INFECTED_YEAR (and any similar dynamic fields) to enforce correct
visibility/access.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java`:
- Around line 168-170: The serotype field in PathogenTestDto is too narrow
(typed as Serotype) and loses non-dengue values; change the field to a generic
String (e.g., replace private Serotype serotype with private String serotype)
and add the same `@Size`(max = FieldConstraints.CHARACTER_LIMIT_DEFAULT, message =
Validations.textTooLong) validation as used for serotypeText, update the
corresponding getter/setter and any JSON/serialization annotations, and adjust
callers (including PathogenTestForm.SEROTYPE_VISIBILITY_MAP and any code
handling Disease.INVASIVE_PNEUMOCOCCAL_INFECTION) to treat serotype as a raw
string value so raw/non-dengue serotypes are preserved.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.java`:
- Around line 1440-1442: The current check in CaseController using
!diseaseConfigurationDto.getIncubationPeriodEnabled() can NPE when
getIncubationPeriodEnabled() returns null; change the null-unsafe negation to a
null-safe comparison such as using
Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled()) (e.g.,
if (!Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled()))
return;), or explicitly handle null (check for null first) so
DiseaseConfigurationDto.getIncubationPeriodEnabled() is never auto-unboxed.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java`:
- Around line 593-598: PERFORMED_BY_REFERENCE_LABORATORY and RETEST_REQUESTED
are currently added unconditionally and thus show for all pathogen tests; update
PathogenTestForm so these fields (added via
addField(PathogenTestDto.PERFORMED_BY_REFERENCE_LABORATORY,
NullableOptionGroup.class) and addField(PathogenTestDto.RETEST_REQUESTED,
NullableOptionGroup.class)) are only visible for the malaria/dengue
workflows—e.g. after adding the fields, call setVisible(false) by default and
add the same disease-gating logic used for RESULT_DETAILS (or use the form's
pathogen/disease getter to setVisible(true) when pathogen type indicates malaria
or dengue), ensuring the visibility check uses the DTO/pathogen value in
PathogenTestForm rather than leaving them global.
- Around line 1227-1230: When toggling visibility for resultDetailsField and
seroTypeMetCB based on disease/testType, also clear their bound values when they
are hidden to avoid saving stale data; update the logic around
resultDetailsField.setVisible(...) and seroTypeMetCB.setVisible(...) so that if
visibility is set to false you call the respective clear/reset method on the
underlying model/binder (e.g., null or empty value via the field's setter or
binder) for resultDetails and serotyping method, using the same identifiers
resultDetailsField, seroTypeMetCB, testType and disease/PathogenTestType/Disease
to locate the code.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.java`:
- Around line 57-68: The VARIANT_MAP entry for Disease.MALARIA is missing
several PathogenTestType values that PathogenTestForm.SPECIE_VISIBILITY_MAP
treats as specie-driven; update the Disease.MALARIA list inside VARIANT_MAP to
include PathogenTestType.ANTIGEN_DETECTION,
PathogenTestType.ENZYME_LINKED_IMMUNOSORBENT_ASSAY and the OTHER_* malaria test
types (the exact OTHER_* enum names used elsewhere) so the map aligns with
PathogenTestForm.SPECIE_VISIBILITY_MAP and those records will render
species-specific results; keep the list wrapped with
Collections.unmodifiableList and maintain the same construction pattern used for
the other disease entries.
- Around line 153-159: In PathogenTestListEntry where resultText is set for
genoType/specie/serotype, update the logic to use the free-text companion fields
when the enum value is OTHER instead of rendering the literal "Other": for the
genoType branch use pathogenTest.getGenoTypeText() when
pathogenTest.getGenoType() == OTHER (otherwise use
pathogenTest.getGenoType().toString()), for the specie branch use
pathogenTest.getSpecieText() when pathogenTest.getSpecie() == OTHER (otherwise
use pathogenTest.getSpecie().toString()), and for the serotype branch use
pathogenTest.getSerotypeText() when pathogenTest.getSerotype() == OTHER
(otherwise use pathogenTest.getSerotype().toString()); keep the existing
VARIANT_MAP and testType checks and still pass the chosen text through
StringUtils.abbreviate before assigning to resultText.
🪄 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: 0dc46871-42a0-4e19-87b8-edff2ff91873
📒 Files selected for processing (9)
sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/Serotype.javasormas-api/src/main/resources/enum.propertiessormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseSymptomSideViewComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.java
🚧 Files skipped from review as they are similar to previous changes (2)
- sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseSymptomSideViewComponent.java
- sormas-api/src/main/resources/enum.properties
Comment on lines
+168
to
+170
| private Serotype serotype; | ||
| @Size(max = FieldConstraints.CHARACTER_LIMIT_DEFAULT, message = Validations.textTooLong) | ||
| private String serotype; | ||
| private String serotypeText; |
There was a problem hiding this comment.
serotype is now too narrow for the shared DTO contract.
This field is still reused outside dengue — PathogenTestForm.SEROTYPE_VISIBILITY_MAP still exposes it for Disease.INVASIVE_PNEUMOCOCCAL_INFECTION — but the backing enum only contains DENV_1..4/OTHER. Non-dengue or raw serotype values can now only degrade to OTHER, which loses the actual result unless every caller also keeps serotypeText in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java`
around lines 168 - 170, The serotype field in PathogenTestDto is too narrow
(typed as Serotype) and loses non-dengue values; change the field to a generic
String (e.g., replace private Serotype serotype with private String serotype)
and add the same `@Size`(max = FieldConstraints.CHARACTER_LIMIT_DEFAULT, message =
Validations.textTooLong) validation as used for serotypeText, update the
corresponding getter/setter and any JSON/serialization annotations, and adjust
callers (including PathogenTestForm.SEROTYPE_VISIBILITY_MAP and any code
handling Disease.INVASIVE_PNEUMOCOCCAL_INFECTION) to treat serotype as a raw
string value so raw/non-dengue serotypes are preserved.
Comment on lines
+1440
to
+1442
| if (!diseaseConfigurationDto.getIncubationPeriodEnabled()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the return type of getIncubationPeriodEnabled()
ast-grep --pattern $'class DiseaseConfigurationDto {
$$$
getIncubationPeriodEnabled() {
$$$
}
$$$
}'
# Alternative: search for the field and getter declaration
rg -n -A3 'incubationPeriodEnabled' --glob '**/DiseaseConfigurationDto.java'Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 2143
Potential NPE if getIncubationPeriodEnabled() returns null.
The field incubationPeriodEnabled is declared as Boolean (wrapper type) in DiseaseConfigurationDto. When the getter returns null and the code applies the ! operator, it triggers auto-unboxing and throws a NullPointerException.
Consider using a null-safe comparison:
Suggested fix
- if (!diseaseConfigurationDto.getIncubationPeriodEnabled()) {
+ if (!Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled())) {
return;
}📝 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.
Suggested change
| if (!diseaseConfigurationDto.getIncubationPeriodEnabled()) { | |
| return; | |
| } | |
| if (!Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled())) { | |
| return; | |
| } |
🤖 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/CaseController.java` around
lines 1440 - 1442, The current check in CaseController using
!diseaseConfigurationDto.getIncubationPeriodEnabled() can NPE when
getIncubationPeriodEnabled() returns null; change the null-unsafe negation to a
null-safe comparison such as using
Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled()) (e.g.,
if (!Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled()))
return;), or explicitly handle null (check for null first) so
DiseaseConfigurationDto.getIncubationPeriodEnabled() is never auto-unboxed.
Comment on lines
+593
to
+598
| // Malaria and Dengue fields | ||
| addField(PathogenTestDto.ANTIBODY_TITRE, TextField.class); | ||
| addField(PathogenTestDto.PERFORMED_BY_REFERENCE_LABORATORY, NullableOptionGroup.class); | ||
| addField(PathogenTestDto.RETEST_REQUESTED, NullableOptionGroup.class); | ||
| Field<?> resultDetailsField = addField(PathogenTestDto.RESULT_DETAILS); | ||
| resultDetailsField.setVisible(false); |
There was a problem hiding this comment.
These new controls are globally visible.
Only resultDetailsField is hidden here. PERFORMED_BY_REFERENCE_LABORATORY and RETEST_REQUESTED have no disease gating in the DTO or the form, so they render on every pathogen-test form instead of just the malaria/dengue workflows introduced by this PR.
🤖 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/samples/PathogenTestForm.java`
around lines 593 - 598, PERFORMED_BY_REFERENCE_LABORATORY and RETEST_REQUESTED
are currently added unconditionally and thus show for all pathogen tests; update
PathogenTestForm so these fields (added via
addField(PathogenTestDto.PERFORMED_BY_REFERENCE_LABORATORY,
NullableOptionGroup.class) and addField(PathogenTestDto.RETEST_REQUESTED,
NullableOptionGroup.class)) are only visible for the malaria/dengue
workflows—e.g. after adding the fields, call setVisible(false) by default and
add the same disease-gating logic used for RESULT_DETAILS (or use the form's
pathogen/disease getter to setVisible(true) when pathogen type indicates malaria
or dengue), ensuring the visibility check uses the DTO/pathogen value in
PathogenTestForm rather than leaving them global.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
92-98:⚠️ Potential issue | 🟠 MajorDuplicate
CASE_IMPORTED_STATUSin layout definition.
EpiDataDto.CASE_IMPORTED_STATUSappears twice in the layout:
- Line 93:
fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"")- Line 98:
fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"")This will cause issues with duplicate component placement in the Vaadin layout. The first occurrence (line 93) appears to be unintentional and should be removed.
Proposed fix
private static final String MAIN_HTML_LAYOUT = loc(LOC_EXPOSURE_INVESTIGATION_HEADING) + fluidRowLocs("EXP_DATES_LAYOUT") + - fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"") + loc(LOC_EXP_PERIOD_HEADING) + loc(EpiDataDto.EXPOSURE_DETAILS_KNOWN) + loc(EpiDataDto.EXPOSURES) + loc(LOC_CONCLUSION_HEADING) + fluidRowLocs(6,EpiDataDto.CASE_IMPORTED_STATUS,6,"")+🤖 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/epidata/EpiDataForm.java` around lines 92 - 98, Remove the duplicated CASE_IMPORTED_STATUS component from the layout by deleting the earlier fluidRowLocs(6, EpiDataDto.CASE_IMPORTED_STATUS, 6, "") entry so the form only contains a single occurrence; locate the sequence building the layout that includes fluidRowLocs("EXP_DATES_LAYOUT"), fluidRowLocs(6, EpiDataDto.CASE_IMPORTED_STATUS,6,""), loc(LOC_EXP_PERIOD_HEADING), loc(EpiDataDto.EXPOSURE_DETAILS_KNOWN), loc(EpiDataDto.EXPOSURES), loc(LOC_CONCLUSION_HEADING) and remove the unintended first fluidRowLocs call referencing EpiDataDto.CASE_IMPORTED_STATUS.
🧹 Nitpick comments (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (2)
129-130: Rename parameterdatetosymptomOnsetDatefor clarity.The parameter name
dateis vague. Renaming it tosymptomOnsetDatewould match the field name (line 121) and improve readability.Proposed fix
public EpiDataForm( Disease disease, Class<? extends EntityDto> parentClass, boolean isPseudonymized, boolean inJurisdiction, Consumer<Boolean> sourceContactsToggleCallback, boolean isEditAllowed, - Date date) { + Date symptomOnsetDate) { super( EpiDataDto.class, EpiDataDto.I18N_PREFIX, false, FieldVisibilityCheckers.withDisease(disease).andWithCountry(FacadeProvider.getConfigFacade().getCountryLocale()), FieldAccessHelper.getFieldAccessCheckers(inJurisdiction, isPseudonymized), isEditAllowed); this.disease = disease; this.parentClass = parentClass; this.sourceContactsToggleCallback = sourceContactsToggleCallback; this.isPseudonymized = isPseudonymized; - this.symptomOnsetDate = date; + this.symptomOnsetDate = symptomOnsetDate; addFields(); }🤖 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/epidata/EpiDataForm.java` around lines 129 - 130, Rename the vague constructor parameter `date` to `symptomOnsetDate` in EpiDataForm (the constructor that currently takes boolean isEditAllowed, Date date) and update all usages inside the constructor and any calls to this constructor to use the new name; ensure the parameter maps to the existing field `symptomOnsetDate` and update any internal references (assignments, validations, UI bindings) and Javadoc/comments that mention `date` to keep naming consistent.
247-266: Consider extracting layout slot identifiers to constants.The layout slot identifiers (
"EXPOSURE_START_DATE_LABEL","EXPOSURE_START_DATE_VALUE", etc.) are repeated between the template definition (line 87) and the component placement calls. Extracting these tostatic final Stringconstants would reduce the risk of typos and improve maintainability.🤖 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/epidata/EpiDataForm.java` around lines 247 - 266, The repeated literal layout slot identifiers ("EXPOSURE_START_DATE_LABEL", "EXPOSURE_START_DATE_VALUE", "EXPOSURE_END_DATE_LABEL", "EXPOSURE_END_DATE_VALUE", "EXP_DATES_LAYOUT") in EpiDataForm should be extracted to static final String constants in the EpiDataForm class; add clearly named constants (e.g., EXPOSURE_START_DATE_LABEL, EXPOSURE_START_DATE_VALUE, etc.), replace the string literals used in exposureDatesLayout.addComponent(...) and getContent().addComponent(...) with these constants, and update any template definition references that use the same names to use the newly created constants to avoid typos and improve maintainability.sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java (1)
54-183: Consider extracting the common malaria test types to reduce repetition.The same 9-element
@ApplicableToPathogenTestslist is duplicated across all 10 malaria species constants. While Java annotations don't support variable references directly, you could document the canonical list in a comment or consider a helper method that returns the malaria-applicable test types for validation purposes.This is a minor maintainability concern — if a new test type needs to be added for malaria, it must be updated in 10 places.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java` around lines 54 - 183, The enum has the same 9-element `@ApplicableToPathogenTests` list repeated for all malaria species; add a single shared source of truth and switch validation to use it: create a public static final EnumSet<PathogenTestType> MALARIA_APPLICABLE_TESTS (or a method getMalariaApplicablePathogenTests()) in PathogenSpecie and populate it with the nine PathogenTestType values, add a brief comment above the malaria constants pointing to that shared constant, and update any validation/lookup code that currently reads the `@ApplicableToPathogenTests` annotation for Disease.MALARIA to use MALARIA_APPLICABLE_TESTS instead (keep the annotations if needed for other tooling but centralize runtime logic in the new constant/method).
🤖 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-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java`:
- Around line 184-195: The UNKNOWN enum constant in PathogenSpecie lacks a
`@Diseases` annotation so it gets filtered out by the disease-aware UI; update the
UNKNOWN enum entry to include an appropriate `@Diseases` annotation (matching TB
and Malaria as needed) so FieldHelper.updateItems' reflection-based filtering
will include it. Locate the UNKNOWN constant in the PathogenSpecie enum and add
the `@Diseases`(...) annotation with the correct disease values.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java`:
- Around line 221-227: Fix the typo in the Javadoc of EpiDataForm: change
"exposire" to "exposure" in the comment above the method that mentions including
the exposure start and dates when symptomOnsetDate is present; update the
Javadoc text around that sentence to read "Include the exposure start and dates
when symptomOnsetDate is present." to keep spelling correct and documentation
clear.
- Around line 237-238: The current check uses the negation operator on
diseaseConfigurationDto.getIncubationPeriodEnabled(), which unboxes a
potentially null Boolean and can throw NPE; change the guard to a null-safe
boolean test such as using
Boolean.TRUE.equals(diseaseConfigurationDto.getIncubationPeriodEnabled()) (or
explicitly check for null before unboxing) so the method returns when the flag
is not true without risking NullPointerException.
---
Outside diff comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java`:
- Around line 92-98: Remove the duplicated CASE_IMPORTED_STATUS component from
the layout by deleting the earlier fluidRowLocs(6,
EpiDataDto.CASE_IMPORTED_STATUS, 6, "") entry so the form only contains a single
occurrence; locate the sequence building the layout that includes
fluidRowLocs("EXP_DATES_LAYOUT"), fluidRowLocs(6,
EpiDataDto.CASE_IMPORTED_STATUS,6,""), loc(LOC_EXP_PERIOD_HEADING),
loc(EpiDataDto.EXPOSURE_DETAILS_KNOWN), loc(EpiDataDto.EXPOSURES),
loc(LOC_CONCLUSION_HEADING) and remove the unintended first fluidRowLocs call
referencing EpiDataDto.CASE_IMPORTED_STATUS.
---
Nitpick comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.java`:
- Around line 54-183: The enum has the same 9-element `@ApplicableToPathogenTests`
list repeated for all malaria species; add a single shared source of truth and
switch validation to use it: create a public static final
EnumSet<PathogenTestType> MALARIA_APPLICABLE_TESTS (or a method
getMalariaApplicablePathogenTests()) in PathogenSpecie and populate it with the
nine PathogenTestType values, add a brief comment above the malaria constants
pointing to that shared constant, and update any validation/lookup code that
currently reads the `@ApplicableToPathogenTests` annotation for Disease.MALARIA to
use MALARIA_APPLICABLE_TESTS instead (keep the annotations if needed for other
tooling but centralize runtime logic in the new constant/method).
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java`:
- Around line 129-130: Rename the vague constructor parameter `date` to
`symptomOnsetDate` in EpiDataForm (the constructor that currently takes boolean
isEditAllowed, Date date) and update all usages inside the constructor and any
calls to this constructor to use the new name; ensure the parameter maps to the
existing field `symptomOnsetDate` and update any internal references
(assignments, validations, UI bindings) and Javadoc/comments that mention `date`
to keep naming consistent.
- Around line 247-266: The repeated literal layout slot identifiers
("EXPOSURE_START_DATE_LABEL", "EXPOSURE_START_DATE_VALUE",
"EXPOSURE_END_DATE_LABEL", "EXPOSURE_END_DATE_VALUE", "EXP_DATES_LAYOUT") in
EpiDataForm should be extracted to static final String constants in the
EpiDataForm class; add clearly named constants (e.g., EXPOSURE_START_DATE_LABEL,
EXPOSURE_START_DATE_VALUE, etc.), replace the string literals used in
exposureDatesLayout.addComponent(...) and getContent().addComponent(...) with
these constants, and update any template definition references that use the same
names to use the newly created constants to avoid typos and improve
maintainability.
🪄 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: 90c03b96-f715-4617-ab52-7c01fac97b63
📒 Files selected for processing (9)
sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/resources/enum.propertiessormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.javasormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactController.javasormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/sampleLink/SampleListEntry.java
✅ Files skipped from review due to trivial changes (3)
- sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactController.java
- sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java
- sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.java
🚧 Files skipped from review as they are similar to previous changes (4)
- sormas-ui/src/main/java/de/symeda/sormas/ui/samples/sampleLink/SampleListEntry.java
- sormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.java
- sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
- sormas-api/src/main/resources/enum.properties
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/epidata/EpiDataForm.java`:
- Around line 240-245: Tighten the existing guards in EpiDataForm around
diseaseConfigurationDto incubation periods: in the block that currently checks
getMaxIncubationPeriod() and getMinIncubationPeriod(), also treat null, zero, or
negative max/min as invalid and return early, and additionally return early when
getMinIncubationPeriod() > getMaxIncubationPeriod(); update the logic that
computes exposure windows (the same method using
diseaseConfigurationDto.getMaxIncubationPeriod()/getMinIncubationPeriod()) so it
only runs when both values are present, positive, and min <= max.
🪄 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: ab37a347-9986-40ea-98e8-9c53da89a49a
📒 Files selected for processing (2)
sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java
Comment on lines
+240
to
+245
| if (diseaseConfigurationDto.getMaxIncubationPeriod() == null || diseaseConfigurationDto.getMaxIncubationPeriod() == 0) { | ||
| return; | ||
| } | ||
| if (diseaseConfigurationDto.getMinIncubationPeriod() == null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Reject invalid incubation ranges before computing dates.
Negative values or minIncubationPeriod > maxIncubationPeriod currently pass the guard and will render future or inverted exposure windows. Since these come from disease configuration, it's safer to skip the section when the range is invalid.
Proposed guard tightening
- if (diseaseConfigurationDto.getMaxIncubationPeriod() == null || diseaseConfigurationDto.getMaxIncubationPeriod() == 0) {
+ if (diseaseConfigurationDto.getMaxIncubationPeriod() == null || diseaseConfigurationDto.getMaxIncubationPeriod() <= 0) {
return;
}
- if (diseaseConfigurationDto.getMinIncubationPeriod() == null) {
+ if (diseaseConfigurationDto.getMinIncubationPeriod() == null
+ || diseaseConfigurationDto.getMinIncubationPeriod() < 0
+ || diseaseConfigurationDto.getMinIncubationPeriod() > diseaseConfigurationDto.getMaxIncubationPeriod()) {
return;
}📝 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.
Suggested change
| if (diseaseConfigurationDto.getMaxIncubationPeriod() == null || diseaseConfigurationDto.getMaxIncubationPeriod() == 0) { | |
| return; | |
| } | |
| if (diseaseConfigurationDto.getMinIncubationPeriod() == null) { | |
| return; | |
| } | |
| if (diseaseConfigurationDto.getMaxIncubationPeriod() == null || diseaseConfigurationDto.getMaxIncubationPeriod() <= 0) { | |
| return; | |
| } | |
| if (diseaseConfigurationDto.getMinIncubationPeriod() == null | |
| || diseaseConfigurationDto.getMinIncubationPeriod() < 0 | |
| || diseaseConfigurationDto.getMinIncubationPeriod() > diseaseConfigurationDto.getMaxIncubationPeriod()) { | |
| return; | |
| } |
🤖 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/epidata/EpiDataForm.java` around
lines 240 - 245, Tighten the existing guards in EpiDataForm around
diseaseConfigurationDto incubation periods: in the block that currently checks
getMaxIncubationPeriod() and getMinIncubationPeriod(), also treat null, zero, or
negative max/min as invalid and return early, and additionally return early when
getMinIncubationPeriod() > getMaxIncubationPeriod(); update the logic that
computes exposure windows (the same method using
diseaseConfigurationDto.getMaxIncubationPeriod()/getMinIncubationPeriod()) so it
only runs when both values are present, positive, and min <= max.
raulbob
approved these changes
Apr 1, 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
2 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.
Changes:
#13801 #13814
Fixes # Enhancing the sample and pathogen changes for Malaria and Dengue diseases
Summary by CodeRabbit
New Features
Refactor
Documentation