Conversation
story: amm-1668 task - 1754
WalkthroughBumped tm-api version to 3.4.0 in pom.xml. Added fatherName and preferredPhoneNum fields to BeneficiaryFlowStatus constructor and mapped them in getBeneficiaryFlowStatusForLeftPanel. Updated repository query to select t.fatherName and t.preferredPhoneNum. Removed constructor’s explicit bloodGroup null assignment. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Repo as BeneficiaryFlowStatusRepo
participant DB
participant Helper as BeneficiaryFlowStatus.getBeneficiaryFlowStatusForLeftPanel
participant Entity as BeneficiaryFlowStatus
Client->>Repo: getBenDetailsForLeftSidePanel(params)
Repo->>DB: SELECT ... , t.fatherName, t.preferredPhoneNum
DB-->>Repo: Object[] rows (… + fatherName, preferredPhoneNum)
Repo-->>Client: ArrayList<Object[]>
Client->>Helper: map Object[] to BeneficiaryFlowStatus
Note over Helper,Entity: Uses objArr[19], objArr[20] for new fields
Helper->>Entity: new BeneficiaryFlowStatus(..., fatherName, preferredPhoneNum)
Entity-->>Client: BeneficiaryFlowStatus instance
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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java (1)
332-354: Assign the unused visitDate parameter and remove redundant nulling of bloodGroupI’ve confirmed that the constructor signature includes a
Timestamp visitDateparameter, but there is no correspondingthis.visitDate = visitDate;assignment. In addition,this.bloodGroup = null;is unnecessary because instance fields default tonull. Please apply the following refactor insrc/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java(around lines 332–354):• After setting
this.serviceDate, add:- this.serviceDate = benVisitDate; + this.serviceDate = benVisitDate; + this.visitDate = visitDate;• Remove the explicit null assignment for bloodGroup:
- this.bloodGroup = null; + // bloodGroup left as default (null) unless set elsewhereIf the
visitDateparameter is intentionally unused, please annotate it (e.g.@SuppressWarnings("unused")) or document why it’s omitted to prevent future confusion.
🧹 Nitpick comments (2)
src/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java (2)
352-354: Null/format safeguards for fatherName and preferredPhoneNumIf these values can be null or malformed from the DB, consider normalizing (trim, empty-to-null) and, for preferredPhoneNum, optional format validation before exposing in APIs.
367-380: Guard against multiple/empty rows and avoid needless loopThe query (by benRegID and benFlowID) should return at most one row. Looping assigns the last row if more exist and returns null if empty. Prefer explicit handling.
Apply:
public static BeneficiaryFlowStatus getBeneficiaryFlowStatusForLeftPanel(ArrayList<Object[]> objList) { - BeneficiaryFlowStatus obj = null; - if (objList != null && objList.size() > 0) { - for (Object[] objArr : objList) { - obj = new BeneficiaryFlowStatus((Long) objArr[0], (Long) objArr[1], (Timestamp) objArr[2], - (String) objArr[3], (String) objArr[4], (Integer) objArr[5], (Short) objArr[6], - (String) objArr[7], (String) objArr[8], (String) objArr[9], (Long) objArr[10], - (String) objArr[11], (String) objArr[12], (String) objArr[13], (Long) objArr[14], - (Timestamp) objArr[15], (Timestamp) objArr[16], (Long) objArr[17], (Timestamp) objArr[18], - (String) objArr[19], (String) objArr[20]); - } - } - return obj; + if (objList == null || objList.isEmpty()) { + return null; + } + if (objList.size() > 1) { + // Optional: log a warning; unexpected multiple rows for unique keys + } + Object[] objArr = objList.get(0); + return new BeneficiaryFlowStatus((Long) objArr[0], (Long) objArr[1], (Timestamp) objArr[2], + (String) objArr[3], (String) objArr[4], (Integer) objArr[5], (Short) objArr[6], + (String) objArr[7], (String) objArr[8], (String) objArr[9], (Long) objArr[10], + (String) objArr[11], (String) objArr[12], (String) objArr[13], (Long) objArr[14], + (Timestamp) objArr[15], (Timestamp) objArr[16], (Long) objArr[17], (Timestamp) objArr[18], + (String) objArr[19], (String) objArr[20]); }
📜 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 (3)
pom.xml(1 hunks)src/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java(3 hunks)src/main/java/com/iemr/tm/repo/benFlowStatus/BeneficiaryFlowStatusRepo.java(1 hunks)
🔇 Additional comments (2)
pom.xml (1)
9-9: No hardcoded tm-api WAR references foundI ran ripgrep across the repository (excluding build outputs) for any hard-coded
tm-api-*.waroccurrences and for any.warmentions in CI/CD configs, and found none.• No
tm-api-3.x.x.warpatterns in source files
• NoJenkinsfile,.ymlor.yamlCI/CD config files in the repoPlease ensure there aren’t any external packaging or deployment scripts (outside this repository) that assume the older WAR name.
src/main/java/com/iemr/tm/repo/benFlowStatus/BeneficiaryFlowStatusRepo.java (1)
119-122: Mapping Verified:fatherNameandpreferredPhoneNumalign with entity column mappings
The @column annotations insrc/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java
- Line 165:
@Column(name = "ben_phone_no") private String preferredPhoneNum;- Line 169:
@Column(name = "father_name") private String fatherName;match the two fields appended to the end of the SELECT list in
BeneficiaryFlowStatusRepo.java(lines 119–122), and they correspond toobjArr[19]andobjArr[20]ingetBeneficiaryFlowStatusForLeftPanel. No drift detected—changes approved.
| (Timestamp) objArr[15], (Timestamp) objArr[16], (Long) objArr[17], (Timestamp) objArr[18], | ||
| (String) objArr[19], (String) objArr[20]); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Brittle index-based mapping — prefer JPQL constructor projection
The indices align with the SELECT list today (fatherName at 19, preferredPhoneNum at 20). To eliminate future off-by-one risks, switch the repository query to a constructor projection and return BeneficiaryFlowStatus directly (no Object[] casts).
In the repo file, change the query to:
-@Query("SELECT t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, "
- + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, "
- + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum FROM BeneficiaryFlowStatus t "
- + " Where t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ")
-public ArrayList<Object[]> getBenDetailsForLeftSidePanel(...)
+@Query("SELECT new com.iemr.tm.data.benFlowStatus.BeneficiaryFlowStatus("
+ + " t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, "
+ + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, "
+ + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum) "
+ + " FROM BeneficiaryFlowStatus t WHERE t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ")
+public ArrayList<BeneficiaryFlowStatus> getBenDetailsForLeftSidePanel(...)Then this method can simply return the first element (or Optional).
📝 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.
| (Timestamp) objArr[15], (Timestamp) objArr[16], (Long) objArr[17], (Timestamp) objArr[18], | |
| (String) objArr[19], (String) objArr[20]); | |
| } | |
| // Before | |
| - @Query("SELECT t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, " | |
| - + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, " | |
| - + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum FROM BeneficiaryFlowStatus t " | |
| - + " Where t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ") | |
| - public ArrayList<Object[]> getBenDetailsForLeftSidePanel(...); | |
| // After | |
| @Query("SELECT new com.iemr.tm.data.benFlowStatus.BeneficiaryFlowStatus(" | |
| + " t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, " | |
| + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, " | |
| + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum) " | |
| + " FROM BeneficiaryFlowStatus t " | |
| + "WHERE t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ") | |
| public ArrayList<BeneficiaryFlowStatus> getBenDetailsForLeftSidePanel(...); |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/tm/data/benFlowStatus/BeneficiaryFlowStatus.java
around lines 375 to 377, the code relies on fragile Object[] index-based mapping
from a native/JPQL select; replace that by changing the repository query to use
a JPQL constructor projection (SELECT new
com.iemr.tm.data.benFlowStatus.BeneficiaryFlowStatus(...matching fields... )
...) so the repository returns BeneficiaryFlowStatus instances instead of
Object[], then update this method to accept the repository result and simply
return the first element or Optional<BeneficiaryFlowStatus> (no casts or index
extraction). Ensure the constructor parameter order matches the select list and
update the repository method signature to the concrete type.
| @Query("SELECT t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, " | ||
| + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, " | ||
| + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate FROM BeneficiaryFlowStatus t " | ||
| + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum FROM BeneficiaryFlowStatus t " | ||
| + " Where t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce mapping fragility with JPQL constructor projection
To avoid Object[] casting and index coupling, consider returning BeneficiaryFlowStatus directly with a constructor expression.
Apply:
- @Query("SELECT t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, "
- + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, "
- + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum FROM BeneficiaryFlowStatus t "
- + " Where t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ")
- public ArrayList<Object[]> getBenDetailsForLeftSidePanel(@Param("benRegID") Long benRegID,
- @Param("benFlowID") Long benFlowID);
+ @Query("SELECT new com.iemr.tm.data.benFlowStatus.BeneficiaryFlowStatus("
+ + " t.benFlowID, t.beneficiaryRegID, t.visitDate, t.benName, t.age, t.ben_age_val, t.genderID, t.genderName, "
+ + " t.villageName, t.districtName, t.beneficiaryID, t.servicePointName, t.VisitReason, t.VisitCategory, t.benVisitID, "
+ + " t.registrationDate, t.benVisitDate, t.visitCode, t.consultationDate, t.fatherName, t.preferredPhoneNum) "
+ + " FROM BeneficiaryFlowStatus t WHERE t.beneficiaryRegID = :benRegID AND t.benFlowID = :benFlowID ")
+ public ArrayList<BeneficiaryFlowStatus> getBenDetailsForLeftSidePanel(@Param("benRegID") Long benRegID,
+ @Param("benFlowID") Long benFlowID);🤖 Prompt for AI Agents
In src/main/java/com/iemr/tm/repo/benFlowStatus/BeneficiaryFlowStatusRepo.java
around lines 119 to 122, the JPQL currently selects individual fields which
forces consumers to handle Object[] and fragile index-based mapping; change the
query to use a JPQL constructor expression (SELECT new
com.iemr.tm.model.benFlowStatus.BeneficiaryFlowStatus(...)) listing the same
fields in the same order, ensure the BeneficiaryFlowStatus class has a matching
public constructor accepting those types in that order, and update the
repository method signature to return BeneficiaryFlowStatus (or
List<BeneficiaryFlowStatus>) instead of Object[] so callers receive typed
instances directly.
3 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Chores
Tests