AMM-1139 | Scheduler for fetching L3 to L5 grievance details #148
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)
|
Warning Rate limit exceeded@srishtigrp78 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 53 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 grievance data synchronization feature, adding new classes and interfaces to manage grievance details, transactions, and synchronization processes. It includes enhancements to Quartz scheduling configuration, new data repositories, and a service for processing grievance data from an external system. These changes enable automated periodic synchronization of grievance information while ensuring robust error handling and transaction management. Changes
Sequence DiagramsequenceDiagram
participant Scheduler
participant GrievanceJob
participant GrievanceDataSyncService
participant ExternalAPI
participant Database
Scheduler->>GrievanceJob: Trigger Job
GrievanceJob->>GrievanceDataSyncService: Initiate Data Sync
GrievanceDataSyncService->>GrievanceDataSyncService: Generate Auth Token
GrievanceDataSyncService->>ExternalAPI: Fetch Grievance Data
ExternalAPI-->>GrievanceDataSyncService: Return Grievance Details
GrievanceDataSyncService->>Database: Save Grievance Details
GrievanceDataSyncService->>Database: Save Grievance Transactions
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: 11
🧹 Nitpick comments (6)
src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java (1)
1-20: Verify repository's role in grievance data synchronization.Given that this PR implements a scheduler for fetching L3-L5 grievance details:
- Consider adding methods specific to L3-L5 grievance filtering
- Add transaction support for batch operations
- Consider implementing pagination for large data sets
Let's verify the integration with the scheduler:
#!/bin/bash # Search for scheduler-related configurations and usage rg -A 5 "GrievanceFetchBenDetailsRepo|@Scheduled.*grievance" --type java # Search for transaction management in related files rg "@Transactional.*grievance" --type javasrc/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (2)
45-45: Declare the logger asprivate static finalfor proper usage.For consistency and adherence to best practices, the logger should be declared as
private static final.Apply this diff to update the logger declaration:
- Logger logger = LoggerFactory.getLogger(this.getClass().getName()); + private static final Logger logger = LoggerFactory.getLogger(GrievanceDataSyncImpl.class);
141-179: Remove commented-out code to improve maintainability.The block of code between lines 141 and 179 is commented out. Removing unused code enhances readability and maintainability.
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java (1)
23-29: Add exception handling in theexecutemethod to handle potential errors.To ensure that any exceptions during job execution are properly logged and do not cause the scheduler to fail, wrap the code in a try-catch block.
Apply this diff to include exception handling:
@Override public void execute(JobExecutionContext arg0) throws JobExecutionException { logger.info("Started job for grievance data sync " + arg0.getClass().getName()); + try { grievanceDataSync.dataSyncToGrievance(); + } catch (Exception e) { + logger.error("Error during grievance data sync job execution", e); + throw new JobExecutionException(e); + } logger.info("Completed job for grievance data sync " + arg0.getClass().getName()); }src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java (1)
34-36: Maintain consistent field naming conventions.Some fields don't follow Java naming conventions:
Processedstarts with uppercase- Inconsistent use of camelCase vs PascalCase
Apply consistent naming:
-private Character Processed = 'N'; +private Character processed = 'N';Also applies to: 57-59, 98-99
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (1)
116-127: Maintain consistent field naming conventions.Several fields don't follow Java naming conventions:
VanID,VanSerialNo,ParkingPlaceIDuse inconsistent casingApply consistent camelCase naming:
-private Integer VanID; -private Integer VanSerialNo; -private Integer ParkingPlaceID; +private Integer vanId; +private Integer vanSerialNo; +private Integer parkingPlaceId;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/iemr/common/config/quartz/QuartzConfig.java(2 hunks)src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceDataRepo.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceTransactionRepo.java(1 hunks)src/main/java/com/iemr/common/repository/location/LocationStateRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/com/iemr/common/repository/grievance/GrievanceTransactionRepo.java
- src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (java)
- GitHub Check: Build
- GitHub Check: Package-test
🔇 Additional comments (3)
src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java (2)
1-11: LGTM! Package structure and imports are well-organized.
13-14: Verify repository naming and entity relationship.The repository name suggests it handles grievance beneficiary details, but it extends
CrudRepository<FeedbackDetails, Long>. This might indicate:
- A potential naming inconsistency between grievance and feedback domains
- A design decision to store grievance data in the feedback table
Please clarify if this is intentional or if we should consider:
- Renaming to
FeedbackDetailsRepositoryfor clarity- Creating a separate entity for grievance data
Let's verify the relationship between feedback and grievance domains:
✅ Verification successful
Repository name and entity relationship are architecturally sound
The codebase shows that grievances are intentionally modeled as part of the feedback domain, with
FeedbackDetailsserving as the underlying entity. The repository nameGrievanceFetchBenDetailsRepoappropriately describes its specialized purpose of fetching beneficiary-related grievance data from the feedback store.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of FeedbackDetails to understand the domain relationship rg -l "FeedbackDetails" --type java # Search for any existing grievance-specific entities rg -l "Grievance.*\.java$" --type javaLength of output: 2731
src/main/java/com/iemr/common/config/quartz/QuartzConfig.java (1)
232-253: LGTM! Well-structured scheduler configuration.The implementation follows the established pattern and includes proper configuration for both job and trigger beans.
src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java
Outdated
Show resolved
Hide resolved
| if (response != null && response.hasBody()) { | ||
| JSONObject obj = new JSONObject(response.getBody()); | ||
| if (obj != null && obj.has("data") && obj.has("statusCode") && obj.getInt("statusCode") == 200) { | ||
| logger.info("Grievance data details response: " + response.getBody()); |
There was a problem hiding this comment.
Avoid logging sensitive data in production logs.
Logging the entire response body may expose sensitive information. Consider removing or sanitizing the response before logging.
Apply this diff to adjust the logging:
- logger.info("Grievance data details response: " + response.getBody());
+ logger.info("Grievance data details response received successfully.");📝 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.
| logger.info("Grievance data details response: " + response.getBody()); | |
| logger.info("Grievance data details response received successfully."); |
|
|
||
| @Service | ||
| @PropertySource("classpath:application.properties") | ||
| public class GrievanceDataSyncImpl { |
There was a problem hiding this comment.
Implement the GrievanceDataSync interface in the class declaration.
To ensure that GrievanceDataSyncImpl correctly implements the GrievanceDataSync interface and follows the defined contract, the class declaration should explicitly implement the interface.
Apply this diff to fix the issue:
-public class GrievanceDataSyncImpl {
+public class GrievanceDataSyncImpl implements GrievanceDataSync {Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@srishtigrp78 Thank you for implementing the interface! This will help ensure type safety and contract adherence.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| private static String GRIEVANCE_AUTH_TOKEN; | ||
| private static Long GRIEVANCE_TOKEN_EXP; | ||
|
|
There was a problem hiding this comment.
Avoid using static variables for instance-specific state to ensure thread safety.
Using static variables GRIEVANCE_AUTH_TOKEN and GRIEVANCE_TOKEN_EXP in a service class can lead to thread safety issues in a multi-threaded environment. Consider changing them to instance variables or using thread-safe mechanisms.
Apply this diff to fix the issue:
-private static String GRIEVANCE_AUTH_TOKEN;
-private static Long GRIEVANCE_TOKEN_EXP;
+private String grievanceAuthToken;
+private Long grievanceTokenExp;Also, update references to these variables throughout the class accordingly.
📝 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 static String GRIEVANCE_AUTH_TOKEN; | |
| private static Long GRIEVANCE_TOKEN_EXP; | |
| private String grievanceAuthToken; | |
| private Long grievanceTokenExp; | |
There was a problem hiding this comment.
@srishtigrp78 Thank you for addressing the thread safety concern by making the variables instance-scoped! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
|
||
| HttpEntity<Object> request = new HttpEntity<Object>(headers); | ||
| RestTemplate restTemplate = new RestTemplate(); | ||
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails, HttpMethod.POST, request, String.class); |
There was a problem hiding this comment.
Include complaintId in the API call to fetch transaction details.
The complaintId parameter is not being used in the API request. Ensure that the complaintId is included in the request to fetch the correct grievance transactions.
Apply this diff to include complaintId in the request:
ResponseEntity<String> response = restTemplate.exchange(
- updateGrievanceTransactionDetails, HttpMethod.POST, request, String.class);
+ updateGrievanceTransactionDetails + "?complaintId=" + URLEncoder.encode(complaintId, "UTF-8"),
+ HttpMethod.POST, request, String.class);Ensure to handle UnsupportedEncodingException and properly encode the parameter.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@srishtigrp78 Great! Thank you for implementing the fix. 🎉
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/repository/grievance/GrievanceDataRepo.java
Show resolved
Hide resolved
| @Query("select state from States state where state.stateID = :id") | ||
| public String findByStateIDForGrievance(@Param("id") int id); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate query methods.
The new method findByStateIDForGrievance uses the same query as findByStateID but returns String instead of States. This creates unnecessary duplication and could lead to confusion.
Consider one of these approaches:
- If you need a specific field, modify the query to select only that field:
-@Query("select state from States state where state.stateID = :id")
+@Query("select state.stateName from States state where state.stateID = :id")- Or use the existing
findByStateIDmethod and extract the required field in the service layer.
📝 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 state from States state where state.stateID = :id") | |
| public String findByStateIDForGrievance(@Param("id") int id); | |
| @Query("select state.stateName from States state where state.stateID = :id") | |
| public String findByStateIDForGrievance(@Param("id") int id); |
There was a problem hiding this comment.
@srishtigrp78 Thank you for addressing the duplicate query methods! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| public class GrievanceTransaction { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "gtid") | ||
| private Long gtid; | ||
|
|
||
| @Column(name = "gwid", nullable = false) | ||
| @Expose | ||
| private Long gwid; | ||
|
|
||
| @Column(name = "FileName", nullable = false, length = 500) | ||
| @Expose | ||
| private String fileName; | ||
|
|
||
| @Column(name = "FileType", nullable = false, length = 300) | ||
| @Expose | ||
| private String fileType; | ||
|
|
||
| @Column(name = "Redressed", nullable = false) | ||
| @Expose | ||
| private String redressed; | ||
|
|
||
| @Column(name = "createdAt", nullable = false) | ||
| @Expose | ||
| private Date createdAt; | ||
|
|
||
| @Column(name = "updatedAt", nullable = false) | ||
| @Expose | ||
| private Date updatedAt; | ||
|
|
||
| @Column(name = "Comments", length = 500) | ||
| private String comments; | ||
|
|
||
| @Column(name = "ProviderServiceMapID") | ||
| @Expose | ||
| private Integer providerServiceMapID; | ||
|
|
||
| @Column(name = "Deleted", insertable = false, updatable = true) | ||
| @Expose | ||
| private Boolean deleted; | ||
|
|
||
| @Column(name = "Processed", nullable = false) | ||
| @Expose | ||
| private String processed; | ||
|
|
||
|
|
||
| @Column(name = "ActionTakenBy") | ||
| @Expose | ||
| private String actionTakenBy; | ||
|
|
||
| @Column(name = "Status") | ||
| @Expose | ||
| private String status; | ||
|
|
||
| @Column(name = "Comment") | ||
| @Expose | ||
| private String comment; | ||
|
|
||
| @Column(name = "CreatedBy", nullable = false) | ||
| @Expose | ||
| private String createdBy; | ||
|
|
||
| @Column(name = "CreatedDate", nullable = false) | ||
| private Date createdDate; | ||
|
|
||
| @Column(name = "ModifiedBy") | ||
| @Expose | ||
| private String modifiedBy; | ||
|
|
||
| @Column(name = "LastModDate") | ||
| @Expose | ||
| private Date lastModDate; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JPA relationship mapping with GrievanceDetails.
The entity lacks a proper JPA relationship mapping with its parent GrievanceDetails entity.
Add the following relationship mapping:
+@ManyToOne(fetch = FetchType.LAZY)
+@JoinColumn(name = "gwid", insertable = false, updatable = false)
+private GrievanceDetails grievanceDetails;📝 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 class GrievanceTransaction { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "gtid") | |
| private Long gtid; | |
| @Column(name = "gwid", nullable = false) | |
| @Expose | |
| private Long gwid; | |
| @Column(name = "FileName", nullable = false, length = 500) | |
| @Expose | |
| private String fileName; | |
| @Column(name = "FileType", nullable = false, length = 300) | |
| @Expose | |
| private String fileType; | |
| @Column(name = "Redressed", nullable = false) | |
| @Expose | |
| private String redressed; | |
| @Column(name = "createdAt", nullable = false) | |
| @Expose | |
| private Date createdAt; | |
| @Column(name = "updatedAt", nullable = false) | |
| @Expose | |
| private Date updatedAt; | |
| @Column(name = "Comments", length = 500) | |
| private String comments; | |
| @Column(name = "ProviderServiceMapID") | |
| @Expose | |
| private Integer providerServiceMapID; | |
| @Column(name = "Deleted", insertable = false, updatable = true) | |
| @Expose | |
| private Boolean deleted; | |
| @Column(name = "Processed", nullable = false) | |
| @Expose | |
| private String processed; | |
| @Column(name = "ActionTakenBy") | |
| @Expose | |
| private String actionTakenBy; | |
| @Column(name = "Status") | |
| @Expose | |
| private String status; | |
| @Column(name = "Comment") | |
| @Expose | |
| private String comment; | |
| @Column(name = "CreatedBy", nullable = false) | |
| @Expose | |
| private String createdBy; | |
| @Column(name = "CreatedDate", nullable = false) | |
| private Date createdDate; | |
| @Column(name = "ModifiedBy") | |
| @Expose | |
| private String modifiedBy; | |
| @Column(name = "LastModDate") | |
| @Expose | |
| private Date lastModDate; | |
| public class GrievanceTransaction { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "gtid") | |
| private Long gtid; | |
| @Column(name = "gwid", nullable = false) | |
| @Expose | |
| private Long gwid; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "gwid", insertable = false, updatable = false) | |
| private GrievanceDetails grievanceDetails; | |
| @Column(name = "FileName", nullable = false, length = 500) | |
| @Expose | |
| private String fileName; | |
| @Column(name = "FileType", nullable = false, length = 300) | |
| @Expose | |
| private String fileType; | |
| @Column(name = "Redressed", nullable = false) | |
| @Expose | |
| private String redressed; | |
| @Column(name = "createdAt", nullable = false) | |
| @Expose | |
| private Date createdAt; | |
| @Column(name = "updatedAt", nullable = false) | |
| @Expose | |
| private Date updatedAt; | |
| @Column(name = "Comments", length = 500) | |
| private String comments; | |
| @Column(name = "ProviderServiceMapID") | |
| @Expose | |
| private Integer providerServiceMapID; | |
| @Column(name = "Deleted", insertable = false, updatable = true) | |
| @Expose | |
| private Boolean deleted; | |
| @Column(name = "Processed", nullable = false) | |
| @Expose | |
| private String processed; | |
| @Column(name = "ActionTakenBy") | |
| @Expose | |
| private String actionTakenBy; | |
| @Column(name = "Status") | |
| @Expose | |
| private String status; | |
| @Column(name = "Comment") | |
| @Expose | |
| private String comment; | |
| @Column(name = "CreatedBy", nullable = false) | |
| @Expose | |
| private String createdBy; | |
| @Column(name = "CreatedDate", nullable = false) | |
| private Date createdDate; | |
| @Column(name = "ModifiedBy") | |
| @Expose | |
| private String modifiedBy; | |
| @Column(name = "LastModDate") | |
| @Expose | |
| private Date lastModDate; |
There was a problem hiding this comment.
@srishtigrp78 Thank you for implementing the JPA relationship mapping! 🎉
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.
@srishtigrp78 Great! Thank you for implementing the JPA relationship mapping. 🎉
(｀∇´ฅ)
| @Column(name = "isCompleted") | ||
| private Boolean isCompleted = false; | ||
|
|
||
| private List<GrievanceTransaction> grievanceTransactionDetails; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add JPA relationship mapping for grievanceTransactionDetails.
The field lacks proper JPA relationship mapping with GrievanceTransaction entity.
Add the following annotations:
+@OneToMany(mappedBy = "grievanceDetails", cascade = CascadeType.ALL, fetch = FetchType.LAZY)
+@JsonManagedReference
private List<GrievanceTransaction> grievanceTransactionDetails;📝 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<GrievanceTransaction> grievanceTransactionDetails; | |
| @OneToMany(mappedBy = "grievanceDetails", cascade = CascadeType.ALL, fetch = FetchType.LAZY) | |
| @JsonManagedReference | |
| private List<GrievanceTransaction> grievanceTransactionDetails; |
There was a problem hiding this comment.
@srishtigrp78 Thank you for confirming the fix! The JPA relationship mapping is now properly configured.
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: 6
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java (1)
6-9: Consider using more descriptive parameter names and a custom return type.The interface could benefit from:
- More descriptive parameter names to indicate their purpose
- A custom return type instead of
List<Map<String, Object>>for better type safetyApply this diff to improve the interface:
-public List<Map<String, Object>> dataSyncToGrievance(String grievanceAuthorization, String registeringUser, - String Authorization); +public List<GrievanceResponse> dataSyncToGrievance(String grievanceToken, String registeredUsername, + String authorizationKey); +public class GrievanceResponse { + private String complaintID; + private String subjectOfComplaint; + private String complaint; + private Long beneficiaryRegID; + private Integer providerServiceMapId; + private List<GrievanceTransactionResponse> transactions; + // Add other fields, getters, and setters +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java(1 hunks)src/main/java/com/iemr/common/repository/location/LocationStateRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/iemr/common/repository/location/LocationStateRepository.java
- src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java
⏰ 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 (2)
src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java (1)
8-14: Remove redundant code generation.The class uses both Lombok's
@Dataannotation and manual getters/setters, creating redundancy.src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
79-80:⚠️ Potential issueUse instance variables for token management to ensure thread safety.
Static variables for token management can cause issues in a multi-threaded environment.
Apply this diff to fix thread safety:
-private String GRIEVANCE_AUTH_TOKEN; -private Long GRIEVANCE_TOKEN_EXP; +private volatile String grievanceAuthToken; +private volatile Long grievanceTokenExp;Likely invalid or redundant comment.
| public String comment( ) { | ||
| return comment; | ||
| } | ||
|
|
||
| public void setComment(String comment ) { | ||
| this.comment = comment; | ||
| } |
There was a problem hiding this comment.
Fix method name inconsistency.
The method name comment() doesn't follow the getter naming convention.
Apply this diff to fix the method name:
-public String comment( ) {
+public String getComment() {📝 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 String comment( ) { | |
| return comment; | |
| } | |
| public void setComment(String comment ) { | |
| this.comment = comment; | |
| } | |
| public String getComment() { | |
| return comment; | |
| } | |
| public void setComment(String comment ) { | |
| this.comment = comment; | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for fixing the method name! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| @Column(name = "FileName", nullable = false, length = 500) | ||
| @Expose | ||
| private String fileName; | ||
|
|
||
| @Column(name = "FileType", nullable = false, length = 300) | ||
| @Expose | ||
| private String fileType; | ||
|
|
||
| @Column(name = "Redressed", nullable = false) | ||
| @Expose | ||
| private String redressed; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation constraints for file-related fields.
The file-related fields should have proper validation constraints to ensure data integrity.
Apply this diff to add validation:
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.Size;
@Column(name = "FileName", nullable = false, length = 500)
@Expose
+@NotBlank(message = "File name is required")
+@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 is required")
+@Size(max = 300, message = "File type cannot exceed 300 characters")
private String fileType;📝 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.
| @Column(name = "FileName", nullable = false, length = 500) | |
| @Expose | |
| private String fileName; | |
| @Column(name = "FileType", nullable = false, length = 300) | |
| @Expose | |
| private String fileType; | |
| @Column(name = "Redressed", nullable = false) | |
| @Expose | |
| private String redressed; | |
| import javax.validation.constraints.NotBlank; | |
| import javax.validation.constraints.Size; | |
| @Column(name = "FileName", nullable = false, length = 500) | |
| @Expose | |
| @NotBlank(message = "File name is required") | |
| @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 is required") | |
| @Size(max = 300, message = "File type cannot exceed 300 characters") | |
| private String fileType; | |
| @Column(name = "Redressed", nullable = false) | |
| @Expose | |
| private String redressed; |
| @Expose | ||
| @Column(name = "Severety") | ||
| private String severety; | ||
|
|
||
| @Expose | ||
| @Column(name = "Level") | ||
| private String level; |
There was a problem hiding this comment.
Fix typo in column name and add validation.
The column name "Severety" is misspelled and lacks validation.
Apply this diff to fix the issue:
@Expose
-@Column(name = "Severety")
-private String severety;
+@Column(name = "Severity")
+@NotBlank(message = "Severity is required")
+private String severity;📝 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.
| @Expose | |
| @Column(name = "Severety") | |
| private String severety; | |
| @Expose | |
| @Column(name = "Level") | |
| private String level; | |
| @Expose | |
| @Column(name = "Severity") | |
| @NotBlank(message = "Severity is required") | |
| private String severity; | |
| @Expose | |
| @Column(name = "Level") | |
| private String level; |
| @Expose | ||
| @Column(name = "VanSerialNo") | ||
| private Integer VanSerialNo; | ||
| @Expose | ||
| @Column(name = "VanID") | ||
| private Integer VanID; | ||
|
|
||
| @Expose | ||
| @Column(name = "VehicalNo") | ||
| private String VehicalNo; | ||
| @Expose | ||
| @Column(name = "ParkingPlaceID") | ||
| private Integer ParkingPlaceID; | ||
| @Expose |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Follow Java naming conventions for field names.
Field names should follow camelCase convention in Java.
Apply this diff to fix field naming:
-private Integer VanSerialNo;
-private Integer VanID;
-private String VehicalNo;
-private Integer ParkingPlaceID;
+private Integer vanSerialNo;
+private Integer vanId;
+private String vehicleNo; // Also fixed typo in 'Vehical'
+private Integer parkingPlaceId;📝 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.
| @Expose | |
| @Column(name = "VanSerialNo") | |
| private Integer VanSerialNo; | |
| @Expose | |
| @Column(name = "VanID") | |
| private Integer VanID; | |
| @Expose | |
| @Column(name = "VehicalNo") | |
| private String VehicalNo; | |
| @Expose | |
| @Column(name = "ParkingPlaceID") | |
| private Integer ParkingPlaceID; | |
| @Expose | |
| @Expose | |
| @Column(name = "VanSerialNo") | |
| private Integer vanSerialNo; | |
| @Expose | |
| @Column(name = "VanID") | |
| private Integer vanId; | |
| @Expose | |
| @Column(name = "VehicalNo") | |
| private String vehicleNo; | |
| @Expose | |
| @Column(name = "ParkingPlaceID") | |
| private Integer parkingPlaceId; | |
| @Expose |
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (2)
21-24: Replace @DaTa with more specific Lombok annotations.Using
@Datawith JPA entities can lead to performance issues due to lazy loading. Consider using a combination of@Getter,@Setter, and@EqualsAndHashCode(exclude = "grievanceTransactionDetails")instead.-@Data +@Getter +@Setter +@EqualsAndHashCode(exclude = "grievanceTransactionDetails")
24-24: Add JPA auditing for better tracking of changes.Since this entity handles sensitive grievance data, consider adding JPA auditing annotations for better tracking:
+@EntityListeners(AuditingEntityListener.class) public class GrievanceDetails {Also add the following imports:
import org.springframework.data.jpa.domain.support.AuditingEntityListener;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/iemr/common/data/grievance/GrievanceTransaction.java
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (2)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (2)
65-67: Fix typos and inconsistent field naming.The field names contain typos and don't follow Java naming conventions:
- "Severety" should be "Severity"
- "VehicalNo" should be "vehicleNo"
Also applies to: 129-130
146-148: LGTM! Well-structured relationship mapping.The one-to-many relationship with GrievanceTransaction is properly configured with appropriate cascade type and fetch strategy.
| public GrievanceDetails(Long gwid, Long grievanceId, Long beneficiaryRegID, Long benCallID, | ||
| Integer providerServiceMapID, String complaintID, String subjectOfComplaint, String complaint, | ||
| String primaryNumber, String severety, String state, String agentID, String userid, Boolean isAllocated, | ||
| Boolean retryNeeded, Boolean isRegistered, Integer callCounter, Boolean deleted, Character processed, | ||
| String createdBy, Timestamp createdDate, String modifiedBy, Timestamp lastModDate, Integer vanSerialNo, | ||
| Integer vanID, String vehicalNo, Integer parkingPlaceID, String syncedBy, Timestamp syncedDate, | ||
| Boolean isCompleted) { | ||
| super(); | ||
| this.gwid = gwid; | ||
| this.grievanceId = grievanceId; | ||
| this.beneficiaryRegID = beneficiaryRegID; | ||
| this.benCallID = benCallID; | ||
| this.providerServiceMapID = providerServiceMapID; | ||
| this.complaintID = complaintID; | ||
| this.subjectOfComplaint = subjectOfComplaint; | ||
| this.complaint = complaint; | ||
| this.primaryNumber = primaryNumber; | ||
| this.severety = severety; | ||
| this.state = state; | ||
| this.agentid = agentID; | ||
| this.userid = userid; | ||
| this.isAllocated = isAllocated; | ||
| this.retryNeeded = retryNeeded; | ||
| this.isRegistered = isRegistered; | ||
| this.callCounter = callCounter; | ||
| this.deleted = deleted; | ||
| this.Processed = processed; | ||
| this.createdBy = createdBy; | ||
| this.createdDate = createdDate; | ||
| this.modifiedBy = modifiedBy; | ||
| this.lastModDate = lastModDate; | ||
| this.vanSerialNo = vanSerialNo; | ||
| this.vanID = vanID; | ||
| this.vehicalNo = vehicalNo; | ||
| this.parkingPlaceID = parkingPlaceID; | ||
| this.syncedBy = syncedBy; | ||
| this.syncedDate = syncedDate; | ||
| this.isCompleted = isCompleted; | ||
| } |
There was a problem hiding this comment.
Fix inconsistencies in constructor parameters and setter methods.
There are naming inconsistencies between fields, constructor parameters, and setter methods:
- Constructor parameter
agentIDdoesn't match field nameagentid - Method
setSeveritydoesn't match field nameseverety
-public void setSeverity(String severety) {
+public void setSeverety(String severety) {
this.severety = severety;
}Also applies to: 268-270
There was a problem hiding this comment.
@srishtigrp78 Thank you for addressing the naming inconsistencies! 🎉
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: 1
♻️ Duplicate comments (7)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (4)
65-67:⚠️ Potential issueFix typo in column name and add validation.
The column name "Severety" is misspelled and should be "Severity".
57-62:⚠️ Potential issueConsider encrypting sensitive PII data.
Fields containing PII should be encrypted:
primaryNumber: Contains contact information
122-133:⚠️ Potential issueFollow Java naming conventions for field names.
Field names should follow camelCase convention and be consistent with their column names.
268-270:⚠️ Potential issueFix inconsistencies in setter method name.
The setter method name doesn't match the field name:
- Method:
setSeverity- Field:
severetysrc/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (3)
84-85:⚠️ Potential issueUse instance variables instead of static variables.
Using static variables for token storage can lead to thread safety issues in a multi-threaded environment.
284-286:⚠️ Potential issueImprove exception handling.
The catch block swallows exceptions after logging them. Consider rethrowing a custom exception.
158-162: 🛠️ Refactor suggestionEnhance error handling for duplicate complaints.
Consider using a custom exception for duplicate complaints instead of RuntimeException.
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (1)
21-24: Add class-level documentation.Consider adding Javadoc to describe the purpose and functionality of this entity class.
+/** + * Entity class representing grievance details in the system. + * Maps to the t_grievanceworklist table in the database. + */ @Entity @Table(name = "t_grievanceworklist") @Data public class GrievanceDetails {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/iemr/common/config/quartz/QuartzConfig.java(3 hunks)src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/iemr/common/service/grievance/GrievanceDataSync.java
- src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java
- src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java
🔇 Additional comments (4)
src/main/java/com/iemr/common/data/grievance/GrievanceDetails.java (1)
146-148: LGTM! Well-structured relationship mapping.The one-to-many relationship with GrievanceTransaction is properly configured with appropriate JPA annotations.
src/main/java/com/iemr/common/config/quartz/QuartzConfig.java (2)
100-101: LGTM! Proper integration of new triggers.The grievance data sync trigger is correctly integrated into the scheduler configuration.
173-173: Improved null check using Boolean.TRUE.equals().Good practice using
Boolean.TRUE.equals(startJob)for null-safe comparison.src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
353-353:⚠️ Potential issueAvoid logging sensitive data.
The auth token should not be logged in production environments.
-logger.info("Auth key generated at : " + System.currentTimeMillis() + ", Key : " + GRIEVANCE_AUTH_TOKEN); +logger.info("Auth key generated at : " + System.currentTimeMillis());Likely invalid or redundant comment.
| while (count > 0) { | ||
| List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); | ||
| if (savedGrievanceData != null) | ||
| break; | ||
| else | ||
| count--; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout mechanism for retry logic.
The retry loop should include a delay between attempts and a timeout mechanism.
while (count > 0) {
+ try {
List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance();
if (savedGrievanceData != null)
break;
- else
- count--;
+ else {
+ count--;
+ if (count > 0) {
+ Thread.sleep(5000); // 5 seconds delay between retries
+ }
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ break;
+ }
}📝 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.
| while (count > 0) { | |
| List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); | |
| if (savedGrievanceData != null) | |
| break; | |
| else | |
| count--; | |
| } | |
| while (count > 0) { | |
| try { | |
| List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); | |
| if (savedGrievanceData != null) | |
| break; | |
| else { | |
| count--; | |
| if (count > 0) { | |
| Thread.sleep(5000); // 5 seconds delay between retries | |
| } | |
| } | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| break; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (2)
359-365: Improve retry mechanism with exponential backoff.The current retry mechanism doesn't include any delay between attempts. Implement exponential backoff to avoid overwhelming the system.
int count = 3; while (count > 0) { + try { List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); if (savedGrievanceData != null) break; - else - count--; + else { + count--; + if (count > 0) { + Thread.sleep((3 - count) * 1000); // Exponential backoff + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } }
349-356: Simplify token expiration calculation using Java 8 Time API.The current date manipulation code is verbose and uses the legacy Calendar API. Consider using the Java 8 Time API for better readability and maintainability.
- Date date = new Date(); - java.sql.Date sqlDate = new java.sql.Date(date.getTime()); - Calendar grievanceCalendar = Calendar.getInstance(); - grievanceCalendar.setTime(sqlDate); - grievanceCalendar.add(Calendar.DATE, 29); - Date grievanceTokenEndDate = grievanceCalendar.getTime(); - GRIEVANCE_TOKEN_EXP = grievanceTokenEndDate.getTime(); + GRIEVANCE_TOKEN_EXP = LocalDateTime.now() + .plusDays(29) + .atZone(ZoneId.systemDefault()) + .toInstant() + .toEpochMilli();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/iemr/common/repository/grievance/GrievanceFetchBenDetailsRepo.java
- src/main/java/com/iemr/common/config/quartz/ScheduleForGrievanceDataSync.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (java)
- GitHub Check: Build
- GitHub Check: Package-test
🔇 Additional comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
88-89: 🛠️ Refactor suggestionConsider using instance variables for token management.
Using instance variables for
GRIEVANCE_AUTH_TOKENandGRIEVANCE_TOKEN_EXPwould be more appropriate in a multi-instance environment. Also, follow Java naming conventions for instance variables.- private String GRIEVANCE_AUTH_TOKEN; - private Long GRIEVANCE_TOKEN_EXP; + private String grievanceAuthToken; + private Long grievanceTokenExp;Likely invalid or redundant comment.
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | ||
|
|
||
| // Adding properties for each transaction detail | ||
| // transactionDetails.setComplaintId(formattedComplaintId); | ||
| // Assuming these fields are coming from your API response | ||
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | ||
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | ||
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | ||
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | ||
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | ||
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | ||
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | ||
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | ||
|
|
There was a problem hiding this comment.
Potential NullPointerException in transaction processing.
The code assumes that all fields exist in the transaction JSON. Add null checks before accessing JSON fields.
- transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString());
+ if (transactionDetailsJson.has("actionTakenBy")) {
+ transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString());
+ }📝 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.
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | |
| // Adding properties for each transaction detail | |
| // transactionDetails.setComplaintId(formattedComplaintId); | |
| // Assuming these fields are coming from your API response | |
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | |
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | |
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | |
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | |
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | |
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | |
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | |
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | |
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | |
| // Adding properties for each transaction detail | |
| // transactionDetails.setComplaintId(formattedComplaintId); | |
| // Assuming these fields are coming from your API response | |
| if (transactionDetailsJson.has("actionTakenBy")) { | |
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | |
| } | |
| if (transactionDetailsJson.has("status")) { | |
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | |
| } | |
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | |
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | |
| if (transactionDetailsJson.has("redressed")) { | |
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | |
| } | |
| if (transactionDetailsJson.has("createdAt")) { | |
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | |
| } | |
| if (transactionDetailsJson.has("updatedAt")) { | |
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | |
| } | |
| if (transactionDetailsJson.has("comment")) { | |
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | |
| } |
| HttpEntity<Object> request = new HttpEntity<Object>(headers); | ||
| RestTemplate restTemplate = new RestTemplate(); | ||
|
|
||
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails + grievanceId, HttpMethod.POST, request, String.class); |
There was a problem hiding this comment.
Secure URL construction for transaction details.
The grievance ID is directly concatenated to the URL, which could be vulnerable to injection attacks. Use UriComponentsBuilder to safely construct the URL.
- ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails + grievanceId, HttpMethod.POST, request, String.class);
+ String url = UriComponentsBuilder.fromUriString(updateGrievanceTransactionDetails)
+ .queryParam("grievanceId", grievanceId)
+ .build()
+ .toUriString();
+ ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.POST, request, String.class);📝 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.
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails + grievanceId, HttpMethod.POST, request, String.class); | |
| String url = UriComponentsBuilder.fromUriString(updateGrievanceTransactionDetails) | |
| .queryParam("grievanceId", grievanceId) | |
| .build() | |
| .toUriString(); | |
| ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.POST, request, String.class); |
| public List<Map<String, Object>> dataSyncToGrievance() { | ||
|
|
||
| int count = 0; | ||
| String registeringUser = ""; | ||
| List<Map<String, Object>> responseData = new ArrayList<>(); | ||
| List<GrievanceDetails> grievanceDetailsListAS = null; | ||
| try { | ||
| // Loop to fetch data for multiple pages | ||
| while (count >= 0) { | ||
| RestTemplate restTemplate = new RestTemplate(); | ||
|
|
||
| if (GRIEVANCE_AUTH_TOKEN != null && GRIEVANCE_TOKEN_EXP != null | ||
| && GRIEVANCE_TOKEN_EXP > System.currentTimeMillis()) { | ||
| // no need of calling auth API | ||
| } else { | ||
| // call method to generate Auth Token at Everwell end | ||
| generateGrievanceAuthToken(); | ||
| } | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); | ||
| headers.add(USER_AGENT_HEADER, USER_AGENT_VALUE); | ||
| headers.add("AUTHORIZATION", GRIEVANCE_AUTH_TOKEN); | ||
|
|
||
| Date date = new Date(); | ||
| java.sql.Date sqlDate = new java.sql.Date(date.getTime()); | ||
| Calendar calendar = Calendar.getInstance(); | ||
| calendar.setTime(sqlDate); | ||
| calendar.add(Calendar.DATE, -Integer.parseInt(grievanceDataSyncDuration)); | ||
|
|
||
| // Request object | ||
| HttpEntity<Object> request = new HttpEntity<Object>(headers); | ||
|
|
||
| // Call rest-template to call API to download master data for given table | ||
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceDetails, HttpMethod.POST, request, | ||
| String.class); | ||
|
|
||
| if (response != null && response.hasBody()) { | ||
| JSONObject obj = new JSONObject(response.getBody()); | ||
| if (obj != null && obj.has("data") && obj.has(STATUS_CODE) && obj.getInt(STATUS_CODE) == 200) { | ||
| logger.info("Grievance data details response received successfully "); | ||
|
|
||
| String responseStr = response.getBody(); | ||
| JsonObject jsnOBJ = new JsonObject(); | ||
| JsonParser jsnParser = new JsonParser(); | ||
| JsonElement jsnElmnt = jsnParser.parse(responseStr); | ||
| jsnOBJ = jsnElmnt.getAsJsonObject(); | ||
| JsonObject grievanceJsonData = jsnOBJ.getAsJsonObject("data"); | ||
|
|
||
| registeringUser = grievanceJsonData.get("userName").getAsString(); | ||
|
|
||
| if (Integer.parseInt(jsnOBJ.get("TotalRecords").toString()) > 0) { | ||
| GrievanceDetails[] grievanceDetailsArray = InputMapper.gson().fromJson(jsnOBJ.get("Data").toString(), GrievanceDetails[].class); | ||
| List<GrievanceDetails> grievanceDetailsList = Arrays.asList(grievanceDetailsArray); | ||
|
|
||
|
|
||
| ////////////////////// | ||
| // Loop through the fetched grievance list and integrate transaction details | ||
| for (GrievanceDetails grievance : grievanceDetailsList) { | ||
| String complaintId = grievanceJsonData.get("complainId").getAsString(); | ||
| String formattedComplaintId = complaintId.replace("\\/", "/"); | ||
|
|
||
| // Check if the complaintId is already present in the t_grievance_worklist table | ||
| boolean complaintExists = grievanceDataRepo.existsByComplaintId(formattedComplaintId); | ||
| if (complaintExists) { | ||
| throw new RuntimeException("Complaint ID " + formattedComplaintId + " already exists in the grievance worklist table."); | ||
| } | ||
|
|
||
|
|
||
| grievance.setComplaintId(formattedComplaintId); | ||
|
|
||
| // Fetch related grievance transaction details | ||
| List<GrievanceTransaction> transactionDetailsList = fetchGrievanceTransactions(grievance.getGrievanceId()); | ||
|
|
||
| if (transactionDetailsList != null && !transactionDetailsList.isEmpty()) { | ||
| // Loop through each transaction and set individual properties | ||
| for (GrievanceTransaction transactionDetails : transactionDetailsList) { | ||
|
|
||
| // Assuming transactionDetailsJson is the JSON object representing a single transaction from the API response | ||
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | ||
|
|
||
| // Adding properties for each transaction detail | ||
| // transactionDetails.setComplaintId(formattedComplaintId); | ||
| // Assuming these fields are coming from your API response | ||
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | ||
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | ||
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | ||
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | ||
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | ||
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | ||
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | ||
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | ||
|
|
||
| // Save individual transaction detail in the t_grievance_transaction table | ||
| grievanceTransactionRepo.save(transactionDetails); | ||
| } | ||
|
|
||
| // Add the transaction list to the grievance object | ||
| grievance.setGrievanceTransactionDetails(transactionDetailsList); | ||
| } | ||
|
|
||
| // Adding other grievance-related fields (similar to the existing code) | ||
| grievance.setSubjectOfComplaint(grievanceJsonData.get("subject").getAsString()); | ||
| ArrayList<Object[]> lists = grievanceFetchBenDetailsRepo.findByComplaintId(formattedComplaintId); | ||
|
|
||
| for (Object[] objects : lists) { | ||
| if (objects != null && objects.length <= 4) { | ||
| grievance.setComplaintId((String) objects[0]); | ||
| grievance.setBeneficiaryRegId((Long) objects[1]); | ||
| grievance.setBencallId((Long) objects[2]); | ||
| grievance.setProviderServiceMapId((Integer) objects[3]); | ||
| String state = locationStateRepository.findByStateIDForGrievance((Integer) objects[4]); | ||
| grievance.setState(state); | ||
| } | ||
| } | ||
|
|
||
| // Setting remaining grievance properties (similar to the existing code) | ||
| grievance.setAgentId(grievance.getAgentId()); | ||
| grievance.setDeleted(grievance.getDeleted()); | ||
| grievance.setCreatedBy(registeringUser); | ||
| grievance.setProcessed('N'); | ||
| grievance.setIsAllocated(false); | ||
| grievance.setCallCounter(0); | ||
| grievance.setRetryNeeded(true); | ||
| } | ||
|
|
||
| // Save the grievance details to the t_grievance table | ||
| grievanceDetailsListAS = (List<GrievanceDetails>) grievanceDataRepo.saveAll(grievanceDetailsList); | ||
|
|
||
| // Combine grievance and transaction data for response | ||
|
|
||
| for (GrievanceDetails grievance : grievanceDetailsListAS) { | ||
| Map<String, Object> combinedData = new HashMap<>(); | ||
| combinedData.put("complaintID", grievance.getGrievanceId()); | ||
| combinedData.put("subjectOfComplaint", grievance.getSubjectOfComplaint()); | ||
| combinedData.put("complaint", grievance.getComplaint()); | ||
| combinedData.put("beneficiaryRegID", grievance.getBeneficiaryRegId()); | ||
| combinedData.put("providerServiceMapId", grievance.getProviderServiceMapId()); | ||
| // combinedData.put("firstName", grievance.getFirstName()); | ||
| // combinedData.put("lastName", grievance.getLastName()); | ||
| combinedData.put("primaryNumber", grievance.getPrimaryNumber()); | ||
|
|
||
| // Add transaction data | ||
| List<Map<String, Object>> transactions = new ArrayList<>(); | ||
| for (GrievanceTransaction transaction : grievance.getGrievanceTransactionDetails()) { | ||
| Map<String, Object> transactionData = new HashMap<>(); | ||
| transactionData.put("actionTakenBy", transaction.getActionTakenBy()); | ||
| transactionData.put("status", transaction.getStatus()); | ||
| transactionData.put("fileName", transaction.getFileName()); | ||
| transactionData.put("fileType", transaction.getFileType()); | ||
| transactionData.put("redressed", transaction.getRedressed()); | ||
| transactionData.put("createdAt", transaction.getCreatedAt().toString()); | ||
| transactionData.put("updatedAt", transaction.getUpdatedAt().toString()); | ||
| transactionData.put("comment", transaction.getComment()); | ||
| transactions.add(transactionData); | ||
| } | ||
|
|
||
| combinedData.put("transaction", transactions); | ||
| combinedData.put("severity", grievance.getSeverety()); | ||
| combinedData.put("state", grievance.getState()); | ||
| combinedData.put("agentId", grievance.getAgentId()); | ||
| combinedData.put("deleted", grievance.getDeleted()); | ||
| combinedData.put("createdBy", grievance.getCreatedBy()); | ||
| combinedData.put("createdDate", grievance.getCreatedDate()); | ||
| combinedData.put("lastModDate", grievance.getLastModDate()); | ||
| combinedData.put("isCompleted", grievance.getIsCompleted()); | ||
| // combinedData.put("gender", grievance.getGender()); | ||
| // combinedData.put("district", grievance.getDistrict()); | ||
| // combinedData.put("beneficiaryID", grievance.getBeneficiaryId()); | ||
| // combinedData.put("age", grievance.getAge()); | ||
| combinedData.put("retryNeeded", grievance.getRetryNeeded()); | ||
| combinedData.put("callCounter", grievance.getCallCounter()); | ||
| // combinedData.put("lastCall", grievance.getLastCall()); | ||
|
|
||
| responseData.add(combinedData); | ||
| } | ||
|
|
||
|
|
||
| // Return the combined response as required | ||
| // return responseData; | ||
|
|
||
| } else { | ||
| logger.info("No records found for page = " + count); | ||
| count = -1; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| logger.error("Error in saving data into t_grievanceworklist: ", e); | ||
| } | ||
| return responseData; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor long method to improve maintainability.
The dataSyncToGrievance method is too long and handles multiple responsibilities. Consider breaking it down into smaller, focused methods.
- Extract grievance processing logic (lines 150-216) into a separate method
- Extract response mapping logic (lines 223-267) into a separate method
- Consider using a builder pattern for response construction
Also, improve error handling to avoid swallowing exceptions:
} catch (Exception e) {
logger.error("Error in saving data into t_grievanceworklist: ", e);
+ throw new GrievanceProcessingException("Failed to process grievance data", e);
}📝 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<Map<String, Object>> dataSyncToGrievance() { | |
| int count = 0; | |
| String registeringUser = ""; | |
| List<Map<String, Object>> responseData = new ArrayList<>(); | |
| List<GrievanceDetails> grievanceDetailsListAS = null; | |
| try { | |
| // Loop to fetch data for multiple pages | |
| while (count >= 0) { | |
| RestTemplate restTemplate = new RestTemplate(); | |
| if (GRIEVANCE_AUTH_TOKEN != null && GRIEVANCE_TOKEN_EXP != null | |
| && GRIEVANCE_TOKEN_EXP > System.currentTimeMillis()) { | |
| // no need of calling auth API | |
| } else { | |
| // call method to generate Auth Token at Everwell end | |
| generateGrievanceAuthToken(); | |
| } | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); | |
| headers.add(USER_AGENT_HEADER, USER_AGENT_VALUE); | |
| headers.add("AUTHORIZATION", GRIEVANCE_AUTH_TOKEN); | |
| Date date = new Date(); | |
| java.sql.Date sqlDate = new java.sql.Date(date.getTime()); | |
| Calendar calendar = Calendar.getInstance(); | |
| calendar.setTime(sqlDate); | |
| calendar.add(Calendar.DATE, -Integer.parseInt(grievanceDataSyncDuration)); | |
| // Request object | |
| HttpEntity<Object> request = new HttpEntity<Object>(headers); | |
| // Call rest-template to call API to download master data for given table | |
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceDetails, HttpMethod.POST, request, | |
| String.class); | |
| if (response != null && response.hasBody()) { | |
| JSONObject obj = new JSONObject(response.getBody()); | |
| if (obj != null && obj.has("data") && obj.has(STATUS_CODE) && obj.getInt(STATUS_CODE) == 200) { | |
| logger.info("Grievance data details response received successfully "); | |
| String responseStr = response.getBody(); | |
| JsonObject jsnOBJ = new JsonObject(); | |
| JsonParser jsnParser = new JsonParser(); | |
| JsonElement jsnElmnt = jsnParser.parse(responseStr); | |
| jsnOBJ = jsnElmnt.getAsJsonObject(); | |
| JsonObject grievanceJsonData = jsnOBJ.getAsJsonObject("data"); | |
| registeringUser = grievanceJsonData.get("userName").getAsString(); | |
| if (Integer.parseInt(jsnOBJ.get("TotalRecords").toString()) > 0) { | |
| GrievanceDetails[] grievanceDetailsArray = InputMapper.gson().fromJson(jsnOBJ.get("Data").toString(), GrievanceDetails[].class); | |
| List<GrievanceDetails> grievanceDetailsList = Arrays.asList(grievanceDetailsArray); | |
| ////////////////////// | |
| // Loop through the fetched grievance list and integrate transaction details | |
| for (GrievanceDetails grievance : grievanceDetailsList) { | |
| String complaintId = grievanceJsonData.get("complainId").getAsString(); | |
| String formattedComplaintId = complaintId.replace("\\/", "/"); | |
| // Check if the complaintId is already present in the t_grievance_worklist table | |
| boolean complaintExists = grievanceDataRepo.existsByComplaintId(formattedComplaintId); | |
| if (complaintExists) { | |
| throw new RuntimeException("Complaint ID " + formattedComplaintId + " already exists in the grievance worklist table."); | |
| } | |
| grievance.setComplaintId(formattedComplaintId); | |
| // Fetch related grievance transaction details | |
| List<GrievanceTransaction> transactionDetailsList = fetchGrievanceTransactions(grievance.getGrievanceId()); | |
| if (transactionDetailsList != null && !transactionDetailsList.isEmpty()) { | |
| // Loop through each transaction and set individual properties | |
| for (GrievanceTransaction transactionDetails : transactionDetailsList) { | |
| // Assuming transactionDetailsJson is the JSON object representing a single transaction from the API response | |
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | |
| // Adding properties for each transaction detail | |
| // transactionDetails.setComplaintId(formattedComplaintId); | |
| // Assuming these fields are coming from your API response | |
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | |
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | |
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | |
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | |
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | |
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | |
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | |
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | |
| // Save individual transaction detail in the t_grievance_transaction table | |
| grievanceTransactionRepo.save(transactionDetails); | |
| } | |
| // Add the transaction list to the grievance object | |
| grievance.setGrievanceTransactionDetails(transactionDetailsList); | |
| } | |
| // Adding other grievance-related fields (similar to the existing code) | |
| grievance.setSubjectOfComplaint(grievanceJsonData.get("subject").getAsString()); | |
| ArrayList<Object[]> lists = grievanceFetchBenDetailsRepo.findByComplaintId(formattedComplaintId); | |
| for (Object[] objects : lists) { | |
| if (objects != null && objects.length <= 4) { | |
| grievance.setComplaintId((String) objects[0]); | |
| grievance.setBeneficiaryRegId((Long) objects[1]); | |
| grievance.setBencallId((Long) objects[2]); | |
| grievance.setProviderServiceMapId((Integer) objects[3]); | |
| String state = locationStateRepository.findByStateIDForGrievance((Integer) objects[4]); | |
| grievance.setState(state); | |
| } | |
| } | |
| // Setting remaining grievance properties (similar to the existing code) | |
| grievance.setAgentId(grievance.getAgentId()); | |
| grievance.setDeleted(grievance.getDeleted()); | |
| grievance.setCreatedBy(registeringUser); | |
| grievance.setProcessed('N'); | |
| grievance.setIsAllocated(false); | |
| grievance.setCallCounter(0); | |
| grievance.setRetryNeeded(true); | |
| } | |
| // Save the grievance details to the t_grievance table | |
| grievanceDetailsListAS = (List<GrievanceDetails>) grievanceDataRepo.saveAll(grievanceDetailsList); | |
| // Combine grievance and transaction data for response | |
| for (GrievanceDetails grievance : grievanceDetailsListAS) { | |
| Map<String, Object> combinedData = new HashMap<>(); | |
| combinedData.put("complaintID", grievance.getGrievanceId()); | |
| combinedData.put("subjectOfComplaint", grievance.getSubjectOfComplaint()); | |
| combinedData.put("complaint", grievance.getComplaint()); | |
| combinedData.put("beneficiaryRegID", grievance.getBeneficiaryRegId()); | |
| combinedData.put("providerServiceMapId", grievance.getProviderServiceMapId()); | |
| // combinedData.put("firstName", grievance.getFirstName()); | |
| // combinedData.put("lastName", grievance.getLastName()); | |
| combinedData.put("primaryNumber", grievance.getPrimaryNumber()); | |
| // Add transaction data | |
| List<Map<String, Object>> transactions = new ArrayList<>(); | |
| for (GrievanceTransaction transaction : grievance.getGrievanceTransactionDetails()) { | |
| Map<String, Object> transactionData = new HashMap<>(); | |
| transactionData.put("actionTakenBy", transaction.getActionTakenBy()); | |
| transactionData.put("status", transaction.getStatus()); | |
| transactionData.put("fileName", transaction.getFileName()); | |
| transactionData.put("fileType", transaction.getFileType()); | |
| transactionData.put("redressed", transaction.getRedressed()); | |
| transactionData.put("createdAt", transaction.getCreatedAt().toString()); | |
| transactionData.put("updatedAt", transaction.getUpdatedAt().toString()); | |
| transactionData.put("comment", transaction.getComment()); | |
| transactions.add(transactionData); | |
| } | |
| combinedData.put("transaction", transactions); | |
| combinedData.put("severity", grievance.getSeverety()); | |
| combinedData.put("state", grievance.getState()); | |
| combinedData.put("agentId", grievance.getAgentId()); | |
| combinedData.put("deleted", grievance.getDeleted()); | |
| combinedData.put("createdBy", grievance.getCreatedBy()); | |
| combinedData.put("createdDate", grievance.getCreatedDate()); | |
| combinedData.put("lastModDate", grievance.getLastModDate()); | |
| combinedData.put("isCompleted", grievance.getIsCompleted()); | |
| // combinedData.put("gender", grievance.getGender()); | |
| // combinedData.put("district", grievance.getDistrict()); | |
| // combinedData.put("beneficiaryID", grievance.getBeneficiaryId()); | |
| // combinedData.put("age", grievance.getAge()); | |
| combinedData.put("retryNeeded", grievance.getRetryNeeded()); | |
| combinedData.put("callCounter", grievance.getCallCounter()); | |
| // combinedData.put("lastCall", grievance.getLastCall()); | |
| responseData.add(combinedData); | |
| } | |
| // Return the combined response as required | |
| // return responseData; | |
| } else { | |
| logger.info("No records found for page = " + count); | |
| count = -1; | |
| } | |
| } | |
| } | |
| } | |
| } catch (Exception e) { | |
| logger.error("Error in saving data into t_grievanceworklist: ", e); | |
| } | |
| return responseData; | |
| } | |
| public List<Map<String, Object>> dataSyncToGrievance() { | |
| int count = 0; | |
| String registeringUser = ""; | |
| List<Map<String, Object>> responseData = new ArrayList<>(); | |
| List<GrievanceDetails> grievanceDetailsListAS = null; | |
| try { | |
| // Loop to fetch data for multiple pages | |
| while (count >= 0) { | |
| RestTemplate restTemplate = new RestTemplate(); | |
| if (GRIEVANCE_AUTH_TOKEN != null && GRIEVANCE_TOKEN_EXP != null | |
| && GRIEVANCE_TOKEN_EXP > System.currentTimeMillis()) { | |
| // no need of calling auth API | |
| } else { | |
| // call method to generate Auth Token at Everwell end | |
| generateGrievanceAuthToken(); | |
| } | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); | |
| headers.add(USER_AGENT_HEADER, USER_AGENT_VALUE); | |
| headers.add("AUTHORIZATION", GRIEVANCE_AUTH_TOKEN); | |
| Date date = new Date(); | |
| java.sql.Date sqlDate = new java.sql.Date(date.getTime()); | |
| Calendar calendar = Calendar.getInstance(); | |
| calendar.setTime(sqlDate); | |
| calendar.add(Calendar.DATE, -Integer.parseInt(grievanceDataSyncDuration)); | |
| // Request object | |
| HttpEntity<Object> request = new HttpEntity<Object>(headers); | |
| // Call rest-template to call API to download master data for given table | |
| ResponseEntity<String> response = restTemplate.exchange(updateGrievanceDetails, HttpMethod.POST, request, | |
| String.class); | |
| if (response != null && response.hasBody()) { | |
| JSONObject obj = new JSONObject(response.getBody()); | |
| if (obj != null && obj.has("data") && obj.has(STATUS_CODE) && obj.getInt(STATUS_CODE) == 200) { | |
| logger.info("Grievance data details response received successfully "); | |
| String responseStr = response.getBody(); | |
| JsonObject jsnOBJ = new JsonObject(); | |
| JsonParser jsnParser = new JsonParser(); | |
| JsonElement jsnElmnt = jsnParser.parse(responseStr); | |
| jsnOBJ = jsnElmnt.getAsJsonObject(); | |
| JsonObject grievanceJsonData = jsnOBJ.getAsJsonObject("data"); | |
| registeringUser = grievanceJsonData.get("userName").getAsString(); | |
| if (Integer.parseInt(jsnOBJ.get("TotalRecords").toString()) > 0) { | |
| GrievanceDetails[] grievanceDetailsArray = InputMapper.gson().fromJson(jsnOBJ.get("Data").toString(), GrievanceDetails[].class); | |
| List<GrievanceDetails> grievanceDetailsList = Arrays.asList(grievanceDetailsArray); | |
| ////////////////////// | |
| // Loop through the fetched grievance list and integrate transaction details | |
| for (GrievanceDetails grievance : grievanceDetailsList) { | |
| String complaintId = grievanceJsonData.get("complainId").getAsString(); | |
| String formattedComplaintId = complaintId.replace("\\/", "/"); | |
| // Check if the complaintId is already present in the t_grievance_worklist table | |
| boolean complaintExists = grievanceDataRepo.existsByComplaintId(formattedComplaintId); | |
| if (complaintExists) { | |
| throw new RuntimeException("Complaint ID " + formattedComplaintId + " already exists in the grievance worklist table."); | |
| } | |
| grievance.setComplaintId(formattedComplaintId); | |
| // Fetch related grievance transaction details | |
| List<GrievanceTransaction> transactionDetailsList = fetchGrievanceTransactions(grievance.getGrievanceId()); | |
| if (transactionDetailsList != null && !transactionDetailsList.isEmpty()) { | |
| // Loop through each transaction and set individual properties | |
| for (GrievanceTransaction transactionDetails : transactionDetailsList) { | |
| // Assuming transactionDetailsJson is the JSON object representing a single transaction from the API response | |
| JsonObject transactionDetailsJson = grievanceJsonData.getAsJsonObject("transactionDetails"); // or another relevant path | |
| // Adding properties for each transaction detail | |
| // transactionDetails.setComplaintId(formattedComplaintId); | |
| // Assuming these fields are coming from your API response | |
| transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); | |
| transactionDetails.setStatus(transactionDetailsJson.get("status").getAsString()); | |
| transactionDetails.setFileName(transactionDetailsJson.has("fileName") ? transactionDetailsJson.get("fileName").getAsString() : null); | |
| transactionDetails.setFileType(transactionDetailsJson.has("fileType") ? transactionDetailsJson.get("fileType").getAsString() : null); | |
| transactionDetails.setRedressed(transactionDetailsJson.get("redressed").getAsString()); | |
| transactionDetails.setCreatedAt(Timestamp.valueOf(transactionDetailsJson.get("createdAt").getAsString())); | |
| transactionDetails.setUpdatedAt(Timestamp.valueOf(transactionDetailsJson.get("updatedAt").getAsString())); | |
| transactionDetails.setComment(transactionDetailsJson.get("comment").getAsString()); | |
| // Save individual transaction detail in the t_grievance_transaction table | |
| grievanceTransactionRepo.save(transactionDetails); | |
| } | |
| // Add the transaction list to the grievance object | |
| grievance.setGrievanceTransactionDetails(transactionDetailsList); | |
| } | |
| // Adding other grievance-related fields (similar to the existing code) | |
| grievance.setSubjectOfComplaint(grievanceJsonData.get("subject").getAsString()); | |
| ArrayList<Object[]> lists = grievanceFetchBenDetailsRepo.findByComplaintId(formattedComplaintId); | |
| for (Object[] objects : lists) { | |
| if (objects != null && objects.length <= 4) { | |
| grievance.setComplaintId((String) objects[0]); | |
| grievance.setBeneficiaryRegId((Long) objects[1]); | |
| grievance.setBencallId((Long) objects[2]); | |
| grievance.setProviderServiceMapId((Integer) objects[3]); | |
| String state = locationStateRepository.findByStateIDForGrievance((Integer) objects[4]); | |
| grievance.setState(state); | |
| } | |
| } | |
| // Setting remaining grievance properties (similar to the existing code) | |
| grievance.setAgentId(grievance.getAgentId()); | |
| grievance.setDeleted(grievance.getDeleted()); | |
| grievance.setCreatedBy(registeringUser); | |
| grievance.setProcessed('N'); | |
| grievance.setIsAllocated(false); | |
| grievance.setCallCounter(0); | |
| grievance.setRetryNeeded(true); | |
| } | |
| // Save the grievance details to the t_grievance table | |
| grievanceDetailsListAS = (List<GrievanceDetails>) grievanceDataRepo.saveAll(grievanceDetailsList); | |
| // Combine grievance and transaction data for response | |
| for (GrievanceDetails grievance : grievanceDetailsListAS) { | |
| Map<String, Object> combinedData = new HashMap<>(); | |
| combinedData.put("complaintID", grievance.getGrievanceId()); | |
| combinedData.put("subjectOfComplaint", grievance.getSubjectOfComplaint()); | |
| combinedData.put("complaint", grievance.getComplaint()); | |
| combinedData.put("beneficiaryRegID", grievance.getBeneficiaryRegId()); | |
| combinedData.put("providerServiceMapId", grievance.getProviderServiceMapId()); | |
| // combinedData.put("firstName", grievance.getFirstName()); | |
| // combinedData.put("lastName", grievance.getLastName()); | |
| combinedData.put("primaryNumber", grievance.getPrimaryNumber()); | |
| // Add transaction data | |
| List<Map<String, Object>> transactions = new ArrayList<>(); | |
| for (GrievanceTransaction transaction : grievance.getGrievanceTransactionDetails()) { | |
| Map<String, Object> transactionData = new HashMap<>(); | |
| transactionData.put("actionTakenBy", transaction.getActionTakenBy()); | |
| transactionData.put("status", transaction.getStatus()); | |
| transactionData.put("fileName", transaction.getFileName()); | |
| transactionData.put("fileType", transaction.getFileType()); | |
| transactionData.put("redressed", transaction.getRedressed()); | |
| transactionData.put("createdAt", transaction.getCreatedAt().toString()); | |
| transactionData.put("updatedAt", transaction.getUpdatedAt().toString()); | |
| transactionData.put("comment", transaction.getComment()); | |
| transactions.add(transactionData); | |
| } | |
| combinedData.put("transaction", transactions); | |
| combinedData.put("severity", grievance.getSeverety()); | |
| combinedData.put("state", grievance.getState()); | |
| combinedData.put("agentId", grievance.getAgentId()); | |
| combinedData.put("deleted", grievance.getDeleted()); | |
| combinedData.put("createdBy", grievance.getCreatedBy()); | |
| combinedData.put("createdDate", grievance.getCreatedDate()); | |
| combinedData.put("lastModDate", grievance.getLastModDate()); | |
| combinedData.put("isCompleted", grievance.getIsCompleted()); | |
| // combinedData.put("gender", grievance.getGender()); | |
| // combinedData.put("district", grievance.getDistrict()); | |
| // combinedData.put("beneficiaryID", grievance.getBeneficiaryId()); | |
| // combinedData.put("age", grievance.getAge()); | |
| combinedData.put("retryNeeded", grievance.getRetryNeeded()); | |
| combinedData.put("callCounter", grievance.getCallCounter()); | |
| // combinedData.put("lastCall", grievance.getLastCall()); | |
| responseData.add(combinedData); | |
| } | |
| // Return the combined response as required | |
| // return responseData; | |
| } else { | |
| logger.info("No records found for page = " + count); | |
| count = -1; | |
| } | |
| } | |
| } | |
| } | |
| } catch (Exception e) { | |
| logger.error("Error in saving data into t_grievanceworklist: ", e); | |
| throw new GrievanceProcessingException("Failed to process grievance data", e); | |
| } | |
| return responseData; | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for addressing the refactoring suggestions! This will help improve the code maintainability.
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: 2
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
344-347: Make token expiration time configurable.The token expiration time is hardcoded to 29 days. This should be configurable through application properties.
+ @Value("${grievance.token.expiration.days:29}") + private int tokenExpirationDays; - grievanceCalendar.add(Calendar.DATE, 29); + grievanceCalendar.add(Calendar.DATE, tokenExpirationDays);
📜 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(1 hunks)
🔇 Additional comments (4)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (4)
290-290:⚠️ Potential issueUse UriComponentsBuilder for safe URL construction.
Direct string concatenation in URLs is unsafe and could lead to injection vulnerabilities.
- ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails + grievanceId, HttpMethod.POST, request, String.class); + String url = UriComponentsBuilder.fromUriString(updateGrievanceTransactionDetails) + .queryParam("grievanceId", grievanceId) + .build() + .toUriString(); + ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.POST, request, String.class);Likely invalid or redundant comment.
273-275:⚠️ Potential issueImprove exception handling.
The catch block swallows exceptions after logging them. This could hide critical errors and make debugging difficult.
} catch (Exception e) { logger.error("Error in saving data into t_grievanceworklist: ", e); + throw new GrievanceProcessingException("Failed to process grievance data", e); }Likely invalid or redundant comment.
350-356: 🛠️ Refactor suggestionImprove retry mechanism with proper delay and timeout.
The retry logic should include a delay between attempts and a proper timeout mechanism.
- while (count > 0) { - List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); - if (savedGrievanceData != null) - break; - else - count--; - } + while (count > 0) { + try { + List<Map<String, Object>> savedGrievanceData = dataSyncToGrievance(); + if (savedGrievanceData != null) { + break; + } + count--; + if (count > 0) { + Thread.sleep(5000); // 5 seconds delay between retries + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + }Likely invalid or redundant comment.
88-89:⚠️ Potential issueConvert static fields to instance variables for thread safety.
The authentication token and expiration time should be instance variables to ensure thread safety in a multi-threaded environment.
- private String GRIEVANCE_AUTH_TOKEN; - private Long GRIEVANCE_TOKEN_EXP; + private volatile String grievanceAuthToken; + private volatile Long grievanceTokenExp;Likely invalid or redundant comment.
| @Value("${greivanceUserAuthenticate}") | ||
| private String grievanceUserAuthenticate; | ||
|
|
||
| @Value("${updateGrievanceDetails}") | ||
| private String updateGrievanceDetails; | ||
|
|
||
| @Value("${updateGrievanceTransactionDetails}") | ||
| private String updateGrievanceTransactionDetails; | ||
|
|
||
| @Value("${grievanceUserName}") | ||
| private String grievanceUserName; | ||
|
|
||
| @Value("${grievancePassword}") | ||
| private String grievancePassword; | ||
|
|
||
| @Value("${grievanceDataSyncDuration}") | ||
| private String grievanceDataSyncDuration; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for injected configuration values.
The @value injected properties should be validated during initialization to fail fast if any required configuration is missing.
Add a @PostConstruct method to validate the configuration:
@PostConstruct
private void validateConfiguration() {
Assert.hasText(grievanceUserAuthenticate, "grievanceUserAuthenticate must not be empty");
Assert.hasText(updateGrievanceDetails, "updateGrievanceDetails must not be empty");
Assert.hasText(updateGrievanceTransactionDetails, "updateGrievanceTransactionDetails must not be empty");
Assert.hasText(grievanceUserName, "grievanceUserName must not be empty");
Assert.hasText(grievancePassword, "grievancePassword must not be empty");
Assert.hasText(grievanceDataSyncDuration, "grievanceDataSyncDuration must not be empty");
}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (1)
42-44: Add class-level documentation.Add Javadoc to describe the class's purpose, responsibilities, and its role in the grievance synchronization process.
+/** + * Service implementation responsible for synchronizing grievance data with external systems. + * Handles authentication, data fetching, and persistence of grievance details and transactions. + */ @Service @PropertySource("classpath:application.properties") public class GrievanceDataSyncImpl implements GrievanceDataSync {
📜 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(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (5)
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java (5)
349-367: Well-implemented retry mechanism with proper thread handling.The retry logic includes appropriate delay between attempts and proper handling of thread interruption.
290-290:⚠️ Potential issueUse UriComponentsBuilder for secure URL construction.
Direct string concatenation for URL construction could be vulnerable to injection attacks.
- ResponseEntity<String> response = restTemplate.exchange(updateGrievanceTransactionDetails + grievanceId, HttpMethod.POST, request, String.class); + String url = UriComponentsBuilder.fromUriString(updateGrievanceTransactionDetails) + .queryParam("grievanceId", grievanceId) + .build() + .toUriString(); + ResponseEntity<String> response = restTemplate.exchange(url, HttpMethod.POST, request, String.class);Likely invalid or redundant comment.
273-275:⚠️ Potential issueImprove exception handling.
The catch block swallows exceptions after logging them, which might hide important errors.
} catch (Exception e) { logger.error("Error in saving data into t_grievanceworklist: ", e); + throw new GrievanceProcessingException("Failed to process grievance data", e); }Likely invalid or redundant comment.
70-90: 🛠️ Refactor suggestionAdd validation for required configuration properties.
Add a @PostConstruct method to validate all required properties during initialization to fail fast if any configuration is missing.
+import javax.annotation.PostConstruct; +import org.springframework.util.Assert; + +@PostConstruct +private void validateConfiguration() { + Assert.hasText(grievanceUserAuthenticate, "grievanceUserAuthenticate must not be empty"); + Assert.hasText(updateGrievanceDetails, "updateGrievanceDetails must not be empty"); + Assert.hasText(updateGrievanceTransactionDetails, "updateGrievanceTransactionDetails must not be empty"); + Assert.hasText(grievanceUserName, "grievanceUserName must not be empty"); + Assert.hasText(grievancePassword, "grievancePassword must not be empty"); + Assert.hasText(grievanceDataSyncDuration, "grievanceDataSyncDuration must not be empty"); +}Likely invalid or redundant comment.
171-183:⚠️ Potential issueAdd null checks for JSON fields.
Direct access to JSON fields without null checks could lead to NullPointerException.
- transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); + if (transactionDetailsJson.has("actionTakenBy")) { + transactionDetails.setActionTakenBy(transactionDetailsJson.get("actionTakenBy").getAsString()); + }Likely invalid or redundant comment.
src/main/java/com/iemr/common/service/grievance/GrievanceDataSyncImpl.java
Show resolved
Hide resolved
|
📋 Description
JIRA ID: AMM-1139
Scheduler for fetching L3 to L5 grievance details from GR portal and saving it into the DB alongwith transactions for each grievances.
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
Repositories
Services