Feature 13294 doctor declarations xml esante adapter #13575
Conversation
- Added missing fields for doctor declaration hospitalization data. `ExternalMessageDto` `ExternalMessage` `ExternalMessageFacadeEjbMappingTest` - Added `caseComments` field to support comments from the doctor declaration. `ExternalMessageDto` `ExternalMessage` `ExternalMessageFacadeEjbMappingTest` - Corrected `treatmentStarted` to YesNoUnknown type. `ExternalMessageDto` `ExternalMessage` `ExternalMessageFacadeEjbMappingTest` - Added a generic infrastructure check method `AbstractProcessingFlow` - Added methods for retrieving hospitals by UUID and name. `ExternalMessageProcessingFacade` `FacilityFacade` `FacilityFacadeEjb` `FacilityService` `ExternalMessageFacadeEjb` - Added support for doctor declaration hospitalization data. `AbstractDoctorDeclarationMessageProcessingFlow` `DoctorDeclarationMessageProcessingFlow` - Added `healthFacilityDepartment` field to support department information. `PreviousHospitalizationDto` `PreviousHospitalization` `Captions` `captions.properties` `HospitalizationFacadeEjbPseudonymizationTest` `HospitalizationFacadeEjb` `PreviousHospitalizationEditForm` - Added additional fields for hospitalization data. `sormas_schema.sql`
…the message adapter.
- Introduced a new form for editing notifications - Updated side view components to support notification editing - Improved treatment options handling
- Fixed the button label for creating a new notification to use the correct caption. - Added a badge to indicate the type of notification (phone or external).
…be processed in the same flow. Moved the sample and test processing logic into the common base class `AbstractMessageProcessingFlowBase`. Updated `DoctorDeclarationMessageProcessingFlow` and `LabMessageProcessingFlow` to make use of the shared logic. Added test type details field to `TestReportDto` to accommodate additional information in case of Other test types.
- Added additional fields to `ExternalMessageDto` and `ExternalMessage` - Updated `AbstractDoctorDeclarationMessageProcessingFlow` to handle additional fields - Updated `AbstractDoctorDeclarationMessageProcessingFlow` to handle health conditions - Fixed `PathogenTestForm` to display updated values for test type and test results plus text
WalkthroughExtends external message, hospitalization, TB/HIV, therapy, notifier, symptoms, and lab-report models and UI; adds facility lookup and infra-data check hook; refactors processing orchestration for sampling and doctor-declaration post-build; updates mappings, DB schema/histories, i18n keys, and notifier UI flows. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI
participant Flow as AbstractProcessingFlow
participant Impl as Flow subclass
UI->>Flow: startProcessing(message)
Flow->>Flow: checkDisease()
Flow->>Impl: handleInfraDataChecks()
alt infra checks pass
Impl-->>Flow: completion(true)
Flow->>Flow: continue processing
else infra checks fail
Impl-->>Flow: completion(false)
Flow-->>UI: cancel processing (default result)
end
sequenceDiagram
participant Processor as AbstractMessageProcessingFlowBase
participant Handler as concrete doXFlow
participant UI as ExternalMessageProcessingUIHelper
participant Lab as Lab/Sample services
Processor->>Handler: doCreateEventParticipantFlow(...)
Handler->>UI: show/create participant
Handler->>Processor: doCreateEventParticipantAndSamplesFlow(...)
Processor->>Lab: doPickOrCreateSamplesFlow(...)
alt multiple sample reports
Lab->>UI: handleMultipleSampleConfirmation()
UI-->>Lab: user confirms/cancels
end
Lab->>Processor: create/pick samples -> createOneSampleAndPathogenTests(...)
Processor->>Processor: continue processing
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🔭 Outside diff range comments (9)
sormas-api/src/main/java/de/symeda/sormas/api/hospitalization/PreviousHospitalizationDto.java (1)
39-39: Enum constant typo likely breaks compilation: CASE_SURVEILANCE → CASE_SURVEILLANCEThe FeatureType name appears misspelled. If the enum is FeatureType.CASE_SURVEILLANCE (double “l”), this will not compile or will silently disable the feature gating.
-@DependingOnFeatureType(featureType = FeatureType.CASE_SURVEILANCE) +@DependingOnFeatureType(featureType = FeatureType.CASE_SURVEILLANCE)sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/NotifierService.java (1)
103-108: Avoid NoResultException by using getResultListgetSingleResult throws NoResultException if no row is found. The method contract allows returning null; switch to getResultList to avoid exception-driven flow.
Apply this diff:
- Query query = em.createNativeQuery(sql, Notifier.class); - query.setParameter("uuid", uuid); - query.setParameter("changeDate", timestamp); - - return (Notifier) query.getSingleResult(); + Query query = em.createNativeQuery(sql, Notifier.class); + query.setParameter("uuid", uuid); + query.setParameter("changeDate", timestamp); + + @SuppressWarnings("unchecked") + java.util.List<Notifier> results = query.getResultList(); + return results.isEmpty() ? null : results.get(0);Note: You’ll need to ensure java.util.List is imported or fully qualify as in the snippet.
sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java (1)
730-736: Exclude UNKNOWN_SYMPTOM from symptomatic triggersThe list
unconditionalSymptomFieldIdsdrives:
- soft‐required styling (
addSoftRequiredStyleWhenSymptomaticAndCooperative)- onset‐date/onset‐symptom enabling
- the “clear all” and bulk‐set‐cleared actions
Treating
UNKNOWN_SYMPTOMas a symptom‐presence trigger causes onset requirements and styling to fire when “Unknown symptom” is marked. It should not. Please remove it:File: sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java
Around line 594:unconditonalSymptomFieldIds = Arrays.asList( …, - UNKNOWN_SYMPTOM, OTHER_CLINICAL_PRESENTATION);Follow-up: if you still want “Unknown symptom” cleared by your bulk‐clear buttons, split this into two lists—one for symptomatic triggers (without UNKNOWN_SYMPTOM) and one for clear-all/bulk-set actions (including it).
sormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjb.java (1)
161-219: Missing round-trip test fordeceasedDate
Mapping tests covercaseComments,treatmentNotApplicable, andtreatmentStartedDate, but I don’t see any assertion fordeceasedDatein ExternalMessageFacadeEjbMappingTest.• File:
sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjbMappingTest.java• Suggested fix:
Add an assertion fordeceasedDatein both DTO→entity and entity→DTO sections, for example:assertEquals(source.getDeceasedDate(), result.getDeceasedDate());sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewComponent.java (1)
47-47: Compile error: missing Sizeable.Unit import
Unitis unresolved here. Either re-importcom.vaadin.server.Sizeable.Unitor fully qualify in the method call.Apply this minimal diff to avoid adding an import:
- setWidth(100, Unit.PERCENTAGE); + setWidth(100, com.vaadin.server.Sizeable.Unit.PERCENTAGE);sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java (2)
118-136: Typo in FeatureType constant likely breaks compilation
FeatureType.CASE_SURVEILANCElooks misspelled. In SORMAS it’s typicallyCASE_SURVEILLANCE(double “l”). Please verify and fix.Apply this diff:
- .addCreateEntry(EntrySelectionField.OptionType.CREATE_CASE, FeatureType.CASE_SURVEILANCE, UserRight.CASE_CREATE, UserRight.CASE_EDIT) + .addCreateEntry(EntrySelectionField.OptionType.CREATE_CASE, FeatureType.CASE_SURVEILLANCE, UserRight.CASE_CREATE, UserRight.CASE_EDIT)
416-418: Returning null for CompletionStage can cause NPEs; return a completed futureIf callers chain on the returned stage,
nullwill break them. Return a completed future instead.Apply this diff:
- @Override - protected CompletionStage<Void> notifyCorrectionsSaved() { - return null; - } + @Override + protected CompletionStage<Void> notifyCorrectionsSaved() { + return java.util.concurrent.CompletableFuture.completedFuture(null); + }sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/TreatmentOption.java (1)
80-87: Compute captions dynamically to respect current user languageCaptions are resolved at class-load time; this locks them to whatever language was active then. Resolve captions on access so they reflect the current thread’s language.
Update getCaption/toString to compute dynamically (no diff since this block wasn’t part of the changed hunk). Example implementation:
@Override public String getCaption() { if ("NA".equals(value)) { return I18nProperties.getString(Strings.notApplicable); } if (YesNoUnknown.YES.toString().equals(value)) { return I18nProperties.getEnumCaption(YesNoUnknown.YES); } else if (YesNoUnknown.NO.toString().equals(value)) { return I18nProperties.getEnumCaption(YesNoUnknown.NO); } else if (YesNoUnknown.UNKNOWN.toString().equals(value)) { return I18nProperties.getEnumCaption(YesNoUnknown.UNKNOWN); } return value; // fallback } @Override public String toString() { return getCaption(); }Optionally, drop the
captionfield entirely to avoid stale state.sormas-api/src/main/resources/captions.properties (1)
1151-1151: Bulk action caption addedThe caption
bulkActionCreatDocuments=Create quarantine order documentshas been added. Note there's a typo in the key name.Fix typo in caption key
The key
bulkActionCreatDocumentsappears to have a typo - it should bebulkActionCreateDocuments(missing 'e' in 'Create').-bulkActionCreatDocuments=Create quarantine order documents +bulkActionCreateDocuments=Create quarantine order documents
🧹 Nitpick comments (41)
sormas-ui/src/main/java/de/symeda/sormas/ui/person/PersonCreateForm.java (1)
373-373: Death date sync added — verify consistency with present conditionCopying deathDate is fine, but please verify that presentCondition is consistently set when a death date is present (and cleared when removed). Otherwise, you can end up with a non-deceased present condition while a death date is set.
Would you like a follow-up patch that adjusts presentCondition when deathDate is provided/cleared?
sormas-api/src/main/resources/strings.properties (1)
1974-1975: Wording consistency: “Notifier” vs “Notification” and capitalization
- Keys are about “notification…”, but the value for notificationCreationNotAllowedWithoutSurveillanceReport uses “Notifier creation…”, which can be confusing. Consider aligning the noun (“notification” vs “notifier”) with the rest of the UI.
- Consider sentence case for consistency with the rest of strings (e.g., “Cannot create or edit notification”).
Proposed wording (if “notification” is the intended concept):
- notificationCannotCreate=Cannot create or edit notification
- notificationCreationNotAllowedWithoutSurveillanceReport=Notification creation or modification is not allowed when a surveillance report already exists for this case.
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java (1)
83-92: Javadoc nit: mention CompletionStage instead of CompletableFutureThe method returns a CompletionStage. Update the doc to avoid confusion.
Apply:
- * @return A CompletableFuture containing the processing result. + * @return A CompletionStage containing the processing result.sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java (1)
74-94: Clarify intent of forceSampleCreation and prefer primitive booleanUsing a boxed Boolean for forceSampleCreation introduces a third state (null) that is not needed and complicates reasoning. If null is not intentionally used, switch to a primitive boolean and simplify checks.
Example changes:
- Field: private final boolean forceSampleCreation;
- Ctor param: boolean forceSampleCreation
- Usage: if (forceSampleCreation) { ... } instead of Boolean.TRUE.equals(forceSampleCreation)
sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/NotifierService.java (2)
7-7: Fix GPL header punctuationMinor nit: The sentence reads “(at any later version.” and is missing a closing parenthesis. Consider aligning with the standard GPL header text.
98-101: SQL concatenation is correct; consider readability and avoid accidental whitespace issuesString concatenation creates a valid query. For maintainability, consider using a multiline string or StringBuilder to avoid subtle spacing bugs and improve readability, especially as more columns are projected.
Example using String.join or text block (if using Java 15+):
- Use a text block with proper aliases and line breaks.
sormas-ui/src/main/java/de/symeda/sormas/ui/hospitalization/PreviousHospitalizationEditForm.java (1)
125-126: Consider visibility/required rules for department fieldDepending on requirements, HEALTH_FACILITY_DEPARTMENT might only be relevant when a concrete health facility (not OTHER/NONE/unknown) is selected. If so, add FieldHelper visibility and required rules similar to healthFacilityDetails.
I can provide a small patch to bind visibility/required to healthFacilityCombo selection if needed.
sormas-backend/src/main/java/de/symeda/sormas/backend/therapy/TherapyFacadeEjb.java (1)
37-39: Map new therapy fields and consider enforcing invariants when not applicableThe new fields treatmentStarted, treatmentNotApplicable, and treatmentStartDate are mapped correctly in both directions.
Optional improvement: if treatmentNotApplicable is true, consider normalizing state by clearing treatmentStarted/treatmentStartDate to avoid contradictory state.
Apply this after setting the three fields in fillOrBuildEntity:
entity.setTreatmentStarted(source.getTreatmentStarted()); entity.setTreatmentNotApplicable(source.isTreatmentNotApplicable()); entity.setTreatmentStartDate(source.getTreatmentStartDate()); +// Normalize contradictory state +if (entity.isTreatmentNotApplicable()) { + entity.setTreatmentStarted(null); + entity.setTreatmentStartDate(null); +}Also applies to: 57-59
sormas-backend/src/main/java/de/symeda/sormas/backend/symptoms/Symptoms.java (1)
263-265: Clarify the comment for unknownSymptomThe current wording is hard to parse. Suggest rephrasing for clarity.
Proposed:
- // Added in case, enrolling person is not known about the symptoms, but sure of the case. + // Captures cases where the enrolling person does not know the symptoms, but the case classification is certain.sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/labmessage/TestReportFacadeEjbMappingTest.java (1)
107-107: Add symmetric coverage for fromDto mapping of testTypeDetailsCurrently, fromDto() isn’t asserting testTypeDetails propagation. Add it to prevent regressions.
Suggested change within testFromDto():
@Test public void testFromDto() { @@ source.setTestType(PathogenTestType.ANTIBODY_DETECTION); + source.setTestTypeDetails("Test Type Details"); source.setTestDateTime(new Date(9999999L)); @@ assertEquals(source.getTestType(), result.getTestType()); + assertEquals(source.getTestTypeDetails(), result.getTestTypeDetails());Also applies to: 125-125
sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/Notifier.java (1)
49-51: Agent first/last name fields added — OK, consider minor consistency tweaksThe new fields and accessors look correct.
Optional refinements:
- Add public static field name constants for agentFirstName/agentLastName to mirror the existing pattern (e.g., FIRST_NAME_FIELD_NAME, etc.).
- If applicable, specify a length using FieldConstraints (e.g., CHARACTER_LIMIT_DEFAULT) to align with other string columns in the project.
Given the PR context indicates DTO, EJB, and service updates, no functional issues here.
Also applies to: 106-122
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java (1)
70-75: Death date mapping added — verify normalization and condition alignmentGood to map PersonDto.DEATH_DATE from external messages before approximate age calculation.
Two follow-ups to consider:
- If PersonDto.DEATH_DATE is a date-only field in UI/DB, normalize the time (e.g., start of day) with the Mapping.of overload that applies a converter.
- Optionally, when a death date is provided but present condition isn’t, align presentCondition with DECEASED (unless the team decided to map it strictly from the message only).
Example normalization (if needed):
- Mapping.of(person::setDeathDate, person.getDeathDate(), externalMessage.getDeceasedDate(), PersonDto.DEATH_DATE), + Mapping.of(person::setDeathDate, person.getDeathDate(), externalMessage.getDeceasedDate(), DateHelper::getStartOfDay, PersonDto.DEATH_DATE),sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/processing/ExternalMessageProcessingUIHelper.java (1)
544-552: Helper addLabelIfAvailable — concise and safe
- Skips blank text, uses i18n caption with fallback behavior, and keeps Label in TEXT mode by default (safe against HTML injection).
- WidthUndefined is appropriate for inline labels.
Optional: If captionKey might be null in any callsites, defensively handle it (e.g., requireNonNull or a default caption) to avoid odd “prefix.null” lookups.
sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java (1)
2579-2583: Consider adding @order to unknownSymptom for stable import/export positioning.Many SymptomsDto fields define @order to keep CSV/import templates stable. unknownSymptom currently has none. Optional, but adding an @order near related clinical presentation fields avoids future reordering surprises.
sormas-backend/src/main/java/de/symeda/sormas/backend/infrastructure/facility/FacilityFacadeEjb.java (1)
114-131: Consider pushing name filtering down to the service/DBFiltering all active facilities in-memory with a regex can be costly in large deployments. If feasible, consider extending the service to support a name filter (e.g., case-insensitive LIKE or DB-side regex) to reduce payload and CPU usage on the application node.
sormas-api/src/main/java/de/symeda/sormas/api/therapy/TherapyDto.java (1)
46-49: Add missing constant for treatmentStartDate for consistencyOther properties expose String constants; adding one for treatmentStartDate keeps API symmetry and eases binder usage.
Apply this diff just below the existing constants:
public static final String TREATMENT_STARTED = "treatmentStarted"; public static final String TREATMENT_NOT_APPLICABLE = "treatmentNotApplicable"; +public static final String TREATMENT_START_DATE = "treatmentStartDate";Also applies to: 85-107
sormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjb.java (1)
1-15: License header year looks off (2026)Align the end year with the current year to avoid tooling/license checks flagging it.
Apply this diff:
- * Copyright © 2016-2026 SORMAS Foundation gGmbH + * Copyright © 2016-2025 SORMAS Foundation gGmbHsormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageProcessingFacade.java (1)
200-206: Null-guard externalId to avoid unexpected service call with nullIf externalId is null, short-circuit to null to avoid passing null to the facade and relying on its null-handling.
Apply this diff:
public FacilityReferenceDto getHospitalFacilityReferenceByExternalId(String externalId) { - return facilityFacade.getByExternalIdAndType(externalId, FacilityType.HOSPITAL, false) + if (externalId == null) { + return null; + } + return facilityFacade.getByExternalIdAndType(externalId, FacilityType.HOSPITAL, false) .stream() .filter(Objects::nonNull) .findFirst() .orElse(null); }sormas-backend/src/main/java/de/symeda/sormas/backend/therapy/Therapy.java (1)
36-39: Consider nullable type for not-applicable and align naming with related models
- treatmentNotApplicable is a primitive boolean here, while ExternalMessage uses a boxed Boolean for the same concept. If “not applicable” needs a tri-state (true/false/unknown) or to represent absence, use Boolean for consistency and nullability.
- Therapy uses treatmentStartDate while ExternalMessage uses treatmentStartedDate. The divergence is easy to miss and can introduce mapping confusions.
Do we need a tri-state for treatmentNotApplicable in Therapy? If yes, switch to
Boolean. Also confirm the intended naming across layers; if alignment is desired, consider renaming totreatmentStartedDateor adjusting mapping helpers accordingly.sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java (1)
331-346: Prefer localized formatting for date and enum captions
sampleDateTime.toString()is not localized; use a date formatter aligned with user locale.sampleMaterial.toString()likely prints enum name; better to useI18nProperties.getEnumCaption(sampleMaterial).Example minimal changes within this block:
- ExternalMessageProcessingUIHelper.addLabelIfAvailable( - sampleDetailsLayout, - sampleDateTime == null ? null : sampleDateTime.toString(), + ExternalMessageProcessingUIHelper.addLabelIfAvailable( + sampleDetailsLayout, + sampleDateTime == null ? null : de.symeda.sormas.ui.utils.DateFormatHelper.formatDateTime(sampleDateTime), ExternalMessageDto.I18N_PREFIX, SampleReportDto.SAMPLE_DATE_TIME); - ExternalMessageProcessingUIHelper - .addLabelIfAvailable(sampleDetailsLayout, sampleReport.getLabSampleId(), ExternalMessageDto.I18N_PREFIX, SampleReportDto.LAB_SAMPLE_ID); + ExternalMessageProcessingUIHelper.addLabelIfAvailable( + sampleDetailsLayout, + sampleReport.getLabSampleId(), + ExternalMessageDto.I18N_PREFIX, + SampleReportDto.LAB_SAMPLE_ID); ... - ExternalMessageProcessingUIHelper.addLabelIfAvailable( - sampleDetailsLayout, - sampleMaterial == null ? null : sampleMaterial.toString(), + ExternalMessageProcessingUIHelper.addLabelIfAvailable( + sampleDetailsLayout, + sampleMaterial == null ? null : I18nProperties.getEnumCaption(sampleMaterial), ExternalMessageDto.I18N_PREFIX, SampleReportDto.SAMPLE_MATERIAL);If
DateFormatHelperdoesn’t exist, I can provide a small helper for localized formatting.sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjbMappingTest.java (4)
115-118: Fix duplicated birth date setter: set MM instead of DD twiceYou’re calling
setPersonBirthDateDDtwice; the second should set month (MM).Apply this diff:
- source.setPersonBirthDateDD(1); + source.setPersonBirthDateMM(1);
228-241: Fix duplicated birth date setter in toDto test as wellSame issue as above.
Apply this diff:
- source.setPersonBirthDateDD(1); + source.setPersonBirthDateMM(1);
215-217: Strengthen mapping test by stubbing SampleReport mapping to a non-null DTOCurrently
sampleReportFacade.toDto(sampleReport)returns null (mock default), so the assertion only checks a list with null. Stub the mock to return a real DTO to actually verify the mapping.Apply this diff:
- SampleReportDto sampleReportDto = sampleReportFacade.toDto(sampleReport); + SampleReportDto sampleReportDto = new SampleReportDto(); + when(sampleReportFacade.toDto(sampleReport)).thenReturn(sampleReportDto);
268-271: Also assert treatmentNotApplicable on toDto pathYou added assertions for
treatmentStartedand dates; includetreatmentNotApplicableto cover the new field both ways.Apply this diff:
- source.setTreatmentStarted(YesNoUnknown.YES); + source.setTreatmentStarted(YesNoUnknown.YES); + source.setTreatmentNotApplicable(Boolean.TRUE); ... - assertEquals(source.getTreatmentStarted(), result.getTreatmentStarted()); + assertEquals(source.getTreatmentStarted(), result.getTreatmentStarted()); + assertEquals(source.getTreatmentNotApplicable(), result.getTreatmentNotApplicable());Also applies to: 315-316
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierForm.java (3)
147-151: Use i18n key instead of hardcoded “Notifier Information”Avoid hardcoded English strings to maintain localization.
Apply this diff (adjust key if different in your bundles):
- Label datesLabel = new Label("Notifier Information"); + Label datesLabel = new Label(I18nProperties.getString(Strings.headingNotifierInformation));
158-166: Externalize field descriptions to i18n and avoid hardcoded EnglishDescriptions for notification/diagnostic dates are hardcoded. Move them to I18n resources for consistency.
I can provide the necessary keys and wiring if you confirm the target bundles (Strings vs Captions) and phrasing.
299-324: Use localized validation messages instead of hardcoded “Required field”To keep consistency with the rest of the UI, use i18n-based messages.
Example:
firstNameField.setComponentError(new UserError(I18nProperties.getString(Strings.messageRequiredField)));If there’s a canonical validation key (e.g., Validations.fieldIsRequired), prefer that.
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
117-172: Infrastructure data validation implementationThe
handleInfraDataChecks()method implements a comprehensive hospital facility validation flow:
- Attempts to retrieve hospital facility reference
- Shows appropriate warning dialogs based on missing data
- Returns false to skip hospitalization if data is incomplete
The implementation correctly handles both missing hospital data and unmatched facilities.
Consider extracting UI message strings to constants
The hardcoded messages in lines 142-143 and 155-156 could be moved to the i18n properties files for better maintainability and localization support.
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (2)
261-304: Consider extracting magic dates to constantsThe therapy update logic is well-structured with clear conditional paths. However, consider extracting the default date (
new java.util.Date()) to a method parameter or configuration for better testability.private void updateTherapyBasedOnSelection(CaseDataDto caze, TreatmentOption selectedOption) { + updateTherapyBasedOnSelection(caze, selectedOption, new Date()); +} + +private void updateTherapyBasedOnSelection(CaseDataDto caze, TreatmentOption selectedOption, Date defaultStartDate) { if (selectedOption == null) { return; } TherapyDto therapy = caze.getTherapy(); if (therapy == null) { therapy = TherapyDto.build(); } if (selectedOption.equals(TreatmentOption.YES)) { therapy.setTreatmentStarted(YesNoUnknown.YES); therapy.setTreatmentNotApplicable(false); if (therapy.getTreatmentStartDate() == null) { - therapy.setTreatmentStartDate(new java.util.Date()); + therapy.setTreatmentStartDate(defaultStartDate); } return; }
315-317: TODO comment needs implementation planThe placeholder method for diagnostic date handling contains a TODO without any implementation details or tracking reference.
Would you like me to create an issue to track the implementation of diagnostic date handling functionality?
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/doctordeclaration/AbstractDoctorDeclarationMessageProcessingFlow.java (2)
447-478: Complex facility matching logic could benefit from documentationThe facility reference lookup implements a three-tier fallback strategy (external ID → ID in name → name match). While the implementation is correct, consider adding method-level documentation explaining the matching priority.
/** * Retrieves the hospital facility reference based on the external message data. * It first tries to find the facility by external ID and then by name containing external ID. + * + * Matching priority: + * 1. Exact match by external ID + * 2. Name contains external ID (with word boundaries) + * 3. Name contains the provided hospital name * * @param externalMessageDto * The external message containing hospitalization facility information. * @return The facility reference DTO for the hospital, or null if not found. */ protected FacilityReferenceDto getHospitalFacilityReference(ExternalMessageDto externalMessageDto) {
461-464: Consider extracting regex pattern to constantThe complex regex pattern for hospital ID matching could be extracted to a constant for better readability and reusability.
+private static final String HOSPITAL_ID_NAME_PATTERN_FORMAT = "^(\\b%s\\b)?.*?(\\b%s\\b).*?$"; + protected FacilityReferenceDto getHospitalFacilityReference(ExternalMessageDto externalMessageDto) { // ... existing code ... // Search for a hospital facility containing the external ID in name if not found by external ID if (hospitalFacilityReference == null && StringUtils.isNotBlank(hospitalExternalId)) { - final Pattern hospitalIdInNamePattern = Pattern.compile( - "^(\\b" + Pattern.quote(hospitalExternalId) + "\\b)?.*?(\\b" + Pattern.quote(hospitalExternalId) + "\\b).*?$", - Pattern.CASE_INSENSITIVE); + final String quotedId = Pattern.quote(hospitalExternalId); + final Pattern hospitalIdInNamePattern = Pattern.compile( + String.format(HOSPITAL_ID_NAME_PATTERN_FORMAT, quotedId, quotedId), + Pattern.CASE_INSENSITIVE); hospitalFacilityReference = getExternalMessageProcessingFacade().getHospitalFacilityReferenceNameMatching(hospitalIdInNamePattern)sormas-backend/src/main/resources/sql/sormas_schema.sql (4)
14426-14427: Case comments length likely too smallFree-text comments are often longer than 255. We’ve moved many comment fields to 512 or even 4096 across the schema for similar reasons.
Consider increasing to at least 512 to be consistent and prevent truncation.
-ALTER TABLE externalmessage ADD COLUMN casecomments varchar(255); -ALTER TABLE externalmessage_history ADD COLUMN casecomments varchar(255); +ALTER TABLE externalmessage ADD COLUMN casecomments varchar(512); +ALTER TABLE externalmessage_history ADD COLUMN casecomments varchar(512);
14488-14490: Column name casing: will become lowercase in PostgreSQL; verify ORM/mappersPostgreSQL folds unquoted identifiers to lowercase. Adding testTypeDetails will create testtypedetails on disk. Ensure your mappers/SQL reference the lowercase name, and consider aligning with existing testtypetext naming for consistency.
-ALTER TABLE testreport ADD COLUMN testTypeDetails varchar(255); -ALTER TABLE testreport_history ADD COLUMN testTypeDetails varchar(255); +ALTER TABLE testreport ADD COLUMN testtypedetails varchar(255); +ALTER TABLE testreport_history ADD COLUMN testtypedetails varchar(255);
14446-14455: Make DDL idempotent with IF NOT EXISTS for repeatable deploymentsYou’ve used IF NOT EXISTS here, but the surrounding blocks (e.g., Lines 14426-14444) don’t. For operational safety when migrations are re-run or applied to partially-upgraded environments, prefer IF NOT EXISTS across the board in this file section.
-ALTER TABLE externalmessage ADD COLUMN casecomments varchar(255); +ALTER TABLE externalmessage ADD COLUMN IF NOT EXISTS casecomments varchar(255); ... -ALTER TABLE externalmessage_history ADD COLUMN casecomments varchar(255); +ALTER TABLE externalmessage_history ADD COLUMN IF NOT EXISTS casecomments varchar(255);Apply the same pattern to the newly added hospitalization* and previoushospitalization.healthfacilitydepartment columns in this section.
14426-14438: Optional: include case comments in full-text search (tsv) if used for message searchThe externalmessage tsv is currently built from externalmessagedetails only. If casecomments should be discoverable via content search, extend the tsv trigger function to include it.
Outside this hunk (function defined around Lines 12334-12344), update trigger function:
CREATE OR REPLACE FUNCTION externalmessage_tsv_update() RETURNS TRIGGER AS $$ BEGIN IF TG_OP = 'INSERT' OR TG_OP = 'UPDATE' THEN NEW.tsv = to_tsvector( 'simple', unaccent( regexp_replace( coalesce(NEW.externalmessagedetails, '') || ' ' || coalesce(NEW.casecomments, ''), E'[<>]', ' ', 'g' ) ) ); END IF; RETURN NEW; END $$ LANGUAGE 'plpgsql';If you want, I can prepare a full diff adjusting the trigger and recreating it safely (drop/recreate) in a new schema version step.
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/ExternalMessageDto.java (5)
106-119: Public constants added — consider adding a constant for caseComments as wellThe new constants look consistent and useful. If caseComments is referenced in UI/mapping layers similarly to other fields, consider adding:
- public static final String CASE_COMMENTS = "caseComments";
If not used anywhere, feel free to ignore.
188-188: Deceased date semantics and cross-field consistency
- Clarify whether deceasedDate is a date-only or date-time; the field name implies date-only. If date-only is intended, consider normalizing time or documenting the expected semantics.
- Consider validating consistency with personPresentCondition to avoid states like deceasedDate set while condition is ALIVE.
Also applies to: 832-838
197-199: caseComments field and accessors look fineLength constraint matches other long-text fields. If caseComments is used in UI bindings similarly to other fields, see earlier note about adding a CASE_COMMENTS constant.
Also applies to: 546-552
221-229: Hospitalization fields: add date-range and presence validationThe new fields are valuable. Consider adding:
- Date-range validation to ensure discharge date is not before admission date.
- If admittedToHealthFacility is YES, require an admission date (and optionally at least one of facilityName/externalId).
You can add object-level Bean Validation methods (outside the touched lines):
import javax.validation.constraints.AssertTrue; @AssertTrue public boolean isHospitalizationDateRangeValid() { if (hospitalizationAdmissionDate != null && hospitalizationDischargeDate != null) { return !hospitalizationDischargeDate.before(hospitalizationAdmissionDate); } return true; } @AssertTrue public boolean isHospitalizationAdmissionPresenceValid() { return admittedToHealthFacility != YesNoUnknown.YES || hospitalizationAdmissionDate != null; }Also applies to: 610-648
250-257: TB/HIV fields: consider cross-field consistency checksPotential rules to prevent contradictory states:
- hivArt should not be YES when hiv is NO.
- If previousTuberculosisTreatment is YES, tuberculosis should likely not be NO (or enforce domain-specific logic).
- tuberculosisInfectionYear should be a reasonable year (not in the future).
If you want to enforce some of these with Bean Validation (outside the touched lines):
import javax.validation.constraints.AssertTrue; @AssertTrue public boolean isHivArtConsistent() { return hiv != YesNoUnknown.NO || hivArt != YesNoUnknown.YES; }For tuberculosisInfectionYear, since @max requires a compile-time constant, confirm validation happens elsewhere (e.g., service layer) to ensure it’s not in the future and within a sensible range.
Also applies to: 856-902
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/ExternalMessageDto.java
Show resolved
Hide resolved
.../java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java
Show resolved
Hide resolved
...pi/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java
Show resolved
Hide resolved
...rnalmessage/processing/doctordeclaration/AbstractDoctorDeclarationMessageProcessingFlow.java
Show resolved
Hide resolved
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewContent.java
Show resolved
Hide resolved
| // TODO - NotifierFacade doesn't seem to have delete functionality yet | ||
| // if (canDelete) { | ||
| // editView.addDeleteListener(() -> { | ||
| // // Delete functionality would go here | ||
| // window.close(); | ||
| // callback.run(); | ||
| // }, "Notifier"); | ||
| // } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Commented delete functionality needs clarification
The delete functionality is commented out with a TODO noting that NotifierFacade doesn't have delete capability. This should either be implemented or the commented code should be removed to avoid confusion.
🏁 Script executed:
#!/bin/bash
# Check if NotifierFacade has any delete methods
ast-grep --pattern 'interface NotifierFacade {
$$$
delete($$$) {
$$$
}
$$$
}'
# Also check for any delete-related methods
rg "delete|remove" --type java -A 2 -B 2 $(fd NotifierFacade.java)Length of output: 242
To ensure we’re accurately detecting any delete/remove capabilities in NotifierFacade, let’s print out its contents and search more exhaustively:
🏁 Script executed:
#!/bin/bash
# 1) Locate the interface file
file=$(fd NotifierFacade.java)
# 2) Show the first 200 lines of the interface
echo "=== ${file} (lines 1–200) ==="
sed -n '1,200p' "$file"
# 3) Search for any delete/remove method signatures
echo "=== delete/remove occurrences ==="
rg -nE "delete|remove" --type java "$file"Length of output: 5681
Let’s verify whether NotifierFacade inherits any delete/remove methods from its parent BaseFacade:
🏁 Script executed:
#!/bin/bash
# Locate and inspect BaseFacade
file=$(fd BaseFacade.java)
echo "=== ${file} (first 200 lines) ==="
sed -n '1,200p' "$file"
echo "=== delete/remove occurrences in BaseFacade ==="
rg -nE "delete|remove" --type java "$file"Length of output: 1676
Delete functionality missing in NotifierFacade
The commented-out delete listener in CaseNotifierSideViewController refers to a delete operation, but neither NotifierFacade nor its BaseFacade parent declares any delete/remove method. Please address this by either:
- Adding and implementing a
delete(String uuid)(or similar) method in NotifierFacade (and its server-side implementation) before re-enabling the UI code, or - Removing the commented delete block (and its TODO) to avoid leaving dead code.
Locations:
- sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (lines 239–246)
- sormas-api/src/main/java/de/symeda/sormas/api/person/notifier/NotifierFacade.java
🤖 Prompt for AI Agents
In
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
around lines 239–246 the commented delete listener references a delete operation
that does not exist in NotifierFacade/BaseFacade; either implement delete
functionality in the API and server or remove the dead code: Option A - add a
delete(String uuid) (or deleteById) method to
sormas-api/src/main/java/de/symeda/sormas/api/person/notifier/NotifierFacade.java
(and its BaseFacade parent if appropriate), implement the server-side removal
logic and tests, then re-enable the UI delete listener to call that method and
invoke callback on success; Option B - if deletion is not required now, delete
the commented block and its TODO in the UI controller to avoid dead code and
leave a clear TODO or issue reference for future implementation.
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/TreatmentOption.java
Outdated
Show resolved
Hide resolved
...meda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java
Outdated
Show resolved
Hide resolved
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13575 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13575 |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
699-702: Missing implementation for notifyCorrectionsSavedThe
notifyCorrectionsSavedmethod returns null but should return a completed CompletionStage. This could cause issues in the processing flow.@Override protected CompletionStage<Void> notifyCorrectionsSaved() { LOGGER.debug("Notifying corrections saved for externalMessage: {}", getExternalMessage()); - return null; + return CompletableFuture.completedFuture(null); }
♻️ Duplicate comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
323-329: Improved reporter name parsing logicThe updated implementation correctly handles hyphenated names by treating all parts except the last as the first name, which is more robust than the previous approach. This addresses the concerns raised in the past review comment about handling names like "Marie-Claire" or "Jean-Pierre".
🧹 Nitpick comments (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (3)
3-3: Update copyright year range to 2024The copyright year should be 2024, not 2026, as we are currently in 2024.
- * Copyright © 2016-2026 SORMAS Foundation gGmbH + * Copyright © 2016-2024 SORMAS Foundation gGmbH
164-164: Remove commented-out codeThis commented line appears to be leftover debug code and should be removed.
- //commitDiscardWrapperComponent.addCommitListener(() -> ret.complete(true));
639-663: Consider extracting pathogen test logic to a helper methodThe
getSampleCreateComponentmethod is doing multiple responsibilities. Consider extracting the pathogen test setup logic (lines 648-659) into a separate helper method for better modularity and testability.private CommitDiscardWrapperComponent<SampleCreateForm> getSampleCreateComponent( SampleDto sample, boolean lastSample, List<PathogenTestDto> pathogenTests, Disease disease, Consumer<PathogenTestDto> pathogenTestSaveHandler) { SampleController sampleController = ControllerProvider.getSampleController(); CommitDiscardWrapperComponent<SampleCreateForm> sampleCreateComponent = sampleController.getSampleCreateComponent(sample, disease, null); - // add pathogen test create components - List<PathogenTestDto> pathogenTestsToAdd = new ArrayList<>(pathogenTests); - // always build at least one PathogenTestDto - if (pathogenTestsToAdd.isEmpty()) { - pathogenTestsToAdd.add(LabMessageProcessingHelper.buildPathogenTest(null, getMapper(), sample, getUser())); - } - - ExternalMessageProcessingUIHelper.addNewPathogenTests(pathogenTestsToAdd, sampleCreateComponent, true, pathogenTestSaveHandler, null); - - // add option to create additional pathogen tests - sampleController.addPathogenTestButton(sampleCreateComponent, true, null, null, pathogenTestSaveHandler); + setupPathogenTests(sampleCreateComponent, sample, pathogenTests, pathogenTestSaveHandler, sampleController); LabMessageUiHelper.establishCommitButtons(sampleCreateComponent, lastSample); return sampleCreateComponent; } +private void setupPathogenTests( + CommitDiscardWrapperComponent<SampleCreateForm> sampleCreateComponent, + SampleDto sample, + List<PathogenTestDto> pathogenTests, + Consumer<PathogenTestDto> pathogenTestSaveHandler, + SampleController sampleController) { + + // add pathogen test create components + List<PathogenTestDto> pathogenTestsToAdd = new ArrayList<>(pathogenTests); + // always build at least one PathogenTestDto + if (pathogenTestsToAdd.isEmpty()) { + pathogenTestsToAdd.add(LabMessageProcessingHelper.buildPathogenTest(null, getMapper(), sample, getUser())); + } + + ExternalMessageProcessingUIHelper.addNewPathogenTests(pathogenTestsToAdd, sampleCreateComponent, true, pathogenTestSaveHandler, null); + + // add option to create additional pathogen tests + sampleController.addPathogenTestButton(sampleCreateComponent, true, null, null, pathogenTestSaveHandler); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/TreatmentOption.java(2 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/TreatmentOption.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (12)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/PickOrCreateEventResult.java (1)
PickOrCreateEventResult(20-45)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/PickOrCreateSampleResult.java (1)
PickOrCreateSampleResult(20-45)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/doctordeclaration/AbstractDoctorDeclarationMessageProcessingFlow.java (1)
AbstractDoctorDeclarationMessageProcessingFlow(69-480)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/labmessage/LabMessageProcessingHelper.java (1)
LabMessageProcessingHelper(31-56)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/labmessage/SampleAndPathogenTests.java (1)
SampleAndPathogenTests(23-40)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/processing/EntrySelectionComponentForExternalMessage.java (1)
EntrySelectionComponentForExternalMessage(26-55)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/processing/ExternalMessageProcessingUIHelper.java (1)
ExternalMessageProcessingUIHelper(88-554)sormas-ui/src/main/java/de/symeda/sormas/ui/samples/humansample/SampleController.java (1)
SampleController(100-746)sormas-ui/src/main/java/de/symeda/sormas/ui/samples/humansample/SampleCreateForm.java (1)
SampleCreateForm(24-51)sormas-ui/src/main/java/de/symeda/sormas/ui/samples/humansample/SampleEditPathogenTestListHandler.java (1)
SampleEditPathogenTestListHandler(25-46)sormas-ui/src/main/java/de/symeda/sormas/ui/samples/humansample/SampleSelectionField.java (1)
SampleSelectionField(33-212)sormas-ui/src/main/java/de/symeda/sormas/ui/utils/processing/ProcessingUiHelper.java (1)
ProcessingUiHelper(34-68)
⏰ 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). (5)
- GitHub Check: Lint Code Base
- GitHub Check: SORMAS CI
- GitHub Check: android app test (28)
- GitHub Check: android app test (27)
- GitHub Check: android app test (26)
🔇 Additional comments (4)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (4)
119-174: LGTM! Well-structured infrastructure data validationThe
handleInfraDataChecksmethod properly validates hospital facility references and provides appropriate user feedback through modal dialogs. The logic correctly handles the different scenarios:
- Hospital facility found - continues processing
- Missing hospital data - allows continuation with a warning
- Hospital not found with valid identifier - blocks processing
211-237: LGTM! Enhanced person matching logicThe implementation intelligently handles cases where patient name fields are missing or match the national health ID, attempting to find existing persons by their health ID first. This prevents duplicate person records and improves data quality.
533-589: LGTM! Comprehensive sample selection UIThe
handlePickOrCreateSamplemethod provides a well-structured UI for sample selection with appropriate details display including sample date/time, lab sample ID, and sample material. The implementation follows the established UI patterns and provides clear user interaction flow.
609-637: LGTM! Well-integrated sample and pathogen test creationThe
handleCreateSampleAndPathogenTestsmethod properly orchestrates the creation of samples with associated pathogen tests. The post-commit listener ensures that pathogen tests are saved after the sample is created, maintaining referential integrity.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13575 |
raulbob
deleted the
feature-13294_doctor_declarations_xml_esante_adapter
branch
raulbob
restored the
feature-13294_doctor_declarations_xml_esante_adapter
branch
Summary by CodeRabbit
New Features
Improvements