#13589 - Added update of person address when external message is proc…#13644
Conversation
…essed - Deprecated PhysiciansReport classes due to not being used anymore - Deprecated ExternalMessageController.processPhysiciansReport() due to not being used anymore
WalkthroughAdds a person-update hook to the external message processing flow: an abstract Changes
Sequence Diagram(s)sequenceDiagram
participant Flow as AbstractProcessingFlow
participant Base as AbstractMessageProcessingFlowBase
participant Facade as ExternalMessageProcessingFacade
participant PersonSvc as PersonFacade
Flow->>Flow: personSelection continuation
Flow->>Base: doPersonUpdates(personSelection)
activate Base
Base->>Base: extract PersonDto & message Location
opt address fields present
Base->>Base: compare and set houseNo/street/city/postal/country
alt any change
Base->>Facade: updatePerson(personDto)
activate Facade
Facade->>PersonSvc: save(personDto)
Facade-->>Base: saved
deactivate Facade
else no change
Base-->>Flow: return (no-op)
end
end
deactivate Base
Flow->>Flow: continue processing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.39.7)sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/processing/labmessage/AbstractLabMessageProcessingFlowTest.javaThanks 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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java (1)
40-40: Document the deprecation with JavaDoc.The
@Deprecatedannotation should be accompanied by a JavaDoc@deprecatedtag explaining why this class is deprecated, what should be used instead, and when it will be removed.Apply this diff to add proper deprecation documentation:
+/** + * @deprecated This physicians report processing flow is deprecated. Use the new external message processing flow instead. + */ @Deprecated public abstract class AbstractPhysiciansReportProcessingFlow extends AbstractProcessingFlow {sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseImmunizationsComponent.java (1)
63-63: Document the deprecation with JavaDoc.The
@Deprecatedannotation should be accompanied by a JavaDoc@deprecatedtag explaining why this component is deprecated, what should be used instead, and when it will be removed.Apply this diff to add proper deprecation documentation:
+/** + * @deprecated This component is deprecated as part of the physicians report flow deprecation. + */ @Deprecated public class PhysiciansReportCaseImmunizationsComponent extends CommitDiscardWrapperComponent<VerticalLayout> {sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.java (1)
212-212: Document the deprecation with JavaDoc.The
@Deprecatedannotation should be accompanied by a JavaDoc@deprecatedtag explaining why this method is deprecated, what should be used instead, and when it will be removed.Apply this diff to add proper deprecation documentation:
+ /** + * @deprecated This method is deprecated as part of the physicians report flow deprecation. + */ @Deprecated public void processPhysiciansReport(String uuid) {sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseEditComponent.java (1)
45-45: Document the deprecation with JavaDoc.The
@Deprecatedannotation should be accompanied by a JavaDoc@deprecatedtag explaining why this component is deprecated, what should be used instead, and when it will be removed.Apply this diff to add proper deprecation documentation:
+/** + * @deprecated This component is deprecated as part of the physicians report flow deprecation. + */ @Deprecated public class PhysiciansReportCaseEditComponent extends CommitDiscardWrapperComponent<VerticalLayout> {sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java (1)
44-44: Document the deprecation with JavaDoc.The
@Deprecatedannotation should be accompanied by a JavaDoc@deprecatedtag explaining why this class is deprecated, what should be used instead, and when it will be removed.Apply this diff to add proper deprecation documentation:
+/** + * @deprecated This processing flow is deprecated as part of the physicians report flow deprecation. + */ @Deprecated public class PhysiciansReportProcessingFlow extends AbstractPhysiciansReportProcessingFlow {sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageProcessingFacade.java (1)
250-259: Remove redundant null check and consider adding logging.The
personFacadenull check at line 251 is unnecessary sincepersonFacadeis aprotected finalfield initialized in the constructor (line 108). This check can never be true in normal operation.Additionally, consider adding debug logging to track when person updates occur for better observability.
Apply this diff to improve the implementation:
public void updatePerson(PersonDto personDto) { - if(personFacade == null) { - return; - } if(personDto == null) { return; } personFacade.save(personDto); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java(3 hunks)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java(2 hunks)sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/ExternalMessageProcessingFacade.java(1 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.java(1 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java(2 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseEditComponent.java(1 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseImmunizationsComponent.java(1 hunks)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseEditComponent.java (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java (1)
Deprecated(44-109)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java (3)
sormas-api/src/main/java/de/symeda/sormas/api/utils/dataprocessing/EntitySelection.java (1)
EntitySelection(18-40)sormas-api/src/main/java/de/symeda/sormas/api/utils/dataprocessing/HandlerCallback.java (1)
HandlerCallback(20-35)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java (1)
Deprecated(44-109)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/ExternalMessageController.java (4)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java (1)
Deprecated(40-146)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseEditComponent.java (1)
Deprecated(45-239)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseImmunizationsComponent.java (1)
Deprecated(63-353)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java (1)
Deprecated(44-109)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportProcessingFlow.java (4)
sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/AbstractPhysiciansReportProcessingFlow.java (1)
Deprecated(40-146)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseEditComponent.java (1)
Deprecated(45-239)sormas-ui/src/main/java/de/symeda/sormas/ui/externalmessage/physiciansreport/PhysiciansReportCaseImmunizationsComponent.java (1)
Deprecated(63-353)sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java (1)
Deprecated(2494-2508)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/utils/dataprocessing/EntitySelection.java (1)
EntitySelection(18-40)
⏰ 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). (4)
- GitHub Check: android app test (26)
- GitHub Check: android app test (27)
- GitHub Check: android app test (28)
- GitHub Check: SORMAS CI
🔇 Additional comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractProcessingFlow.java (1)
154-178: LGTM! Well-designed extension point.The addition of the
doPersonUpdateshook is well-placed in the processing flow, invoked after person selection and before building the result. The abstract method ensures all subclasses must provide an implementation (even if empty), making the contract explicit.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/processing/labmessage/AbstractLabMessageProcessingFlowTest.java (1)
448-451: LGTM! Test data enrichment looks good.The addition of
lastNameandsexfields enriches the test data for the person selection scenario, which is helpful for ensuring the lab message processing flow handles these fields correctly.Minor observation: The lastName "Ftest" matches the firstName, creating "Ftest Ftest", while
testCreatePerson()uses distinct values ("Ftest"/"Ltest"). This inconsistency is minor since it's just test fixture data, but using distinct values might make the test data clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/externalmessage/processing/labmessage/AbstractLabMessageProcessingFlowTest.java(1 hunks)
⏰ 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). (4)
- GitHub Check: android app test (28)
- GitHub Check: android app test (26)
- GitHub Check: android app test (27)
- GitHub Check: SORMAS CI
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13644 |
Implements #13589.
Summary by CodeRabbit