#13589 - Adds person phone and email update to external message processing flow#13834
Merged
raulbob merged 1 commit intodevelopmentfrom Feb 17, 2026
Merged
Conversation
…ssing flow - Additionally to the existing address update, the phone and email are now updated as well.
📝 WalkthroughWalkthroughThe change enhances person data processing in external messages by adding automatic updates for phone numbers and email addresses alongside existing address updates. A doUpdate flag tracks whether changes occurred, and contact details are created or promoted to primary as needed before persisting changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
🧹 Nitpick comments (4)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java (4)
1123-1137: DuplicatesetPrimaryContact(true)call and ordering issue when creating a new phoneLine 1125 and Line 1127 both call
setPrimaryContact(true)— the second is redundant. More importantly, the old primary is demoted after the new entry is added to the list (Line 1131 vs Line 1134). While functionally this still works because the objects are independent, demoting the old primary before adding the new one would be clearer and consistent with theexistingPhone != nullbranch above.Proposed cleanup
} else { // we do not have the new phone number in the list so we need to create a new one + // demote old primary first + if (primaryPhone != null) { + primaryPhone.setPrimaryContact(false); + } final PersonContactDetailDto personContactDetail = new PersonContactDetailDto(); personContactDetail.setPerson(person.toReference()); personContactDetail.setPrimaryContact(true); personContactDetail.setPersonContactDetailType(PersonContactDetailType.PHONE); - personContactDetail.setPrimaryContact(true); personContactDetail.setPhoneNumberType(phoneNumberType); personContactDetail.setContactInformation(phoneNumber); personContactDetail.setThirdParty(false); personContactDetails.add(personContactDetail); - - // we need to set the old primary to false - if(primaryPhone != null) { - primaryPhone.setPrimaryContact(false); - } }🤖 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 1123 - 1137, Remove the duplicate personContactDetail.setPrimaryContact(true) call and move the demotion of the previous primary before adding the new phone to the list: in AbstractMessageProcessingFlowBase where personContactDetail is created, call setPrimaryContact(true) only once on the new PersonContactDetailDto, ensure primaryPhone.setPrimaryContact(false) is executed before personContactDetails.add(personContactDetail), and keep behavior consistent with the existingPhone != null branch so the old primary is demoted prior to adding the new entry.
1097-1181: Phone and email update blocks are near-identical — extract a helperThe phone block (Lines 1102–1140) and email block (Lines 1144–1181) share the same structure: find existing primary, find matching entry, promote or create. The only differences are the
PersonContactDetailType, the contact information value, and the optionalphoneNumberType. Extracting a common method (e.g.,upsertPrimaryContactDetail(...)) would eliminate ~40 lines of duplication and make this easier to maintain.Sketch of extracted helper
private boolean upsertPrimaryContactDetail( PersonDto person, List<PersonContactDetailDto> contactDetails, PersonContactDetailType type, String contactInfo, PhoneNumberType phoneNumberType) { if (contactInfo == null || contactInfo.isBlank()) { return false; } PersonContactDetailDto primary = contactDetails.stream() .filter(cd -> cd.getPersonContactDetailType() == type && !cd.isThirdParty() && cd.isPrimaryContact()) .findFirst().orElse(null); PersonContactDetailDto existing = contactDetails.stream() .filter(cd -> cd.getPersonContactDetailType() == type && !cd.isThirdParty() && contactInfo.equals(cd.getContactInformation())) .findFirst().orElse(null); if (existing != null) { if (primary != null) { primary.setPrimaryContact(false); } existing.setPrimaryContact(true); } else { if (primary != null) { primary.setPrimaryContact(false); } PersonContactDetailDto newDetail = new PersonContactDetailDto(); newDetail.setPerson(person.toReference()); newDetail.setPrimaryContact(true); newDetail.setPersonContactDetailType(type); newDetail.setPhoneNumberType(phoneNumberType); newDetail.setContactInformation(contactInfo); newDetail.setThirdParty(false); contactDetails.add(newDetail); } return true; }🤖 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 1097 - 1181, The phone and email handling in AbstractMessageProcessingFlowBase are duplicated; implement a helper method upsertPrimaryContactDetail(PersonDto person, List<PersonContactDetailDto> contactDetails, PersonContactDetailType type, String contactInfo, PhoneNumberType phoneNumberType) that encapsulates the shared logic (skip if null/blank, find primary and existing via getPersonContactDetailType/isThirdParty/isPrimaryContact and contactInformation, promote existing or create new PersonContactDetailDto, ensure old primary is unset, set phoneNumberType only when provided) and returns a boolean indicating whether an update was made; then replace both inline blocks to call this helper (pass PhoneNumberType for phone and null for email) and set doUpdate |= upsertPrimaryContactDetail(...).
1092-1095:doUpdateset unconditionally whenpersonAddressis non-null
doUpdateis set totrueeven when every address field from the external message isnull(i.e., nothing actually changed). This causes a needlessupdatePersoncall. Consider tracking whether at least one field was actually written.🤖 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 1092 - 1095, The code currently sets doUpdate = true whenever personAddress is non-null, causing updatePerson to be called even if no address fields were actually modified; change the logic in AbstractMessageProcessingFlowBase so that you track whether at least one field was written (e.g., introduce a boolean flag like addressFieldsChanged) while assigning to personAddress fields and only set doUpdate = true if that flag is true (or if other change flags indicate modifications); ensure this check is performed before calling updatePerson and reference the doUpdate variable, personAddress assignments, and the updatePerson invocation when making the change.
1163-1178: Same duplicatesetPrimaryContact(true)and ordering issue in the email branchLines 1167 and 1169 duplicate
setPrimaryContact(true). Same cleanup as the phone branch applies here.Proposed cleanup
} else { + if (primaryEmail != null) { + primaryEmail.setPrimaryContact(false); + } final PersonContactDetailDto personContactDetail = new PersonContactDetailDto(); personContactDetail.setPerson(person.toReference()); personContactDetail.setPrimaryContact(true); personContactDetail.setPersonContactDetailType(PersonContactDetailType.EMAIL); - personContactDetail.setPrimaryContact(true); personContactDetail.setContactInformation(emailAddress); personContactDetail.setThirdParty(false); personContactDetails.add(personContactDetail); - - if(primaryEmail != null) { - primaryEmail.setPrimaryContact(false); - } }🤖 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 1163 - 1178, Remove the duplicate personContactDetail.setPrimaryContact(true) call and ensure primaryEmail is demoted before marking the new contact as primary: in the email branch involving PersonContactDetailDto, call primaryEmail.setPrimaryContact(false) (if primaryEmail != null) before setting personContactDetail.setPrimaryContact(true) and adding personContactDetail to personContactDetails, and only call setPrimaryContact(true) once.
🤖 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-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java`:
- Around line 1123-1137: Remove the duplicate
personContactDetail.setPrimaryContact(true) call and move the demotion of the
previous primary before adding the new phone to the list: in
AbstractMessageProcessingFlowBase where personContactDetail is created, call
setPrimaryContact(true) only once on the new PersonContactDetailDto, ensure
primaryPhone.setPrimaryContact(false) is executed before
personContactDetails.add(personContactDetail), and keep behavior consistent with
the existingPhone != null branch so the old primary is demoted prior to adding
the new entry.
- Around line 1097-1181: The phone and email handling in
AbstractMessageProcessingFlowBase are duplicated; implement a helper method
upsertPrimaryContactDetail(PersonDto person, List<PersonContactDetailDto>
contactDetails, PersonContactDetailType type, String contactInfo,
PhoneNumberType phoneNumberType) that encapsulates the shared logic (skip if
null/blank, find primary and existing via
getPersonContactDetailType/isThirdParty/isPrimaryContact and contactInformation,
promote existing or create new PersonContactDetailDto, ensure old primary is
unset, set phoneNumberType only when provided) and returns a boolean indicating
whether an update was made; then replace both inline blocks to call this helper
(pass PhoneNumberType for phone and null for email) and set doUpdate |=
upsertPrimaryContactDetail(...).
- Around line 1092-1095: The code currently sets doUpdate = true whenever
personAddress is non-null, causing updatePerson to be called even if no address
fields were actually modified; change the logic in
AbstractMessageProcessingFlowBase so that you track whether at least one field
was written (e.g., introduce a boolean flag like addressFieldsChanged) while
assigning to personAddress fields and only set doUpdate = true if that flag is
true (or if other change flags indicate modifications); ensure this check is
performed before calling updatePerson and reference the doUpdate variable,
personAddress assignments, and the updatePerson invocation when making the
change.
- Around line 1163-1178: Remove the duplicate
personContactDetail.setPrimaryContact(true) call and ensure primaryEmail is
demoted before marking the new contact as primary: in the email branch involving
PersonContactDetailDto, call primaryEmail.setPrimaryContact(false) (if
primaryEmail != null) before setting personContactDetail.setPrimaryContact(true)
and adding personContactDetail to personContactDetails, and only call
setPrimaryContact(true) once.
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13834 |
KarnaiahPesula
approved these changes
Feb 17, 2026
raulbob
deleted the
bugfix-13589-external_message_update_person_phone_email
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 #13589
Summary by CodeRabbit