Skip to content

Conversation

@roldy
Copy link

@roldy roldy commented Dec 18, 2025

… the medical history of a case

Fixes #13456

Summary by CodeRabbit

  • New Features
    • Added mutual exclusivity constraints: PREGNANT and POSTPARTUM fields cannot both be selected; CURRENT_SMOKER and FORMER_SMOKER are also mutually exclusive
    • Implemented automatic field updates: selecting certain health conditions now automatically sets related health fields when present

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

Walkthrough

This PR implements field-level constraints to prevent contradictory medical history data entry. It introduces mutual exclusivity between paired fields (Current/Former smoker, Pregnancy/Postpartum) and auto-enables immunodeficiency when asplenia is selected, via a reusable helper method added to the base form class.

Changes

Cohort / File(s) Summary
Base form utility
sormas-ui/src/main/java/de/symeda/sormas/ui/utils/AbstractEditForm.java
Adds setupMutuallyExclusiveFields() helper method to wire bidirectional mutual exclusivity between two NullableOptionGroup fields; includes HashSet/Set/Arrays imports.
Form implementations
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java, sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java
CaseDataForm enforces mutual exclusivity for PREGNANT/POSTPARTUM fields. HealthConditionsForm enforces mutual exclusivity for CURRENT_SMOKER/FORMER_SMOKER fields and adds auto-check: when ASPLENIA is set, IMMUNODEFICIENCY_OTHER_THAN_HIV is automatically enabled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • New helper method logic requires verification of bidirectional field state synchronization
  • Dual-form implementation of mutual exclusivity pattern should be validated for consistency
  • Auto-check logic in HealthConditionsForm needs testing to ensure proper field visibility/state dependencies

Possibly related PRs

Suggested reviewers

  • raulbob
  • obinna-h-n
  • KarnaiahPesula

Poem

🐰 Hop along, fields now dance in sync,
No more pregnancy and postpartum's mixed link!
Asplenia checks immunity true,
Contradictions fade—medical logic shines through!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title references issue #13456 and clearly summarizes the main change: fixing inconsistent/contradictory data entry in medical history.
Description check ✅ Passed The description follows the repository template correctly, specifying 'Fixes #13456' to link to the related issue as required.
Linked Issues check ✅ Passed The changes fully address all objectives: mutual exclusivity for Current/Former smoker [#13456], Pregnancy/Postpartum [#13456], and auto-linking Asplenia to Immunodeficiency [#13456].
Out of Scope Changes check ✅ Passed All changes are directly related to issue #13456: adding mutual exclusivity logic and auto-linking fields as specified in the requirements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix-13456-contradictory-data-entry

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af6249b and 6a99f3a.

📒 Files selected for processing (3)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java (2 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java (1 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/utils/AbstractEditForm.java (4 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). (1)
  • GitHub Check: SORMAS CI
🔇 Additional comments (6)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.java (2)

40-40: LGTM! Minimal import additions.

The new imports (HashSet, Set) are necessary to support the mutual exclusivity logic introduced later in the form.

Also applies to: 42-42


967-976: LGTM! Proper implementation of mutual exclusivity.

The mutual exclusivity between Pregnancy and Postpartum fields is correctly implemented with appropriate null guards and a clear explanatory comment. This aligns with the PR objective to prevent contradictory data entry.

sormas-ui/src/main/java/de/symeda/sormas/ui/utils/AbstractEditForm.java (2)

21-21: LGTM! Required imports for the new helper method.

The added imports (Arrays, HashSet, Set) are minimal and necessary to support the new mutual exclusivity functionality.

Also applies to: 24-24, 27-27


692-715: Verify read-only field handling and confirm NO vs UNKNOWN design choice.

The mutual exclusivity logic is sound and prevents infinite loops. However, consider these aspects:

  1. Read-only fields: The method doesn't check if fields are read-only before calling setValue(). While this is probably fine in practice (read-only fields typically aren't modified by users), verify this doesn't cause issues in edge cases where fields might be programmatically set while read-only.

  2. Design choice - NO vs UNKNOWN: When one field is set to YES, the other is forced to NO rather than UNKNOWN. This is a design decision that may be too opinionated - for example, if a user sets "Pregnant: YES", the system forces "Postpartum: NO" even if the actual state might be UNKNOWN. Verify this aligns with the expected behavior and domain requirements.

Optional: Consider adding read-only guards

If read-only field handling is a concern, you could add guards:

 protected void setupMutuallyExclusiveFields(NullableOptionGroup field1, NullableOptionGroup field2) {
   field1.addValueChangeListener(e -> {
     Set<Object> value = (Set<Object>) e.getProperty().getValue();
     if (value != null && value.contains(de.symeda.sormas.api.utils.YesNoUnknown.YES)) {
+      if (!field2.isReadOnly()) {
         field2.setValue(new HashSet<>(Arrays.asList(de.symeda.sormas.api.utils.YesNoUnknown.NO)));
+      }
     }
   });
   field2.addValueChangeListener(e -> {
     Set<Object> value = (Set<Object>) e.getProperty().getValue();
     if (value != null && value.contains(de.symeda.sormas.api.utils.YesNoUnknown.YES)) {
+      if (!field1.isReadOnly()) {
         field1.setValue(new HashSet<>(Arrays.asList(de.symeda.sormas.api.utils.YesNoUnknown.NO)));
+      }
     }
   });
 }
sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java (2)

151-156: LGTM! Proper mutual exclusivity implementation.

The mutual exclusivity between Current Smoker and Former Smoker is correctly implemented with appropriate null guards and a clear comment, aligning with the PR objectives.


158-174: Verify the one-way auto-check behavior is intentional.

The auto-check implementation correctly sets "Immunodeficiency (other than HIV)" to YES when Asplenia is selected, with appropriate visibility guards. However, note the following behavior:

One-way logic: When Asplenia is set to YES, Immunodeficiency is automatically set to YES. However, if the user later changes Asplenia to NO or UNKNOWN, the Immunodeficiency field remains YES and is not automatically cleared.

This might be intentional (once alerted to immunodeficiency via asplenia selection, we don't automatically remove it if they reconsider), but verify this aligns with the expected medical workflow and user experience.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sormas-vitagroup
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent or contradictory data entry is possible in the medical history of a case

3 participants