-
Notifications
You must be signed in to change notification settings - Fork 160
#13717 - Added additional missing fields for person #13792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
#13717 - Added additional missing fields for person #13792
Conversation
📝 WalkthroughWalkthroughAdds support for a personOccupation field across API DTO, backend entity, DB schema, and processing flow; updates country facade access rights and augments doctor-declaration processing to map occupation, guardian, and third-party contact details during case creation. Changes
Sequence Diagram(s)sequenceDiagram
actor Sender
participant UI as DoctorDeclarationFlow
participant FACADE as ExternalMessageProcessingFacade
participant ENUM as CustomizableEnumFacade
participant PERSON as PersonService
participant DB as Database
Sender->>UI: Submit doctor declaration (includes occupation, guardian, contacts)
UI->>FACADE: Process external message
FACADE->>ENUM: lookup OCCUPATION_TYPE "OTHER" (getOccupationTypeOther)
ENUM-->>FACADE: OccupationType ("OTHER")
FACADE->>PERSON: Create/Update Person (set occupation, guardian name, add contact details)
PERSON->>DB: Persist person and contact details
DB-->>PERSON: Persisted
PERSON-->>FACADE: Person updated
FACADE->>DB: Create/Update Case referencing updated person
DB-->>FACADE: Case created/updated
FACADE-->>UI: Processing complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageProcessingFacade.java`:
- Around line 245-247: Replace the non-throwing call in getOccupationTypeOther()
that uses
customizableEnumFacade.getEnumValue(CustomizableEnumType.OCCUPATION_TYPE, null,
"OTHER") with the throwing variant used elsewhere (see how getDiseaseVariant
calls the facade) so that a missing "OTHER" occupation constant triggers
CustomEnumNotFoundException instead of returning null; update
getOccupationTypeOther() to call the same throwing signature on
customizableEnumFacade for OCCUPATION_TYPE/"OTHER".
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java`:
- Around line 385-419: Replace the null-unsafe contains() checks in
DoctorDeclarationMessageProcessingFlow with exact, null-safe equality checks:
for the email block use
externalMessage.getPersonGuardianEmail().equals(pc.getContactInformation())
inside the stream noneMatch (since externalMessage.getPersonGuardianEmail() is
already checked non-null/non-blank), and do the same for the phone block using
externalMessage.getPersonGuardianPhone().equals(pc.getContactInformation());
this prevents partial matches and avoids NPE when
PersonContactDetailDto.getContactInformation() is null.
- Around line 366-368: The code fetches PersonDto casePerson via
getExternalMessageProcessingFacade().getPersonByContext(PersonContext.CASE,
result.getUuid()) but only checks for null later, which risks NPE because
casePerson is used between lines 369–426; immediately after calling
getPersonByContext move the null check for casePerson (and early-return or
handle accordingly) so no subsequent code that references casePerson runs when
it's null, and then remove the delayed null check further down; keep doUpdate
handling and any subsequent logic intact.
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java (1)
385-419: Consider extracting contact detail creation into a helper method.The logic for adding guardian email (lines 385-401) and phone (lines 403-419) contact details is nearly identical. Extracting this into a helper method would reduce duplication and improve maintainability.
♻️ Example helper method
private boolean addGuardianContactDetailIfAbsent( PersonDto casePerson, String contactInfo, PersonContactDetailType type, String guardianName, String guardianRelationship) { if (contactInfo == null || contactInfo.isBlank()) { return false; } List<PersonContactDetailDto> contactDetails = casePerson.getPersonContactDetails(); if (contactDetails.stream().noneMatch(pc -> contactInfo.equals(pc.getContactInformation()))) { PersonContactDetailDto pcd = new PersonContactDetailDto(); pcd.setPerson(casePerson.toReference()); pcd.setPrimaryContact(false); pcd.setPersonContactDetailType(type); pcd.setContactInformation(contactInfo); pcd.setThirdParty(true); pcd.setThirdPartyRole(guardianRelationship); pcd.setThirdPartyName(guardianName); contactDetails.add(pcd); return true; } return false; }
...in/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageProcessingFacade.java
Outdated
Show resolved
Hide resolved
...meda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java
Show resolved
Hide resolved
...meda/sormas/ui/externalmessage/doctordeclaration/DoctorDeclarationMessageProcessingFlow.java
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13792 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13792 |
Fixes #13717
Summary by CodeRabbit
New Features
System Updates
✏️ Tip: You can customize this high-level summary in your review settings.