Feature/grievence/changes #175
Conversation
WalkthroughThis update revises several configuration properties and Java methods for processing grievance data. The configuration files (common_ci, common_dev, common_example, common_test, common_uat) now append pagination parameters to the grievance API URL and adjust scheduler cron expressions to trigger more frequently. In the Java code, repository interfaces and service implementations have simplified method signatures (removing redundant parameters), updated SQL queries, and improved data synchronization by introducing a new helper method for building JSON requests and enhanced error logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Sync as GrievanceDataSyncImpl
participant API as Grievance Data API
participant Repo as Repository
Sync->>Sync: prepareRequestObject()
Sync->>API: Send HTTP request with JSON headers
API-->>Sync: Return response (status & data)
alt Status is 200
Sync->>Repo: Save grievance details directly
else Error occurs
Sync->>Sync: Log error for the specific complaint ID
end
Sync->>Sync: Return result string
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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: 1
🧹 Nitpick comments (9)
src/main/environment/common_example.properties (1)
176-176: Added pagination parameters to the grievance API URL.The addition of pagination parameters is a good practice for handling large datasets.
The parameter naming seems inconsistent:
page=PageNumbersuggests a variable to be substituted at runtime, whilecurrentpage=1is a hard-coded value. Consider either:-updateGrievanceDetails = <ENTER GRIEVANCE_API_BASE_URL>/grsbepro/igemr1097/public/api/v1/state-wise/grievance-list?page=PageNumber¤tpage=1 +updateGrievanceDetails = <ENTER GRIEVANCE_API_BASE_URL>/grsbepro/igemr1097/public/api/v1/state-wise/grievance-list?page=${pageNumber}¤tpage=${currentPage}src/main/environment/common_ci.properties (1)
165-167: Added grievance data sync scheduler configuration.These configuration properties establish consistent scheduler behavior across environments.
Setting
start-grievancedatasync-scheduler=falseis appropriate for the CI environment to avoid interference with tests. Would you like me to suggest configuration improvements for specific environments?src/main/environment/common_test.properties (1)
87-87: Scheduler frequency increased to every 2 minutesThe cron expression for the grievance data sync has been updated to run more frequently (every 2 minutes instead of every 5 minutes).
Consider evaluating whether this increased polling frequency is necessary and what impact it may have on system resources and API rate limits, especially in a production environment.
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (6)
143-145: Avoid manually setting the Content-Length header.
Spring typically handles the content length automatically. Maintaining a manual count (lines 144-145) can lead to mismatches if the payload is altered before transmission or if compression/transfer-encoding is applied.- int contentLength = json.getBytes(StandardCharsets.UTF_8).length; - headers.add("Content-Length", String.valueOf(contentLength));
147-148: Use parameterization instead of string replacement for pagination.
Replacing"PageNumber"in the URL may become brittle if the parameter name changes or multiple placeholders are introduced. Consider using URI parameters (e.g.,UriComponentsBuilder) orString.formatfor clarity and scalability.- String url = updateGrievanceDetails.replace("PageNumber", "1"); ... - String GrievanceUrl = updateGrievanceDetails.replace("PageNumber", String.valueOf(total)); + // Example with String.format for clarity: + String url = String.format("%s?page=%d", baseUrl, page);Also applies to: 161-162
163-170: Check for the presence of "total" in the JSON response before parsing.
At line 160 and onward, the code assumes"total"always exists. UsingjsnOBJ.has("total")or a default fallback would handle unexpected responses more gracefully and prevent errors.
259-297: Validate timestamp fields to avoid runtime parse errors.
While you safely check the presence of most JSON attributes, the code at lines 281–284 directly parses time fields ("createdAt","updatedAt") without verifying they exist or contain valid date strings. Consider adding a null or format check to avoid potentialDateTimeParseException.
313-313: Remove or clarify commented-out return code.
Leaving commented-out code (// return responseData;) can cause confusion about the intended structure. If it's no longer relevant, remove it for cleaner readability.
317-346: Externalize magic "level=3" and handle JSON conversion errors more robustly.
Hardcoding"level"as"3"can reduce flexibility. Also, ifobjectMapper.writeValueAsString(bodyMap)fails, the method currently returns an empty string. Consider surfacing or logging the exception cause more prominently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/environment/common_ci.properties(2 hunks)src/main/environment/common_dev.properties(2 hunks)src/main/environment/common_example.properties(2 hunks)src/main/environment/common_test.properties(2 hunks)src/main/environment/common_uat.properties(2 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java(4 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(11 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java (1)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#166
File: src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java:139-140
Timestamp: 2025-03-20T06:20:41.576Z
Learning: The query method `fetchGrievanceWorklistByComplaintID` in GrievanceDataRepo does not require filtering for deleted records (g.deleted = false) as per developer's preference.
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#161
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:256-261
Timestamp: 2025-03-20T06:20:41.576Z
Learning: Type safety checks for array values from stored procedure 'Pr_Grievanceworklist' are not required as the data types are guaranteed by the procedure contract.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Build
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
🔇 Additional comments (16)
src/main/environment/common_example.properties (1)
91-91:❓ Verification inconclusive
Increased frequency of grievance data synchronization.
Changing the cron schedule from every 5 minutes to every 2 minutes will make the data more up-to-date but might increase load on both your system and the external API.
Consider monitoring the impact of this increased frequency on system performance:
🏁 Script executed:
#!/bin/bash # Check if there are any logs showing potential performance issues with the increased frequency rg -A 5 "grievancedatasync" | grep -i "error\|exceed\|timeout\|slow"Length of output: 68
Action Required: Manual Verification of Performance Impact
The increased frequency change in the cron schedule has been implemented as shown below:
cron-scheduler-grievancedatasync=0 0/2 * * * ?While the performance monitoring script was executed:
#!/bin/bash # Check if there are any logs showing potential performance issues with the increased frequency rg -A 5 "grievancedatasync" | grep -i "error\|exceed\|timeout\|slow"the command produced no output. This lack of output is not conclusive evidence that there are no performance issues. Please manually verify the logs and system metrics to ensure that the increased synchronization frequency (from every 5 minutes to every 2 minutes) does not introduce any adverse load on the system or the external API.
- File Affected:
src/main/environment/common_example.properties(Line 91)- Action Steps:
- Manually inspect recent logs for any errors, timeouts, slow responses, or load exceeding issues related to
grievancedatasync.- Monitor system and API performance metrics over a sufficient period to confirm that the behavior remains acceptable.
src/main/environment/common_uat.properties (2)
62-62: Increased frequency of grievance data synchronization.Same change as in common_example.properties - the cron schedule has been adjusted to run every 2 minutes instead of every 5 minutes.
149-149: Added pagination parameters to the grievance API URL.Same change as in common_example.properties - pagination parameters have been added to the API URL.
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java (1)
18-19:❓ Verification inconclusive
Removed providerServiceMapID parameter from the stored procedure call.
The method signature has been simplified to only require the userId parameter.
Verify that the stored procedure
db_iemr.Pr_Grievanceworklistcan function correctly with only the userId parameter:
🏁 Script executed:
#!/bin/bash # Check the stored procedure definition rg -A 10 "CREATE PROCEDURE.*Pr_Grievanceworklist" --include="*.sql" # Check if any other code is still passing the providerServiceMapID parameter rg -A 3 "Pr_Grievanceworklist.*providerServiceMapID"Length of output: 566
Attention: Verify Stored Procedure Definition for Simplified Parameter Usage
The updated method in
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.javanow only accepts auserIdparameter for invokingdb_iemr.Pr_Grievanceworklist. Please ensure that:
- The stored procedure’s definition (in your SQL files) accepts only the
userIdparameter.- There are no lingering references in the codebase that pass the previously used
providerServiceMapIDparameter.Kindly run the revised shell script above or manually verify the stored procedure definition and related invocations to confirm correct functionality.
🏁 Script executed:
#!/bin/bash echo "Checking stored procedure definition for Pr_Grievanceworklist in SQL files..." rg -A 10 "CREATE PROCEDURE.*Pr_Grievanceworklist" -g "*.sql" echo "Checking for any references where providerServiceMapID is passed to Pr_Grievanceworklist..." rg -A 3 "Pr_Grievanceworklist.*providerServiceMapID"Length of output: 746
Action Required: Manually Verify SQL Procedure Definitions
The update in
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.javanow callsdb_iemr.Pr_Grievanceworklistwith only theuserIdparameter. Please verify that:
- The stored procedure definition (in your SQL files) only accepts the
userIdparameter.- There are no references anywhere in the codebase that are still passing the old
providerServiceMapIDparameter.Automated searches did not produce any output, so a manual review is recommended to ensure the SQL definitions and invocations are fully updated.
src/main/environment/common_ci.properties (1)
151-151: Added pagination parameters to the grievance API URL.Consistent with changes in other property files.
src/main/environment/common_test.properties (1)
178-178: Pagination parameters added to grievance API URLThe grievance API URL now includes pagination parameters, which is good for handling large datasets.
However, note that the
PageNumberparameter appears to be a placeholder. Make sure this gets properly replaced with the actual page number during API calls.src/main/environment/common_dev.properties (2)
121-121: Scheduler frequency increased to every 2 minutesThe cron expression for the grievance data sync has been updated to run more frequently (every 2 minutes instead of every 5 minutes), consistent with changes in the test environment.
176-176: Pagination parameters added to grievance API URLThe grievance API URL now includes pagination parameters, which helps with handling large datasets.
src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java (3)
37-38: Query updated to use beneficiarydetailsid parameterThe SQL query has been modified to use
beneficiarydetailsidinstead ofBeneficiaryRegID, and field names are now in lowercase, improving consistency.
46-50: Method signature simplified by removing providerServiceMapID parameterThe
fetchGrievanceRecordsCountmethod has been simplified to only require theuserIDparameter, making the interface cleaner.
163-164: Added helper method to retrieve BenDetailsId from BenRegIdThis new method provides a convenient way to map between the beneficiary registration ID and details ID.
This is a useful addition that supports the transition to using
beneficiarydetailsidas the primary identifier.src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
126-126: Simplified method call by removing providerServiceMapID parameterThe call to
grievanceDataRepo.fetchGrievanceRecordsCounthas been updated to only pass theuserIDparameter, matching the repository interface change.
280-283: Simplified validation logic in getFormattedGrievanceData methodThe validation has been updated to only check for
userIdand notproviderServiceMapID, aligning with other changes in the application.src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (3)
79-80: Constructor injection looks appropriate.
The newly injected repositories (GrievanceFetchBenDetailsRepo,LocationStateRepository, andIEMRCalltypeRepositoryImplCustom) are properly referenced below and appear to be used correctly.
112-112:✅ Verification successful
Confirm all callers of
dataSyncToGrievanceare updated for the changed return type.
Previously, this method returned aList<Map<String,Object>>; now it returns aString. This is a breaking change for any consumers expecting a list or structured data. Please verify that all references to this method are updated accordingly.Generate a script to locate all references of
dataSyncToGrievanceacross the codebase:
🏁 Script executed:
#!/bin/bash rg -A 3 "dataSyncToGrievance"Length of output: 2101
Action Required: Confirm Caller Updates for
dataSyncToGrievanceMethodAfter reviewing the codebase:
- The interface in GrievanceDataSync.java now only exposes
public String dataSyncToGrievance()(with the old signature commented out).- In GrievanceDataSyncImpl.java, the method call assignment has been updated from the former list type to a
String(the previous assignment usingList<Map<String, Object>>is properly commented out).- The scheduling class in ScheduleForGrievanceDataSync.java simply calls the method without expecting a list—this remains compatible with the new return type.
All references appear to be updated accordingly.
134-134:✅ Verification successful
Verify that the server endpoint expects JSON requests.
The content type is now set toMediaType.APPLICATION_JSONinstead ofAPPLICATION_FORM_URLENCODED. Ensure the grievance API accepts JSON payloads to avoid 4xx errors.
🏁 Script executed:
#!/bin/bash # Search relevant configuration or usage references to confirm JSON acceptance rg "MediaType.APPLICATION_JSON" -A 5Length of output: 71336
Confirm API JSON Consumption in GrievanceDataSyncImpl.java
The change to set the content type toMediaType.APPLICATION_JSONinsrc/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(line 134) is in line with the pattern observed in other parts of the codebase (e.g., in UptsuServiceImpl and various controller endpoints). Please double-check that the corresponding grievance API endpoint is configured with a JSON request contract (i.e., it ‘consumes’ JSON) to prevent any potential 4xx errors.
|
|
||
| GrievanceCallRequest grievanceCallRequest = InputMapper.gson().fromJson(request, GrievanceCallRequest.class); | ||
| String complaintID = grievanceCallRequest.getComplaintID(); | ||
| Integer userID = grievanceCallRequest.getUserID(); | ||
| Boolean isCompleted = grievanceCallRequest.getIsCompleted(); | ||
| Long beneficiaryRegID = grievanceCallRequest.getBeneficiaryRegID(); | ||
| Integer callTypeID = grievanceCallRequest.getCallTypeID(); | ||
| Integer providerServiceMapID = grievanceCallRequest.getProviderServiceMapID(); | ||
|
|
||
| CallType callTypeObj = new CallType(); | ||
| String response = "failure"; | ||
| int updateCount = 0; | ||
| int updateCallCounter = 0; | ||
| try { | ||
|
|
||
| GrievanceDetails grievanceCallStatus = new GrievanceDetails(); | ||
|
|
||
| List<Object[]> lists = grievanceDataRepo.getCallCounter(complaintID); | ||
| for (Object[] objects : lists) { | ||
| if (objects != null && objects.length >= 2) { | ||
| grievanceCallStatus.setCallCounter((Integer) objects[0]); | ||
| grievanceCallStatus.setRetryNeeded((Boolean)objects[1]); | ||
| } | ||
| String complaintID = grievanceCallRequest.getComplaintID(); | ||
| Integer userID = grievanceCallRequest.getUserID(); | ||
| Boolean isCompleted = grievanceCallRequest.getIsCompleted(); | ||
| Long beneficiaryRegID = grievanceCallRequest.getBeneficiaryRegID(); | ||
| Integer callTypeID = grievanceCallRequest.getCallTypeID(); | ||
| Integer providerServiceMapID = grievanceCallRequest.getProviderServiceMapID(); | ||
|
|
||
| CallType callTypeObj = new CallType(); | ||
| String response = "failure"; | ||
| int updateCount = 0; | ||
| int updateCallCounter = 0; | ||
| try { | ||
|
|
||
| GrievanceDetails grievanceCallStatus = new GrievanceDetails(); | ||
|
|
||
| List<Object[]> lists = grievanceDataRepo.getCallCounter(complaintID); | ||
| for (Object[] objects : lists) { | ||
| if (objects != null && objects.length >= 2) { | ||
| grievanceCallStatus.setCallCounter((Integer) objects[0]); | ||
| grievanceCallStatus.setRetryNeeded((Boolean) objects[1]); | ||
| } | ||
|
|
||
| // Fetching CallDetails using BenCallID and CallTypeID | ||
| Set<Object[]> callTypesArray = new HashSet<Object[]>(); | ||
| callTypesArray = iEMRCalltypeRepositoryImplCustom.getCallDetails(callTypeID); | ||
| for (Object[] object : callTypesArray) | ||
| { | ||
| if (object != null && object.length >= 2) | ||
| { | ||
| callTypeObj.setCallGroupType((String) object[0]); | ||
| callTypeObj.setCallType((String) object[1]); | ||
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| String callGroupType = callTypeObj.getCallGroupType(); | ||
| String callType = callTypeObj.getCallType(); | ||
|
|
||
|
|
||
| // Logic for reattempt based on call group type and call type | ||
|
|
||
| boolean isRetryNeeded = grievanceCallStatus.getRetryNeeded(); | ||
| if (callGroupType.equals("Valid")) { | ||
| // Conditions when no reattempt is needed | ||
| if (callType.equals("Valid") || callType.equals("Test Call")) { | ||
| isRetryNeeded = false; | ||
| } else if (callType.equals("Disconnected Call") || callType.equals("Serviced Call") || | ||
| callType.equals("Silent Call") || callType.equals("Call Back")) { | ||
| // Reattempt is needed for these call subtypes | ||
| isRetryNeeded = true; | ||
| } | ||
| } | ||
| if (callGroupType.equals("Invalid") && callType.equals("Wrong Number")) { | ||
| isRetryNeeded = false; | ||
| //isCompleted = true; | ||
| grievanceDataRepo.updateCompletedStatusInCall(isCompleted, isRetryNeeded, complaintID, userID, beneficiaryRegID, providerServiceMapID); | ||
| } | ||
|
|
||
| // Check if max attempts (3) are reached | ||
| if (isRetryNeeded == true && grievanceCallStatus.getCallCounter() < grievanceAllocationRetryConfiguration) { | ||
| // Increment the call counter for reattempt | ||
| grievanceCallStatus.setCallCounter(grievanceCallStatus.getCallCounter() + 1); | ||
| // Update the retryNeeded flag | ||
| isRetryNeeded = true; | ||
| //isCompleted = false; | ||
| updateCallCounter = grievanceDataRepo.updateCallCounter(grievanceCallStatus.getCallCounter(), isRetryNeeded, grievanceCallRequest.getComplaintID(), | ||
| grievanceCallRequest.getBeneficiaryRegID(), grievanceCallRequest.getProviderServiceMapID(), | ||
| grievanceCallRequest.getUserID()); | ||
| // Return success when reattempt logic is applied successfully. The grievance call needs to be retried, and a reattempt is performed. | ||
| if (updateCallCounter > 0) | ||
| response = "Successfully closing call"; | ||
| else { | ||
| response = "failure in closing call"; | ||
| } | ||
| } else if (grievanceCallStatus.getCallCounter()== grievanceAllocationRetryConfiguration) { | ||
| // Max attempts reached, no further reattempt | ||
| isRetryNeeded = false; | ||
| //isCompleted = true; | ||
| updateCount = grievanceDataRepo.updateCompletedStatusInCall(isCompleted, isRetryNeeded, complaintID, userID, beneficiaryRegID, providerServiceMapID); | ||
| response = "max_attempts_reached"; // Indicate that max attempts are reached | ||
|
|
||
|
|
||
| } else { | ||
| } | ||
|
|
||
| response = "no_reattempt_needed"; // No reattempt needed | ||
| } | ||
| // Fetching CallDetails using BenCallID and CallTypeID | ||
| Set<Object[]> callTypesArray = new HashSet<Object[]>(); | ||
| callTypesArray = iEMRCalltypeRepositoryImplCustom.getCallDetails(callTypeID); | ||
| for (Object[] object : callTypesArray) { | ||
| if (object != null && object.length >= 2) { | ||
| callTypeObj.setCallGroupType((String) object[0]); | ||
| callTypeObj.setCallType((String) object[1]); | ||
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
| catch (Exception e) { | ||
| response = "error: " + e.getMessage(); | ||
| } | ||
| String callGroupType = callTypeObj.getCallGroupType(); | ||
| String callType = callTypeObj.getCallType(); | ||
|
|
||
| return response; // Return the response (either success or error message) | ||
| } | ||
| // Logic for reattempt based on call group type and call type | ||
|
|
||
| boolean isRetryNeeded = grievanceCallStatus.getRetryNeeded(); | ||
| if (callGroupType.equals("Valid")) { | ||
| // Conditions when no reattempt is needed | ||
| if (callType.equals("Valid") || callType.equals("Test Call")) { | ||
| isRetryNeeded = false; | ||
| } else if (callType.equals("Disconnected Call") || callType.equals("Serviced Call") | ||
| || callType.equals("Silent Call") || callType.equals("Call Back")) { | ||
| // Reattempt is needed for these call subtypes | ||
| isRetryNeeded = true; | ||
| } | ||
| } | ||
| if (callGroupType.equals("Invalid") && callType.equals("Wrong Number")) { | ||
| isRetryNeeded = false; | ||
| // isCompleted = true; | ||
| grievanceDataRepo.updateCompletedStatusInCall(isCompleted, isRetryNeeded, complaintID, userID, | ||
| beneficiaryRegID, providerServiceMapID); | ||
| } | ||
|
|
||
| // Check if max attempts (3) are reached | ||
| if (isRetryNeeded == true && grievanceCallStatus.getCallCounter() < grievanceAllocationRetryConfiguration) { | ||
| // Increment the call counter for reattempt | ||
| grievanceCallStatus.setCallCounter(grievanceCallStatus.getCallCounter() + 1); | ||
| // Update the retryNeeded flag | ||
| isRetryNeeded = true; | ||
| // isCompleted = false; | ||
| updateCallCounter = grievanceDataRepo.updateCallCounter(grievanceCallStatus.getCallCounter(), | ||
| isRetryNeeded, grievanceCallRequest.getComplaintID(), | ||
| grievanceCallRequest.getBeneficiaryRegID(), grievanceCallRequest.getProviderServiceMapID(), | ||
| grievanceCallRequest.getUserID()); | ||
| // Return success when reattempt logic is applied successfully. The grievance | ||
| // call needs to be retried, and a reattempt is performed. | ||
| if (updateCallCounter > 0) | ||
| response = "Successfully closing call"; | ||
| else { | ||
| response = "failure in closing call"; | ||
| } | ||
| } else if (grievanceCallStatus.getCallCounter() == grievanceAllocationRetryConfiguration) { | ||
| // Max attempts reached, no further reattempt | ||
| isRetryNeeded = false; | ||
| // isCompleted = true; | ||
| updateCount = grievanceDataRepo.updateCompletedStatusInCall(isCompleted, isRetryNeeded, complaintID, | ||
| userID, beneficiaryRegID, providerServiceMapID); | ||
| response = "max_attempts_reached"; // Indicate that max attempts are reached | ||
|
|
||
| } else { | ||
|
|
||
| response = "no_reattempt_needed"; // No reattempt needed | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
| response = "error: " + e.getMessage(); | ||
| } | ||
|
|
||
| return response; // Return the response (either success or error message) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Evaluate concurrency handling for call completion.
If completeGrievanceCall can be invoked by multiple threads for the same complaint, there's a potential race when reading and incrementing callCounter. Consider using transaction locking or other concurrency controls to prevent data races and inconsistent states.
There was a problem hiding this comment.
Scheduler will run only once in a day so multiple threads can not be invoke
There was a problem hiding this comment.
@ravishanigarapu Thanks for clarifying. I understand that the scheduler runs only once per day, which eliminates the concurrency concerns I had raised. This context is helpful for understanding the system design choices.
✏️ Learnings added
Learnt from: ravishanigarapu
PR: PSMRI/Common-API#175
File: src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java:541-637
Timestamp: 2025-03-20T07:16:43.734Z
Learning: In the PSMRI/Common-API grievance system, the scheduler runs only once per day, so concurrent thread execution is not a concern for methods like `completeGrievanceCall`.
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.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
451-455: Redundant recursive data sync call.The
dataSyncToGrievance()method is called recursively from withingenerateGrievanceAuthToken(), which could lead to multiple synchronization attempts during a single scheduled run. This might be intentional for retry purposes, but consider using a boolean flag parameter to prevent unintended recursive behavior.- // List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); - String savedGrievanceData = dataSyncToGrievance(); + String savedGrievanceData = dataSyncToGrievance(true); // Pass a flag to indicate it's a retry // Then update the method signature: - public String dataSyncToGrievance() { + public String dataSyncToGrievance(boolean isRetry) {And add a check at the beginning of the method to prevent deep recursion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(10 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
Learnt from: ravishanigarapu
PR: PSMRI/Common-API#175
File: src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java:541-637
Timestamp: 2025-03-20T07:16:43.865Z
Learning: In the PSMRI/Common-API grievance system, the scheduler runs only once per day, so concurrent thread execution is not a concern for methods like `completeGrievanceCall`.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (java)
- GitHub Check: Package-test
- GitHub Check: Build
🔇 Additional comments (10)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (10)
3-4: New imports for enhanced request processing.The additions of
BigIntegerandStandardCharsetsimports support the improvements in the request handling, particularly with the explicit character encoding for content length calculation.
112-117: Method signature change improves API clarity.The return type has been changed from
List<Map<String, Object>>toString, providing a simpler and more consistent interface. The method now returns a success message rather than the detailed data, which aligns better with its purpose as a synchronization method.
134-148: Updated request format to use JSON instead of form data.The content type has been changed from
APPLICATION_FORM_URLENCODEDtoAPPLICATION_JSON, which is more appropriate for structured data. The new approach with explicit content length calculation usingStandardCharsets.UTF_8ensures proper encoding handling.
152-169: Improved response handling with pagination support.The code now correctly checks for HTTP status 200 instead of a custom status code and implements pagination by:
- Retrieving the total count from the initial response
- Making a second request with the proper page number
- Processing all returned records efficiently
This enhancement ensures complete data retrieval from the grievance API.
180-183: Good defensive coding with duplicate detection.The code now checks if a complaint already exists before processing it, avoiding duplicate entries and unnecessary processing. The detailed logging helps with troubleshooting.
301-304: Enhanced error handling with complaint-specific logging.The try-catch block now captures the specific complaint ID in the error log, making it easier to identify and troubleshoot issues with particular grievance records without failing the entire batch.
320-348: Well-structured request object creation.The new
prepareRequestObject()method creates a clean separation of concerns by encapsulating the JSON request building logic. This improves code organization and maintainability. The method handles all necessary pagination and search parameters required by the API.
414-431: Improved authentication token extraction with error handling.The updated code properly handles the nested JSON structure for token extraction and includes robust error handling with detailed logging. This prevents null pointer exceptions and provides clear diagnostic information when authentication fails.
541-638: No concurrency concerns incompleteGrievanceCallmethod.As confirmed in previous feedback, the scheduler runs only once per day, so concurrent thread execution is not a concern for this method. The implementation correctly handles the call counter logic and retry mechanism based on call types and configured retry limits.
605-622: Potential business logic issue with call retry mechanism.The condition checks if
isRetryNeeded == truebut doesn't handle the case where it might be false due to the conditions in lines 587-602. Consider adding an explicit else branch for whenisRetryNeededis false but the counter hasn't reached the max attempts.
📋 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
Refactor