Fixed the case classification when a negative test result is detected.#13985
Conversation
📝 WalkthroughWalkthroughAdds UI and i18n pieces to allow reclassifying cases as "not a case" after verified negative pathogen tests, enables negative-case criteria for two diseases in backend rules, and tweaks contact view panel gating. ChangesNegative Case Classification Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
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/PathogenTestController.java (1)
455-467:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid triggering both confirm and negative dialogs in the same save flow.
Line 455 and Line 462 can both execute when verified positive and negative tests are present, leading to conflicting classification prompts in one action. Gate this to a single branch (based on the selected/latest result), and pass the refreshed case (
c) consistently.Suggested fix
- if (hasVerifiedPositiveTest) { + if (hasVerifiedPositiveTest) { this.showConfirmCaseDialog(c); // Case classification - } - // Show the confirmation dialog if there are verified negative tests. - if (hasVerifiedNegativeTest) { - this.showNegativeCaseDialog(caze); + } else if (hasVerifiedNegativeTest) { + this.showNegativeCaseDialog(c); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java` around lines 455 - 467, The save flow may call both branches when hasVerifiedPositiveTest and hasVerifiedNegativeTest are true, causing conflicting dialogs; update the logic in PathogenTestController (the block checking hasVerifiedPositiveTest / hasVerifiedNegativeTest) to choose a single branch (e.g., prefer the selected/latest test result) and invoke only one dialog: either showConfirmCaseDialog(c) or showNegativeCaseDialog(c) (use the refreshed case variable c consistently instead of caze). Ensure the selection criterion (latest/selected result) is used to gate the if/else so only one dialog path runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java`:
- Around line 798-813: showNegativeCaseDialog is missing the early short-circuit
used in showConfirmCaseDialog and it unboxes a nullable Boolean (confirmed)
which can NPE; add the same guard checks as in showConfirmCaseDialog (e.g.,
return early if caze.getCountryCode() is null/empty or if
caze.getCaseClassification() == CaseClassification.NO_CASE) before showing the
popup, and change the confirmation check to a null-safe comparison (use
Boolean.TRUE.equals(confirmed)) so the callback cannot NPE; reference the
methods/fields showNegativeCaseDialog, showConfirmCaseDialog,
caze.getCountryCode(), caze.getCaseClassification(), and
CaseClassification.NO_CASE when making the changes.
---
Outside diff comments:
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java`:
- Around line 455-467: The save flow may call both branches when
hasVerifiedPositiveTest and hasVerifiedNegativeTest are true, causing
conflicting dialogs; update the logic in PathogenTestController (the block
checking hasVerifiedPositiveTest / hasVerifiedNegativeTest) to choose a single
branch (e.g., prefer the selected/latest test result) and invoke only one
dialog: either showConfirmCaseDialog(c) or showNegativeCaseDialog(c) (use the
refreshed case variable c consistently instead of caze). Ensure the selection
criterion (latest/selected result) is used to gate the if/else so only one
dialog path runs.
🪄 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: 8ed02873-d088-4f15-845a-2aa9625af39f
📒 Files selected for processing (6)
sormas-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/resources/captions.propertiessormas-api/src/main/resources/strings.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/caze/classification/CaseClassificationFacadeEjb.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataView.java (1)
231-233: 💤 Low valueConsider simplifying single-element disease check.
For a single-disease exclusion check, a direct equality comparison would be clearer and slightly more efficient than creating a single-element list.
♻️ Suggested simplification
- // Allowing the samples component for LUX Measles. if (!(FacadeProvider.getConfigFacade().isConfiguredCountry(CountryHelper.COUNTRY_CODE_LUXEMBOURG) - && List.of(Disease.INVASIVE_MENINGOCOCCAL_INFECTION).contains(contactDto.getDisease()))) { + && contactDto.getDisease() == Disease.INVASIVE_MENINGOCOCCAL_INFECTION)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataView.java` around lines 231 - 233, The conditional in ContactDataView that currently does: List.of(Disease.INVASIVE_MENINGOCOCCAL_INFECTION).contains(contactDto.getDisease()) should be simplified to a direct enum comparison; replace the single-element list containment check with contactDto.getDisease() == Disease.INVASIVE_MENINGOCOCCAL_INFECTION (or .equals(...) if not an enum) inside the existing if to make the intent clearer and slightly more efficient.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataView.java`:
- Around line 231-233: The conditional in ContactDataView that currently does:
List.of(Disease.INVASIVE_MENINGOCOCCAL_INFECTION).contains(contactDto.getDisease())
should be simplified to a direct enum comparison; replace the single-element
list containment check with contactDto.getDisease() ==
Disease.INVASIVE_MENINGOCOCCAL_INFECTION (or .equals(...) if not an enum) inside
the existing if to make the intent clearer and slightly more efficient.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0cfc9469-3d0f-4691-bb9a-53c51716728d
📒 Files selected for processing (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactDataView.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestController.java
Fixes #
Summary by CodeRabbit
New Features
Bug Fixes / Improvements