If call disconnected call status open - #82
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CallClosureImpl
participant Repository
Client->>CallClosureImpl: closeCall(request)
alt Call type is "introductory" and disconnected with preferred language
CallClosureImpl->>CallClosureImpl: Set status to "OPEN"\n(Comment out allocation update)
else Call is answered
CallClosureImpl->>CallClosureImpl: Set status to "COMPLETED"
end
CallClosureImpl->>Repository: Update call record
alt Language not mapped and call type is "introductory"
CallClosureImpl->>CallClosureImpl: Reset allocated user ID\nSet status to "OPEN"\nReset attempt number\nSet allocation to "UNALLOCATED"
end
CallClosureImpl->>Repository: Finalize updates
CallClosureImpl-->>Client: Return result
Possibly related PRs
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://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (1)
87-297: 🛠️ Refactor suggestionConsider refactoring this method for better maintainability
The
closeCallmethod is over 200 lines long and handles multiple complex scenarios. This makes it difficult to understand, test, and maintain. Consider:
- Extract method for call status logic: Lines 160-212 contain complex logic for determining call status that could be extracted
- Extract method for allocation handling: The allocation status logic could be separated
- Extract method for language preference updates: Lines 299-329 already exist as a separate method, which is good
- Consider using a state machine pattern for managing the various call states and transitions
Example refactoring approach:
+private void updateCallStatusAndAllocation(OutboundCalls callObj, Bencall bencall, + CallClosureDTO request, boolean isMaxCallsAttempted) { + // Extract lines 160-212 logic here +} + +private boolean shouldUnallocateCall(OutboundCalls callObj, CallClosureDTO request, + boolean isLanguageMapped) { + // Define clear rules for when to unallocate +}
🧹 Nitpick comments (1)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (1)
252-268: Fix error message formattingThe error messages in the catch blocks have improper string concatenation:
-throw new ECDException("Failed to update isFurtherCallRequired upcoming calls for child" + e); +throw new ECDException("Failed to update isFurtherCallRequired upcoming calls for child: " + e.getMessage()); -throw new ECDException("Failed to update isFurtherCallRequired upcoming calls for mother" + e); +throw new ECDException("Failed to update isFurtherCallRequired upcoming calls for mother: " + e.getMessage());
📜 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(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (1)
src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java (1)
205-207: Good formatting improvementAdding a blank line after the
isLanguageMappedassignment improves code readability by visually separating the variable assignment from the conditional logic.
| if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) && Boolean.TRUE.equals(obj.getIsCallDisconnected()) && StringUtils.hasText(request.getPreferredLanguage())) { | ||
| callObj.setCallStatus(Constants.OPEN); | ||
| callObj.setAllocationStatus(Constants.UNALLOCATED); | ||
| //callObj.setAllocationStatus(Constants.UNALLOCATED); | ||
| }else if(Boolean.TRUE.equals(obj.getIsCallAnswered())){ | ||
| callObj.setCallStatus(Constants.COMPLETED); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential inconsistency in allocation status handling
The allocation status update has been commented out for introductory calls that are disconnected with a preferred language. However, this creates an inconsistency with the logic at lines 207-212, where allocation status IS set to UNALLOCATED for introductory calls when language is not mapped.
This could lead to scenarios where:
- Disconnected introductory calls with preferred language remain allocated to the original user
- But similar calls without language mapping get unallocated
Consider either:
- Uncommenting this line to maintain consistency
- Documenting why allocation status should remain unchanged in this specific scenario
🏁 Script executed:
#!/bin/bash
# Description: Check for other occurrences of allocation status updates to understand the pattern
# Search for all occurrences of setAllocationStatus to understand the allocation logic
echo "=== Searching for setAllocationStatus usage patterns ==="
rg -A 3 -B 3 "setAllocationStatus" --type java
# Search for Constants.UNALLOCATED usage
echo -e "\n=== Searching for UNALLOCATED constant usage ==="
rg -A 2 -B 2 "Constants\.UNALLOCATED" --type java
# Check if there are any other places where allocation status is managed for introductory calls
echo -e "\n=== Searching for introductory call handling ==="
rg -A 5 -B 5 "introductory.*allocation|allocation.*introductory" --type java -iLength of output: 18963
Ensure consistent allocation status for disconnected introductory calls
In CallClosureImpl.java (around lines 160–165), the callObj.setAllocationStatus(Constants.UNALLOCATED); line is commented out for disconnected introductory calls with a preferred language. Later in the same method, introductory calls without a language mapping do get unallocated, leading to inconsistent behavior.
Please address this by either:
- Re-enabling the unallocation for disconnected introductory calls:
@@ src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java:160 if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) && Boolean.TRUE.equals(obj.getIsCallDisconnected()) && StringUtils.hasText(request.getPreferredLanguage())) { callObj.setCallStatus(Constants.OPEN); - //callObj.setAllocationStatus(Constants.UNALLOCATED); + callObj.setAllocationStatus(Constants.UNALLOCATED); } else if (Boolean.TRUE.equals(obj.getIsCallAnswered())) { callObj.setCallStatus(Constants.COMPLETED); } - Or add a clear comment explaining why allocation must remain unchanged when a preferred language is present.
📝 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.
| if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) && Boolean.TRUE.equals(obj.getIsCallDisconnected()) && StringUtils.hasText(request.getPreferredLanguage())) { | |
| callObj.setCallStatus(Constants.OPEN); | |
| callObj.setAllocationStatus(Constants.UNALLOCATED); | |
| //callObj.setAllocationStatus(Constants.UNALLOCATED); | |
| }else if(Boolean.TRUE.equals(obj.getIsCallAnswered())){ | |
| callObj.setCallStatus(Constants.COMPLETED); | |
| } | |
| if ("introductory".equalsIgnoreCase(callObj.getEcdCallType()) | |
| && Boolean.TRUE.equals(obj.getIsCallDisconnected()) | |
| && StringUtils.hasText(request.getPreferredLanguage())) { | |
| callObj.setCallStatus(Constants.OPEN); | |
| callObj.setAllocationStatus(Constants.UNALLOCATED); | |
| } else if (Boolean.TRUE.equals(obj.getIsCallAnswered())) { | |
| callObj.setCallStatus(Constants.COMPLETED); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/iemr/ecd/service/associate/CallClosureImpl.java between
lines 160 and 165, the line that sets the call's allocation status to
UNALLOCATED for disconnected introductory calls with a preferred language is
commented out, creating inconsistency with other parts of the code where similar
calls are unallocated. To fix this, uncomment the line to ensure consistent
allocation handling for such cases or add a comment explaining why the
allocation status should remain unchanged when a preferred language is present,
maintaining clarity and consistency in call state management.
2 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