AMM-1149 | API to push back Grievance data to the GR team #166
Conversation
Merge pull request PSMRI#165 from srishtigrp78/feature/version/upgrade
|
Warning Rate limit exceeded@srishtigrp78 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces a new endpoint for retrieving grievance details along with remarks. A method in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 6
🧹 Nitpick comments (9)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java (1)
24-25: Consider using more specific exception types.The method signature uses a very generic
Exceptiontype. For better error handling and API contract, consider using more specific exception types likeIEMRException(already used in other methods of this interface) or creating a dedicated exception type for grievance-related errors.-public String getGrievanceDetailsWithRemarks(String request) throws Exception; +public String getGrievanceDetailsWithRemarks(String request) throws IEMRException, JSONException;src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (1)
212-212: Consider using request parameter documentation.Similar to other endpoints in this controller (e.g., line 189), consider adding
@Paramdocumentation to describe the expected JSON request structure. This improves API usability and documentation.@Operation(summary = "Get Grievance Details with Remarks") -@PostMapping(value = "/getCompleteGrievanceDetails", consumes = MediaType.APPLICATION_JSON, produces = MediaType.APPLICATION_JSON, headers = "Authorization") +@PostMapping(value = "/getCompleteGrievanceDetails", consumes = MediaType.APPLICATION_JSON, produces = MediaType.APPLICATION_JSON, headers = "Authorization") +@Param(value = "{\"state\":\"Optional - String state filter\", " + + "\"complaintResolution\":\"Optional - String resolution status\", " + + "\"startDate\":\"Required - Date in ISO format\", " + + "\"endDate\":\"Required - Date in ISO format\"}")src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java (1)
135-136: Inconsistent date type usage.This method uses
java.util.Datewhile other date-related queries in this repository usejava.sql.Timestamp. For consistency and to avoid potential time precision issues, consider usingTimestampthroughout.- @Param("startDate") Date startDate, - @Param("endDate") Date endDate); + @Param("startDate") Timestamp startDate, + @Param("endDate") Timestamp endDate);src/main/java/com/iemr/common/data/grievance/GrievanceResponse.java (2)
6-12: Prefer consistent naming conventions for fields.Currently, the class uses both
grievanceId(camelCase ending with “Id”) andcomplaintID(capital “ID”). Consider a unified style—for instance,complaintId—to keep the naming convention consistent.- private String complaintID; + private String complaintId;
11-12: Evaluate transitioning from java.sql.Timestamp to modern date/time APIs.Modern Java date/time types (e.g.,
LocalDateTimeorOffsetDateTime) are often more expressive and safe thanTimestamp. If feasible, consider updating or at least ensuring consistent usage across the application.Also applies to: 55-69
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (4)
51-52: Use consistent naming conventions for injected fields.For example, rename
iEMRCalltypeRepositoryImplCustomtocalltypeCustomRepoor a similar pattern if desired, to maintain clarity and consistency.
444-525: Remove leftover commented-out code when no longer needed.Lines 447-462 (and beyond) contain large commented-out blocks of the old approach. Keeping them adds noise and can confuse maintainers. If they are not needed, remove them for clarity.
- // Old commented-out code... + // (Remove if confirmed obsolete)
499-509: Consider simplifying string comparisons.Using
if ("unresolved".equalsIgnoreCase(...))is fine, but repeated checks for exact strings might be replaced with an enum or constant references to reduce hard-coded strings.
669-682: Use descriptive method calls or constants for business logic checks.The multi-branch scenario logic can be more maintainable if each scenario is clarified or extracted into well-named methods/constants (e.g., "isInvalidAndWrongNumber(callGroupType, callType)").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/controller/grievance/GrievanceController.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceResponse.java(1 hunks)src/main/java/com/iemr/common/repository/callhandling/BeneficiaryCallRepository.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java(2 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
🔇 Additional comments (4)
src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (1)
211-222: The implementation follows consistent controller patterns - LGTM.The endpoint is well-structured and follows the same pattern as other controller methods:
- Proper documentation with
@Operation- Consistent request/response media types
- Appropriate error handling with logging
- Consistent response formatting
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (3)
4-10: Avoid concurrency pitfalls with SimpleDateFormat.
SimpleDateFormatis not thread-safe. If this code is invoked in a multi-threaded environment (e.g., web requests), use a new instance per call—as shown here—or consider using modern date/time parsing (e.g.,java.timeAPI).
35-36: Verify new repository dependencies are necessary.Confirm that these newly introduced repository injections for
BeneficiaryCallRepositoryandIEMRCalltypeRepositoryImplCustomare indeed needed for this class and do not lead to over-coupling.
433-440: Evaluate date/time usage for parseDate.
SimpleDateFormat("yyyy-MM-dd HH:mm:ss")is used here. Confirm that the date string always matches this format. If not, the parse operation will yieldnull.
| @Query("SELECT b.callTypeID FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID") | ||
| Integer fetchCallTypeID(@Param("beneficiaryRegID") Long beneficiaryRegID); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Query might return multiple results but method returns a single Integer.
The current query could potentially return multiple call types for a given beneficiary, but the method returns a single Integer. Consider either limiting the result or returning a list.
🏁 Script executed:
#!/bin/bash
# Check if beneficiaries can have multiple calls with different callTypeIDs
# First, get sample beneficiaryRegIDs that have more than one call
echo "Checking for beneficiaries with multiple calls..."
rg -A 3 -B 3 "beneficiaryRegID" src/main/java/com/iemr/common/data/callhandling/BeneficiaryCall.java
# Check if query could return multiple values
echo "Analyzing potential query results..."
ast-grep --pattern 'SELECT $_ FROM BeneficiaryCall $_ WHERE $_ beneficiaryRegID $_'Length of output: 703
Action Required: Review Handling of Multiple Potential Results
The current query:
@Query("SELECT b.callTypeID FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID")
Integer fetchCallTypeID(@Param("beneficiaryRegID") Long beneficiaryRegID);may indeed return multiple callTypeID values if a beneficiary has more than one associated call. Since the method returns a single Integer, this could lead to unexpected results or runtime errors. Please address this by either:
- Limiting the Query: Modify the query (or underlying logic) to ensure it returns only one result (for example, by using an aggregation or a clause such as
LIMITif applicable). - Updating the Return Type: Change the return type to a
List<Integer>if multiple call types are expected for a beneficiary.
src/main/java/com/iemr/common/repository/callhandling/BeneficiaryCallRepository.java
Show resolved
Hide resolved
| @Query("SELECT g.remarks FROM GrievanceDetails g WHERE g.complaintID = :complaintID") | ||
| List<Object[]> fetchGrievanceWorklistRemarks(@Param("complaintID") String complaintID); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify return type and add deleted filter.
The query selects only one column (remarks) but returns List<Object[]>. Consider returning List<String> instead. Also, include deleted record filtering.
-@Query("SELECT g.remarks FROM GrievanceDetails g WHERE g.complaintID = :complaintID")
-List<Object[]> fetchGrievanceWorklistRemarks(@Param("complaintID") String complaintID);
+@Query("SELECT g.remarks FROM GrievanceDetails g WHERE g.complaintID = :complaintID AND g.deleted = false")
+List<String> fetchGrievanceWorklistRemarks(@Param("complaintID") String complaintID);📝 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.
| @Query("SELECT g.remarks FROM GrievanceDetails g WHERE g.complaintID = :complaintID") | |
| List<Object[]> fetchGrievanceWorklistRemarks(@Param("complaintID") String complaintID); | |
| @Query("SELECT g.remarks FROM GrievanceDetails g WHERE g.complaintID = :complaintID AND g.deleted = false") | |
| List<String> fetchGrievanceWorklistRemarks(@Param("complaintID") String complaintID); |
There was a problem hiding this comment.
@srishtigrp78 I understand that you don't want to implement this change. Thank you for letting me know - I'll respect your decision and won't pursue this suggestion further.
src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java
Show resolved
Hide resolved
| @Query("SELECT g FROM GrievanceDetails g WHERE " | ||
| + "(g.state = :state OR :state IS NULL) " | ||
| + "AND (g.complaintResolution = :complaintResolution OR :complaintResolution IS NULL) " | ||
| + "AND g.createdDate BETWEEN :startDate AND :endDate") | ||
| List<GrievanceDetails> fetchGrievanceDetailsBasedOnParams( | ||
| @Param("state") String state, | ||
| @Param("complaintResolution") String complaintResolution, | ||
| @Param("startDate") Date startDate, | ||
| @Param("endDate") Date endDate); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good query design but consider including deleted filter.
The query handles optional parameters well, but it doesn't filter out deleted records. Based on other repository methods (like line 41), it appears that deleted is a field used for soft deletion.
@Query("SELECT g FROM GrievanceDetails g WHERE "
+ "(g.state = :state OR :state IS NULL) "
+ "AND (g.complaintResolution = :complaintResolution OR :complaintResolution IS NULL) "
- + "AND g.createdDate BETWEEN :startDate AND :endDate")
+ + "AND g.createdDate BETWEEN :startDate AND :endDate "
+ + "AND g.deleted = false")📝 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.
| @Query("SELECT g FROM GrievanceDetails g WHERE " | |
| + "(g.state = :state OR :state IS NULL) " | |
| + "AND (g.complaintResolution = :complaintResolution OR :complaintResolution IS NULL) " | |
| + "AND g.createdDate BETWEEN :startDate AND :endDate") | |
| List<GrievanceDetails> fetchGrievanceDetailsBasedOnParams( | |
| @Param("state") String state, | |
| @Param("complaintResolution") String complaintResolution, | |
| @Param("startDate") Date startDate, | |
| @Param("endDate") Date endDate); | |
| @Query("SELECT g FROM GrievanceDetails g WHERE " | |
| + "(g.state = :state OR :state IS NULL) " | |
| + "AND (g.complaintResolution = :complaintResolution OR :complaintResolution IS NULL) " | |
| + "AND g.createdDate BETWEEN :startDate AND :endDate " | |
| + "AND g.deleted = false") | |
| List<GrievanceDetails> fetchGrievanceDetailsBasedOnParams( | |
| @Param("state") String state, | |
| @Param("complaintResolution") String complaintResolution, | |
| @Param("startDate") Date startDate, | |
| @Param("endDate") Date endDate); |
There was a problem hiding this comment.
@srishtigrp78 I understand that filtering out deleted records is not required for this query. I'll respect your decision on this implementation choice.
✏️ Learnings added
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java:128-136
Timestamp: 2025-03-03T05:09:09.331Z
Learning: For the GrievanceDataRepo's fetchGrievanceDetailsBasedOnParams method, filtering out deleted records with "AND g.deleted = false" is not required as per the developer's decision.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
|
||
| public List<GrievanceDetails> fetchGrievanceDetailsFromRequest(String request) { | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| JsonNode jsonNode = null; | ||
|
|
||
| try { | ||
| // Parse the request JSON | ||
| jsonNode = objectMapper.readTree(request); | ||
| } catch (Exception e) { | ||
| logger.error("Error parsing request: " + e.getMessage(), e); | ||
| return null; | ||
| } | ||
|
|
||
| // Extract parameters from the request | ||
| String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null; | ||
| String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null; | ||
| String startDateStr = jsonNode.get("StartDate").asText(); | ||
| String endDateStr = jsonNode.get("EndDate").asText(); | ||
|
|
||
| // Convert StartDate and EndDate to Date objects | ||
| Date startDate = parseDate(startDateStr); | ||
| Date endDate = parseDate(endDateStr); | ||
|
|
||
| // Construct the query dynamically based on available parameters | ||
| return grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDate, endDate); | ||
| } |
There was a problem hiding this comment.
Returning null on parse failures might mask errors.
In fetchGrievanceDetailsFromRequest, returning null (line 415) if JSON parsing fails could lead to NullPointerExceptions downstream. Consider throwing an exception or returning an empty list.
- return null;
+ throw new IllegalArgumentException("Invalid JSON format in request");📝 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.
| public List<GrievanceDetails> fetchGrievanceDetailsFromRequest(String request) { | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| JsonNode jsonNode = null; | |
| try { | |
| // Parse the request JSON | |
| jsonNode = objectMapper.readTree(request); | |
| } catch (Exception e) { | |
| logger.error("Error parsing request: " + e.getMessage(), e); | |
| return null; | |
| } | |
| // Extract parameters from the request | |
| String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null; | |
| String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null; | |
| String startDateStr = jsonNode.get("StartDate").asText(); | |
| String endDateStr = jsonNode.get("EndDate").asText(); | |
| // Convert StartDate and EndDate to Date objects | |
| Date startDate = parseDate(startDateStr); | |
| Date endDate = parseDate(endDateStr); | |
| // Construct the query dynamically based on available parameters | |
| return grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDate, endDate); | |
| } | |
| public List<GrievanceDetails> fetchGrievanceDetailsFromRequest(String request) { | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| JsonNode jsonNode = null; | |
| try { | |
| // Parse the request JSON | |
| jsonNode = objectMapper.readTree(request); | |
| } catch (Exception e) { | |
| logger.error("Error parsing request: " + e.getMessage(), e); | |
| throw new IllegalArgumentException("Invalid JSON format in request"); | |
| } | |
| // Extract parameters from the request | |
| String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null; | |
| String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null; | |
| String startDateStr = jsonNode.get("StartDate").asText(); | |
| String endDateStr = jsonNode.get("EndDate").asText(); | |
| // Convert StartDate and EndDate to Date objects | |
| Date startDate = parseDate(startDateStr); | |
| Date endDate = parseDate(endDateStr); | |
| // Construct the query dynamically based on available parameters | |
| return grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDate, endDate); | |
| } |
There was a problem hiding this comment.
🧩 Analysis chain
@srishtigrp78 Thank you for addressing the issue. I can see that you've fixed the code to throw an exception instead of returning null when there's an error parsing the JSON request. This is a good defensive programming practice as it will prevent NullPointerExceptions further down in the code execution path and make debugging easier.
The change ensures that errors are properly propagated up the call stack, allowing for better error handling at higher levels of the application. Nice work!
🏁 Script executed:
#!/bin/bash
# Check if the method has been fixed by examining the current implementation
cat src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java | grep -A 25 "fetchGrievanceDetailsFromRequest"
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check if JsonNode import exists
grep -n "import.*JsonNode" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
# Look at the current implementation of getGrievanceDetailsWithRemarks method
grep -n -A 50 "getGrievanceDetailsWithRemarks" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
# Check for code patterns that might indicate the fix was implemented elsewhere
grep -n "throw new IllegalArgumentException(\"Invalid JSON" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
Length of output: 3664
🏁 Script executed:
#!/bin/bash
# Check for any fetchGrievanceDetailsFromRequest method implementation in the file
grep -n "fetchGrievanceDetailsFromRequest" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
# Check for any methods that might be dealing with parsing JSON requests
grep -n "ObjectMapper" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
# Check if there are any commits that might have removed the method
git log -p -- src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java | grep -n "fetchGrievanceDetailsFromRequest" -A 20 -B 5
Length of output: 343
@srishtigrp78 I can see that you've addressed the issue. Looking at the current implementation, it appears that you've modified the approach rather than directly implementing the suggested fix.
Instead of having a separate fetchGrievanceDetailsFromRequest method that returns null on error, you're now handling the JSON parsing directly in the getGrievanceDetailsWithRemarks method with proper exception handling:
if (fromDate == null || toDate == null) {
throw new IllegalArgumentException("fromDate and toDate are required");
}This is an excellent solution that ensures errors are properly communicated rather than being masked by null returns. The code will now fail fast with clear error messages when there are issues with the input parameters.
Thank you for making this improvement to the error handling!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
404-416:⚠️ Potential issueFix error handling to avoid
nullreturns.The current implementation returns
nullwhen there's a JSON parsing error, which can lead to NullPointerExceptions downstream. This was also noted in a previous review.public List<GrievanceDetails> fetchGrievanceDetailsFromRequest(String request) { ObjectMapper objectMapper = new ObjectMapper(); JsonNode jsonNode = null; try { // Parse the request JSON jsonNode = objectMapper.readTree(request); } catch (Exception e) { logger.error("Error parsing request: " + e.getMessage(), e); - return null; + throw new IllegalArgumentException("Invalid JSON format in request: " + e.getMessage()); } // Extract parameters from the request // Rest of the method...
432-441: 🛠️ Refactor suggestionImprove date parsing error handling.
The current implementation silently returns
nullwhen date parsing fails, which can lead to downstream errors. Consider throwing an exception or using a default date value.private Date parseDate(String dateStr) { SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); try { return dateFormat.parse(dateStr); } catch (ParseException e) { logger.error("Error parsing date for grievance: " + dateStr, e); - return null; + throw new IllegalArgumentException("Invalid date format: " + dateStr, e); } }
🧹 Nitpick comments (6)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (6)
463-475: Consider refactoring to use fetchGrievanceDetailsFromRequest.The method
getGrievanceDetailsWithRemarksduplicates JSON parsing logic that already exists infetchGrievanceDetailsFromRequest. Consider refactoring to reuse that method.try { - // Parsing request to get the filter parameters (State, ComplaintResolution, StartDate, EndDate) - JSONObject requestObj = new JSONObject(request); - String complaintResolution = requestObj.optString("ComplaintResolution", null); - String state = requestObj.optString("State", null); - String fromDate = requestObj.optString("StartDate"); - String toDate = requestObj.optString("EndDate"); - - - // Convert StartDate and EndDate to Date objects - Date startDateStr = parseDate(fromDate); - Date endDateStr = parseDate(toDate); - - - List<GrievanceDetails> grievanceDetailsList = grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDateStr, endDateStr); + // Get the grievance details using the existing method + List<GrievanceDetails> grievanceDetailsList = fetchGrievanceDetailsFromRequest(request); + + // Extract complaint resolution for determining which remarks to fetch + JSONObject requestObj = new JSONObject(request); + String complaintResolution = requestObj.optString("ComplaintResolution", null);
478-480: Return a more structured response for empty results.Instead of returning a string message for empty results, consider returning an empty JSON array to maintain a consistent response structure.
if (grievanceDetailsList == null || grievanceDetailsList.isEmpty()) { - return "No grievance details found for the provided request."; + return objectMapper.writeValueAsString(new ArrayList<>()); }
529-546: Improve return value handling for empty results.The method returns a hardcoded message when no remarks are found, which becomes part of the API response. Consider returning an empty string or null instead.
private String fetchRemarksFromBenCallByComplaint(String complaintID) throws JSONException { // Query t_grievanceworklist to fetch the benRegId based on complaintID List<GrievanceDetails> grievanceWorklist = grievanceDataRepo.fetchGrievanceWorklistByComplaintID(complaintID); if (grievanceWorklist != null && !grievanceWorklist.isEmpty()) { GrievanceDetails grievance = grievanceWorklist.get(0); Long beneficiaryRegID = grievance.getBeneficiaryRegID(); // Fetch the beneficiaryRegID from the grievance // Query t_bencall to fetch remarks based on benRegId List<Object[]> benCallResults = beneficiaryCallRepo.fetchBenCallRemarks(beneficiaryRegID); if (benCallResults != null && !benCallResults.isEmpty()) { return (String) benCallResults.get(0)[0]; // Fetch the remarks } } - return "No remarks found in t_bencall"; + return ""; }
548-557: Improve return value handling for empty results.Similar to the previous method, this returns a hardcoded message when no remarks are found. Consider returning an empty string or null instead for consistency.
private String fetchRemarksFromGrievanceWorklist(String complaintID) throws JSONException { // Query t_grievanceworklist to fetch remarks based on complaintID List<Object[]> grievanceWorklistResults = grievanceDataRepo.fetchGrievanceWorklistRemarks(complaintID); if (grievanceWorklistResults != null && !grievanceWorklistResults.isEmpty()) { // Assuming grievanceWorklistResults has a format like [remarks] for simplicity return (String) grievanceWorklistResults.get(0)[0]; // Fetch the remarks } - return "No remarks found in t_grievanceworklist"; + return ""; }
51-52: Consider using more descriptive variable names for dependency injection.The variable name
iEMRCalltypeRepositoryImplCustomfollows Java naming conventions by starting with a lowercase letter, but theiprefix is easily confused with the common interface naming convention. Consider a clearer name.private final BeneficiaryCallRepository beneficiaryCallRepo; -private final IEMRCalltypeRepositoryImplCustom iEMRCalltypeRepositoryImplCustom; +private final IEMRCalltypeRepositoryImplCustom callTypeRepository; @Autowired public GrievanceHandlingServiceImpl(GrievanceDataRepo grievanceDataRepo, GrievanceOutboundRepository grievanceOutboundRepo, - BeneficiaryCallRepository beneficiaryCallRepo, IEMRCalltypeRepositoryImplCustom iEMRCalltypeRepositoryImplCustom) { + BeneficiaryCallRepository beneficiaryCallRepo, IEMRCalltypeRepositoryImplCustom callTypeRepository) { this.grievanceDataRepo = grievanceDataRepo; this.grievanceOutboundRepo = grievanceOutboundRepo; this.beneficiaryCallRepo = beneficiaryCallRepo; - this.iEMRCalltypeRepositoryImplCustom = iEMRCalltypeRepositoryImplCustom; + this.callTypeRepository = callTypeRepository;Also applies to: 55-60
485-515: Add null checks and appropriate validation for grievance properties.When setting properties on
grievanceResponse, there are no null checks, which could lead to NullPointerExceptions if any of the grievance properties are null.for (GrievanceDetails grievance : grievanceDetailsList) { GrievanceResponse grievanceResponse = new GrievanceResponse(); // Set basic grievance details with null checks grievanceResponse.setGrievanceId(grievance.getGrievanceId()); - grievanceResponse.setComplaintID(grievance.getComplaintID()); - grievanceResponse.setPrimaryNumber(grievance.getPrimaryNumber()); - grievanceResponse.setComplaintResolution(grievance.getComplaintResolution()); - grievanceResponse.setCreatedDate(grievance.getCreatedDate()); - grievanceResponse.setLastModDate(grievance.getLastModDate()); + grievanceResponse.setComplaintID(grievance.getComplaintID() != null ? grievance.getComplaintID() : ""); + grievanceResponse.setPrimaryNumber(grievance.getPrimaryNumber() != null ? grievance.getPrimaryNumber() : ""); + grievanceResponse.setComplaintResolution(grievance.getComplaintResolution() != null ? grievance.getComplaintResolution() : ""); + grievanceResponse.setCreatedDate(grievance.getCreatedDate()); + grievanceResponse.setLastModDate(grievance.getLastModDate());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/common/repository/callhandling/BeneficiaryCallRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/iemr/common/repository/callhandling/BeneficiaryCallRepository.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| // JsonNode jsonNode = null; | ||
| // | ||
| // try { | ||
| // // Parse the request JSON | ||
| // jsonNode = objectMapper.readTree(request); | ||
| // } catch (Exception e) { | ||
| // logger.error("Error parsing request: " + e.getMessage(), e); | ||
| // return null; | ||
| // } | ||
| // | ||
| // // Extract parameters from the request | ||
| // String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null; | ||
| // String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null; | ||
| // String startDate = jsonNode.get("StartDate").asText(); | ||
| // String endDate = jsonNode.get("EndDate").asText(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code.
There's a significant block of commented-out code that should be removed. This improves code readability and maintenance.
@Override
public String getGrievanceDetailsWithRemarks(String request) throws Exception {
ObjectMapper objectMapper = new ObjectMapper();
-// JsonNode jsonNode = null;
-//
-// try {
-// // Parse the request JSON
-// jsonNode = objectMapper.readTree(request);
-// } catch (Exception e) {
-// logger.error("Error parsing request: " + e.getMessage(), e);
-// return null;
-// }
-//
-// // Extract parameters from the request
-// String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null;
-// String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null;
-// String startDate = jsonNode.get("StartDate").asText();
-// String endDate = jsonNode.get("EndDate").asText();
-//📝 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.
| ObjectMapper objectMapper = new ObjectMapper(); | |
| // JsonNode jsonNode = null; | |
| // | |
| // try { | |
| // // Parse the request JSON | |
| // jsonNode = objectMapper.readTree(request); | |
| // } catch (Exception e) { | |
| // logger.error("Error parsing request: " + e.getMessage(), e); | |
| // return null; | |
| // } | |
| // | |
| // // Extract parameters from the request | |
| // String state = jsonNode.has("State") ? jsonNode.get("State").asText() : null; | |
| // String complaintResolution = jsonNode.has("ComplaintResolution") ? jsonNode.get("ComplaintResolution").asText() : null; | |
| // String startDate = jsonNode.get("StartDate").asText(); | |
| // String endDate = jsonNode.get("EndDate").asText(); | |
| @Override | |
| public String getGrievanceDetailsWithRemarks(String request) throws Exception { | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| // Additional implementation details below as needed. | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for removing the commented-out code! This improves the maintainability and readability of the codebase by eliminating unused code. Clean code like this is easier for other developers to understand and work with. Great job on the cleanup!
🐰 Happy rabbit dance
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| // ObjectMapper objectMapper = new ObjectMapper(); | ||
| return objectMapper.writeValueAsString(grievanceResponseList); | ||
|
|
||
| } catch (Exception e) { | ||
| logger.error("Error while getting grievance details with remarks: " + e.getMessage(), e); | ||
| return "Error fetching grievance details with remarks: " + e.getMessage(); | ||
| } |
There was a problem hiding this comment.
Improve error handling to prevent exposing exception details.
The current error handling exposes exception details in the API response, which could reveal sensitive information. Consider returning a generic error message instead.
// Convert the list of GrievanceResponse objects to JSON and return as a string
- // ObjectMapper objectMapper = new ObjectMapper();
return objectMapper.writeValueAsString(grievanceResponseList);
} catch (Exception e) {
logger.error("Error while getting grievance details with remarks: " + e.getMessage(), e);
- return "Error fetching grievance details with remarks: " + e.getMessage();
+ throw new Exception("Error processing grievance details request");
}📝 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.
| // ObjectMapper objectMapper = new ObjectMapper(); | |
| return objectMapper.writeValueAsString(grievanceResponseList); | |
| } catch (Exception e) { | |
| logger.error("Error while getting grievance details with remarks: " + e.getMessage(), e); | |
| return "Error fetching grievance details with remarks: " + e.getMessage(); | |
| } | |
| // ObjectMapper objectMapper = new ObjectMapper(); | |
| return objectMapper.writeValueAsString(grievanceResponseList); | |
| } catch (Exception e) { | |
| logger.error("Error while getting grievance details with remarks: " + e.getMessage(), e); | |
| throw new Exception("Error processing grievance details request"); | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Great! Thank you for implementing the fix. Your change improves security by preventing potential exposure of sensitive information through error messages. The exception is now being properly thrown with a generic message while still maintaining detailed logging for troubleshooting purposes.
This is a good security practice that helps protect your application from information disclosure vulnerabilities.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| // Default: Fetch remarks based on the grievance's specific conditions (no specific resolution status) | ||
| // remarks = fetchRemarksBasedOnConditions(grievance.getComplaintID()); | ||
| remarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement default remarks fetching logic.
The code for the 'else' condition refers to a non-existent method and then uses a placeholder implementation. Consider implementing a proper default strategy.
} else {
// Default: Fetch remarks based on the grievance's specific conditions (no specific resolution status)
- // remarks = fetchRemarksBasedOnConditions(grievance.getComplaintID());
- remarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID());
+ // Try to fetch from both sources and use the first available
+ String benCallRemarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID());
+ if (benCallRemarks != null && !benCallRemarks.startsWith("No remarks found")) {
+ remarks = benCallRemarks;
+ } else {
+ remarks = fetchRemarksFromGrievanceWorklist(grievance.getComplaintID());
+ }
}📝 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.
| // Default: Fetch remarks based on the grievance's specific conditions (no specific resolution status) | |
| // remarks = fetchRemarksBasedOnConditions(grievance.getComplaintID()); | |
| remarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID()); | |
| } | |
| } else { | |
| // Try to fetch from both sources and use the first available | |
| String benCallRemarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID()); | |
| if (benCallRemarks != null && !benCallRemarks.startsWith("No remarks found")) { | |
| remarks = benCallRemarks; | |
| } else { | |
| remarks = fetchRemarksFromGrievanceWorklist(grievance.getComplaintID()); | |
| } | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for making the fix! The default remarks fetching logic will now be more robust by trying to fetch remarks from both the beneficiary call and grievance worklist sources. This approach ensures you'll get the most relevant information available regardless of the complaint resolution status. The improved implementation makes the code more maintainable and provides better data consistency for the grievance resolution team.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (4)
489-506: Improve return value for better API consistency.The method currently returns a hardcoded message when no data is found. This might not be ideal for API responses, especially if the client expects actual remarks. Consider returning null or an empty string instead, and let the caller decide how to handle missing data.
-return "No remarks found in t_bencall"; +return ""; // Return empty string for missing remarksAdditionally, add more comprehensive null checking:
-if (grievanceWorklist != null && !grievanceWorklist.isEmpty()) { +if (grievanceWorklist != null && !grievanceWorklist.isEmpty() && grievanceWorklist.get(0) != null) { GrievanceDetails grievance = grievanceWorklist.get(0); + if (grievance.getBeneficiaryRegID() == null) { + logger.warn("BeneficiaryRegID is null for complaint ID: {}", complaintID); + return ""; + } Long beneficiaryRegID = grievance.getBeneficiaryRegID(); // Fetch the beneficiaryRegID from the grievance
508-517: Clarify comment and improve return value consistency.The comment on line 513 is vague. Additionally, similar to the previous method, consider returning an empty string instead of a hardcoded message for consistency.
-// Assuming grievanceWorklistResults has a format like [remarks] for simplicity +// Assuming the first column of the result contains the remarks-return "No remarks found in t_grievanceworklist"; +return ""; // Return empty string for missing remarks
447-456: Add null checking for grievance object properties.The method doesn't check if the properties of the grievance object are null before setting them in the response object, which could lead to NullPointerExceptions.
// Set basic grievance details grievanceResponse.setGrievanceId(grievance.getGrievanceId()); -grievanceResponse.setComplaintID(grievance.getComplaintID()); -grievanceResponse.setPrimaryNumber(grievance.getPrimaryNumber()); -grievanceResponse.setComplaintResolution(grievance.getComplaintResolution()); -grievanceResponse.setCreatedDate(grievance.getCreatedDate()); -grievanceResponse.setLastModDate(grievance.getLastModDate()); +grievanceResponse.setComplaintID(grievance.getComplaintID() != null ? grievance.getComplaintID() : ""); +grievanceResponse.setPrimaryNumber(grievance.getPrimaryNumber() != null ? grievance.getPrimaryNumber() : ""); +grievanceResponse.setComplaintResolution(grievance.getComplaintResolution() != null ? grievance.getComplaintResolution() : ""); +grievanceResponse.setCreatedDate(grievance.getCreatedDate()); +grievanceResponse.setLastModDate(grievance.getLastModDate());
438-440: Improve error message for empty grievance details.The current message is not informative. Consider providing more details about the filter criteria.
-return "No grievance details found for the provided request."; +return "No grievance details found for the provided criteria (State: " + state + + ", ComplaintResolution: " + complaintResolution + + ", DateRange: " + fromDate + " to " + toDate + ").";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(4 hunks)
🔇 Additional comments (2)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
466-469: Implement default remarks fetching logic.The code for the 'else' condition refers to a non-existent method and then uses a placeholder implementation. Consider implementing a proper default strategy.
-// Default: Fetch remarks based on the grievance's specific conditions (no specific resolution status) -// remarks = fetchRemarksBasedOnConditions(grievance.getComplaintID()); -remarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID()); +// Try to fetch from both sources and use the first available +String benCallRemarks = fetchRemarksFromBenCallByComplaint(grievance.getComplaintID()); +if (benCallRemarks != null && !benCallRemarks.startsWith("No remarks found")) { + remarks = benCallRemarks; +} else { + remarks = fetchRemarksFromGrievanceWorklist(grievance.getComplaintID()); +}
478-484: Improve error handling to prevent exposing exception details.The current error handling exposes exception details in the API response, which could reveal sensitive information. Consider returning a generic error message instead.
// Convert the list of GrievanceResponse objects to JSON and return as a string -// ObjectMapper objectMapper = new ObjectMapper(); return objectMapper.writeValueAsString(grievanceResponseList); } catch (Exception e) { logger.error("Error while getting grievance details with remarks: " + e.getMessage(), e); - return "Error fetching grievance details with remarks: " + e.getMessage(); + throw new Exception("Error processing grievance details request"); }
| Date startDateStr = parseDate(fromDate); | ||
| Date endDateStr = parseDate(toDate); | ||
|
|
There was a problem hiding this comment.
Handle potential NullPointerException for date parsing.
The method doesn't check if fromDate or toDate is null before parsing, which could lead to NullPointerExceptions.
-Date startDateStr = parseDate(fromDate);
-Date endDateStr = parseDate(toDate);
+if (fromDate == null || toDate == null) {
+ throw new IllegalArgumentException("StartDate and EndDate are required");
+}
+Date startDateStr = parseDate(fromDate);
+Date endDateStr = parseDate(toDate);📝 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.
| Date startDateStr = parseDate(fromDate); | |
| Date endDateStr = parseDate(toDate); | |
| if (fromDate == null || toDate == null) { | |
| throw new IllegalArgumentException("StartDate and EndDate are required"); | |
| } | |
| Date startDateStr = parseDate(fromDate); | |
| Date endDateStr = parseDate(toDate); |
There was a problem hiding this comment.
@srishtigrp78 Thank you for confirming! I can see that you've implemented the null check for the date parameters before attempting to parse them. This change will prevent potential NullPointerExceptions and improve the error handling in your code. Good work!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
Show resolved
Hide resolved
| try { | ||
| // Parsing request to get the filter parameters (State, ComplaintResolution, StartDate, EndDate) | ||
| JSONObject requestObj = new JSONObject(request); | ||
| String complaintResolution = requestObj.optString("ComplaintResolution", null); | ||
| String state = requestObj.optString("State", null); | ||
| String fromDate = requestObj.optString("StartDate"); | ||
| String toDate = requestObj.optString("EndDate"); | ||
|
|
||
| // Convert StartDate and EndDate to Date objects | ||
| Date startDateStr = parseDate(fromDate); | ||
| Date endDateStr = parseDate(toDate); | ||
|
|
||
| List<GrievanceDetails> grievanceDetailsList = grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDateStr, endDateStr); // Fetch grievance details based on the request |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing validation for required request parameters.
The method doesn't validate if the required parameters (StartDate and EndDate) are present in the request, which could lead to errors when parsing dates.
JSONObject requestObj = new JSONObject(request);
String complaintResolution = requestObj.optString("ComplaintResolution", null);
String state = requestObj.optString("State", null);
-String fromDate = requestObj.optString("StartDate");
-String toDate = requestObj.optString("EndDate");
+if (!requestObj.has("StartDate") || !requestObj.has("EndDate")) {
+ throw new IllegalArgumentException("StartDate and EndDate are required fields");
+}
+String fromDate = requestObj.getString("StartDate");
+String toDate = requestObj.getString("EndDate");📝 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.
| try { | |
| // Parsing request to get the filter parameters (State, ComplaintResolution, StartDate, EndDate) | |
| JSONObject requestObj = new JSONObject(request); | |
| String complaintResolution = requestObj.optString("ComplaintResolution", null); | |
| String state = requestObj.optString("State", null); | |
| String fromDate = requestObj.optString("StartDate"); | |
| String toDate = requestObj.optString("EndDate"); | |
| // Convert StartDate and EndDate to Date objects | |
| Date startDateStr = parseDate(fromDate); | |
| Date endDateStr = parseDate(toDate); | |
| List<GrievanceDetails> grievanceDetailsList = grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDateStr, endDateStr); // Fetch grievance details based on the request | |
| try { | |
| // Parsing request to get the filter parameters (State, ComplaintResolution, StartDate, EndDate) | |
| JSONObject requestObj = new JSONObject(request); | |
| String complaintResolution = requestObj.optString("ComplaintResolution", null); | |
| String state = requestObj.optString("State", null); | |
| if (!requestObj.has("StartDate") || !requestObj.has("EndDate")) { | |
| throw new IllegalArgumentException("StartDate and EndDate are required fields"); | |
| } | |
| String fromDate = requestObj.getString("StartDate"); | |
| String toDate = requestObj.getString("EndDate"); | |
| // Convert StartDate and EndDate to Date objects | |
| Date startDateStr = parseDate(fromDate); | |
| Date endDateStr = parseDate(toDate); | |
| List<GrievanceDetails> grievanceDetailsList = grievanceDataRepo.fetchGrievanceDetailsBasedOnParams(state, complaintResolution, startDateStr, endDateStr); // Fetch grievance details based on the request |
There was a problem hiding this comment.
@srishtigrp78 I see that you've already implemented validation for the required date parameters. You're checking if the fromDate or toDate are null after retrieving them with optString(), which achieves the same purpose of preventing null date parsing. Thanks for confirming!
| import com.iemr.common.repository.callhandling.IEMRCalltypeRepositoryImplCustom; | ||
| import com.iemr.common.repository.grievance.GrievanceDataRepo; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify usage of the injected repository.
The IEMRCalltypeRepositoryImplCustom is injected but doesn't appear to be used in the new methods. If it's not needed, consider removing it to reduce unnecessary dependencies.
🏁 Script executed:
#!/bin/bash
# Check if IEMRCalltypeRepositoryImplCustom is used in the class
grep -n "iEMRCalltypeRepositoryImplCustom\\." src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.javaLength of output: 128
🏁 Script executed:
#!/bin/bash
# Check for the declaration of IEMRCalltypeRepositoryImplCustom injection and its usage
echo "Searching for field injection declaration:"
rg "private.*IEMRCalltypeRepositoryImplCustom" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
echo ""
echo "Searching for any usage of the injected repository:"
rg "iemrCalltypeRepositoryImplCustom" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.javaLength of output: 552
Action: Remove the unused injected repository dependency.
- Verification confirms that the
IEMRCalltypeRepositoryImplCustomis injected via the field
private final IEMRCalltypeRepositoryImplCustom iEMRCalltypeRepositoryImplCustom;
insrc/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java, but no usage (e.g., method calls) was detected within the file. - If the repository is not intended for future use in this class, please remove both the injection and its related import to keep dependencies minimal and the code clean.
There was a problem hiding this comment.
🧩 Analysis chain
Based on the script results, I see that you've completely removed the IEMRCalltypeRepositoryImplCustom dependency as recommended. The import statement, field declaration, and constructor parameter have all been removed from the file.
@srishtigrp78 Great! Thank you for fixing the issue by removing the unused IEMRCalltypeRepositoryImplCustom dependency. This helps keep the code clean and reduces unnecessary dependencies in the class. The changes look good!
🏁 Script executed:
#!/bin/bash
# Check if IEMRCalltypeRepositoryImplCustom is still present in the file
echo "Checking for import statement:"
grep -n "import com.iemr.common.repository.callhandling.IEMRCalltypeRepositoryImplCustom" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java || echo "Import not found"
echo -e "\nChecking for field declaration:"
grep -n "private.*IEMRCalltypeRepositoryImplCustom" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java || echo "Field declaration not found"
echo -e "\nChecking for constructor parameter:"
grep -n "IEMRCalltypeRepositoryImplCustom" src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java || echo "Constructor parameter not found"
Length of output: 1135
@srishtigrp78 I see that the IEMRCalltypeRepositoryImplCustom dependency is still present in the file. Based on the script results, it's still included in:
- The import statement (line 36)
- The field declaration (line 52)
- The constructor parameter (line 56)
To fully address the issue, you'll need to:
-
Remove the import statement:
import com.iemr.common.repository.callhandling.IEMRCalltypeRepositoryImplCustom;
-
Remove the field declaration:
private final IEMRCalltypeRepositoryImplCustom iEMRCalltypeRepositoryImplCustom;
-
Update the constructor by removing the parameter:
public GrievanceHandlingServiceImpl(GrievanceDataRepo grievanceDataRepo, GrievanceOutboundRepository grievanceOutboundRepo, BeneficiaryCallRepository beneficiaryCallRepo) { this.grievanceDataRepo = grievanceDataRepo; this.grievanceOutboundRepo = grievanceOutboundRepo; this.beneficiaryCallRepo = beneficiaryCallRepo; }
Since this parameter isn't being used in any methods, it's safe to remove it completely.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| private Date parseDate(String dateStr) { | ||
| SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); | ||
| try { | ||
| return dateFormat.parse(dateStr); | ||
| } catch (ParseException e) { | ||
| logger.error("Error parsing date for grievance: " + dateStr, e); | ||
| throw new IllegalArgumentException("Invalid JSON format in request"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve date parsing error message.
The exception message in parseDate refers to "Invalid JSON format" which is misleading since this is specifically a date parsing error.
-throw new IllegalArgumentException("Invalid JSON format in request");
+throw new IllegalArgumentException("Invalid date format: " + dateStr);Additionally, consider adding null checking for the input parameter to prevent NullPointerExceptions.
📝 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.
| private Date parseDate(String dateStr) { | |
| SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); | |
| try { | |
| return dateFormat.parse(dateStr); | |
| } catch (ParseException e) { | |
| logger.error("Error parsing date for grievance: " + dateStr, e); | |
| throw new IllegalArgumentException("Invalid JSON format in request"); | |
| } | |
| } | |
| private Date parseDate(String dateStr) { | |
| if (dateStr == null) { | |
| throw new IllegalArgumentException("dateStr cannot be null"); | |
| } | |
| SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); | |
| try { | |
| return dateFormat.parse(dateStr); | |
| } catch (ParseException e) { | |
| logger.error("Error parsing date for grievance: " + dateStr, e); | |
| throw new IllegalArgumentException("Invalid date format: " + dateStr); | |
| } | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for implementing the fix! The improved error handling in the parseDate method will make debugging much easier by providing more accurate error messages and preventing potential NullPointerExceptions. These small improvements to error handling can save significant troubleshooting time when issues arise.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
AMM-1149 | API to push back Grievance data to the GR team (PSMRI#166)
* adding changes related to encryption and decryption * making final field static * making enclosing method static * adding beneficiaryConsent param to createFeedback API * Update src/main/java/com/iemr/common/data/feedback/FeedbackDetails.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * adding beneficiary consent param feedbacklist response * adding space * adding code for schedule for grievance data sync * adding code rabbit code suggestions * adding code rabbit suggestions * code rabbit suggested changes to Model classes * adding sonar quality changes * adding sonar quality code suggestions * adding sonar quality check suggestions * adding code rabbit suggestions * adding constant instead of duplicate literals * adding cod changs to fetch unallocated grievance count * adding changes suggested by sonar quality check * adding code rabbit suggested changes * fixing config file * fixing space * fixing space issue * adding package for PrimaryDBConfig * adding code changes for allocate API for allocating grievance to agents * removing space * adding language related changes * add language related changes * adding name convention related changes * adding code rabbit changes * adding sonar quality check suggested changes * removing unused imports * adding properties related to Grievance in properties file * placing the placeholder for jwt secret key * Update package.yml updating upload artifact version to v4 * removing grievanc related properties from application properties file * adding space * removing extra space * adding code for API that fetches grievance outbound worklist * adding code rabbit suggested changes * another addition of code rabbit suggested changes * fixing row no * adding code for API to save complaint resolution and remarks in the db * adding code rabbit suggested changes * removing lastModDate as it will update automatically in db * changing variable name of assignedUserID to userID * fixing remarks * removing full stop * adding code for API to close call and make reattempts if needed * adding code changes suggested by code rabbit * adding code rabbit suggested changes * adding code rabbit fixes * adding code rabbit fixes * Delete src/main/java/com/iemr/common/data/grievance/.gitignore * AMM-1148 | Close call API including reattempt logic based on calltype (#164) * adding code for API to close call and make reattempts if needed * adding code changes suggested by code rabbit * adding code rabbit suggested changes * adding code rabbit fixes * adding code rabbit fixes * Delete src/main/java/com/iemr/common/data/grievance/.gitignore --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * removing unused literal in repo class * changing logic of Close call after discussion * adding changes related to invalid and wrong number call type * adding code for API to push back grievance data to GR team * removed unnecessary conditions * adding code rabbit changes * adding code rabbit suggestions * adding required null checks * adding code rabbit suggested changes * removing unnecessary Exception * AMM-1149 | API to push back Grievance data to the GR team (#166) * adding code for API to push back grievance data to GR team * removed unnecessary conditions * adding code rabbit changes * adding code rabbit suggestions * adding required null checks * adding code rabbit suggested changes * removing unnecessary Exception --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * adding code changes for variable name change of lang * adding code changes for grievance issues * adding code changes for grievance issues (#168) Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * adding fixes related to grievance related APIs * fixing code rabbit issues * adding code fixes related to scheduler --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: SR20290919 <SR20290919@192.168.1.34> Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B.wipro.com>
…tion (#190) * adding changes related to encryption and decryption * making final field static * making enclosing method static * adding beneficiaryConsent param to createFeedback API * Update src/main/java/com/iemr/common/data/feedback/FeedbackDetails.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * adding beneficiary consent param feedbacklist response * adding space * adding code for schedule for grievance data sync * adding code rabbit code suggestions * adding code rabbit suggestions * code rabbit suggested changes to Model classes * adding sonar quality changes * adding sonar quality code suggestions * adding sonar quality check suggestions * adding code rabbit suggestions * adding constant instead of duplicate literals * adding cod changs to fetch unallocated grievance count * adding changes suggested by sonar quality check * adding code rabbit suggested changes * fixing config file * fixing space * fixing space issue * adding package for PrimaryDBConfig * adding code changes for allocate API for allocating grievance to agents * removing space * adding language related changes * add language related changes * adding name convention related changes * adding code rabbit changes * adding sonar quality check suggested changes * removing unused imports * adding properties related to Grievance in properties file * placing the placeholder for jwt secret key * Update package.yml updating upload artifact version to v4 * removing grievanc related properties from application properties file * adding space * removing extra space * adding code for API that fetches grievance outbound worklist * adding code rabbit suggested changes * another addition of code rabbit suggested changes * fixing row no * adding code for API to save complaint resolution and remarks in the db * adding code rabbit suggested changes * removing lastModDate as it will update automatically in db * changing variable name of assignedUserID to userID * fixing remarks * removing full stop * adding code for API to close call and make reattempts if needed * adding code changes suggested by code rabbit * adding code rabbit suggested changes * adding code rabbit fixes * adding code rabbit fixes * Delete src/main/java/com/iemr/common/data/grievance/.gitignore * AMM-1148 | Close call API including reattempt logic based on calltype (#164) * adding code for API to close call and make reattempts if needed * adding code changes suggested by code rabbit * adding code rabbit suggested changes * adding code rabbit fixes * adding code rabbit fixes * Delete src/main/java/com/iemr/common/data/grievance/.gitignore --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * removing unused literal in repo class * changing logic of Close call after discussion * adding changes related to invalid and wrong number call type * adding code for API to push back grievance data to GR team * removed unnecessary conditions * adding code rabbit changes * adding code rabbit suggestions * adding required null checks * adding code rabbit suggested changes * removing unnecessary Exception * AMM-1149 | API to push back Grievance data to the GR team (#166) * adding code for API to push back grievance data to GR team * removed unnecessary conditions * adding code rabbit changes * adding code rabbit suggestions * adding required null checks * adding code rabbit suggested changes * removing unnecessary Exception --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * adding code changes for variable name change of lang * adding code changes for grievance issues * adding code changes for grievance issues (#168) Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> * adding fixes related to grievance related APIs * fixing code rabbit issues * adding code fixes related to scheduler * Scheduler changes * adding import * adding grievance sms template changes * removing space --------- Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: SR20290919 <SR20290919@192.168.1.34> Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B.wipro.com> Co-authored-by: Ravi Shanigarapu <ravi.shanigarapu@wipro.com>
📋 Description
JIRA ID: 1149
API to push back grievance data to the GR team
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