Logic change for isCallDisconnected and preferredLanguage in close call API#78
Logic change for isCallDisconnected and preferredLanguage in close call API#78srishtigrp78 wants to merge 6 commits intoPSMRI:developfrom
Conversation
WalkthroughThe change updates the call closure logic by adding a condition to check if an introductory call has been disconnected. If so, it resets the call's allocation, status, and attempt number, ensuring that only disconnected introductory calls are marked as open and unallocated for reassignment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CallClosureImpl
participant CallObj
User->>CallClosureImpl: closeCall(request)
CallClosureImpl->>CallObj: getCallType(), getIsCallDisconnected()
alt Call is introductory AND disconnected
CallClosureImpl->>CallObj: setAllocatedUserId(null)
CallClosureImpl->>CallObj: setCallStatus("OPEN")
CallClosureImpl->>CallObj: setCallAttemptNo(0)
end
CallClosureImpl-->>User: return result
Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Nitpick comments (2)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (2)
207-210: The logic enhancement looks correct but consider using consistent null-safe checking pattern.The addition of the disconnection check aligns well with the PR objectives. However, for consistency with line 160, consider using
Boolean.TRUE.equals()for null-safe boolean checking:-if (!isLanguageMapped - && callObj.getEcdCallType().equalsIgnoreCase("introductory") - && obj.getIsCallDisconnected() != null - && obj.getIsCallDisconnected()) { +if (!isLanguageMapped + && callObj.getEcdCallType().equalsIgnoreCase("introductory") + && Boolean.TRUE.equals(obj.getIsCallDisconnected())) {This pattern is already used elsewhere in the code (line 160) and provides cleaner null-safe checking.
87-300: Consider refactoring this method for better maintainability.The
closeCallmethod is handling multiple responsibilities and has grown to over 300 lines. For future improvements, consider:
- Extracting the call status update logic into separate methods
- Creating dedicated methods for handling different call types (introductory, follow-up, etc.)
- Consolidating the duplicate disconnection handling logic
This would make the code more testable and easier to maintain.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (1)
208-216: 💡 Verification agent🧩 Analysis chain
Potential logic conflict with existing introductory call handling.
The new conditional block adds logic for handling disconnected introductory calls when language is not mapped. However, there's similar logic at lines 160-163 that also handles introductory calls that are disconnected. This could lead to conflicting state changes or redundant processing.
Additionally, the Boolean comparison
obj.getIsCallDisconnected()should useBoolean.TRUE.equals()for safer null handling, consistent with the pattern used at line 160.Consider consolidating the introductory call logic or ensuring clear separation of concerns:
-if (!isLanguageMapped - && callObj.getEcdCallType().equalsIgnoreCase("introductory") - && obj.getIsCallDisconnected() != null - && obj.getIsCallDisconnected()) { +if (!isLanguageMapped + && callObj.getEcdCallType().equalsIgnoreCase("introductory") + && Boolean.TRUE.equals(obj.getIsCallDisconnected())) {Let me verify if there are other similar patterns in the codebase and check for potential conflicts:
🏁 Script executed:
#!/bin/bash # Description: Check for other introductory call handling patterns and potential conflicts # Search for other introductory call handling logic rg -A 10 -B 2 "introductory.*getIsCallDisconnected|getIsCallDisconnected.*introductory" # Search for other Boolean.TRUE.equals patterns rg "Boolean\.TRUE\.equals" # Look for other call status setting patterns rg -A 3 "setCallStatus.*OPEN"Length of output: 4601
Clarify and unify introductory‐call disconnection handling
- The two blocks in CallClosureImpl.java handle disconnected “introductory” calls in complementary scenarios (mapped vs. unmapped language) and are not duplicates—but they share the same state changes.
- The unmapped block (lines 208–216) uses
obj.getIsCallDisconnected() != null && obj.getIsCallDisconnected(), whereas the mapped block (around line 160) usesBoolean.TRUE.equals(obj.getIsCallDisconnected()).- To prevent NPEs and reduce duplication, extract the shared state changes into a helper and align null‐safety checks.
Locations to address:
- src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java
• Mapped case: ~lines 159–165
• Unmapped case: lines 208–216Proposed diff for the unmapped block:
-if (!isLanguageMapped - && callObj.getEcdCallType().equalsIgnoreCase("introductory") - && obj.getIsCallDisconnected() != null - && obj.getIsCallDisconnected()) { +if (!isLanguageMapped + && "introductory".equalsIgnoreCase(callObj.getEcdCallType()) + && Boolean.TRUE.equals(obj.getIsCallDisconnected())) { callObj.setAllocatedUserId(null); callObj.setCallStatus(Constants.OPEN); callObj.setCallAttemptNo(0); callObj.setAllocationStatus(Constants.UNALLOCATED); }Consider extracting into a helper to DRY up both branches:
private void openUnallocatedIntroCall(CallObj callObj) { callObj.setAllocatedUserId(null); callObj.setCallStatus(Constants.OPEN); callObj.setCallAttemptNo(0); callObj.setAllocationStatus(Constants.UNALLOCATED); }
🧹 Nitpick comments (2)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (2)
160-165: Review interaction with new language mapping logic.This existing logic for introductory calls that are disconnected might interact unexpectedly with the new language mapping logic added later (lines 208-216). Both conditions could potentially apply to the same call, leading to multiple state changes.
Consider consolidating both introductory call handling blocks into a single, more comprehensive condition that handles all scenarios:
-if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) && Boolean.TRUE.equals(obj.getIsCallDisconnected()) && StringUtils.hasText(request.getPreferredLanguage())) { - callObj.setCallStatus(Constants.OPEN); - callObj.setAllocationStatus(Constants.UNALLOCATED); -} +if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) && Boolean.TRUE.equals(obj.getIsCallDisconnected())) { + callObj.setCallStatus(Constants.OPEN); + callObj.setAllocationStatus(Constants.UNALLOCATED); + // Additional logic for language mapping will be handled later +}
207-207: Consider moving language mapping check closer to usage.The
isLanguageMappedvariable is calculated here but only used in the condition below. For better code readability and performance, consider moving this check directly into the conditional or storing it in a more descriptive variable name.-isLanguageMapped = isLanguageMappedWithUser(request); +boolean isLanguageMapped = isLanguageMappedWithUser(request);Or move it directly into the condition if not used elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (java)
- GitHub Check: Build
|
not required anymore. |
2 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
Logic change for isCallDisconnected and preferredLanguage in close call API
✅ 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