Bugfix mala deng issues#13994
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds the ChangesMalaria, Dengue and Lab Message Processing Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![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
🤖 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/exposure/ExposureForm.java`:
- Around line 425-428: The prophylaxis visibility logic in the new listener gate
(lines 425-428) correctly restricts prophylaxis fields to malaria-only travelers
by checking both disease == Disease.MALARIA and TRAVELED_ABROAD, but the load
path in the setValue method (around line 707 onward) only checks TRAVELED_ABROAD
without the disease restriction. Apply the same disease == Disease.MALARIA check
in the load path where it currently controls the visibility of
PROPHYLAXIS_ADHERENCE and TRAVEL_PURPOSE fields to ensure non-malaria disease
records cannot display prophylaxis fields through the load path, maintaining
consistency with the new malaria-only rule.
🪄 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: 5680faab-1fcf-406a-a947-83a465dd2fa0
📒 Files selected for processing (12)
sormas-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/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-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/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.java
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
1 similar comment
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
# Conflicts: # sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java # sormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.java # sormas-backend/src/main/resources/sql/sormas_schema.sql
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 16412-16413: The ALTER TABLE statements for testreport and
testreport_history are adding fourfoldincreaseantibodytiter as nullable boolean
columns with DEFAULT false, but the Java entity uses a primitive boolean type
which cannot be null, creating a schema-Java mismatch. To fix this, follow the
established pattern in the schema: first add an UPDATE statement to set any
existing NULL values to false in both the testreport and testreport_history
tables, then modify both ALTER TABLE statements to include the NOT NULL
constraint on the fourfoldincreaseantibodytiter columns to enforce the
non-nullable contract expected by the Java primitive boolean field.
🪄 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: ac11a9ad-39ae-4ef9-8c2b-895df47e5914
📒 Files selected for processing (8)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.java
✅ Files skipped from review due to trivial changes (1)
- sormas-api/src/main/resources/enum.properties
🚧 Files skipped from review as they are similar to previous changes (5)
- sormas-api/src/main/resources/captions.properties
- sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java
- sormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.java
- sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java
- sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java
Comment on lines
+16412
to
+16413
| ALTER TABLE testreport ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; | ||
| ALTER TABLE testreport_history ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current nullability/defaults for the added columns in schema SQL
rg -n -C2 'fourfoldincreaseantibodytiter|ALTER COLUMN .* SET NOT NULL' sormas-backend/src/main/resources/sql/sormas_schema.sqlRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 16223
🏁 Script executed:
fd -i testReport.java -type fRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 247
🏁 Script executed:
find . -type f -name "*.java" | xargs grep -l "class TestReport" | head -5Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 602
🏁 Script executed:
rg -l "class TestReport\b" --type javaRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 173
🏁 Script executed:
cat -n sormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/labmessage/TestReport.java | grep -A5 -B5 "fourFoldIncreaseAntibodyTiter"Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 988
Enforce NOT NULL on the new boolean columns to match the Java primitive contract.
TestReport uses primitive boolean for fourFoldIncreaseAntibodyTiter (line 186), but the new DB columns allow NULL. This creates a schema-Java mismatch at the persistence boundary.
The established pattern in the schema file (see epidata.clusterrelated at lines 14651-14656) is to UPDATE existing NULLs before adding the NOT NULL constraint:
Suggested migration adjustment
ALTER TABLE testreport ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false;
ALTER TABLE testreport_history ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false;
+UPDATE testreport SET fourfoldincreaseantibodytiter = false WHERE fourfoldincreaseantibodytiter IS NULL;
+UPDATE testreport_history SET fourfoldincreaseantibodytiter = false WHERE fourfoldincreaseantibodytiter IS NULL;
+ALTER TABLE testreport ALTER COLUMN fourfoldincreaseantibodytiter SET NOT NULL;
+ALTER TABLE testreport_history ALTER COLUMN fourfoldincreaseantibodytiter SET NOT NULL;📝 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
| ALTER TABLE testreport ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; | |
| ALTER TABLE testreport_history ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; | |
| ALTER TABLE testreport ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; | |
| ALTER TABLE testreport_history ADD COLUMN IF NOT EXISTS fourfoldincreaseantibodytiter boolean DEFAULT false; | |
| UPDATE testreport SET fourfoldincreaseantibodytiter = false WHERE fourfoldincreaseantibodytiter IS NULL; | |
| UPDATE testreport_history SET fourfoldincreaseantibodytiter = false WHERE fourfoldincreaseantibodytiter IS NULL; | |
| ALTER TABLE testreport ALTER COLUMN fourfoldincreaseantibodytiter SET NOT NULL; | |
| ALTER TABLE testreport_history ALTER COLUMN fourfoldincreaseantibodytiter SET NOT NULL; |
🤖 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-backend/src/main/resources/sql/sormas_schema.sql` around lines 16412 -
16413, The ALTER TABLE statements for testreport and testreport_history are
adding fourfoldincreaseantibodytiter as nullable boolean columns with DEFAULT
false, but the Java entity uses a primitive boolean type which cannot be null,
creating a schema-Java mismatch. To fix this, follow the established pattern in
the schema: first add an UPDATE statement to set any existing NULL values to
false in both the testreport and testreport_history tables, then modify both
ALTER TABLE statements to include the NOT NULL constraint on the
fourfoldincreaseantibodytiter columns to enforce the non-nullable contract
expected by the Java primitive boolean field.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-backend/src/test/java/de/symeda/sormas/backend/user/UserFacadeEjbTest.java`:
- Around line 83-84: Replace the weak assertions using
assertFalse(result.isEmpty()) at lines 83, 103, 124, and 425 with stronger
assertions that verify specific user inclusion and exclusion behavior. Instead
of only checking that the result is not empty, add assertions that verify the
expected users are present in the results and unwanted users are absent.
Additionally, assert the result size where the expected outcome is deterministic
to prevent false positives when incorrect users are returned by the filter.
- Around line 91-92: The assertion in the test method that checks result.get(1)
is order-dependent and will fail if the query results return in a different
order. Replace the index-based assertion that uses get(1) to check
natUser.toReference() with a containment assertion that verifies the
natUser.toReference() is a member of the result collection, regardless of its
position. Use an appropriate matcher like hasItem or contains to check for
membership rather than specific index positions.
🪄 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: 12527b86-2914-487b-b7f5-b0b63a6ea6db
📒 Files selected for processing (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/user/UserFacadeEjbTest.java
Comment on lines
+83
to
84
| assertFalse(result.isEmpty()); | ||
|
|
There was a problem hiding this comment.
These assertions are too weak for filter-behavior tests
Using only assertFalse(result.isEmpty()) (Lines 83, 103, 124, 425) can pass even when wrong users are returned. Since these tests validate filtering contracts, assert specific inclusion/exclusion (and size where deterministic) to prevent false positives.
Also applies to: 103-104, 124-125, 425-426
🤖 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-backend/src/test/java/de/symeda/sormas/backend/user/UserFacadeEjbTest.java`
around lines 83 - 84, Replace the weak assertions using
assertFalse(result.isEmpty()) at lines 83, 103, 124, and 425 with stronger
assertions that verify specific user inclusion and exclusion behavior. Instead
of only checking that the result is not empty, add assertions that verify the
expected users are present in the results and unwanted users are absent.
Additionally, assert the result size where the expected outcome is deterministic
to prevent false positives when incorrect users are returned by the filter.
Comment on lines
+91
to
92
| assertThat(result.get(1), equalTo(natUser.toReference())); | ||
|
|
There was a problem hiding this comment.
Avoid index-based assertion on unordered results
Line 91 asserts result.get(1) equals natUser, which makes this test order-sensitive and flaky if query ordering changes. Assert membership instead of position.
Suggested fix
- assertThat(result.get(1), equalTo(natUser.toReference()));
+ assertThat(result, hasItem(equalTo(natUser.toReference())));📝 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
| assertThat(result.get(1), equalTo(natUser.toReference())); | |
| assertThat(result, hasItem(equalTo(natUser.toReference()))); |
🤖 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-backend/src/test/java/de/symeda/sormas/backend/user/UserFacadeEjbTest.java`
around lines 91 - 92, The assertion in the test method that checks result.get(1)
is order-dependent and will fail if the query results return in a different
order. Replace the index-based assertion that uses get(1) to check
natUser.toReference() with a containment assertion that verifies the
natUser.toReference() is a member of the result collection, regardless of its
position. Use an appropriate matcher like hasItem or contains to check for
membership rather than specific index positions.
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.
Fixes #
Summary by CodeRabbit