Default follow-up is enabled for AFP Influenza subtypes like A & B are inactive by default.#13641
Conversation
Influenza subtypes like A & B are inactive by default.
WalkthroughThis pull request modifies disease enum constants in the Disease.java file, adjusting configuration flags and follow-up durations for AFP, INFLUENZA, INFLUENZA_A, and INFLUENZA_B. Specifically, AFP gains follow-up tracking enabled with a 60-day duration, while INFLUENZA-related constants have their active and primary flags redistributed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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: 1
🧹 Nitpick comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/Disease.java (1)
28-28: AFP follow-up enabled (60 days): confirm requirement and add a regression testChange seems correct given the PR goal. Please confirm 60 days is the intended default and add a small test asserting
AFP.isDefaultFollowUpEnabled()andAFP.getDefaultFollowUpDuration() == 60.I can draft the unit test if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sormas-api/src/main/java/de/symeda/sormas/api/Disease.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: android app test (27)
- GitHub Check: android app test (26)
- GitHub Check: android app test (28)
- GitHub Check: SORMAS CI
| INFLUENZA(true, false, false, false, false, 0, true, false, false), | ||
| INFLUENZA_A(false, true, true, false, false, 0, true, false, false), | ||
| INFLUENZA_B(false, true, true, false, false, 0, true, false, false), |
There was a problem hiding this comment.
🧩 Analysis chain
INFLUENZA tuples likely wrong: case surveillance disabled for INFLUENZA, A/B marked primary while inactive
Per constructor order (lines 106–115), you set:
- INFLUENZA: defaultCaseSurveillanceEnabled=false and defaultPrimary=false
- INFLUENZA_A/B: defaultActive=false but defaultPrimary=true
If the intent is only “INFLUENZA_A and INFLUENZA_B inactive by default” while keeping INFLUENZA as the active, primary disease with case surveillance enabled, adjust as below.
Apply this diff:
- INFLUENZA(true, false, false, false, false, 0, true, false, false),
- INFLUENZA_A(false, true, true, false, false, 0, true, false, false),
- INFLUENZA_B(false, true, true, false, false, 0, true, false, false),
+ INFLUENZA(true, true, true, false, false, 0, true, false, false),
+ INFLUENZA_A(false, false, true, false, false, 0, true, false, false),
+ INFLUENZA_B(false, false, true, false, false, 0, true, false, false),Optional maintainability tweak: add inline argument comments to avoid boolean mis-ordering in future, e.g.
INFLUENZA(/*active*/true, /*primary*/true, /*case*/true, /*aggregate*/false, /*followUp*/false, /*days*/0, /*variant*/true, /*ext*/false, /*extMulti*/false).
Run this quick grep to see where these flags drive behavior and confirm no invariants are violated:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites of case surveillance flag:"
rg -nP '\.isDefaultCaseSurveillanceEnabled\s*\(' -C3 || true
echo "Call sites of primary flag:"
rg -nP '\.isDefaultPrimary\s*\(' -C3 || true
echo "References to INFLUENZA and subtypes:"
rg -nP 'Disease\.(INFLUENZA|INFLUENZA_A|INFLUENZA_B)\b' -n -C2Length of output: 44146
🏁 Script executed:
#!/bin/bash
# Extract Disease enum constructor definition
cat sormas-api/src/main/java/de/symeda/sormas/api/Disease.java | grep -A 20 "Disease(boolean"Length of output: 90
🏁 Script executed:
#!/bin/bash
# Find Disease.java file
fd -name "Disease.java" -type fLength of output: 230
🏁 Script executed:
#!/bin/bash
# Find Disease.java using find
find . -name "Disease.java" -type f 2>/dev/null | head -5Length of output: 64
🏁 Script executed:
#!/bin/bash
# Extract constructor from Disease.java around lines 106-115
sed -n '106,115p' ./git/sormas-api/src/main/java/de/symeda/sormas/api/Disease.javaLength of output: 326
🏁 Script executed:
#!/bin/bash
# Search for tests related to Disease defaults or INFLUENZA configuration
find . -path "*/test/*" -name "*.java" -type f 2>/dev/null | xargs grep -l "INFLUENZA\|defaultPrimary\|defaultCaseSurveillanceEnabled" 2>/dev/null | head -10Length of output: 312
🏁 Script executed:
#!/bin/bash
# Extract DiseasesConfigurationTest.java to check for INFLUENZA assertions
cat ./git/sormas-api/src/test/java/de/symeda/sormas/api/utils/DiseasesConfigurationTest.javaLength of output: 3107
🏁 Script executed:
#!/bin/bash
# Check for any logic combining INFLUENZA_A/B with primary or active checks
rg -n 'INFLUENZA_A|INFLUENZA_B' --type=java -C5 | grep -E '(primary|active|Primary|Active)' | head -20Length of output: 0
INFLUENZA tuples have inconsistent defaults: A/B inactive but marked primary; INFLUENZA active but case surveillance disabled
Constructor parameter order (lines 106–115) is: defaultActive, defaultPrimary, defaultCaseSurveillanceEnabled, ...
Current state:
INFLUENZA(true, false, false, ...): active, not primary, no case surveillanceINFLUENZA_A(false, true, true, ...): inactive, primary, case surveillance enabledINFLUENZA_B(false, true, true, ...): inactive, primary, case surveillance enabled
This contradicts typical enum semantics. If INFLUENZA_A and INFLUENZA_B are inactive by design, they should not be marked as primary. If INFLUENZA is the active parent disease, it should be both primary and have case surveillance enabled by default.
Apply this fix:
- INFLUENZA(true, false, false, false, false, 0, true, false, false),
- INFLUENZA_A(false, true, true, false, false, 0, true, false, false),
- INFLUENZA_B(false, true, true, false, false, 0, true, false, false),
+ INFLUENZA(true, true, true, false, false, 0, true, false, false),
+ INFLUENZA_A(false, false, true, false, false, 0, true, false, false),
+ INFLUENZA_B(false, false, true, false, false, 0, true, false, false),Optional: Add inline comments to prevent future boolean parameter mix-ups:
INFLUENZA(/*active*/true, /*primary*/true, /*case*/true, /*aggregate*/false, /*followUp*/false, /*days*/0, /*variant*/true, /*ext*/false, /*extMulti*/false)1 participant
Fixes #
Summary by CodeRabbit