Bug fixes for samples, tests & SAL #13993
Merged
Merged
Conversation
…ts in the PathogenTestForm components
… Direct Microscopy
…lance interpretation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java (1)
808-812:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
eyeIrritationis now unreachable due to empty disease mapping.
@Diseases({})makes this field fail disease matching, so it won’t be available for any disease flow despite being wired in the form. This looks like an accidental annotation regression.Suggested fix
- `@Diseases`({}) + `@Diseases`({ + SALMONELLOSIS })🤖 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-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java` around lines 808 - 812, The eyeIrritation field in SymptomsDto has an empty `@Diseases`({}) annotation that prevents it from being available for any disease, making it unreachable despite being configured in the form. Remove the empty `@Diseases`({}) annotation from the eyeIrritation field definition, or if specific diseases should support this symptom, populate the annotation with the appropriate disease codes. Since this is noted as an accidental regression, removing the empty annotation is the most likely correct approach.
🧹 Nitpick comments (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsView.java (1)
309-318: 💤 Low valueConsider consistent decimal formatting for Float values.
Using
%swithFloatdelegates toFloat.toString(), which can produce varying decimal precision (e.g.,1.5vs1.5000001depending on floating-point representation). For cleaner display in a medical context, consider using%g(removes trailing zeros) or a fixed precision like%.2f.♻️ Suggested formatting improvement
private static String formatAstValue(Float mic, Float zoneDiameter) { boolean hasMic = mic != null; boolean hasZone = zoneDiameter != null; if (!hasMic && !hasZone) { return null; } if (hasMic && hasZone) { - return String.format("MIC: %s mg/l; Zone: %s mm", mic, zoneDiameter); + return String.format("MIC: %g mg/l; Zone: %g mm", mic, zoneDiameter); } - return hasMic ? String.format("%s mg/l", mic) : String.format("%s mm", zoneDiameter); + return hasMic ? String.format("%g mg/l", mic) : String.format("%g mm", zoneDiameter); }🤖 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/caze/CaseLabResultsView.java` around lines 309 - 318, The formatAstValue method uses %s format specifier for Float values (mic and zoneDiameter), which delegates to Float.toString() and can produce inconsistent decimal precision. Replace all %s format specifiers with %g to remove trailing zeros or use %.2f for fixed two-decimal precision across all three String.format calls in the method (the combined MIC and Zone format, the mic-only format, and the zoneDiameter-only format) to ensure consistent and clean decimal formatting suitable for medical display.sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
316-335: 💤 Low valueConsider ordering exposures by date before selecting the country.
When multiple TRAVEL exposures exist with different countries,
findFirst()returns whichever exposure comes first in the collection's iteration order, which may not be meaningful. For a more predictable user experience, consider sorting exposures by start date (most recent or earliest) before selecting the country:exposures.stream() + .sorted(Comparator.comparing(ExposureDto::getStartDate, Comparator.nullsLast(Comparator.reverseOrder()))) .filter(ex -> ex.getExposureType() == ExposureType.TRAVEL && ex.getLocation() != null && ex.getLocation().getCountry() != null) .map(ex -> ex.getLocation().getCountry()) .findFirst() .ifPresent(country::setValue);This would consistently prefer the most recent travel exposure's country, which is likely more relevant for infection source determination.
🤖 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/epidata/EpiDataForm.java` around lines 316 - 335, The code in the Disease.SALMONELLOSIS exposuresField value change listener uses findFirst() on an unordered stream of TRAVEL exposures, which returns an unpredictable result when multiple exposures exist. Add a sorted() operation to the stream chain before the filter() method that orders exposures by their start date in descending order (most recent first) so that the most relevant travel exposure's country is consistently selected for the probable country of infection field.sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java (1)
68-75: ⚡ Quick winAlign the ELISA Ig-class comment with the actual annotations.
The block comment says these variants have “No
@Diseases” and are visible for every disease, but Lines 74/82/90 add@Diseases(..., hide = true). Please make the comment and behavior description consistent.✏️ Suggested comment-only adjustment
- // workflows already need. Result is qualitative (Pos/Neg) + numeric (titre). No `@Diseases` — visible - // for every disease per `#13951`. The legacy ENZYME_LINKED_IMMUNOSORBENT_ASSAY is now + // workflows already need. Result is qualitative (Pos/Neg) + numeric (titre). Visibility is controlled + // by the `@Diseases` annotations below. The legacy ENZYME_LINKED_IMMUNOSORBENT_ASSAY is nowAlso applies to: 82-83, 90-91
🤖 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-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java` around lines 68 - 75, The block comment describing the ELISA Ig-class variants states "No `@Diseases` — visible for every disease per `#13951`" but the actual code includes `@Diseases` annotations with hide=true on the three affected enum entries. Update the comment above the ELISA Ig-class variants (starting with "Resurrected ELISA Ig-class variants") to accurately reflect that these entries DO have `@Diseases` annotations applied with hide=true, rather than claiming they have no `@Diseases`. This inconsistency also affects the similar comment blocks for the other two related enum entries, so ensure all three are updated consistently.
🤖 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-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java`:
- Around line 1476-1477: The onset-related fields in the SymptomsDto class have
an inconsistency where SALMONELLOSIS is included in the onsetDate annotation but
excluded from the onsetSymptom annotation. To fix this, locate the onsetSymptom
field definition and add SALMONELLOSIS to its list of diseases, ensuring it
matches the same disease list present in the onsetDate field annotation so that
both onset fields have consistent coverage for the same diseases.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/samples/components/CtCqValueComponent.java`:
- Around line 103-110: In the updateCqVisibility() method, the visibility
condition for cqValueField currently only checks if the test result is positive
and if cqInputApplies() returns true for the disease and test type, but it does
not include the regional check that was applied during initial layout building
in buildLayout(). Modify the boolean show assignment to additionally include the
isLuxembourg field check alongside the existing positive and cqInputApplies
conditions, ensuring the field respects the regional constraint even when
visibility is updated based on disease or test type changes.
---
Outside diff comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java`:
- Around line 808-812: The eyeIrritation field in SymptomsDto has an empty
`@Diseases`({}) annotation that prevents it from being available for any disease,
making it unreachable despite being configured in the form. Remove the empty
`@Diseases`({}) annotation from the eyeIrritation field definition, or if specific
diseases should support this symptom, populate the annotation with the
appropriate disease codes. Since this is noted as an accidental regression,
removing the empty annotation is the most likely correct approach.
---
Nitpick comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.java`:
- Around line 68-75: The block comment describing the ELISA Ig-class variants
states "No `@Diseases` — visible for every disease per `#13951`" but the actual code
includes `@Diseases` annotations with hide=true on the three affected enum
entries. Update the comment above the ELISA Ig-class variants (starting with
"Resurrected ELISA Ig-class variants") to accurately reflect that these entries
DO have `@Diseases` annotations applied with hide=true, rather than claiming they
have no `@Diseases`. This inconsistency also affects the similar comment blocks
for the other two related enum entries, so ensure all three are updated
consistently.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsView.java`:
- Around line 309-318: The formatAstValue method uses %s format specifier for
Float values (mic and zoneDiameter), which delegates to Float.toString() and can
produce inconsistent decimal precision. Replace all %s format specifiers with %g
to remove trailing zeros or use %.2f for fixed two-decimal precision across all
three String.format calls in the method (the combined MIC and Zone format, the
mic-only format, and the zoneDiameter-only format) to ensure consistent and
clean decimal formatting suitable for medical display.
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java`:
- Around line 316-335: The code in the Disease.SALMONELLOSIS exposuresField
value change listener uses findFirst() on an unordered stream of TRAVEL
exposures, which returns an unpredictable result when multiple exposures exist.
Add a sorted() operation to the stream chain before the filter() method that
orders exposures by their start date in descending order (most recent first) so
that the most relevant travel exposure's country is consistently selected for
the probable country of infection 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: 9fe61756-c2e4-4272-9366-5a096b7c9b64
📒 Files selected for processing (28)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenSpecie.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.javasormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestType.javasormas-api/src/main/java/de/symeda/sormas/api/sample/ResultValueTypeRel.javasormas-api/src/main/java/de/symeda/sormas/api/sample/SampleMaterial.javasormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-api/src/test/java/de/symeda/sormas/api/sample/PathogenTestTypeTest.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTest.javasormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/symptoms/Symptoms.javasormas-backend/src/main/java/de/symeda/sormas/backend/symptoms/SymptomsFacadeEjb.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-backend/src/test/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjbTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseLabResultsView.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/DevModeView.javasormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/components/CtCqValueComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/components/FourFoldCtCqComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/components/TestResultComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/ImiSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/IpiSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/diseasesection/MalariaSectionComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntry.javasormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.javasormas-ui/src/test/java/de/symeda/sormas/ui/samples/pathogentestlink/PathogenTestListEntryTest.java
💤 Files with no reviewable changes (3)
- sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTestFacadeEjb.java
- sormas-backend/src/main/java/de/symeda/sormas/backend/sample/PathogenTest.java
- sormas-api/src/main/java/de/symeda/sormas/api/sample/PathogenTestDto.java
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
Release Notes
New Features
Improvements