#13771 - Corrections to epipulse export queries & mappings#13866
#13771 - Corrections to epipulse export queries & mappings#13866roldy merged 2 commits intodevelopmentfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds pneumonia handling to Epipulse complication mapping, adjusts sample-material and import-status code mappings, makes SQL ordering null-safe via COALESCE in multiple CTEs, and adds unit tests for laboratory sample material mappings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseLaboratoryMapper.java (1)
249-266: Mapping corrections look good.The updated codes (
IMPREL,IMPUNK,NOTIMP) align with the Javadoc documentation. Note thatmapMeniImportedStatus(lines 886-903) now has an identical implementation—consider extracting shared logic if these mappings are expected to remain consistent.,
♻️ Optional: Extract shared mapping logic
// Both methods could delegate to a shared private helper if they should stay in sync: private static String mapCaseImportedStatusCode(CaseImportedStatus importedStatus) { if (importedStatus == null) { return null; } switch (importedStatus) { case IMPORTED_CASE: return "IMP"; case IMPORT_RELATED_CASE: return "IMPREL"; case UNKNOWN_IMPORTATION_STATUS: return "IMPUNK"; case NOT_IMPORTED_CASE: return "NOTIMP"; default: return null; } }🤖 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/epipulse/EpipulseLaboratoryMapper.java` around lines 249 - 266, Both mapCaseImportedStatusToEpipulseCode and mapMeniImportedStatus contain identical switch logic; extract the mapping into a single private helper (e.g., mapCaseImportedStatusCode) that takes CaseImportedStatus and returns the code, then have both mapCaseImportedStatusToEpipulseCode and mapMeniImportedStatus delegate to that helper to avoid duplication and ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.java`:
- Line 225: The ORDER BY expressions using COALESCE(pt.testdatetime,
pt.reportdate, pt.creationdate) in IpiExportStrategy are currently "DESC" which
in Postgres treats NULLs as first; update each such ORDER BY (the occurrences in
the SQL strings inside IpiExportStrategy) to append "NULLS LAST" and add a
deterministic tiebreaker (e.g. ", pt.id" or ", pt.uuid") after the COALESCE
expression so LIMIT 1 / DISTINCT ON pick the most recent non-null row
consistently; modify the three SQL fragments that contain "ORDER BY
COALESCE(pt.testdatetime, pt.reportdate, pt.creationdate) DESC" to "ORDER BY
COALESCE(pt.testdatetime, pt.reportdate, pt.creationdate) DESC NULLS LAST,
pt.id" (or pt.uuid) as appropriate.
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/MeniExportStrategy.java`:
- Line 256: In MeniExportStrategy, the ORDER BY expressions that use DESC on
COALESCE(pt.testdatetime, pt.reportdate, pt.creationdate) must append "NULLS
LAST" so rows with all-null timestamps aren't chosen as the "latest" (update
both occurrences referenced in the MENI CTE SQL strings inside class
MeniExportStrategy to use COALESCE(... ) DESC NULLS LAST); keep the exact
COALESCE expression and only add "NULLS LAST" to the ORDER BY clauses to
preserve behavior for non-null timestamps.
---
Nitpick comments:
In
`@sormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseLaboratoryMapper.java`:
- Around line 249-266: Both mapCaseImportedStatusToEpipulseCode and
mapMeniImportedStatus contain identical switch logic; extract the mapping into a
single private helper (e.g., mapCaseImportedStatusCode) that takes
CaseImportedStatus and returns the code, then have both
mapCaseImportedStatusToEpipulseCode and mapMeniImportedStatus delegate to that
helper to avoid duplication and ensure consistency.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
sormas-api/src/main/java/de/symeda/sormas/api/epipulse/EpipulseLaboratoryMapper.javasormas-api/src/test/java/de/symeda/sormas/api/epipulse/EpipulseLaboratoryMapperTest.javasormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/EpipulseSqlCteBuilder.javasormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.javasormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/MeaslesExportStrategy.javasormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/MeniExportStrategy.java
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/IpiExportStrategy.java
Outdated
Show resolved
Hide resolved
sormas-backend/src/main/java/de/symeda/sormas/backend/epipulse/strategy/MeniExportStrategy.java
Outdated
Show resolved
Hide resolved
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13866 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13866 |
Fixes #
Summary by CodeRabbit
New Features
Bug Fixes
Tests