Minor issue fixes for Measles#13595
Conversation
WalkthroughAdds a Luxembourg+Measles-specific boolean flag clusterRelated across API, backend, DB, and UI; adds i18n caption; extends PathogenTestType.SEQUENCING to include MEASLES; gates several UI fields/headings for Luxembourg+Measles; refactors immunization form to reflectively filter enum options. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI: EpiDataForm
participant DTO as EpiDataDto
participant EJB as EpiDataFacadeEjb
participant ENT as EpiData (Entity)
participant DB as DB
UI->>DTO: set clusterRelated (checkbox)
UI->>EJB: submit DTO
EJB->>ENT: fillOrBuildEntity(...)\nsetClusterRelated(source.isClusterRelated())
ENT->>DB: persist epidata.clusterRelated
DB-->>ENT: stored
EJB-->>DTO: toDto(...)\nsetClusterRelated(entity.isClusterRelated())
note over UI: Visibility/headings depend on\n(disease == MEASLES && country == LUXEMBOURG)
sequenceDiagram
autonumber
participant UIF as UI: ImmunizationCreationForm
participant FV as FieldVisibilityCheckers
participant FH as FieldHelper
UIF->>UIF: init or disease change
UIF->>UIF: updateMeansOfImmunizationField(disease)
loop for each MeansOfImmunization value
UIF->>FV: isVisible(enumField, disease)
FV-->>UIF: true/false
end
UIF->>FH: updateEnumData(meansOfImmunizationField, filteredValues)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/immunization/components/form/ImmunizationCreationForm.java (3)
225-225: Fix wrong layout slot for person form.Component is added to TravelEntry slot; the template defines ImmunizationDto.PERSON. This likely breaks rendering.
Apply:
- getContent().addComponent(personCreateForm, TravelEntryDto.PERSON); + getContent().addComponent(personCreateForm, ImmunizationDto.PERSON);
218-224: Avoid NPE when disease is cleared.selectedDisease can be null (user clears the field). Either guard here or make updateMeansOfImmunizationField handle null (see below).
Option A (guard here):
final Disease selectedDisease = (Disease) valueChangeEvent.getProperty().getValue(); personCreateForm.updatePresentConditionEnum(selectedDisease); - // Update means of immunization field based on selected disease using filtered enum data - updateMeansOfImmunizationField(selectedDisease); + // Update means of immunization field based on selected disease using filtered enum data + updateMeansOfImmunizationField(selectedDisease);This relies on the method being null-safe as proposed next.
391-403: Make enum filtering null-safe, resilient to reflection issues, and keep selection consistent.Prevents NPE when disease is null, fails open on reflection errors, and clears invalid selection after filtering.
Apply:
- private void updateMeansOfImmunizationField(Disease disease) { - List<MeansOfImmunization> filteredValues = Arrays.stream(MeansOfImmunization.values()).filter(value -> { - try { - java.lang.reflect.Field enumField = MeansOfImmunization.class.getField(value.name()); - return FieldVisibilityCheckers.withDisease(disease).isVisible(MeansOfImmunization.class, enumField); - } catch (NoSuchFieldException e) { - // If field doesn't exist, include it by default - return true; - } - }).collect(Collectors.toList()); - - FieldHelper.updateEnumData(meansOfImmunizationField, filteredValues); - } + private void updateMeansOfImmunizationField(Disease disease) { + // If no disease is selected, expose the full enum set. + if (disease == null) { + FieldHelper.updateEnumData(meansOfImmunizationField, Arrays.asList(MeansOfImmunization.values())); + return; + } + + List<MeansOfImmunization> filteredValues = Arrays.stream(MeansOfImmunization.values()) + .filter(value -> { + try { + java.lang.reflect.Field enumField = MeansOfImmunization.class.getField(value.name()); + return FieldVisibilityCheckers.withDisease(disease).isVisible(MeansOfImmunization.class, enumField); + } catch (ReflectiveOperationException | SecurityException ex) { + // Fail open: include value if reflection/visibility cannot be resolved + return true; + } + }) + .collect(Collectors.toList()); + + Object current = meansOfImmunizationField.getValue(); + FieldHelper.updateEnumData(meansOfImmunizationField, filteredValues); + if (current != null && !filteredValues.contains(current)) { + meansOfImmunizationField.clear(); + } + }
🧹 Nitpick comments (4)
sormas-ui/src/main/java/de/symeda/sormas/ui/immunization/components/form/ImmunizationCreationForm.java (1)
391-403: Optional: cache filtered options per disease to avoid repeated reflection.Memoize Map<Disease, List> and invalidate if visibility rules change.
sormas-backend/src/main/resources/sql/sormas_schema.sql (1)
14650-14653: Optional: consider an index if this flag is queried in filtersIf UI/reporting will frequently filter by this flag (e.g., cluster-related measles in LUX), consider a partial index on
epidata(clusterRelated)fortruein a follow-up migration.sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataForm.java (1)
223-224: Initialize luxMeasles via helper and reuse consistentlyUse the same isConfiguredServer-based check as elsewhere in this class and avoid binding to constructor disease only.
- this.luxMeasles = Disease.MEASLES == disease && FacadeProvider.getConfigFacade().isConfiguredCountry(CountryHelper.COUNTRY_CODE_LUXEMBOURG); + this.luxMeasles = isLuxMeasles(disease);sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
157-157: Typo: rename local variable clustorTypeTF → clusterTypeTFTiny readability nit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java(3 hunks)sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java(1 hunks)sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java(1 hunks)sormas-api/src/main/resources/captions.properties(1 hunks)sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiData.java(2 hunks)sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java(2 hunks)sormas-backend/src/main/resources/sql/sormas_schema.sql(1 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataForm.java(4 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java(4 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/immunization/components/form/ImmunizationCreationForm.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (2)
sormas-api/src/main/java/de/symeda/sormas/api/CountryHelper.java (1)
CountryHelper(22-45)sormas-ui/src/main/java/de/symeda/sormas/ui/utils/FieldHelper.java (1)
FieldHelper(56-1257)
sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataForm.java (3)
sormas-api/src/main/java/de/symeda/sormas/api/FacadeProvider.java (1)
FacadeProvider(127-594)sormas-api/src/main/java/de/symeda/sormas/api/CountryHelper.java (1)
CountryHelper(22-45)sormas-ui/src/main/java/de/symeda/sormas/ui/utils/FieldHelper.java (1)
FieldHelper(56-1257)
sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/CountryHelper.java (1)
CountryHelper(22-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: android app test (28)
- GitHub Check: android app test (27)
- GitHub Check: android app test (26)
- GitHub Check: SORMAS CI
🔇 Additional comments (12)
sormas-ui/src/main/java/de/symeda/sormas/ui/immunization/components/form/ImmunizationCreationForm.java (1)
364-367: LGTM: initialize options based on current disease.Ensures the means-of-immunization list is filtered on initial load.
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (1)
1564-1564: Missing translations: add EpiData.clusterRelated to locale bundles
- Generator output present — sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java:1564: String EpiData_clusterRelated = "EpiData.clusterRelated".
- Only base bundle defines the key — sormas-api/src/main/resources/captions.properties:1148: EpiData.clusterRelated=Cluster Related; all other captions_*.properties in sormas-api/src/main/resources are missing this key.
- Action: add appropriate translations to the missing locale files or regenerate/update the locale bundles so every captions_*.properties includes EpiData.clusterRelated.
sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiData.java (1)
175-181: Accessors & mappings verifiedisClusterRelated/setClusterRelated present in EpiData.java (175–181) and EpiDataDto.java (198–204). EpiDataFacadeEjb maps clusterRelated via target.setClusterRelated(source.isClusterRelated()) at EpiDataFacadeEjb.java:106 and 253.
sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java (1)
136-139: Confirm visibility for SEQUENCING with MEASLESSEQUENCING's @diseases includes Disease.MEASLES but lacks hide = true while many other MEASLES-related types are hidden — confirm it should be user-visible or add hide = true. File: sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java (lines 136–139)
sormas-backend/src/main/resources/sql/sormas_schema.sql (1)
14650-14653: clusterRelated: backend mapping present — verify history trigger & DB identifier
- Backend/API: primitive boolean clusterRelated is declared in sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiData.java and sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java (getter/setter present).
- SQL: sormas-backend/src/main/resources/sql/sormas_schema.sql adds unquoted clusterRelated to epidata and epidata_history (ALTER TABLE ... add column IF NOT EXISTS clusterRelated boolean DEFAULT false). Unquoted identifiers are folded to lowercase by Postgres -> column will be clusterrelated.
- I did not find any CREATE TRIGGER or INSERT INTO epidata_history(...) under sormas-backend/src/main/resources/sql/**; verify any history trigger or code that inserts into epidata_history either (a) uses explicit column lists including clusterRelated or (b) is updated to include the new column.
- Action: confirm ORM naming strategy maps the field to the created lowercase column (or add an explicit @column(name="clusterrelated") / quote the SQL identifier) and update history trigger/INSERT column lists if needed.
sormas-api/src/main/java/de/symeda/sormas/api/epidata/EpiDataDto.java (3)
59-59: Constant for new field looks goodPublic constant aligns with existing naming conventions and is used by UI.
75-79: Scoped field addition is appropriateAnnotations correctly restrict visibility to Luxembourg/Measles. Primitive boolean is fine for a checkbox.
198-204: Accessor methods OKGetter/setter follow bean conventions and match the field name.
sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java (2)
106-107: DTO → Entity mapping added for clusterRelatedSymmetric with other fields; placement is correct.
253-254: Entity → DTO mapping added for clusterRelatedRound-trip completeness confirmed.
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (2)
85-86: Layout extension to include CLUSTER_RELATED looks good3/5/4 split is consistent with surrounding rows.
216-221: Conditional headings are consistent with field scopingRendering the EpiCaseImport and ClusterType headings only for Luxembourg/Measles matches DTO annotations.
| EpiData.largeOutbreaksArea=Residing or travelling to countries/territories/areas experiencing larger outbreaks of local transmission | ||
| EpiData.contactWithSourceCaseKnown=Contacts with source case known | ||
| EpiData.caseImportedStatus=Case imported status | ||
| EpiData.clusterRelated=Cluster Related |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Match caption casing with neighboring keys
Use sentence case to be consistent with adjacent EpiData captions (e.g., “Case imported status”, “Cluster type”) and with existing “contactRelated=Contact related”.
-EpiData.clusterRelated=Cluster Related
+EpiData.clusterRelated=Cluster relatedRun to ensure the new key exists across all locale bundles:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "All caption bundles:"
fd -a 'captions.*\.properties' -t f | sort
echo -e "\nBundles missing EpiData.clusterRelated:"
while read -r f; do
if ! rg -n '^EpiData\.clusterRelated=' "$f" >/dev/null; then
echo " - $f"
fi
done < <(fd -a 'captions.*\.properties' -t f)Length of output: 4084
🏁 Script executed:
#!/bin/bash
set -euo pipefail
caption_files=( $(fd -a 'captions.*\.properties' -t f) )
echo "Caption bundles found: ${#caption_files[@]}"
if [ ${#caption_files[@]} -eq 0 ]; then
echo "No caption bundles found."
exit 0
fi
missing=()
found=()
for f in "${caption_files[@]}"; do
match=$(rg -n '^EpiData\.clusterRelated=' "$f" 2>/dev/null || true)
if [ -n "$match" ]; then
echo "FOUND: $match"
found+=("$f")
else
echo "MISSING: $f"
missing+=("$f")
fi
done
echo
printf 'Summary: %d found, %d missing\n' "${#found[@]}" "${#missing[@]}"
if [ ${#missing[@]} -gt 0 ]; then
printf '\nBundles missing EpiData.clusterRelated:\n'
for mf in "${missing[@]}"; do printf ' - %s\n' "$mf"; done
exit 1
else
echo "All bundles contain EpiData.clusterRelated"
fiLength of output: 8563
Match caption casing with neighboring keys — change value and propagate key
Change in sormas-api/src/main/resources/captions.properties (line 1148):
-EpiData.clusterRelated=Cluster Related
+EpiData.clusterRelated=Cluster relatedAdd EpiData.clusterRelated= to these bundles (currently missing; 52 files):
- sormas-api/src/main/resources/captions_ar-SA.properties
- sormas-api/src/main/resources/captions_ar.properties
- sormas-api/src/main/resources/captions_cs-CZ.properties
- sormas-api/src/main/resources/captions_de-CH.properties
- sormas-api/src/main/resources/captions_de-DE.properties
- sormas-api/src/main/resources/captions_en-AF.properties
- sormas-api/src/main/resources/captions_en-GH.properties
- sormas-api/src/main/resources/captions_en-GM.properties
- sormas-api/src/main/resources/captions_en-KE.properties
- sormas-api/src/main/resources/captions_en-LR.properties
- sormas-api/src/main/resources/captions_en-NG.properties
- sormas-api/src/main/resources/captions_en.properties
- sormas-api/src/main/resources/captions_es-BO.properties
- sormas-api/src/main/resources/captions_es-CU.properties
- sormas-api/src/main/resources/captions_es-EC.properties
- sormas-api/src/main/resources/captions_es-ES.properties
- sormas-api/src/main/resources/captions_fa-AF.properties
- sormas-api/src/main/resources/captions_fi-FI.properties
- sormas-api/src/main/resources/captions_fil-PH.properties
- sormas-api/src/main/resources/captions_fj-FJ.properties
- sormas-api/src/main/resources/captions_fr-CD.properties
- sormas-api/src/main/resources/captions_fr-CH.properties
- sormas-api/src/main/resources/captions_fr-FR.properties
- sormas-api/src/main/resources/captions_fr-TN.properties
- sormas-api/src/main/resources/captions_haw-US.properties
- sormas-api/src/main/resources/captions_hi-IN.properties
- sormas-api/src/main/resources/captions_hr-HR.properties
- sormas-api/src/main/resources/captions_it-CH.properties
- sormas-api/src/main/resources/captions_it-IT.properties
- sormas-api/src/main/resources/captions_ja-JP.properties
- sormas-api/src/main/resources/captions_ks-PK.properties
- sormas-api/src/main/resources/captions_ku-TR.properties
- sormas-api/src/main/resources/captions_la-LA.properties
- sormas-api/src/main/resources/captions_lv-LV.properties
- sormas-api/src/main/resources/captions_mt-MT.properties
- sormas-api/src/main/resources/captions_ne-NP.properties
- sormas-api/src/main/resources/captions_nl-NL.properties
- sormas-api/src/main/resources/captions_no-NO.properties
- sormas-api/src/main/resources/captions_pl-PL.properties
- sormas-api/src/main/resources/captions_ps-AF.properties
- sormas-api/src/main/resources/captions_pt-BR.properties
- sormas-api/src/main/resources/captions_pt-CV.properties
- sormas-api/src/main/resources/captions_pt-PT.properties
- sormas-api/src/main/resources/captions_ro-RO.properties
- sormas-api/src/main/resources/captions_ru-RU.properties
- sormas-api/src/main/resources/captions_sv-SE.properties
- sormas-api/src/main/resources/captions_sw-KE.properties
- sormas-api/src/main/resources/captions_tr-TR.properties
- sormas-api/src/main/resources/captions_uk-UA.properties
- sormas-api/src/main/resources/captions_ur-PK.properties
- sormas-api/src/main/resources/captions_zh-CN.properties
- sormas-api/src/main/resources/captions_zu-ZA.properties
📝 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.
| EpiData.clusterRelated=Cluster Related | |
| EpiData.clusterRelated=Cluster related |
🤖 Prompt for AI Agents
In sormas-api/src/main/resources/captions.properties around line 1148, the key
EpiData.clusterRelated has value "Cluster Related" whose casing should match
neighboring keys; update the value to the correct casing (e.g., "Cluster
related") and add the same key with an appropriate translated value to each of
the listed locale bundles (the 52 captions_*.properties files) so they contain
EpiData.clusterRelated=<translated value>; ensure existing file encoding/format
preserved and keys are added in logical alphabetical group or near related
EpiData keys.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13595 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (1)
1140-1140: Update misleading section comment "Probable"The section still reads “// Probable” but now asserts SUSPECT. Please retitle to avoid confusion, e.g.:
// Epidemiological confirmation alone should remain SUSPECT for IMI
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: SORMAS CI
- GitHub Check: android app test (26)
- GitHub Check: android app test (28)
- GitHub Check: android app test (27)
🔇 Additional comments (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (1)
1082-1082: LGTM: IMI epidemiological confirmation now yields SUSPECTExpectation aligns with the rule change (epi confirmation alone no longer escalates IMI to PROBABLE). Sandbox verification failed: "test: No such file or directory (os error 2)". Run the following locally to confirm no tests still assert PROBABLE for INVASIVE_MENINGOCOCCAL_INFECTION:
rg -nP --type=java -C2 '\bINVASIVE_MENINGOCOCCAL_INFECTION\b' test || true rg -nP --type=java -C2 'CaseClassification\.PROBABLE' test || true rg -nP --type=java -C3 '(INVASIVE_MENINGOCOCCAL_INFECTION).*(PROBABLE)|(PROBABLE).*(INVASIVE_MENINGOCOCCAL_INFECTION)' test || true
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13595 |
3 participants
Fixes #
Summary by CodeRabbit
New Features
UI Changes
Localization
Database
Behavior Changes