Feature 13772 epipulse ipi export#13790
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds Measles (MEAS) and Invasive Pneumococcal Infection (PNEU/IPI) Epipulse export: new DTO fields and CSV helpers, lab/vaccine mapping utilities, SQL CTE builder, orchestrator + strategy pattern for export generation, CSV strategies, and wiring in facade/service/timer to start disease-specific exports. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client / Facade
participant Orchestrator as EpipulseCsvExportOrchestrator
participant Config as EpipulseConfigurationLookupService
participant Strategy as ExportStrategy
participant DB as Database
participant FS as FileSystem
Client->>Orchestrator: orchestrateExport(uuid, ExportFunction, CsvStrategy)
Orchestrator->>Orchestrator: load & validate export by UUID
Orchestrator->>Orchestrator: claim export for processing
Orchestrator->>Config: lookupConfiguration(exportDto, countryIso2, countryName)
Config->>DB: query epipulse_location_configuration / country / subject config
DB-->>Config: EpipulseConfigurationContext
Config-->>Orchestrator: EpipulseConfigurationContext
Orchestrator->>Strategy: execute(exportDto, countryCode, countryName)
Strategy->>DB: run composed SQL query (CTEs + SELECT)
DB-->>Strategy: result rows
Strategy->>Strategy: map rows -> EpipulseDiseaseExportEntryDto list
Strategy-->>Orchestrator: EpipulseDiseaseExportResult
Orchestrator->>Orchestrator: build headers via CsvStrategy.buildColumnNames
Orchestrator->>FS: open/create CSV file
loop per entry
Orchestrator->>CsvStrategy: writeEntryRow(dto, exportLine, exportResult)
Orchestrator->>FS: write row
end
Orchestrator->>DB: update export status + metadata (COMPLETED/FAILED)
Orchestrator-->>Client: export finished
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseExportTimerEjb.java (1)
80-82: Fix broken error logging.The error log statement has placeholder tokens
{}but passes no arguments, and concatenatese.getMessage()instead. This will log malformed messages and lose the stack trace, making it harder to debug export failures for the newly added diseases.🐛 Proposed fix
- } catch (Exception e) { - logger.error("Error during scheduled export for UUID: {} of type: {}: " + e.getMessage()); + } catch (Exception e) { + logger.error("Error during scheduled export for UUID: {} of type: {}", uuid, subjectCodeStr, e);
🤖 Fix all issues with AI agents
In
`@sormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseDiseaseExportEntryDto.java`:
- Around line 1211-1219: The getClinicalPresentationForCsv method currently adds
all items from clinicalPresentation which can exceed maxCount and misalign CSV
columns; change the logic in getClinicalPresentationForCsv to only copy up to
maxCount entries from the clinicalPresentation list (e.g., use
clinicalPresentation.subList(0, Math.min(clinicalPresentation.size(), maxCount))
or loop with a min-bound) and then pad with empty strings until
presentations.size() == maxCount so the returned list is exactly maxCount long.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseCommonDtoMapper.java`:
- Around line 77-81: The code in EpipulseCommonDtoMapper reads subjectCodeFromDb
and silently leaves dto.subjectCode null when blank; change this to fail fast by
validating subjectCodeFromDb after reading it and throwing a clear
IllegalArgumentException (or custom exception) if it is null/blank, otherwise
call dto.setSubjectCode(EpipulseSubjectCode.valueOf(subjectCodeFromDb)); include
the offending value in the exception message to aid debugging.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseConfigurationLookupService.java`:
- Around line 110-125: lookupServerCountryNutsCode uses
countryName.toLowerCase() which depends on the default JVM locale and can
produce incorrect strings (eg. Turkish); change the lowering call to use a
locale-independent form such as countryName.toLowerCase(Locale.ROOT) and add the
necessary import for java.util.Locale so the query parameter uses the
normalized, locale-agnostic lowercased country name before passing it to
em.createNativeQuery.
- Around line 61-67: lookupConfiguration currently passes exportDto,
serverCountryLocale, and serverCountryName into helper lookups without null
checks which can cause NPEs (e.g., exportDto.getSubjectCode() and
subjectCodeEnum.name()); add early null-validation in lookupConfiguration to
throw IllegalArgumentException with clear messages if exportDto,
serverCountryLocale, or serverCountryName are null, and validate
exportDto.getSubjectCode() before using it; also update
lookupServerCountryNutsCode to use toLowerCase(Locale.ENGLISH) instead of
toLowerCase() to avoid locale-dependent behavior.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseSqlCteBuilder.java`:
- Around line 199-214: The returned CTE string in buildVaccinationsCte() lacks
the leading spaces used for alignment with other CTEs; update the string so it
begins with the same leading whitespace prefix as buildImmunizationsCte() (i.e.,
add the same number of spaces before "case_all_vaccinations AS (SELECT...") to
match formatting and keep the //@formatter:off/on blocks intact.
- Around line 176-192: The CTE returned by buildImmunizationsCte() lacks the
leading whitespace/padding other CTEs include, which breaks alignment when
concatenated; update the returned string in buildImmunizationsCte() so the first
token is prefixed with the same leading spaces used by other CTE builders (i.e.,
add the missing spaces before "case_all_immunizations AS ("), preserving the
//@formatter:off/@formatter:on markers and existing string content and
concatenation.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiCsvExportStrategy.java`:
- Around line 26-38: Update the class Javadoc in IpiCsvExportStrategy to reflect
the actual column counts: change the "23 vaccination fields" phrase to "19
vaccination fields" and the "9 AST fields" phrase to "10 AST fields", keeping
the total count noted as 49; ensure the descriptive lines under "Column
structure" match the implemented field distribution in the class.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.java`:
- Around line 270-305: In buildPneuVaccinationDetailsCte() the
pcv_row_num/ppv_row_num computed with ROW_NUMBER() still counts non-PCV/PPV
vaccines, causing mislabeling; replace those ROW_NUMBER() expressions with
running counts that only increment for matching vaccines (e.g., pcv_row_num =
SUM(CASE WHEN is_pcv THEN 1 ELSE 0 END) OVER (PARTITION BY c.id ORDER BY
v.vaccinationdate) and similarly for ppv_row_num) and ensure downstream SELECTs
gate by is_pcv/is_ppv (or include AND is_pcv / AND is_ppv in the CASE WHEN
checks) so only true PCV/PPV doses produce date_pcv#/brand_pcv# and date_ppv.
Keep references to pcv_row_num, ppv_row_num, is_pcv, is_ppv and the method name
IpiExportStrategy.buildPneuVaccinationDetailsCte.
🧹 Nitpick comments (4)
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/util/EpipulseConfigurationContext.java (1)
25-59: LGTM!Clean immutable DTO with proper serialization support and documentation.
Consider adding a
toString()method for easier debugging of export issues.♻️ Optional: Add toString for debugging
public String getSubjectCode() { return subjectCode; } + + `@Override` + public String toString() { + return "EpipulseConfigurationContext{" + + "reportingCountry='" + reportingCountry + '\'' + + ", serverCountryNutsCode='" + serverCountryNutsCode + '\'' + + ", subjectCode='" + subjectCode + '\'' + + '}'; + } }sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/AbstractEpipulseDiseaseExportStrategy.java (1)
117-120: Use parameterized logging instead of string concatenation.String concatenation in log statements incurs overhead even when the log level is disabled.
♻️ Suggested fix
} catch (Exception e) { - logger.error("Error while exporting case based " + exportDto.getSubjectCode() + ":" + e.getMessage(), e); + logger.error("Error while exporting case based {}: {}", exportDto.getSubjectCode(), e.getMessage(), e); throw e; }sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseDiseaseExportService.java (1)
50-50: Remove unused fieldDB_DATE_FORMAT.This field is declared but never used within the file or the broader epipulse export strategy classes. It can be safely removed.
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseCsvExportOrchestrator.java (1)
110-136: Make path handling OS-safe and ensure export directory exists.String concatenation assumes POSIX separators and a pre-existing directory. Creating the directory and using
Paths.get(...)avoids failures on fresh installs or non-standard paths.🛠 Proposed fix
- String generatedFilesPath = configFacadeEjb.getGeneratedFilesPath(); - exportFileName = diseaseExportService.generateDownloadFileName(exportDto, epipulseExport.getId()); - exportFilePath = generatedFilesPath + "/" + exportFileName; + String generatedFilesPath = configFacadeEjb.getGeneratedFilesPath(); + Files.createDirectories(Paths.get(generatedFilesPath)); + exportFileName = diseaseExportService.generateDownloadFileName(exportDto, epipulseExport.getId()); + exportFilePath = Paths.get(generatedFilesPath, exportFileName).toString();
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13790 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.java`:
- Around line 74-205: The Javadoc for mapDiseaseSpecificFields is out of sync:
mapDiseaseSpecificFields and buildPneuSelectClause actually handle 28
PNEU-specific fields (leading to 56 total columns) but the comment says 21 PNEU
fields and 49 total; fix by reconciling documentation with implementation—either
(preferred) update the Javadoc on IpiExportStrategy.mapDiseaseSpecificFields to
state "28 PNEU-specific fields (56 total columns)" and adjust the field number
annotations in the method, or (if you prefer to match docs) remove the 7 extra
fields from buildPneuSelectClause and mapDiseaseSpecificFields so they only map
the documented 21 fields; ensure the index increments in
mapDiseaseSpecificFields remain consistent with buildPneuSelectClause after the
change.
♻️ Duplicate comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.java (1)
270-300: PCV/PPV row numbers never increment past 1.The LATERAL subquery has no FROM, so the window functions run over a single-row result set per joined vaccination row. That makes
pcv_row_num/ppv_row_numeffectively 0/1, which collapses PCV1–4 into a single “latest” dose and leaves later dose slots empty.Consider computing the window functions in a derived table that includes the vaccination rows, then aggregate by case:
🐛 Suggested fix (compute row numbers over vaccination rows)
- return "pneu_vaccination_details AS (" + - " SELECT c.id as case_id," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 1 THEN v.vaccinationdate END) as date_pcv1," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 1 THEN CAST(v.vaccinename AS text) END) as brand_pcv1," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 2 THEN v.vaccinationdate END) as date_pcv2," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 2 THEN CAST(v.vaccinename AS text) END) as brand_pcv2," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 3 THEN v.vaccinationdate END) as date_pcv3," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 3 THEN CAST(v.vaccinename AS text) END) as brand_pcv3," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 4 THEN v.vaccinationdate END) as date_pcv4," + - " MAX(CASE WHEN is_pcv AND pcv_row_num = 4 THEN CAST(v.vaccinename AS text) END) as brand_pcv4," + - " SUM(CASE WHEN is_pcv THEN 1 ELSE 0 END) as pcv_doses," + - " MAX(CASE WHEN is_ppv AND ppv_row_num = 1 THEN v.vaccinationdate END) as date_ppv," + - " SUM(CASE WHEN is_ppv THEN 1 ELSE 0 END) as ppv_doses " + - " FROM filtered_cases c " + - " LEFT JOIN vaccination v ON v.immunization_id IN (SELECT i.id FROM immunization i WHERE i.person_id = c.person_id AND i.disease = 'INVASIVE_PNEUMOCOCCAL_INFECTION') " + - " LEFT JOIN LATERAL (" + - " SELECT CASE " + - " WHEN CAST(v.vaccinename AS text) LIKE '%PCV%' OR CAST(v.vaccinename AS text) LIKE '%CONJUGATE%' THEN true " + - " ELSE false " + - " END as is_pcv," + - " CASE " + - " WHEN CAST(v.vaccinename AS text) LIKE '%PPV%' OR CAST(v.vaccinename AS text) LIKE '%POLYSACCHARIDE%' THEN true " + - " ELSE false " + - " END as is_ppv," + - " SUM(CASE WHEN CAST(v.vaccinename AS text) LIKE '%PCV%' OR CAST(v.vaccinename AS text) LIKE '%CONJUGATE%' THEN 1 ELSE 0 END) " + - " OVER (PARTITION BY c.id ORDER BY v.vaccinationdate) as pcv_row_num," + - " SUM(CASE WHEN CAST(v.vaccinename AS text) LIKE '%PPV%' OR CAST(v.vaccinename AS text) LIKE '%POLYSACCHARIDE%' THEN 1 ELSE 0 END) " + - " OVER (PARTITION BY c.id ORDER BY v.vaccinationdate) as ppv_row_num " + - " ) vax_info ON true " + - " GROUP BY c.id) "; + return "pneu_vaccination_details AS (" + + " SELECT vax.case_id," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 1 THEN vax.vaccinationdate END) as date_pcv1," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 1 THEN CAST(vax.vaccinename AS text) END) as brand_pcv1," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 2 THEN vax.vaccinationdate END) as date_pcv2," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 2 THEN CAST(vax.vaccinename AS text) END) as brand_pcv2," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 3 THEN vax.vaccinationdate END) as date_pcv3," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 3 THEN CAST(vax.vaccinename AS text) END) as brand_pcv3," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 4 THEN vax.vaccinationdate END) as date_pcv4," + + " MAX(CASE WHEN vax.is_pcv AND vax.pcv_row_num = 4 THEN CAST(vax.vaccinename AS text) END) as brand_pcv4," + + " SUM(CASE WHEN vax.is_pcv THEN 1 ELSE 0 END) as pcv_doses," + + " MAX(CASE WHEN vax.is_ppv AND vax.ppv_row_num = 1 THEN vax.vaccinationdate END) as date_ppv," + + " SUM(CASE WHEN vax.is_ppv THEN 1 ELSE 0 END) as ppv_doses " + + " FROM (" + + " SELECT c.id as case_id," + + " v.vaccinationdate," + + " v.vaccinename," + + " CASE " + + " WHEN CAST(v.vaccinename AS text) LIKE '%PCV%' OR CAST(v.vaccinename AS text) LIKE '%CONJUGATE%' THEN true " + + " ELSE false " + + " END as is_pcv," + + " CASE " + + " WHEN CAST(v.vaccinename AS text) LIKE '%PPV%' OR CAST(v.vaccinename AS text) LIKE '%POLYSACCHARIDE%' THEN true " + + " ELSE false " + + " END as is_ppv," + + " SUM(CASE WHEN CAST(v.vaccinename AS text) LIKE '%PCV%' OR CAST(v.vaccinename AS text) LIKE '%CONJUGATE%' THEN 1 ELSE 0 END) " + + " OVER (PARTITION BY c.id ORDER BY v.vaccinationdate) as pcv_row_num," + + " SUM(CASE WHEN CAST(v.vaccinename AS text) LIKE '%PPV%' OR CAST(v.vaccinename AS text) LIKE '%POLYSACCHARIDE%' THEN 1 ELSE 0 END) " + + " OVER (PARTITION BY c.id ORDER BY v.vaccinationdate) as ppv_row_num " + + " FROM filtered_cases c " + + " LEFT JOIN vaccination v ON v.immunization_id IN (SELECT i.id FROM immunization i WHERE i.person_id = c.person_id AND i.disease = 'INVASIVE_PNEUMOCOCCAL_INFECTION') " + + " ) vax " + + " GROUP BY vax.case_id) ";
🧹 Nitpick comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseConfigurationLookupService.java (1)
41-41: Unused logger field.The
loggeris declared but never used in this class. Either add logging statements (e.g., debug logging for lookups) or remove the unused field.
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13790 |
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13790 |
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 #13772
Summary by CodeRabbit
New Features
Improved Exporting
✏️ Tip: You can customize this high-level summary in your review settings.