#13751 - Diagnosis date determined based on surveillance report #13803
Conversation
- Also moved the logic from the UI view to the controller
📝 WalkthroughWalkthroughAPI now sets surveillance report diagnosis dates from external messages during processing; a DB migration backfills existing reports from external messages; UI notifier was refactored to accept explicit diagnosis and notification dates instead of an oldest report DTO. Changes
Sequence Diagram(s)sequenceDiagram
participant ExternalMsg as ExternalMessage Processor
participant API as MessageProcessing API
participant DB as Database
participant UI as Notifier UI
ExternalMsg->>API: ingest external message (includes diagnosticDate)
API->>API: create/finalize SurveillanceReportDto
API->>API: updateSurveillanceReportAdditionalData(survReport, externalMsg, case)
API->>DB: persist SurveillanceReport (dateOfDiagnosis set)
DB-->>API: persist confirmation
UI->>DB: query case / surveillance reports
DB-->>UI: return notificationDate, dateOfDiagnosis
UI-->>User: render notifier with explicit dates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (1)
313-356: Bug:caze.setTherapy(therapy)is never reached.The method modifies the
therapyobject but each branch has an earlyreturnstatement, making line 355 (caze.setTherapy(therapy)) unreachable. The therapy changes are lost and never assigned back to the case.Proposed fix
private void updateTherapyBasedOnSelection(CaseDataDto caze, TreatmentOption selectedOption) { if (selectedOption == null) { return; } TherapyDto therapy = caze.getTherapy(); if (therapy == null) { therapy = TherapyDto.build(); + caze.setTherapy(therapy); } if (selectedOption.equals(TreatmentOption.YES)) { therapy.setTreatmentStarted(YesNoUnknown.YES); therapy.setTreatmentNotApplicable(false); if (therapy.getTreatmentStartDate() == null) { therapy.setTreatmentStartDate(new java.util.Date()); } - return; + } else if (selectedOption.equals(TreatmentOption.NO)) { - } - - if (selectedOption.equals(TreatmentOption.NO)) { therapy.setTreatmentStarted(YesNoUnknown.NO); therapy.setTreatmentNotApplicable(false); therapy.setTreatmentStartDate(null); - return; + } else if (selectedOption.equals(TreatmentOption.NOT_APPLICABLE)) { - } - - if (selectedOption.equals(TreatmentOption.NOT_APPLICABLE)) { therapy.setTreatmentNotApplicable(true); therapy.setTreatmentStarted(null); therapy.setTreatmentStartDate(null); - return; + } else if (selectedOption.equals(TreatmentOption.UNKNOWN)) { - } - - if (selectedOption.equals(TreatmentOption.UNKNOWN)) { therapy.setTreatmentStarted(YesNoUnknown.UNKNOWN); therapy.setTreatmentNotApplicable(false); therapy.setTreatmentStartDate(null); - return; } - - caze.setTherapy(therapy); }sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewContent.java (2)
61-77: Outdated Javadoc.The Javadoc at lines 68-70 still references
oldestReportparameter, but the constructor signature was changed to acceptdateOfDiagnosisandnotificationDate.Proposed fix
/** * Creates a new case notifier side view content. * * `@param` caze * the case data * `@param` notifier * the notifier details - * `@param` oldestReport - * the oldest surveillance report for the case + * `@param` dateOfDiagnosis + * the date of diagnosis (may be null) + * `@param` notificationDate + * the notification date (may be null) */ public CaseNotifierSideViewContent(CaseDataDto caze, NotifierDto notifier, Date dateOfDiagnosis, Date notificationDate) {
149-161: Potential NPE onnotifier.getAgentLastName().toUpperCase().The condition at lines 149-150 uses
||, so the block is entered if eitheragentFirstNameoragentLastNameis non-null/non-blank. However, line 158 unconditionally callstoUpperCase()ongetAgentLastName(), which will throw NPE if onlyagentFirstNameis set.Proposed fix
if ((notifier.getAgentFirstName() != null && !notifier.getAgentFirstName().isBlank()) || (notifier.getAgentLastName() != null && !notifier.getAgentLastName().isBlank())) { // Spacer before Reporting Agent label Label spacerAgent = new Label(); spacerAgent.setHeight("0.1rem"); addComponent(spacerAgent); Label reportingAgent = new Label(I18nProperties.getCaption(Captions.Notification_reportingAgent)); CssStyles.style(reportingAgent, CssStyles.LABEL_BOLD, CssStyles.LABEL_BOTTOM_LINE); addComponent(reportingAgent); - Label agentNameLabel = new Label(notifier.getAgentFirstName() + " " + notifier.getAgentLastName().toUpperCase(Locale.ROOT)); + String firstName = notifier.getAgentFirstName() != null ? notifier.getAgentFirstName() : ""; + String lastName = notifier.getAgentLastName() != null ? notifier.getAgentLastName().toUpperCase(Locale.ROOT) : ""; + Label agentNameLabel = new Label((firstName + " " + lastName).trim()); CssStyles.style(agentNameLabel, CssStyles.LABEL_RELEVANT); addComponent(agentNameLabel); }
🤖 Fix all issues with AI agents
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15068-15073: The UPDATE currently pulls an arbitrary
em.diagnosticdate when multiple externalmessage rows match a surveillancereport
(surveillancereport_id), so change the update to deterministically pick a single
date (e.g., oldest or most recent) by joining against an aggregated subquery
that selects MIN(em.diagnosticdate) or MAX(em.diagnosticdate) grouped by
em.surveillancereport_id and only include rows where sr.dateofdiagnosis IS NULL;
reference the tables/columns surveillancereports sr, externalmessage em,
em.diagnosticdate and em.surveillancereport_id and replace the direct FROM em
with a join to that aggregated result so the same selection logic is applied
consistently for each sr.id.
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java`:
- Around line 112-124: The inline comment in getLatestReport incorrectly says
"Filter to get the oldest report" while the code uses
.max(Comparator.comparing(SurveillanceReportDto::getReportDate)) to return the
latest report; update that comment (inside
CaseNotifierSideViewController.getLatestReport) to accurately state that it
selects the latest report (by report date) or remove the misleading comment
entirely so the behavior and comment match.
- Around line 57-70: The method in CaseNotifierSideViewController calls
getOldestReport(caze) twice and leaves an incomplete comment; fix by calling
getOldestReport(caze) once, store its result in a local variable (e.g.,
oldestReport) and use oldestReport.getReportDate() for notificationDate to avoid
duplicate DB queries, remove the stray comment "// find diagno", and then
construct and return the CaseNotifierSideViewContent(caze, notifier,
dateOfDiagnosis, notificationDate) using the cached oldestReport value; keep
references to getOldestDiagnosisDateFromReports and
FacadeProvider.getNotifierFacade() unchanged.
🧹 Nitpick comments (2)
sormas-api/src/main/java/de/symeda/sormas/api/externalmessage/processing/AbstractMessageProcessingFlowBase.java (1)
694-706: Unusedcazeparameter.The
cazeparameter is declared but never used in the method body. If it's intended for future use or subclass overrides, consider documenting this in the Javadoc. Otherwise, remove it to avoid confusion.Option 1: Remove unused parameter
- protected void updateSurveillanceReportAdditionalData(SurveillanceReportDto surveillanceReport, ExternalMessageDto externalMessage, CaseDataDto caze) { + protected void updateSurveillanceReportAdditionalData(SurveillanceReportDto surveillanceReport, ExternalMessageDto externalMessage) { surveillanceReport.setDateOfDiagnosis(externalMessage.getDiagnosticDate()); }Note: This would also require updating the call site at line 665.
Option 2: Document the parameter for extensibility
/** * Updates the additional data of the surveillance report. * * `@param` surveillanceReport * the surveillance report * `@param` externalMessage * the external message * `@param` caze - * the case + * the case (available for subclass overrides that may need case-specific logic) */sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (1)
367-369: Empty TODO method - verify if implementation is needed.The
handleDiagnosticDateUpdatemethod is called at line 279 but does nothing. If the diagnosis date handling is intentionally delegated elsewhere (e.g., via surveillance reports), consider removing this method and its call site, or document why it's a no-op. Otherwise, this may indicate incomplete implementation of the PR objective.Do you want me to help implement this method or open an issue to track this task?
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
Show resolved
Hide resolved
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
Outdated
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13803 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java`:
- Around line 57-63: The code dereferences caze.getNotifier() in
CaseNotifierSideViewController when calling
FacadeProvider.getNotifierFacade().getByUuidAndTime(...), which can NPE because
CaseDataDto.notifier may be null; add a null guard matching the pattern used in
editNotifier(): check if caze.getNotifier() == null and handle it (e.g., throw
new IllegalArgumentException("Caze notifier is null") or return early) before
calling getUuid() or getVersionDate(), so the getByUuidAndTime call only
executes when notifier is non-null.
♻️ Duplicate comments (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java (2)
68-68: Remove the stray// find diagnocomment.Proposed fix
- // find diagno
122-124: Fix the misleading comment ingetLatestReport.
The method returns the latest report (usingmax), but the comment says “oldest.”Proposed fix
- // Filter to get the oldest report + // Filter to get the latest report
sormas-ui/src/main/java/de/symeda/sormas/ui/caze/notifier/CaseNotifierSideViewController.java
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13803 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13803 |
1 similar comment
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13803 |
Fixes #13751
Summary by CodeRabbit
Bug Fixes
UI
Database
✏️ Tip: You can customize this high-level summary in your review settings.