Fixes for grievance scheduler #173
Conversation
latest pull
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
AMM-1137 | after
latest pull
AMM-1076 and AMM-1078 (PSMRI#146)
latest pull 19 jan
latest pull
changes related to 1148
Merge pull request PSMRI#165 from srishtigrp78/feature/version/upgrade
* 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>
AMM-1149 | API to push back Grievance data to the GR team (PSMRI#166)
supervisor screen variable name change
Co-authored-by: SR20290919 <SR20290919@APL-PG02PW8B>
…rp78/Common-API.git into feature/version/upgrade
adding fixes related to Grievance APIs
Merge pull request PSMRI#169 from srishtigrp78/feature/version/upgrade
pull 120325
WalkthroughThis pull request introduces changes across multiple modules. Configuration files now have the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as GrievanceController
participant Service as GrievanceDataSyncImpl/GrievanceHandlingServiceImpl
participant Repo as GrievanceDataRepo
Client->>Controller: completeGrievanceCall(request)
Controller->>Service: completeGrievanceCall(request)
Service->>Repo: Update call counter/status & process grievance call
Repo-->>Service: Return update result
Service-->>Controller: Response (success/error message)
Controller-->>Client: Return final response
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 5
🔭 Outside diff range comments (1)
src/main/environment/common_dev.properties (1)
177-178:⚠️ Potential issueEnsure Consistency for Grievance Allocation Retry Configuration
It appears that the
grievanceAllocationRetryConfigurationproperty is not present at the end of this file, although the AI summary indicates it should be updated from 1 to 3 as in the other environments. Please verify whether this property must be added to guarantee consistent behavior. If so, consider appending the following:+grievanceAllocationRetryConfiguration=3
🧹 Nitpick comments (25)
src/main/java/com/iemr/common/repository/callhandling/IEMRCalltypeRepositoryImplCustom.java (1)
91-93: Consider adding Javadoc to the new methodThe new
getCallDetailsmethod looks good. It would benefit from Javadoc documentation explaining its purpose and usage.+/** + * Retrieves the call group type and call type for a specific call type ID. + * + * @param callTypeID The ID of the call type to retrieve details for + * @return A set of Object[] containing callGroupType and callType + */ @Query("select callGroupType, callType from CallType " + "where callTypeID = :callTypeID") Set<Object[]> getCallDetails(@Param("callTypeID") Integer callTypeID);src/main/java/com/iemr/common/data/grievance/GrievanceCallRequest.java (3)
8-15: Add field access modifiers and standardize naming conventionsThe fields in this class should have explicit access modifiers (typically
private) and follow consistent naming conventions.- String complaintID; - Integer userID; - Boolean isCompleted; - Long beneficiaryRegID; - Integer callTypeID; - Long benCallID; - Integer providerServiceMapID; - String createdBy; + private String complaintID; + private Integer userID; + private Boolean isCompleted; + private Long beneficiaryRegID; + private Integer callTypeID; + private Long benCallID; + private Integer providerServiceMapID; + private String createdBy;
6-17: Consider adding class-level documentationAdding Javadoc would help explain the purpose of this class in the grievance handling system.
@Data +/** + * Data Transfer Object for handling grievance call requests. + * Used to capture request parameters for completing or updating grievance calls. + */ public class GrievanceCallRequest {
8-15: Consider adding field validationsFor fields that shouldn't be null, consider adding validation annotations (e.g.,
@NotNull).Add validation annotations to critical fields:
+import javax.validation.constraints.NotNull; + @Data public class GrievanceCallRequest { - String complaintID; - Integer userID; + @NotNull(message = "Complaint ID cannot be null") + private String complaintID; + @NotNull(message = "User ID cannot be null") + private Integer userID;src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java (1)
20-20: New method added for completing grievance callsThe new
completeGrievanceCallmethod extends the interface capabilities to handle completion of grievance calls.However, it throws a generic Exception which is not a best practice. Consider:
- Using more specific exception types that reflect the actual failure modes
- Documenting the expected format of the request string (likely JSON)
-public String completeGrievanceCall(String request) throws Exception; +public String completeGrievanceCall(String request) throws IEMRException, JSONException;src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java (2)
116-122: Getter and setter methods for the new grievanceId fieldThe getter and setter methods for grievanceId appear to be correctly implemented.
Note that while Lombok's @DaTa annotation is present on the class which should auto-generate getters and setters, the class still explicitly defines all getter and setter methods. Consider either:
- Removing @DaTa and keeping explicit getters/setters for clarity
- Relying on Lombok to generate all getters/setters and removing the explicit methods
This would improve code consistency and maintainability.
33-36: Inconsistent validation annotationsIt appears the @notblank annotations have been removed from the fileName and fileType fields, while @SiZe constraints remain. This inconsistency could lead to validation issues.
If these fields are truly required, consider reinstating the @notblank annotations to ensure proper validation.
@Column(name = "FileName", nullable = false, length = 500) @Expose +@NotBlank(message = "File name cannot be blank") @Size(max = 500, message = "File name cannot exceed 500 characters") private String fileName; @Column(name = "FileType", nullable = false, length = 300) @Expose +@NotBlank(message = "File type cannot be blank") @Size(max = 300, message = "File type cannot exceed 300 characters") private String fileType;Also applies to: 38-41
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java (1)
17-20:❓ Verification inconclusive
Changed from @procedure to @query for stored procedure call
The method implementation has been modified to use @query with a native SQL call to the stored procedure instead of using the @procedure annotation. Parameter naming has also been standardized (providerServiceMapId → providerServiceMapID).
While both approaches work, this change has potential implications:
- The @transactional annotation has been removed - verify if transaction handling is now managed at a higher level
- The schema name 'db_iemr' is now hardcoded in the query - this could cause issues if deploying to environments with different schema names
Consider:
- Making the schema name configurable through a property
- Ensuring transaction handling is still appropriate
-@Query(value =" call db_iemr.Pr_Grievanceworklist(:providerServiceMapID, :userId)", nativeQuery = true) +@Query(value =" call ${db.schema}.Pr_Grievanceworklist(:providerServiceMapID, :userId)", nativeQuery = true)
🏁 Script executed:
#!/bin/bash # Check for other hardcoded db_iemr references that might need standardization rg "db_iemr\." --glob "*.java" # Check if transaction management is handled at service level for this repository's methods rg -A 3 -B 3 "GrievanceOutboundRepository" --glob "*.java" | grep -A 10 -B 10 "@Transactional"Length of output: 3499
Action Required: Verify Schema Config & Transaction Handling for Stored Procedure Call
- The repository method now uses a native SQL call with @query instead of @procedure, and the parameter name has been standardized.
- Hardcoded Schema: The query uses the hardcoded schema name
db_iemr. Similar occurrences were found in the codebase. For improved flexibility across environments, consider making the schema configurable (for example, replacing"db_iemr"with"${db.schema}").- Transaction Management: The @transactional annotation appears to have been removed. Please confirm that transaction management is appropriately handled at a higher level (e.g., in the service layer or through a global transaction configuration).
-@Query(value =" call db_iemr.Pr_Grievanceworklist(:providerServiceMapID, :userId)", nativeQuery = true) +@Query(value =" call ${db.schema}.Pr_Grievanceworklist(:providerServiceMapID, :userId)", nativeQuery = true)src/main/java/com/iemr/common/dto/grivance/GrievanceWorklistDTO.java (2)
47-47: Updated constructor to include lastCall parameterThe constructor has been properly updated to include and initialize the new lastCall field.
However, there appears to be a package name typo in the path: "grivance" instead of "grievance". Consider fixing this in a future refactoring to maintain consistency.
The package name contains a typo:
com.iemr.common.dto.grivanceshould becom.iemr.common.dto.grievance.Also applies to: 72-72
1-79: Missing a more concise constructor or builder patternThe class has a large constructor with 23 parameters, which makes instantiation error-prone and difficult to maintain.
Consider adding:
- A builder pattern implementation (could leverage Lombok's @builder)
- Multiple constructors for common use cases
- Static factory methods for common creation scenarios
This would improve code readability and maintainability.
You could leverage Lombok to simplify this with:
@Data @NoArgsConstructor @AllArgsConstructor @Builder public class GrievanceWorklistDTO implements Serializable { // fields... }src/main/java/com/iemr/common/data/grievance/GrievanceResponse.java (2)
5-70: Consider migrating fromTimestampto modern Java date/time API
Currently, the class usesjava.sql.Timestampfor date fields (createdDate,lastModDate). It's often recommended to use the Java 8+ date/time types (LocalDateTime,ZonedDateTime, etc.) for clearer semantics and time-zone handling.- private Timestamp createdDate; - private Timestamp lastModDate; + private LocalDateTime createdDate; + private LocalDateTime lastModDate;
5-70: Leverage Lombok for boilerplate
To reduce boilerplate (getters/setters), you could consider using Lombok annotations (@Dataor@Getter/@Setter) if permitted in your project. It will make the code more concise and easier to maintain.src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (5)
103-105: Validate thegrievanceAllocationRetryConfigurationvalue
Ensure that this configuration value is never negative or zero, since it controls the number of reattempts for grievance handling. Consider validating it or providing a safe default when loading.@Value("${grievanceAllocationRetryConfiguration}") private int grievanceAllocationRetryConfiguration; + @PostConstruct + private void validateRetryConfig() { + if (grievanceAllocationRetryConfiguration < 1) { + logger.warn("Invalid 'grievanceAllocationRetryConfiguration'. Setting it to 1 as a fallback."); + grievanceAllocationRetryConfiguration = 1; + } + }
185-210: Avoid throwing genericRuntimeExceptionupon detecting existing complaint
In lines 198–202, throwing aRuntimeExceptionbreaks the entire data-sync loop whenever a duplicate complaint is encountered. Consider handling duplicates more gracefully—e.g., logging and continuing—or throwing a custom exception that you can handle upstream.if (complaintExists) { - throw new RuntimeException("Complaint ID " + formattedComplaintId - + " already exists in the grievance worklist table."); + logger.warn("Duplicate complaint ID {} encountered; skipping insertion.", formattedComplaintId); + continue; // or handle in a safer manner }
289-333: Set transactions on each grievance record to maintain referential clarity
Currently, transactions are saved, but not linked back to theGrievanceDetailsobject in memory (line 335 is commented out). If you plan on retrieving the data from the same objects later, consider setting the relationship field for clarity and consistency, or remove the commented code if it’s no longer needed.
397-434: Thread safety concern with shared auth token storage
GRIEVANCE_AUTH_TOKENandGRIEVANCE_TOKEN_EXPare stored in static fields. If multiple threads invokefetchGrievanceTransactions()andgenerateGrievanceAuthToken(), data races might occur. Consider synchronizing token generation or switching to a thread-safe approach (e.g., store tokens in a thread-local or an injected bean with proper locking).
593-694: Clarify “completed” vs. “retryNeeded” logic
IncompleteGrievanceCall(), the interplay ofisCompletedandisRetryNeededcan be confusing, especially when call counters approach thegrievanceAllocationRetryConfiguration. Provide more explicit documentation or rename variables to clarify whether a grievance is truly finished or awaits further attempts.src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (3)
4-10: Check for unused imports
Imports likejava.text.ParseException,java.text.SimpleDateFormat, and especially any not used in the changed code might indicate dead code or leftover references. If not used, remove them for clarity.
25-25: Avoid potential overhead ofObjectMapper
WhileObjectMapperis powerful, consider re-using a single, shared instance or using your existingInputMapperif appropriate. It might reduce overhead and ensure consistent serialization settings across the codebase.
371-423: Validate fields before callingupdateComplaintResolution
InsaveComplaintResolution(), the code checks for mandatory fields, which is good. Consider adding explicit or more descriptive validation for potential constraints (e.g., max length of remarks) to avoid DB-level errors.src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (3)
133-165: Returning a map-based response
Switching the return type toResponseEntity<Map<String, Object>>provides flexibility but sacrifices compile-time type checks. Consider using a dedicated response DTO for clearer structure.
170-189: NewsaveComplaintResolutionmethod
Implementation follows existing patterns. Returning a raw JSON string is serviceable, but migrating toResponseEntityor a typed DTO can improve consistency and clarity.
221-233: NewgetGrievanceDetailsWithRemarksmethod
This addition is coherent with the new fields. Similar to other endpoints, consider a standardized response object for better maintainability.src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java (2)
74-76:updateGrievanceAllocationStatus
TogglesisAllocated. Validate that updating only the allocation status is sufficient if user references must also be unassigned.
100-112: OverloadedupdateComplaintResolution
This alternative method omits theremarksparameter. Consolidating or documenting the difference between both methods is advisable to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_dev.properties(1 hunks)src/main/environment/common_example.properties(1 hunks)src/main/environment/common_test.properties(1 hunks)src/main/environment/common_uat.properties(1 hunks)src/main/java/com/iemr/common/controller/grievance/GrievanceController.java(4 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceAllocationRequest.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceCallRequest.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java(6 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceResponse.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java(3 hunks)src/main/java/com/iemr/common/data/grievance/MoveToBinRequest.java(1 hunks)src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java(2 hunks)src/main/java/com/iemr/common/dto/grivance/GrievanceWorklistDTO.java(2 hunks)src/main/java/com/iemr/common/repository/callhandling/BeneficiaryCallRepository.java(1 hunks)src/main/java/com/iemr/common/repository/callhandling/IEMRCalltypeRepositoryImplCustom.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java(2 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java(2 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(13 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(10 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (1)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#163
File: src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java:111-118
Timestamp: 2025-03-12T13:28:03.718Z
Learning: The `complaintResolution` and `remarks` fields in GrievanceDetails class are intentionally kept without validation constraints (@NotBlank, @Size) as per the team's decision.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Analyze (java)
- GitHub Check: Build
- GitHub Check: Package-test
🔇 Additional comments (44)
src/main/environment/common_example.properties (1)
189-189: Updated Grievance Allocation Retry ConfigurationThe value for
grievanceAllocationRetryConfigurationhas been updated to 3 in the example environment file. This change increases the number of retry attempts for grievance allocation and is consistent with the intended behavior across environments.src/main/environment/common_ci.properties (1)
163-163: Updated Retry Configuration for CI EnvironmentThe
grievanceAllocationRetryConfigurationis now set to 3 in the CI configuration file, which aligns with the unified approach across different environments.src/main/environment/common_test.properties (1)
187-188: Updated Retry Configuration for Test EnvironmentThe
grievanceAllocationRetryConfigurationvalue has been updated to 3 in the test properties file. This change ensures that the test environment adheres to the new retry logic for grievance allocation.src/main/environment/common_uat.properties (1)
187-188: Updated Retry Configuration for UAT EnvironmentThe UAT configuration now sets
grievanceAllocationRetryConfigurationto 3, ensuring consistency across all deployment environments for how many retry attempts are allowed during grievance allocation.src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java (1)
17-17:❓ Verification inconclusive
Verify that the removal of
f.stateIDdoesn't break existing codeThe query has been modified to remove
f.stateIDfrom the selection list. This changes the structure of the returnedObject[]from 5 elements to 4.
🏁 Script executed:
#!/bin/bash # Find all references to findByComplaintId method to check for potential usage of stateID # Search for all invocations of the method echo "Searching for all references to findByComplaintId method:" rg -A 3 "findByComplaintId\(" --type java # Search specifically for array index access that might be affected echo "Searching for potential array index access on result:" rg -A 5 "findByComplaintId\(.*\).*\[\d\]" --type javaLength of output: 1307
Action Required: Verify Query Result Mapping in Data Sync Implementation
The modification now returns an Object array of 4 elements (without
f.stateID) instead of the previous 5. Automated search results for invocations offindByComplaintIddid not reveal any explicit array index accesses that rely on a fifth element. However, since this method is used inGrievanceDataSyncImpl.java, please manually verify that the code there does not make assumptions about having a fifth element in the returned array (e.g., hard-coded index accesses).
- File to review:
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java- Focus area: Any logic that processes the result of the
findByComplaintIdmethod should be checked for reliance on the original array structure.src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java (1)
23-25: Appropriate interface extensions for grievance handling.The new methods
saveComplaintResolutionandgetGrievanceDetailsWithRemarksfollow the consistent pattern of the existing interface methods (taking String request parameters and throwing Exception). These additions enhance the grievance handling functionality by supporting complaint resolution management and detailed grievance information retrieval with remarks.src/main/java/com/iemr/common/data/grievance/GrievanceAllocationRequest.java (1)
19-20: Update toString method to reflect variable name changes.The toString method has been updated to reflect the variable name changes, which is good practice. However, if you rename
touserIDto follow naming conventions as suggested earlier, this method will need to be updated as well.src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java (3)
13-14: Good addition of transaction metadata fields.Adding
actionTakenByandstatusfields improves the DTO by providing important context about who performed actions on the grievance and its current state. This enhances traceability and reporting capabilities.
25-25: Constructor parameter update for new fields.The constructor signature has been appropriately updated to include the new fields.
29-30: Constructor initialization for new fields.The constructor body correctly initializes the new fields.
src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java (1)
12-14:❓ Verification inconclusive
Return type changed from List<Map<String, Object>> to String
The return type for
dataSyncToGrievance()has been changed from a structured data format to a String. This suggests the method now likely returns a JSON string instead of a list of objects.While this simplifies the interface, ensure that all implementations and calling methods properly handle the new String return type, including parsing the JSON if needed.
🏁 Script executed:
#!/bin/bash # Find implementations of the dataSyncToGrievance method rg -A 5 -B 5 "dataSyncToGrievance\(\)" --glob "*.java" | grep -v "interface"Length of output: 4963
Review Verification: JSON Return Handling Confirmed
The change from returning a structured
List<Map<String, Object>>to a JSONStringhas been validated:
- In
GrievanceDataSync.javaand its implementation inGrievanceDataSyncImpl.java, the method now returns a JSON string. The implementation captures the returned value (e.g., in the retry loop) and checks fornullaccordingly.- In
ScheduleForGrievanceDataSync.java, the method is invoked without capturing the return value. Please verify that this omission is intentional and that the caller does not require direct access to the JSON data.Make sure that any downstream consumers expecting a structured data format perform the necessary JSON parsing. If the JSON string is meant only for logging or status checking, this design is acceptable, but any conversion requirements should be clearly handled.
src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java (1)
27-30:❓ Verification inconclusive
New grievanceId field added
This new field establishes a direct relationship with a grievance record and appears to be correctly mapped with appropriate JPA annotations.
Verify that the database schema has been updated to include this column, and ensure data migration has been addressed if this is being added to an existing table.
🏁 Script executed:
#!/bin/bash # Check for any database migration scripts related to this change fd -e sql -e xml | xargs grep -l "GrievanceId" # Check for references to this new field in service layer rg "getGrievanceId\(\)" --glob "*.java" | grep -v "GrievanceTransaction.java"Length of output: 1716
Database Schema Migration Verification for
grievanceIdField
- The new
grievanceIdfield insrc/main/java/com/iemr/common/data/grievance/GrievanceTransaction.javais correctly added with the appropriate JPA annotations.- Service layer references (e.g., in
GrievanceDataSyncImpl.javaandGrievanceHandlingServiceImpl.java) have been updated to use this field.- However, our search did not locate any migration scripts (SQL or XML) containing "GrievanceId". Please confirm that the database schema is updated appropriately—whether via an automated DDL update or through a migration script—and ensure that proper data migration steps are in place if this change is applied to an existing table.
src/main/java/com/iemr/common/dto/grivance/GrievanceWorklistDTO.java (1)
40-40: Added lastCall timestamp fieldA new timestamp field has been added to track when the last call occurred. This is a useful addition for tracking the history of grievance calls.
Ensure that any JSON serialization/deserialization handles this new field correctly, especially in existing APIs.
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
109-127:❓ Verification inconclusive
Method signature change might break callers
Previously, this method returned aList<Map<String, Object>>. Now it returns aString. Verify all consumers ofdataSyncToGrievance()have been updated accordingly to handle the new return type.
🏁 Script executed:
#!/bin/bash # Verifying references to old method signature rg "dataSyncToGrievance\s*\(" -A 5Length of output: 2992
Action Required: Validate Consumers of
dataSyncToGrievance()Signature ChangeThe method’s signature now returns a
Stringinstead of aList<Map<String, Object>>. In your recent changes, the interface (GrievanceDataSync.java) and the implementation (GrievanceDataSyncImpl.java) have been updated accordingly. However, please ensure that every consumer of this method has been verified:
ScheduleForGrievanceDataSync.java:
- One call invokes
grievanceDataSync.dataSyncToGrievance();without capturing the return value. Confirm that discarding theStringis intentional and does not lead to any side effects.- Another call correctly assigns the result to a
Stringvariable (String savedGrievanceData = dataSyncToGrievance();) and uses it in subsequent conditional logic.Double-check that any use of the method that previously expected a
List<Map<String, Object>>has been fully updated to accommodate the new return type and that your application’s behavior remains as expected.src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
49-49: Initialize final fields in the constructor
private final BeneficiaryCallRepository beneficiaryCallRepo;is declared as final and properly assigned in the constructor. This is a good practice for immutability.
439-544: Confirm mixed usage of remarks retrieval
getGrievanceDetailsWithRemarks()branches logic depending on "resolved" or "unresolved." Double-check that no additional complaint-resolution statuses need coverage. If the application can have more statuses, consider a small refactor to handle them collectively, instead of fallback logic.src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (8)
19-19: Import usage is correct
The newly added import is appropriately used for the@Sizeannotation.
74-74: Ensure integer usage forlevel
Previously this was aString. Verify that external code no longer attempts to set non-integer values.
81-81: Field renamed touserID
Renaming from the olderassignedUserIDtouserIDimproves consistency across the codebase.
106-114: AddedcomplaintResolutionandremarksfields
Including a@Sizelimit onremarkshelps avoid overly large inputs. This aligns with typical data validation practices.
139-139:vanSerialNotype changed toLong
Confirm that dependent code expecting anIntegerhas been updated to handleLong.
168-171: Constructor signature expanded
Parameters forlevel,complaintResolution, andremarksalign with the newly introduced fields.
184-186: AssigninglevelanduserID
These assignments fully align with the changed field types and naming conventions.
193-194: AssigningcomplaintResolutionandremarks
Implementation corresponds correctly with the newly defined fields.src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (3)
51-51: Expanded parameter usage forfetchUnallocatedGrievanceCount
This method now accounts for date filters and the additional provider service map ID. Implementation appears consistent.
90-90: Updated parameter documentation
ChangingassignedUserIDtouserIDclarifies usage and aligns with the refactored field name.
193-218: NewcompleteGrievanceCallmethod
This method parallels other endpoints by returning a stringified response. Ensure that the service call is thoroughly tested for edge cases like invalid complaint IDs or user IDs.src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java (17)
5-5: ImportingDate
This import is required for the new date-based queries.
31-34: Query updated to setuserID
This change properly adjustsallocateGrievanceto useuserIDinstead of the old naming scheme.
39-42: NewgetPrimaryNumberquery
Safely parameterized with@Param("benRegId"); no SQL injection concerns.
47-50:fetchGrievanceRecordsCountadjusted foruserID
Ensures counting is user-specific. Validate no edge cases by user ID = null.
52-53:findAllocatedGrievancesByUserAndLanguage
Method name and parameters are clear. Implementation logic is consistent with the rest of the repository.
57-60:reallocateGrievance
UsesuserIDfor reallocation. Review concurrency conditions if multiple threads attempt to reallocate simultaneously.
62-65:findGrievancesByUserAndLanguage
Straightforward approach. Ensure large result sets are handled efficiently in the service layer if needed.
69-70:unassignGrievance
Assigning auserIDmight effectively remove the previous assignment if it’s set to null or a specific default. Confirm usage in the calling logic.
78-84:fetchUnallocatedGrievanceCountwith date ranges
Implementation extends the query to filter byfilterStartDateandfilterEndDate. Logic is sound for typical date-based queries.
85-98:updateComplaintResolutionwith remarks
Full coverage for complaint resolution updates. Keep an eye on concurrency if multiple updates can occur to the same complaint.
114-115:getCallCounter
Fetching onlycallCounterandretryNeededis efficient. Further expansions should be considered if more info is needed.
117-127:updateCompletedStatusInCall
Correctly setsisCompletedandretryNeeded. Confirm final call flow logic ensures these fields are used consistently.
130-140:updateCallCounter
Adjusts call count andretryNeeded. This is sensible for controlling reattempt logic, but ensure there’s no race condition with concurrent calls.
142-150:fetchGrievanceDetailsBasedOnParams
Permits optional filtering bystateorcomplaintResolution. TheOR :param IS NULLapproach is flexible and conventional.
153-154:fetchGrievanceWorklistByComplaintID
Simple retrieval by complaint ID. Ensure that the service layer checks for empty results.
157-158:fetchGrievanceWorklistRemarks
Directly fetches onlyremarks. This is appropriate if the rest of the record isn’t needed.
160-161:getUniqueGwid
Retrieves a single primary key (gwid). Confirm no collisions or unexpected null results for valid IDs.
|
|
||
| private Integer providerServiceMapID; | ||
| private Integer assignedUserID; | ||
| private Integer userID; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify field rename impact across codebase
The field has been renamed from assignedUserID to userID. While this improves naming consistency, it may impact existing code that uses getters/setters for this field (e.g., getAssignedUserID() to getUserID()).
🏁 Script executed:
#!/bin/bash
# Find references to the old field name (assignedUserID) or its getters/setters
echo "Searching for references to old field name 'assignedUserID':"
rg "assignedUserID|getAssignedUserID|setAssignedUserID" --type java
# Check for references to MoveToBinRequest class
echo "Searching for references to MoveToBinRequest class:"
rg "MoveToBinRequest" -A 3 -B 3 --type javaLength of output: 10767
Action Required: Address Inconsistent Field Naming Across Models
The rename of the field in MoveToBinRequest from assignedUserID to userID does improve naming consistency within that class. However, our search shows that many other parts of the codebase (e.g., in the call handling and repository classes) still reference assignedUserID and its associated getters/setters. This raises several concerns:
- JSON Mapping/Serialization: Since MoveToBinRequest is deserialized (e.g., via Gson in GrievanceHandlingServiceImpl), ensure that any client or service expecting the JSON field
"assignedUserID"is handled appropriately. If clients are still sending"assignedUserID", consider using an alias (e.g., with@JsonProperty("assignedUserID")) on theuserIDfield. - API Consistency: Other models (such as those in OutboundCallRequest or EverwellDetails) continue to use
assignedUserID. Verify whether this rename should be applied across all related models to avoid confusion, or if this change is isolated to MoveToBinRequest alone. - Getter/Setter Expectations: With Lombok’s @DaTa, the getters/setters for MoveToBinRequest become
getUserID()/setUserID(). Confirm that no consuming code is still callinggetAssignedUserID()on this type.
Please review and update the impacted consumers or provide backward compatibility if needed.
| @Query("SELECT b.remarks FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID") | ||
| List<Object[]> fetchBenCallRemarks(@Param("beneficiaryRegID") Long beneficiaryRegID); |
There was a problem hiding this comment.
Return type mismatch with query result.
The query selects only the remarks field (single column), but the return type is List<Object[]> which is typically used for multi-column results. This may cause type conversion issues.
-@Query("SELECT b.remarks FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID")
-List<Object[]> fetchBenCallRemarks(@Param("beneficiaryRegID") Long beneficiaryRegID);
+@Query("SELECT b.remarks FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID")
+List<String> fetchBenCallRemarks(@Param("beneficiaryRegID") Long beneficiaryRegID);📝 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 b.remarks FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID") | |
| List<Object[]> fetchBenCallRemarks(@Param("beneficiaryRegID") Long beneficiaryRegID); | |
| @Query("SELECT b.remarks FROM BeneficiaryCall b WHERE b.beneficiaryRegID = :beneficiaryRegID") | |
| List<String> fetchBenCallRemarks(@Param("beneficiaryRegID") Long beneficiaryRegID); |
| private Timestamp startDate; // Start date for filtering grievances | ||
| private Timestamp endDate; // End date for filtering grievances | ||
| private List<Integer> userID; // List of user IDs (agents) to whom grievances will be allocated | ||
| private List<Integer> touserID; // List of user IDs (agents) to whom grievances will be allocated |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Variable name does not follow Java naming conventions.
The variable name touserID doesn't follow standard Java naming conventions (camelCase). Consider renaming to toUserId or targetUserId for better readability and consistency.
-private List<Integer> touserID; // List of user IDs (agents) to whom grievances will be allocated
+private List<Integer> targetUserId; // List of user IDs (agents) to whom grievances will be allocated📝 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 List<Integer> touserID; // List of user IDs (agents) to whom grievances will be allocated | |
| private List<Integer> targetUserId; // List of user IDs (agents) to whom grievances will be allocated |
| private List<Integer> touserID; // List of user IDs (agents) to whom grievances will be allocated | ||
| private Integer allocateNo; // Number of grievances to be allocated to each user | ||
| private String preferredLanguage; | ||
| private String language; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Field rename from 'preferredLanguage' to 'language'.
The variable has been renamed from preferredLanguage to language. Ensure that all code referencing this field has been updated accordingly to prevent runtime errors.
🏁 Script executed:
#!/bin/bash
# Search for any references to 'preferredLanguage' that might not have been updated
rg "preferredLanguage" --type javaLength of output: 18647
Action Required: Rename Consistency Issue Across Codebase
The field in GrievanceAllocationRequest.java has been correctly renamed from preferredLanguage to language. However, our repository search reveals that multiple files still reference preferredLanguage (or its variants, such as preferredLanguageName). This inconsistency is likely to cause runtime errors. Please update all dependent code to use the new naming convention consistently.
- Affected files include (but are not limited to):
src/main/java/com/iemr/common/service/reports/CallReportsServiceImpl.javasrc/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.javasrc/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java- Other related DTOs, mappers, and repository query definitions referencing
preferredLanguageorpreferredLanguageName
Ensure to verify and test all integration points where this field is used.
|
|
||
| public String fetchUnallocatedGrievanceCount(String preferredLanguage, Timestamp filterStartDate, | ||
| Timestamp filterEndDate, Integer providerServiceMapID) throws IEMRException, JSONException; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Enhanced parameter list for fetchUnallocatedGrievanceCount
The method signature has been expanded to include date filtering (filterStartDate, filterEndDate) and organization context (providerServiceMapID). This provides better filtering capabilities and contextual awareness.
Ensure that all implementations of this method have been updated to handle these new parameters, and all callers are providing the correct arguments.
🏁 Script executed:
#!/bin/bash
# Find implementations of the updated method
rg -A 5 "fetchUnallocatedGrievanceCount\(" --glob "*.java" | grep -v "interface"
# Find callers of this method to verify they've been updated
rg "fetchUnallocatedGrievanceCount\(" --glob "*.java" | grep -v "interface" | grep -v "public.*fetchUnallocatedGrievanceCount"Length of output: 4960
Action Required: Update Handling of Organization Context in fetchUnallocatedGrievanceCount
- The service interface (
GrievanceDataSync.java) now includes the additional parameters (filterStartDate,filterEndDate, andproviderServiceMapID), and the controller (GrievanceController.java) correctly passes all four arguments. - However, in the service implementation (
GrievanceDataSyncImpl.java), the call to the repository is still made with only the date parameters:-
Set<Object[]> resultSet = grievanceDataRepo.fetchUnallocatedGrievanceCount(filterStartDate, filterEndDate);
-
- The repository method in
GrievanceDataRepo.javaalso accepts only two parameters, indicating that the new organization context (providerServiceMapID) is not currently used. - Action needed: Confirm whether
providerServiceMapIDshould be incorporated in the repository query. If it is required for filtering:- Update the repository method to accept
providerServiceMapID(or provide an alternative mechanism to utilize this parameter). - Adjust the service implementation in
GrievanceDataSyncImpl.javato forward the new parameter accordingly.
- Update the repository method to accept
- If the omission is intentional, please document the rationale clearly.
📋 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