Bugfix 13625 add handling person multiple contact info#13872
Merged
KarnaiahPesula merged 7 commits intodevelopmentfrom Mar 11, 2026
Merged
Conversation
- The handling permits merging of multiple contacts for a person - Phone/email contacts are added if they do not match the existing ones - ExternalMessage's person phone/email are taken as primary contacts while the rest are added as secondary contact info - The merge is done by comparing the phone/email with the existing ones - Fixed the sequence handling to avoid persisting the person if the external message processing is canceled by the user
- AutomaticLabMessageProcessorPersonContactTest for automatic processing of person contacts - ExternalMessageMapperPersonTest for mapping of person contacts
📝 WalkthroughWalkthroughAdds JSONB fields for additional person contact details and addresses; moves parsing, mapping, deduplication and merge logic into ExternalMessageMapper; removes the old doPersonUpdates hook and invokes mapper-based merge after person selection; updates facades, UI post-update callbacks, lab processing persistence, schema, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant ExternalMessage as ExternalMessageDto
participant Flow as ProcessingFlow
participant Mapper as ExternalMessageMapper
participant Facade as PersonFacade/DB
ExternalMessage->>Flow: receive message
Flow->>Mapper: buildPerson() / map additional fields
Mapper->>Mapper: deserialize additionalContactDetails & addresses
Mapper-->>Flow: PersonDto with mapped/merged suggestions
Flow->>Facade: mergePerson/save PersonDto (mapper.mergePerson*)
Facade-->>Flow: persisted PersonDto
Flow-->>ExternalMessage: processing result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 5
🧹 Nitpick comments (4)
sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/labmessage/AutomaticLabMessageProcessorPersonContactTest.java (1)
149-169: Assert the extra contact count, not just its existence.This test still passes if
+49-extra-mobilegets stored twice. Counting the matching entries here would turn the new-person duplication path into a deterministic regression test.Diff to tighten the assertion
- boolean hasExtraContact = - persons.get(0).getPersonContactDetails().stream().anyMatch(pcd -> "+49-extra-mobile".equals(pcd.getContactInformation())); - assertThat(hasExtraContact, is(true)); + long extraContactCount = persons.get(0) + .getPersonContactDetails() + .stream() + .filter(pcd -> "+49-extra-mobile".equals(pcd.getContactInformation())) + .count(); + assertThat(extraContactCount, is(1L));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/labmessage/AutomaticLabMessageProcessorPersonContactTest.java` around lines 149 - 169, The test testNewPersonAdditionalContactDetailFromJson currently only asserts existence of a contact with "+49-extra-mobile" which won't catch duplicates; modify the assertion to count matching PersonContactDetails instead of using anyMatch. Locate the test method (testNewPersonAdditionalContactDetailFromJson) and replace the boolean hasExtraContact logic with a count of persons.get(0).getPersonContactDetails().stream().filter(pcd -> "+49-extra-mobile".equals(pcd.getContactInformation())).count() and assert that the count equals 1 (using assertThat(count, is(1))) so the test fails if the extra contact is stored more than once.sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java (1)
183-191: Consider reusing ObjectMapper instance for better performance.A new
ObjectMapperinstance is created each timemapAdditionalPersonContactDetailsis called.ObjectMapperis thread-safe and expensive to instantiate. Consider making it a static final field.♻️ Proposed fix
Add a static ObjectMapper field at class level:
public final class ExternalMessageMapper { private static final Logger logger = LoggerFactory.getLogger(ExternalMessageMapper.class); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); private final ExternalMessageDto externalMessage;Then update the usages:
- List<PersonContactDetailDto> additionalDetails = - new ObjectMapper().readValue(externalMessage.getAdditionalPersonContactDetails(), new TypeReference<List<PersonContactDetailDto>>() { - }); + List<PersonContactDetailDto> additionalDetails = + OBJECT_MAPPER.readValue(externalMessage.getAdditionalPersonContactDetails(), new TypeReference<List<PersonContactDetailDto>>() { + });Apply the same change to
mapAdditionalPersonAddressesat line 203-205.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java` around lines 183 - 191, ExternalMessageMapper instantiates a new ObjectMapper for each call (in mapAdditionalPersonContactDetails and mapAdditionalPersonAddresses); create a single shared thread-safe instance by adding a private static final ObjectMapper (e.g., objectMapper) at class level and replace the two occurrences of new ObjectMapper() in mapAdditionalPersonContactDetails and mapAdditionalPersonAddresses with this static objectMapper to improve performance.sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java (1)
1065-1070: Truncated comment on line 1066.The comment appears to be cut off: "// no merges for new person but we still need to update the additional conta". This should be completed for clarity.
📝 Suggested fix
if (personSelection.isNew()) { - // no merges for new person but we still need to update the additional conta + // no merges for new person but we still need to update the additional contact details and addresses getMapper().mapAdditionalPersonAddresses(person); getMapper().mapAdditionalPersonContactDetails(person); return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java` around lines 1065 - 1070, The inline comment in AbstractMessageProcessingFlowBase near the personSelection.isNew() branch is truncated ("// no merges for new person but we still need to update the additional conta"); update that comment to a complete, clear sentence explaining behavior for new persons (e.g., "// no merges for new person but we still need to update the additional contact details and addresses via the mapper"), placing it immediately above the if (personSelection.isNew()) block and referencing the mapper calls (getMapper().mapAdditionalPersonAddresses and getMapper().mapAdditionalPersonContactDetails) so future readers understand why the method returns early.sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
384-390: Name splitting logic may produce unexpected results for certain name formats.The hyphen-based name splitting assumes the reporter name format is
FirstName-LastName. However:
- Names like "Jean-Pierre Dupont" would incorrectly split as firstName="Jean" and lastName="Pierre Dupont"
- Names without hyphens are not handled (the
contains("-")check passes, but edge cases within the split could occur)- Line 389: When
nameParts.length > 0is checked butnameParts.length - 1could be 0 if there's only one part after splitConsider documenting the expected input format or adding defensive handling.
📝 Suggested improvement for edge case handling
if (externalMessage.getReporterName() != null && externalMessage.getReporterName().contains("-")) { // Split the reporter name into first and last names if it contains a hyphen - // Some names may already contain hyphens, assume first parts are the first name and last parts are the last name + // Format expected: "FirstName-LastName" where hyphen is the delimiter + // For compound names like "Jean-Pierre-Dupont", all but the last segment become firstName final String[] nameParts = externalMessage.getReporterName().split("-"); - notifierDto.setAgentFirstName(Arrays.stream(nameParts).limit(nameParts.length - 1).map(String::trim).collect(Collectors.joining(" "))); - notifierDto.setAgentLastName(nameParts.length > 0 ? nameParts[nameParts.length - 1].trim() : ""); + if (nameParts.length >= 2) { + notifierDto.setAgentFirstName(Arrays.stream(nameParts).limit(nameParts.length - 1).map(String::trim).collect(Collectors.joining(" "))); + notifierDto.setAgentLastName(nameParts[nameParts.length - 1].trim()); + } else if (nameParts.length == 1) { + notifierDto.setAgentLastName(nameParts[0].trim()); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java` around lines 384 - 390, The current hyphen-based parsing in DoctorDeclarationMessageProcessingFlow can mis-split names (e.g., "Jean-Pierre Dupont") and doesn't handle single-part results safely; replace the block that reads externalMessage.getReporterName() and sets notifierDto to robust parsing: first trim the reporterName, if empty set both names empty; otherwise split on whitespace to get tokens and treat the last token as agentLastName and all preceding tokens joined as agentFirstName (this preserves hyphenated given names like "Jean-Pierre"); if the name contains no whitespace but contains a single hyphen still use the same rule (split into parts, last part = last name, rest = first name) and ensure you guard against nameParts.length == 1 by placing the whole string into agentFirstName and leaving agentLastName empty; always trim parts and document the expected input format in a comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java`:
- Around line 180-181: The buildPerson() method currently calls
mapper.mapAdditionalPersonContactDetails(...) and
mapper.mapAdditionalPersonAddresses(...) which causes duplicate add-only
mappings because AbstractMessageProcessingFlowBase.mergePerson(...) also invokes
those when personSelection.isNew(); remove the calls from buildPerson() (leave
mergePerson(...) responsible for applying add-only mappings for new persons) so
additional contacts/addresses are only mapped once, or alternatively make the
mapper methods idempotent if you prefer to keep them in buildPerson().
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java`:
- Around line 401-413: The new PersonContactDetailDto instances created in
ExternalMessageMapper (the personContactDetail for phone and the analogous email
contact block) are missing UUIDs; mirror the approach used in mapGuardianData by
calling setUuid(DataHelper.createUuid()) on the newly created
PersonContactDetailDto before adding it to personContactDetails so persistence
and entity tracking receive a proper UUID (locate the personContactDetail and
email contact creation blocks in ExternalMessageMapper and add
setUuid(DataHelper.createUuid()) to each new PersonContactDetailDto).
- Around line 326-331: When personAddress is null you create a new LocationDto
via LocationDto.build() and call mapToLocation(location) but never assign that
LocationDto back to the person, so the value is lost; fix by assigning the newly
created LocationDto to the person's address field (e.g., set personAddress or
call person.setAddress(location)) before returning mapToLocation(location) so
the person retains the new address; ensure you update the same variable used
elsewhere (personAddress) or call the appropriate setter on the person object.
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Line 15267: The schema_version INSERT currently records the wrong issue
number; update the INSERT INTO schema_version (version_number, comment) VALUES
(611, '...') so the comment references "#13625 - Multiple person contacts and
addresses" instead of "#13754" to match the PR and SQL comment; locate the
VALUES row for version_number 611 and replace the issue number in its comment
field accordingly.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java`:
- Around line 144-164: The existing-case path currently skips persisting the
in-memory result of mergePerson(...) and thus loses merged address/contact
updates; update the selected-case flow (same place where postUpdateCallback is
used for create-case) to load the merged PersonDto and persist it before
finishing: call FacadeProvider.getPersonFacade().getByUuid(...) to reload the
person and then
ExternalMessageProcessingUIHelper.updateAddressAndSavePerson(processedPerson,
getMapper()) (or the equivalent save used in
doCaseSelectedFlow/AutomaticLabMessageProcessor) so that merged changes are
saved when an existing case is selected.
---
Nitpick comments:
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java`:
- Around line 1065-1070: The inline comment in AbstractMessageProcessingFlowBase
near the personSelection.isNew() branch is truncated ("// no merges for new
person but we still need to update the additional conta"); update that comment
to a complete, clear sentence explaining behavior for new persons (e.g., "// no
merges for new person but we still need to update the additional contact details
and addresses via the mapper"), placing it immediately above the if
(personSelection.isNew()) block and referencing the mapper calls
(getMapper().mapAdditionalPersonAddresses and
getMapper().mapAdditionalPersonContactDetails) so future readers understand why
the method returns early.
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java`:
- Around line 183-191: ExternalMessageMapper instantiates a new ObjectMapper for
each call (in mapAdditionalPersonContactDetails and
mapAdditionalPersonAddresses); create a single shared thread-safe instance by
adding a private static final ObjectMapper (e.g., objectMapper) at class level
and replace the two occurrences of new ObjectMapper() in
mapAdditionalPersonContactDetails and mapAdditionalPersonAddresses with this
static objectMapper to improve performance.
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/labmessage/AutomaticLabMessageProcessorPersonContactTest.java`:
- Around line 149-169: The test testNewPersonAdditionalContactDetailFromJson
currently only asserts existence of a contact with "+49-extra-mobile" which
won't catch duplicates; modify the assertion to count matching
PersonContactDetails instead of using anyMatch. Locate the test method
(testNewPersonAdditionalContactDetailFromJson) and replace the boolean
hasExtraContact logic with a count of
persons.get(0).getPersonContactDetails().stream().filter(pcd ->
"+49-extra-mobile".equals(pcd.getContactInformation())).count() and assert that
the count equals 1 (using assertThat(count, is(1))) so the test fails if the
extra contact is stored more than once.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java`:
- Around line 384-390: The current hyphen-based parsing in
DoctorDeclarationMessageProcessingFlow can mis-split names (e.g., "Jean-Pierre
Dupont") and doesn't handle single-part results safely; replace the block that
reads externalMessage.getReporterName() and sets notifierDto to robust parsing:
first trim the reporterName, if empty set both names empty; otherwise split on
whitespace to get tokens and treat the last token as agentLastName and all
preceding tokens joined as agentFirstName (this preserves hyphenated given names
like "Jean-Pierre"); if the name contains no whitespace but contains a single
hyphen still use the same rule (split into parts, last part = last name, rest =
first name) and ensure you guard against nameParts.length == 1 by placing the
whole string into agentFirstName and leaving agentLastName empty; always trim
parts and document the expected input format in a comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2857dc86-3cba-41c7-8d9e-6b3ac30dd00a
📒 Files selected for processing (14)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/ExternalMessageDto.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/ExternalMessage.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/ExternalMessageFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/externalmessage/labmessage/AutomaticLabMessageProcessor.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/labmessage/AutomaticLabMessageProcessorPersonContactTest.javasormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/processing/ExternalMessageMapperPersonTest.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/processing/ExternalMessageProcessingUIHelper.java
💤 Files with no reviewable changes (1)
- sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java
...pi/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java
Show resolved
Hide resolved
...api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java
Show resolved
Hide resolved
...api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java
Show resolved
Hide resolved
...i/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java
Show resolved
Hide resolved
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13872 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java (2)
183-186: Consider reusing ObjectMapper as a static field.A new
ObjectMapperis instantiated for each deserialization call. SinceObjectMapperis thread-safe after configuration, consider extracting it to aprivate static finalfield to avoid repeated instantiation overhead.♻️ Proposed refactor
public final class ExternalMessageMapper { private static final Logger logger = LoggerFactory.getLogger(ExternalMessageMapper.class); + private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); private final ExternalMessageDto externalMessage;Then replace:
- new ObjectMapper().readValue(externalMessage.getAdditionalPersonContactDetails(), new TypeReference<List<PersonContactDetailDto>>() { + JSON_MAPPER.readValue(externalMessage.getAdditionalPersonContactDetails(), new TypeReference<List<PersonContactDetailDto>>() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java` around lines 183 - 186, The code repeatedly instantiates a new ObjectMapper when deserializing additionalPersonContactDetails in ExternalMessageMapper; extract a single shared instance by adding a private static final ObjectMapper (e.g., private static final ObjectMapper OBJECT_MAPPER) to the ExternalMessageMapper class, initialize/configure it once, and replace new ObjectMapper() usages (the call that produces List<PersonContactDetailDto> additionalDetails via readValue) with OBJECT_MAPPER to avoid repeated instantiation and leverage ObjectMapper's thread-safety.
355-358: Minor: Always returns ADDRESS as changed, even when no fields were modified.The method returns
PersonDto.ADDRESSin the changed fields list regardless of whether any address fields were actually updated (e.g., when all external message address fields are null). This differs from other methods that return an empty list when nothing changes. While not incorrect, it may trigger unnecessary UI updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java` around lines 355 - 358, The method in ExternalMessageMapper currently always returns Collections.singletonList(new String[] { PersonDto.ADDRESS }), which causes ADDRESS to be reported changed even when no address data was provided; change the logic so PersonDto.ADDRESS is only included when at least one address subfield from the external message is present and actually differs from the target Person's address (or when any external address field is non-null indicating a change), otherwise return an empty list (Collections.emptyList()). Locate the return that builds the singleton with PersonDto.ADDRESS and replace it with a conditional that inspects the relevant external message address fields and compares them to the existing Person/DTO address values before adding PersonDto.ADDRESS to the changed-fields list.sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
409-415: Name-splitting heuristic may produce unexpected results for compound surnames.The logic splits reporter names by hyphen, treating all parts except the last as the first name. For names like "Jean-Pierre Dupont-Martin", this would yield
firstName="Jean Pierre Dupont"andlastName="Martin", which may not match the intended split.Given the comment acknowledges this limitation, the heuristic seems intentional. Consider documenting expected input format if the source system has a known convention.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java` around lines 409 - 415, The current hyphen-based name split in DoctorDeclarationMessageProcessingFlow (externalMessage.getReporterName(), notifierDto.setAgentFirstName, notifierDto.setAgentLastName) can mis-handle compound surnames; either replace the heuristic with a safer rule (e.g. split on the last hyphen only by using lastIndexOf('-') to take substring(0,lastIndex) as first name and substring(lastIndex+1) as last name) or, if the source has a known convention, add a clear code comment and Javadoc documenting the expected reporterName format and add a defensive log/warning when multiple hyphens are detected so downstream users are aware; update the block around externalMessage.getReporterName() accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java`:
- Around line 1059-1063: The mergePerson(EntitySelection<PersonDto>
personSelection) method risks an NPE because it dereferences personSelection
without checking it; update mergePerson to first check if personSelection is
null and return early (e.g., if (personSelection == null) return;), then proceed
to call personSelection.getEntity() and the existing null check on the
PersonDto; this change applies to the mergePerson method in
AbstractMessageProcessingFlowBase to safely handle
result.getData().getSelectedPerson() possibly being null.
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15261-15265: Update the two new JSON fields
additionalPersonContactDetails and additionalPersonAddresses in the
ExternalMessage entity to use the project-standard JSON mapping: replace any
`@Type`(type = "jsonb") with `@Type`(type = ModelConstants.HIBERNATE_TYPE_JSON) and
replace `@Column`(columnDefinition = "jsonb") with `@Column`(columnDefinition =
ModelConstants.COLUMN_DEFINITION_JSON) in ExternalMessage.java; also update the
SQL schema statements in sormas_schema.sql to use "json" instead of "jsonb" for
these columns so the DB column type matches the entity mappings.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java`:
- Around line 260-264: Update the misleading block comment in
LabMessageProcessingFlow (around the contact re-load logic) to reference
"contact form changes" instead of "case form changes" so it accurately describes
why the person is reloaded; keep the rest of the comment intact and ensure it
still points to ContactController#getContactCreateComponent and the surrounding
rationale for re-fetching the person after automatic processing.
---
Nitpick comments:
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.java`:
- Around line 183-186: The code repeatedly instantiates a new ObjectMapper when
deserializing additionalPersonContactDetails in ExternalMessageMapper; extract a
single shared instance by adding a private static final ObjectMapper (e.g.,
private static final ObjectMapper OBJECT_MAPPER) to the ExternalMessageMapper
class, initialize/configure it once, and replace new ObjectMapper() usages (the
call that produces List<PersonContactDetailDto> additionalDetails via readValue)
with OBJECT_MAPPER to avoid repeated instantiation and leverage ObjectMapper's
thread-safety.
- Around line 355-358: The method in ExternalMessageMapper currently always
returns Collections.singletonList(new String[] { PersonDto.ADDRESS }), which
causes ADDRESS to be reported changed even when no address data was provided;
change the logic so PersonDto.ADDRESS is only included when at least one address
subfield from the external message is present and actually differs from the
target Person's address (or when any external address field is non-null
indicating a change), otherwise return an empty list (Collections.emptyList()).
Locate the return that builds the singleton with PersonDto.ADDRESS and replace
it with a conditional that inspects the relevant external message address fields
and compares them to the existing Person/DTO address values before adding
PersonDto.ADDRESS to the changed-fields list.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java`:
- Around line 409-415: The current hyphen-based name split in
DoctorDeclarationMessageProcessingFlow (externalMessage.getReporterName(),
notifierDto.setAgentFirstName, notifierDto.setAgentLastName) can mis-handle
compound surnames; either replace the heuristic with a safer rule (e.g. split on
the last hyphen only by using lastIndexOf('-') to take substring(0,lastIndex) as
first name and substring(lastIndex+1) as last name) or, if the source has a
known convention, add a clear code comment and Javadoc documenting the expected
reporterName format and add a defensive log/warning when multiple hyphens are
detected so downstream users are aware; update the block around
externalMessage.getReporterName() accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7f63ee8-01b3-4f0b-ab5c-f0e3c6ba7b48
📒 Files selected for processing (5)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.javasormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageMapper.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java
.../java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java
Show resolved
Hide resolved
...i/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java`:
- Around line 347-355: The code in CaseNotifierSideViewController
unconditionally overwrites surveillanceReport.reportDate from
notifierForm.getNotificationDate(), but CaseNotifierForm.populateFields always
seeds notificationDateField so the else branch is dead and existing timestamps
get reset; change the logic to only call surveillanceReport.setReportDate(...)
if the user actually changed the notification date (e.g., check a dirty/modified
flag on the notificationDateField or compare notifierForm.getNotificationDate()
to the original surveillanceReport.getReportDate() date portion and only update
when they differ, or when surveillanceReport.getReportDate() is null), ensuring
you reference notifierForm.getNotificationDate(),
CaseNotifierForm.populateFields, surveillanceReport.getReportDate(), and
surveillanceReport.setReportDate() when locating where to apply this guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30375ce0-b76e-4f87-92d2-117965174ca3
📒 Files selected for processing (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
Show resolved
Hide resolved
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13872 |
2 similar comments
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13872 |
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13872 |
KarnaiahPesula
approved these changes
Mar 10, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java (1)
254-259: Drop the blame from this inline comment.The controller rationale is useful, but the last sentence reads like a commit aside rather than durable source documentation.
✏️ Suggested cleanup
// we need to call again the updates here // compared to automatic processing the person is processed by the contact controller // `@see`{ContactController#getContactCreateComponent} methods // we need to load the person again from the database to get the updates following contact form changes - // ofc. here we have another way to do it because whoever did it was too lazy to fix the contact controller final ContactDto processedContact = contactCreateComponent.getWrappedComponent().getValue();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java` around lines 254 - 259, The inline comment in LabMessageProcessingFlow.java contains an unprofessional/blameful sentence; edit the comment above the line that reads final ContactDto processedContact = contactCreateComponent.getWrappedComponent().getValue(); to keep the useful rationale (why the person must be reloaded and the reference to ContactController#getContactCreateComponent) but remove the last sentence that blames the original author — make the comment concise and factual about reloading the person for contact form updates and why the additional update call is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.java`:
- Around line 254-259: The inline comment in LabMessageProcessingFlow.java
contains an unprofessional/blameful sentence; edit the comment above the line
that reads final ContactDto processedContact =
contactCreateComponent.getWrappedComponent().getValue(); to keep the useful
rationale (why the person must be reloaded and the reference to
ContactController#getContactCreateComponent) but remove the last sentence that
blames the original author — make the comment concise and factual about
reloading the person for contact form updates and why the additional update call
is required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58a6784b-4488-4a82-9b41-60b16add132d
📒 Files selected for processing (4)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/labmessage/LabMessageProcessingFlow.javasormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/processing/ExternalMessageProcessingUIHelper.java
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13872 |
KarnaiahPesula
deleted the
bugfix-13625-add_handling_person_multiple_contact_info
branch
raulbob
restored the
bugfix-13625-add_handling_person_multiple_contact_info
branch
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
3 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 #13625 - Added handling of person multiple contact email and phone
Summary by CodeRabbit
New Features
Improvements
Tests