FIX: Difficulty to process some messages#14154
Draft
Pa-Touche wants to merge 14 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds survey response reprocessing and retry handling, updates backend processing and persistence behavior, refreshes localized status text, refactors DTO save ordering, and adds symptom field visibility and form entries. ChangesSurvey reprocessing and processing updates
DTO save ordering refactor
Symptoms form fields
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.java (1)
172-187: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate "not yet processed" notification on the failure path.
When
reAttemptSurveyProcessingthrows, thecatchat Line 180 shows the notification but does not return;resultremainsnull, so Lines 184–185 immediately show the same notification a second time. Return (or restructure) after handling the exception.🩹 Proposed fix
} catch (RuntimeException e) { Notification.show(I18nProperties.getString(Strings.messageSurveyResponseNotYetProcessed), Notification.Type.HUMANIZED_MESSAGE); + return; } } if (result == null) { Notification.show(I18nProperties.getString(Strings.messageSurveyResponseNotYetProcessed), Notification.Type.HUMANIZED_MESSAGE); return; }🤖 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/externalmessage/ExternalMessageController.java` around lines 172 - 187, The failure path in ExternalMessageController’s survey response handling shows the same “not yet processed” notification twice because the RuntimeException catch after reAttemptSurveyProcessing does not stop execution. Update the try/catch around reAttemptSurveyProcessing so that the catch block returns immediately or otherwise prevents falling through, and keep the existing null-check notification as the only fallback when result is still null.sormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java (1)
312-333: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSave after leaf attachment so nested DTOs are persisted. Roots are saved before
attachLeaf()mutates them, and there’s no second save pass afterward. That drops in-memory additions like exposures/vaccinations and the auto-createdImmunizationDtofrom persistence; the existing tests only assert against the same mutated object reference.🤖 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/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java` around lines 312 - 333, The save flow in BusinessDtoFacade.save currently persists root DTOs before attachLeaf() mutates them, so nested DTOs added during leaf attachment are never saved. Update the ordering so the mutated DTOs are persisted after leaf attachment, using the existing saveDirectEntity path or a second save pass over the affected roots/leaves, and make sure the DTOs produced by attachLeaf() such as exposures, vaccinations, and ImmunizationDto are included in persistence.
🧹 Nitpick comments (1)
sormas-api/src/main/resources/strings_it-CH.properties (1)
2098-2098: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUntranslated string left in English.
The updated
messageSurveyResponseNotYetProcessedtext remains in English rather than Italian, unlike most other entries in this locale file.🤖 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/resources/strings_it-CH.properties` at line 2098, The `messageSurveyResponseNotYetProcessed` entry in the Italian locale file is still in English and needs to be translated to Italian. Update the value for `messageSurveyResponseNotYetProcessed` in `strings_it-CH.properties` so it matches the surrounding localized strings and keep the same key unchanged.
🤖 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/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjb.java`:
- Around line 1057-1060: The overwriteSurveyResponse method in
ExternalMessageFacadeEjb should fail fast when survey-response data is missing
instead of dereferencing it unconditionally. Before calling
externalMessage.getSurveyResponseData().getLatest().getRequest(), add a null
check on getSurveyResponseData() (and any required latest data) and throw a
clear exception for non-SURVEY_RESPONSE messages. Keep the fix localized to
overwriteSurveyResponse so the method only proceeds when the survey-response
payload is present.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/survey/AutomaticSurveyResponseProcessor.java`:
- Around line 77-89: The token grouping in AutomaticSurveyResponseProcessor
currently assumes every ExternalMessageSurveyResponseRequest has a non-null
external survey ID, but request.getExternalSurveyId() can be null and will break
Collectors.groupingBy. Update the stream pipeline that builds
tokensByExternalIds to filter out null external survey IDs before grouping,
keeping the rest of the flow in AutomaticSurveyResponseProcessor unchanged and
ensuring the later surveyFacade.getByExternalIds and
tokenBySurveyReferenceTuples logic only operates on valid IDs.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/surveyresponse/SurveyResponseDetailsWindow.java`:
- Around line 179-185: The retry flow in SurveyResponseDetailsWindow keeps
evaluating the original externalMessage after reAttemptSurveyProcessing(), so
the UI can still show the unprocessed/failure state even when the retry
succeeds. Update the logic in the result/status rendering path to reuse the
retried ExternalMessageDto returned by the facade before checking
externalMessage.getStatus() and rendering the general info/status label, so the
post-retry state is shown consistently.
---
Outside diff comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java`:
- Around line 312-333: The save flow in BusinessDtoFacade.save currently
persists root DTOs before attachLeaf() mutates them, so nested DTOs added during
leaf attachment are never saved. Update the ordering so the mutated DTOs are
persisted after leaf attachment, using the existing saveDirectEntity path or a
second save pass over the affected roots/leaves, and make sure the DTOs produced
by attachLeaf() such as exposures, vaccinations, and ImmunizationDto are
included in persistence.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.java`:
- Around line 172-187: The failure path in ExternalMessageController’s survey
response handling shows the same “not yet processed” notification twice because
the RuntimeException catch after reAttemptSurveyProcessing does not stop
execution. Update the try/catch around reAttemptSurveyProcessing so that the
catch block returns immediately or otherwise prevents falling through, and keep
the existing null-check notification as the only fallback when result is still
null.
---
Nitpick comments:
In `@sormas-api/src/main/resources/strings_it-CH.properties`:
- Line 2098: The `messageSurveyResponseNotYetProcessed` entry in the Italian
locale file is still in English and needs to be translated to Italian. Update
the value for `messageSurveyResponseNotYetProcessed` in
`strings_it-CH.properties` so it matches the surrounding localized strings and
keep the same key unchanged.
🪄 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: fac926c9-e896-4d06-a0af-982583d4e78b
📒 Files selected for processing (39)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/ExternalMessageFacade.javasormas-api/src/main/resources/strings.propertiessormas-api/src/main/resources/strings_ar-SA.propertiessormas-api/src/main/resources/strings_cs-CZ.propertiessormas-api/src/main/resources/strings_de-CH.propertiessormas-api/src/main/resources/strings_de-DE.propertiessormas-api/src/main/resources/strings_en-AF.propertiessormas-api/src/main/resources/strings_en-GH.propertiessormas-api/src/main/resources/strings_en-GM.propertiessormas-api/src/main/resources/strings_en-KE.propertiessormas-api/src/main/resources/strings_en-LR.propertiessormas-api/src/main/resources/strings_en-NG.propertiessormas-api/src/main/resources/strings_es-BO.propertiessormas-api/src/main/resources/strings_fa-AF.propertiessormas-api/src/main/resources/strings_fi-FI.propertiessormas-api/src/main/resources/strings_fr-CH.propertiessormas-api/src/main/resources/strings_fr-FR.propertiessormas-api/src/main/resources/strings_fr-TN.propertiessormas-api/src/main/resources/strings_it-CH.propertiessormas-api/src/main/resources/strings_it-IT.propertiessormas-api/src/main/resources/strings_ka-GE.propertiessormas-api/src/main/resources/strings_pl-PL.propertiessormas-api/src/main/resources/strings_ps-AF.propertiessormas-api/src/main/resources/strings_pt-BR.propertiessormas-api/src/main/resources/strings_pt-CV.propertiessormas-api/src/main/resources/strings_ru-RU.propertiessormas-api/src/main/resources/strings_sr-CS.propertiessormas-api/src/main/resources/strings_sw-TZ.propertiessormas-api/src/main/resources/strings_ur-PK.propertiessormas-api/src/main/resources/strings_zh-CN.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/survey/AutomaticSurveyResponseProcessor.javasormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.javasormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.javasormas-backend/src/main/java/de/symeda/sormas/backend/patch/EntityDtoTypeComparator.javasormas-backend/src/test/java/de/symeda/sormas/backend/ArchitectureTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageGrid.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/surveyresponse/SurveyResponseDetailsWindow.java
TODO: add missing translations
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/symptoms/SymptomsForm.java`:
- Around line 529-534: Add the six new GI symptom fields to
unconditionalSymptomFieldIds in SymptomsForm so they participate in the same
Clear All, Set Cleared to No/Unknown, onset listener, and soft-required handling
as the neighboring symptom fields. Update the SymptomsForm field list to include
the new entries alongside the existing gastrointestinal symptom ids, keeping the
identifiers consistent with the ones added in addFields(...).
🪄 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: 7eaa95dc-1b7c-4096-ba4a-09387c14bdc4
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
sormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java (1)
315-338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the large commented-out code block.
Lines 315–338 contain the entire previous implementation as a commented-out block. This clutters the method and will drift from reality as the active code evolves. If the alternative approach needs to be documented, replace it with a concise comment referencing the rationale or a design doc.
♻️ Proposed cleanup
public void save(`@NotNull` List<Tuple<Integer, EntityDto>> entityDtosByKey) { - // Be careful about "leaf-entities" that can change the case data: - // Person ; Immunization and VaccinationDto - /* - List<Tuple<Integer, EntityDto>> dtosInProgress = new ArrayList<>(entityDtosByKey); - - //entityDtosByKey.stream() .filter(dto -> dto.) - - // ordering is important: root entities must be saved first to avoid OutdatedEntityException as leaf entities seem to update the case. - List<Tuple<Integer, EntityDto>> orderedRootEntities = entityDtosByKey.stream() - .filter(tuple -> leafAttacherDictionary.keySet().stream().noneMatch(leafClass -> leafClass.isInstance(tuple.getSecond()))) - .sorted(Comparator.comparing(Tuple::getSecond, new EntityDtoTypeComparator())) - .collect(Collectors.toList()); - - orderedRootEntities.stream().map(Tuple::getSecond).forEach(this::saveDirectEntity); - - // once it's done, leaf entities can be saved, as updating root entities will not break - leafAttacherDictionary.forEach((leafClass, attacher) -> { - List<Tuple<Integer, EntityDto>> leaves = - dtosInProgress.stream().filter(t -> leafClass.isInstance(t.getSecond())).collect(Collectors.toList()); - - leaves.forEach(leafTuple -> { - dtosInProgress.remove(leafTuple); - attacher.attachLeaf(leafTuple.getSecond(), leafTuple.getFirst(), dtosInProgress); - }); - }); - */ List<Tuple<Integer, EntityDto>> dtosInProgress = new ArrayList<>(entityDtosByKey);🤖 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/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java` around lines 315 - 338, Remove the large commented-out implementation block from BusinessDtoFacade and leave only a brief explanatory comment if needed; the current commented code around dtosInProgress, orderedRootEntities, and leafAttacherDictionary should be deleted to keep the method focused and prevent stale logic from lingering. If the alternate approach still matters, replace it with a short rationale comment or reference to the relevant design note instead of preserving the full old implementation.sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java (2)
1022-1023: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the "outdated range" flakiness comment.
The comment "Might require changing outdated range to reproduce every time" suggests the test may be sensitive to entity version timestamps or timing. If this is a real concern, consider documenting the specific condition or stabilizing the test (e.g., by explicitly setting
changeDate/uuidon the original case). If it's no longer relevant after the save-ordering change, remove the comment to avoid confusion.🤖 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/patch/DataPatcherImplTest.java` around lines 1022 - 1023, The comment in DataPatcherImplTest is too vague about the “outdated range” flakiness and should either be clarified or removed. Update the test around the affected case-update scenario in DataPatcherImplTest to describe the exact condition that makes reproduction timing-sensitive, or stabilize the setup by explicitly controlling the relevant case fields such as changeDate and uuid in the test data. If the save-ordering change has already eliminated the issue, delete the outdated comment entirely so the intent of the test is clear.
1020-1129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd missing assertions for patched fields.
The test patches
CaseData.followUpStatus(FOLLOW_UP),Immunization.validFrom("2020-01-01"),Immunization.validUntil("2035-01-01"), andVaccination.vaccinationDate("2024-05-01") but does not assert any of these values in the CHECK section. ThevalidFrom/validUntilassertions are especially important — they determine whether the immunization is "valid" at the case report date, which drives theupdateDeterminedVaccinationStatusesrecompute. Without asserting them, a silent patch failure on those fields could still produce a green test.Additionally, the
VaccinationStatus.UNVACCINATEDassertion at line 1121 doesn't prove the recompute side-effect occurred ifcreateUnclassifiedCasealready initializesvaccinationStatustoUNVACCINATED. Consider setting it to a different value before the patch to make the assertion meaningful.♻️ Proposed additional assertions
// CaseData () -> Assertions.assertEquals("some comment", actualCase.getQuarantineChangeComment()), + () -> Assertions.assertEquals(FollowUpStatus.FOLLOW_UP, actualCase.getFollowUpStatus()), // Exposures// Immunization / Vaccination () -> Assertions.assertEquals(1, immunizations.size()), () -> Assertions.assertEquals(ImmunizationStatus.ACQUIRED, immunizations.get(0).getImmunizationStatus()), () -> Assertions.assertEquals(MeansOfImmunization.VACCINATION, immunizations.get(0).getMeansOfImmunization()), + () -> Assertions.assertEquals( + Date.from(LocalDate.parse("2020-01-01").atStartOfDay(ZoneId.systemDefault()).toInstant()), + immunizations.get(0).getValidFrom()), + () -> Assertions.assertEquals( + Date.from(LocalDate.parse("2035-01-01").atStartOfDay(ZoneId.systemDefault()).toInstant()), + immunizations.get(0).getValidUntil()), () -> Assertions.assertEquals(1, immunizations.get(0).getVaccinations().size()), () -> Assertions.assertEquals(Vaccine.COMIRNATY, immunizations.get(0).getVaccinations().get(0).getVaccineName()), () -> Assertions.assertEquals("1", immunizations.get(0).getVaccinations().get(0).getVaccineDose()), + () -> Assertions.assertEquals( + Date.from(LocalDate.parse("2024-05-01").atStartOfDay(ZoneId.systemDefault()).toInstant()), + immunizations.get(0).getVaccinations().get(0).getVaccinationDate()),🤖 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/patch/DataPatcherImplTest.java` around lines 1020 - 1129, The test method patch_multipleEntities_caseDataExposureHospitalizationSymptoms is missing checks for several patched values, so add assertions in the CHECK section for CaseDataDto.getFollowUpStatus, ImmunizationDto.getValidFrom, ImmunizationDto.getValidUntil, and VaccinationDto.getVaccinationDate. Also make the VaccinationStatus assertion in the same test meaningful by initializing the case to a different vaccination status before calling victim().patch, so the later assertion on actualCase.getVaccinationStatus() proves CaseService#updateDeterminedVaccinationStatuses was triggered.
🤖 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/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java`:
- Around line 339-353: The root-entity save order in BusinessDtoFacade is no
longer deterministic because dtosInProgress now originates from a HashMap in
DataPatcherImpl.saveDTOsIfAppropriate(). Restore the
EntityDtoTypeComparator-based sorting before the final save loop in
BusinessDtoFacade so direct entities are persisted in a stable order, and keep
the leaf attachment flow via leafAttacherDictionary.attachLeaf unchanged. This
ensures saveDirectEntity is called in the intended sequence and prevents stale
CaseDataDto state that can lead to OutdatedEntityException.
---
Nitpick comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.java`:
- Around line 315-338: Remove the large commented-out implementation block from
BusinessDtoFacade and leave only a brief explanatory comment if needed; the
current commented code around dtosInProgress, orderedRootEntities, and
leafAttacherDictionary should be deleted to keep the method focused and prevent
stale logic from lingering. If the alternate approach still matters, replace it
with a short rationale comment or reference to the relevant design note instead
of preserving the full old implementation.
In
`@sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java`:
- Around line 1022-1023: The comment in DataPatcherImplTest is too vague about
the “outdated range” flakiness and should either be clarified or removed. Update
the test around the affected case-update scenario in DataPatcherImplTest to
describe the exact condition that makes reproduction timing-sensitive, or
stabilize the setup by explicitly controlling the relevant case fields such as
changeDate and uuid in the test data. If the save-ordering change has already
eliminated the issue, delete the outdated comment entirely so the intent of the
test is clear.
- Around line 1020-1129: The test method
patch_multipleEntities_caseDataExposureHospitalizationSymptoms is missing checks
for several patched values, so add assertions in the CHECK section for
CaseDataDto.getFollowUpStatus, ImmunizationDto.getValidFrom,
ImmunizationDto.getValidUntil, and VaccinationDto.getVaccinationDate. Also make
the VaccinationStatus assertion in the same test meaningful by initializing the
case to a different vaccination status before calling victim().patch, so the
later assertion on actualCase.getVaccinationStatus() proves
CaseService#updateDeterminedVaccinationStatuses was triggered.
🪄 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: 2212c15b-6eef-4077-93ad-37b299ae2d49
📒 Files selected for processing (2)
sormas-backend/src/main/java/de/symeda/sormas/backend/patch/BusinessDtoFacade.javasormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java
There was a problem hiding this comment.
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)
5391-5397: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMissing caption for
smellyBurps.The
getSmellyBurps()/setSmellyBurps()accessors are added here, butcaptions.propertiesonly adds entries forlossOfAppetite,flatulence,coughingAttacks,coughingAtNight, andabdominalCramps— noSymptoms.smellyBurpscaption exists. This field will render without a proper label in the UI.Add
Symptoms.smellyBurps=Smelly burps(or appropriate wording) tosormas-api/src/main/resources/captions.properties.🤖 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 5391 - 5397, The new smellyBurps accessors in SymptomsDto are missing the matching UI caption, so add a Symptoms.smellyBurps entry to captions.properties with the appropriate label text. Make sure the new caption follows the existing symptom caption naming pattern alongside the other Symptoms.* entries so the field renders with a proper label.
🧹 Nitpick comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java (1)
2962-2999: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew Luxembourg-only symptom fields lack
@SymptomGrouping.
lossOfAppetite,flatulence,smellyBurps,coughingAttacks,coughingAtNight, andabdominalCrampshave no@SymptomGroupingannotation, unlike sibling fields added in the same block (eggyBurps,weightLoss,bloating) which use@SymptomGrouping(SymptomGroup.GASTROINTESTINAL/GENERAL). Without it, these fields won't be grouped correctly in the symptoms form UI.♻️ Suggested fix
`@Diseases`({ CRYPTOSPORIDIOSIS, GIARDIASIS, RESPIRATORY_SYNCYTIAL_VIRUS }) + `@SymptomGrouping`(SymptomGroup.GASTROINTESTINAL) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState lossOfAppetite; `@Diseases`({ GIARDIASIS }) + `@SymptomGrouping`(SymptomGroup.GASTROINTESTINAL) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState flatulence; `@Diseases`({ GIARDIASIS }) + `@SymptomGrouping`(SymptomGroup.GASTROINTESTINAL) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState smellyBurps; `@Diseases`({ PERTUSSIS }) + `@SymptomGrouping`(SymptomGroup.RESPIRATORY) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState coughingAttacks; `@Diseases`({ PERTUSSIS }) + `@SymptomGrouping`(SymptomGroup.RESPIRATORY) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState coughingAtNight; `@Diseases`({ CRYPTOSPORIDIOSIS, GIARDIASIS }) + `@SymptomGrouping`(SymptomGroup.GASTROINTESTINAL) `@HideForCountriesExcept`(countries = { CountryHelper.COUNTRY_CODE_LUXEMBOURG }) private SymptomState abdominalCramps;🤖 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 2962 - 2999, The Luxembourg-only symptom fields in SymptomsDto are missing `@SymptomGrouping`, so they won’t appear in the same UI groups as the neighboring symptoms. Add the appropriate `@SymptomGrouping` annotations to lossOfAppetite, flatulence, smellyBurps, coughingAttacks, coughingAtNight, and abdominalCramps, matching the existing pattern used by eggyBurps, weightLoss, and bloating in the same block. Use the existing SymptomGroup constants to place each field into the correct gastrointestinal or general group.
🤖 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.
Outside diff comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java`:
- Around line 5391-5397: The new smellyBurps accessors in SymptomsDto are
missing the matching UI caption, so add a Symptoms.smellyBurps entry to
captions.properties with the appropriate label text. Make sure the new caption
follows the existing symptom caption naming pattern alongside the other
Symptoms.* entries so the field renders with a proper label.
---
Nitpick comments:
In `@sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java`:
- Around line 2962-2999: The Luxembourg-only symptom fields in SymptomsDto are
missing `@SymptomGrouping`, so they won’t appear in the same UI groups as the
neighboring symptoms. Add the appropriate `@SymptomGrouping` annotations to
lossOfAppetite, flatulence, smellyBurps, coughingAttacks, coughingAtNight, and
abdominalCramps, matching the existing pattern used by eggyBurps, weightLoss,
and bloating in the same block. Use the existing SymptomGroup constants to place
each field into the correct gastrointestinal or general group.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cad66452-f83c-497c-9547-9589aaccbb02
📒 Files selected for processing (4)
sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.javasormas-api/src/main/resources/captions.propertiessormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java
💤 Files with no reviewable changes (1)
- sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java
🚧 Files skipped from review as they are similar to previous changes (1)
- sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java
…RMAS-Project into fix/14136-processing-issues
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
1 participant
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 #14136
Summary by CodeRabbit