#13555 - Fixed notifier query to correctly get the most recent version#13583
#13555 - Fixed notifier query to correctly get the most recent version#13583raulbob merged 1 commit intodevelopmentfrom
Conversation
WalkthroughRewrote NotifierService.getByUuidAndTime native SQL to use a UNION of notifier_history and notifier filtered by uuid and changeDate, then select the latest row by changedate DESC LIMIT 1. Removed per-column COALESCE logic in favor of selecting whole rows. Method signature and parameter bindings unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Service as NotifierService
participant DB as Database
Service->>DB: Native SQL (UNION notifier_history + notifier)\nWHERE uuid=:uuid AND changedate<=:changeDate
DB-->>Service: Single row (ORDER BY changedate DESC LIMIT 1)
Note over Service: Return mapped Notifier entity
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
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/person/notifier/NotifierService.java (1)
99-103: Handle no-row case; getSingleResult throws NoResultExceptionRequests earlier than the entity’s first change will yield 0 rows;
getSingleResult()will raiseNoResultExceptionand bubble up. Return null instead.- return (Notifier) query.getSingleResult(); + @SuppressWarnings("unchecked") + final java.util.List<Notifier> results = query.getResultList(); + return results.isEmpty() ? null : results.get(0);
🧹 Nitpick comments (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/NotifierService.java (1)
94-97: Ensure supporting index on (uuid, changedate) for both tablesWithout a composite index on
(uuid, changedate)for notifier and notifier_history, this query can degrade under load.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/NotifierService.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). (1)
- GitHub Check: SORMAS CI
| final String sql = "SELECT * FROM (" | ||
| + "SELECT * FROM (SELECT * FROM notifier_history b WHERE b.uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1) " | ||
| + "UNION SELECT * FROM notifier a WHERE a.uuid = :uuid) " | ||
| + "WHERE uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1;"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid SELECT * across UNION; use UNION ALL; drop trailing semicolon; simplify filters
- SELECT * + UNION between base and history risks column-order/type mismatches when mapping to Notifier; verify both tables have identical column sets in identical order. Prefer enumerating columns explicitly.
- UNION incurs unnecessary dedup; UNION ALL is faster here.
- The outer WHERE repeats uuid/changedate; push changedate into both branches and keep a single ORDER BY/LIMIT.
- Trailing semicolon can break some JPA providers.
Apply:
-final String sql = "SELECT * FROM ("
- + "SELECT * FROM (SELECT * FROM notifier_history b WHERE b.uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1) "
- + "UNION SELECT * FROM notifier a WHERE a.uuid = :uuid) "
- + "WHERE uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1;";
+final String sql =
+ "SELECT t.* FROM (" +
+ " SELECT b.* FROM notifier_history b " +
+ " WHERE b.uuid = :uuid AND b.changedate <= CAST(:changeDate AS TIMESTAMP) " +
+ " UNION ALL " +
+ " SELECT a.* FROM notifier a " +
+ " WHERE a.uuid = :uuid AND a.changedate <= CAST(:changeDate AS TIMESTAMP) " +
+ ") t " +
+ "ORDER BY t.changedate DESC LIMIT 1";If history/base schemas diverge, explicitly list columns in the exact Notifier mapping order instead of using *.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final String sql = "SELECT * FROM (" | |
| + "SELECT * FROM (SELECT * FROM notifier_history b WHERE b.uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1) " | |
| + "UNION SELECT * FROM notifier a WHERE a.uuid = :uuid) " | |
| + "WHERE uuid = :uuid AND changedate <= CAST(:changeDate AS TIMESTAMP) ORDER BY changedate DESC LIMIT 1;"; | |
| final String sql = | |
| "SELECT t.* FROM (" + | |
| " SELECT b.* FROM notifier_history b " + | |
| " WHERE b.uuid = :uuid AND b.changedate <= CAST(:changeDate AS TIMESTAMP) " + | |
| " UNION ALL " + | |
| " SELECT a.* FROM notifier a " + | |
| " WHERE a.uuid = :uuid AND a.changedate <= CAST(:changeDate AS TIMESTAMP) " + | |
| ") t " + | |
| "ORDER BY t.changedate DESC LIMIT 1"; |
🤖 Prompt for AI Agents
In
sormas-backend/src/main/java/de/symeda/sormas/backend/person/notifier/NotifierService.java
around lines 94-97, the query uses SELECT * with UNION and a trailing semicolon
and repeats filters; replace SELECT * with an explicit, ordered list of columns
matching the Notifier entity mapping (ensuring identical order and types for
both branches), change UNION to UNION ALL, move the changedate filter into both
the history and base SELECT branches (keep a single ORDER BY ... LIMIT in the
outer query), and remove the trailing semicolon so the JPA provider won’t choke;
if schemas diverge, enumerate columns exactly in the same order for both
SELECTs.
Fixes data was changed error in manual DD notification entry.
Summary by CodeRabbit
Bug Fixes
Refactor