AMM-1145 | API to fetch grievance outbound worklist #161
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
Grievances API changes (PSMRI#159)
latest pull
updating upload artifact version to v4
β¦rp78/Common-API.git into feature/version/upgrade
PULL adding newly added grievance related properties to properties file (#β¦
WalkthroughThis pull request introduces a new POST endpoint in the grievance controller to fetch a formatted grievance worklist. It adds new DTO classes, a request model, and a repository interface to execute a stored procedure. Additionally, the service interface and its implementation are updated to process and structure grievance data retrieved from the repository, with added error handling and logging. Changes
Possibly related PRs
Suggested reviewers
Poem
β¨ Finishing Touches
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: 2
π§Ή Nitpick comments (9)
src/main/java/com/iemr/common/data/grievance/GetGrievanceWorklistRequest.java (1)
7-8: Maintain consistent field naming conventions.The field names are inconsistent - one uses
IDsuffix while the other doesn't. Consider renaming for consistency:- private Integer providerServiceMapID; - private Integer userId; + private Integer providerServiceMapId; + private Integer userId;src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java (1)
22-22: Remove commented out code.Commented out code should be removed if it's not being used. If this is planned for future implementation, consider tracking it in the issue tracker instead.
-// public Map<String, Object> getGrievanceOutboundWorklist(String request) throws Exception;src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java (2)
13-14: Remove or implement commented fields.The commented fields suggest incomplete implementation. Either:
- Remove them if they're not needed, or
- Implement them if they're required for the feature
24-37: Clean up constructor implementation.The constructor contains commented parameters and unnecessary
super()call.- public GrievanceTransactionDTO( - //String actionTakenBy, String status, - String fileName, String fileType, - String redressed, Timestamp createdAt, Timestamp updatedAt, String comment) { - super(); - //this.actionTakenBy = actionTakenBy; - // this.status = status; - this.fileName = fileName; - this.fileType = fileType; - this.redressed = redressed; - this.createdAt = createdAt; - this.updatedAt = updatedAt; - this.comment = comment; - } + public GrievanceTransactionDTO( + String fileName, + String fileType, + String redressed, + Timestamp createdAt, + Timestamp updatedAt, + String comment + ) { + this.fileName = fileName; + this.fileType = fileType; + this.redressed = redressed; + this.createdAt = createdAt; + this.updatedAt = updatedAt; + this.comment = comment; + }src/main/java/com/iemr/common/dto/grivance/GrievanceWorklistDTO.java (2)
26-26: Fix typo in field name.The field name 'severety' is misspelled. It should be 'severity'.
- private String severety; + private String severity;
40-40: Remove commented code.The commented field 'lastCall' should either be implemented or removed to maintain clean code.
src/main/java/com/iemr/common/controller/grievance/GrievanceController.java (2)
131-131: Fix operation summary description.The operation summary is incomplete and contains a typo (extra parenthesis).
- @Operation(summary = "get grievance outbound worklist)") + @Operation(summary = "Get grievance outbound worklist")
142-152: Enhance error handling with specific exceptions.The catch block catches all exceptions generically. Consider catching specific exceptions (e.g.,
IllegalArgumentException,JsonProcessingException) to provide more targeted error handling.-catch (Exception e) { +catch (IllegalArgumentException e) { + logger.error("Invalid argument in grievanceOutboundWorklist: " + e.getMessage(), e); + List<GrievanceWorklistDTO> errorResponse = new ArrayList<>(); + GrievanceWorklistDTO errorDTO = new GrievanceWorklistDTO(); + errorDTO.setComplaint("Invalid request parameters"); + errorDTO.setSubjectOfComplaint(e.getMessage()); + errorResponse.add(errorDTO); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(errorResponse); +} catch (Exception e) { logger.error("grievanceOutboundWorklist failed with error " + e.getMessage(), e); List<GrievanceWorklistDTO> errorResponse = new ArrayList<>(); GrievanceWorklistDTO errorDTO = new GrievanceWorklistDTO(); errorDTO.setComplaint("Error fetching grievance data"); errorDTO.setSubjectOfComplaint(e.getMessage()); errorResponse.add(errorDTO); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(errorResponse); }src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
292-301: Clean up commented code and add validation.The transaction creation has commented fields and lacks validation.
GrievanceTransactionDTO transaction = new GrievanceTransactionDTO( - // (String) row[23], // actionTakenBy - // (String) row[24], // status + // Add validation for required fields + validateField(row[22], "fileName"), + validateField(row[6], "fileType"), // ... continue with other fields ); + private String validateField(Object value, String fieldName) { + if (value == null) { + logger.warn("Missing required field: {}", fieldName); + return ""; + } + return (String) value; + }
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (7)
src/main/java/com/iemr/common/controller/grievance/GrievanceController.java(2 hunks)src/main/java/com/iemr/common/data/grievance/GetGrievanceWorklistRequest.java(1 hunks)src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java(1 hunks)src/main/java/com/iemr/common/dto/grivance/GrievanceWorklistDTO.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java(2 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(3 hunks)
β° Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
π Additional comments (3)
src/main/java/com/iemr/common/data/grievance/GetGrievanceWorklistRequest.java (1)
11-14: LGTM! Constructor implementation is clean and follows best practices.The constructor properly initializes all fields and follows good practices.
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java (1)
24-24: LGTM! Method signature is clear and follows interface design principles.The new method signature is well-defined and follows the pattern of other methods in the interface.
src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java (1)
19-22: LGTM! Stored procedure implementation follows best practices.The stored procedure implementation is well-structured with:
- Proper transaction management
- Clear parameter naming
- Appropriate return type for procedure results
| import com.iemr.common.data.grievance.GrievanceWorklist; | ||
| import com.iemr.common.dto.grivance.GrievanceWorklistDTO; | ||
|
|
||
| import io.lettuce.core.dynamic.annotation.Param; |
There was a problem hiding this comment.
Fix incorrect @param import.
The code is using Lettuce's @param annotation instead of Spring's. This could cause issues with parameter binding.
-import io.lettuce.core.dynamic.annotation.Param;
+import org.springframework.data.repository.query.Param;π 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.
| import io.lettuce.core.dynamic.annotation.Param; | |
| import org.springframework.data.repository.query.Param; |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| for (Object[] row : worklistData) { | ||
| GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( | ||
| (String) row[0], // complaintID | ||
| (String) row[1], // subjectOfComplaint | ||
| (String) row[2], // complaint | ||
| (Long) row[3], // beneficiaryRegID | ||
| (Integer) row[4],// providerServiceMapID | ||
| (String) row[5], // primaryNumber | ||
|
|
||
| (String) row[19], // firstName | ||
| (String) row[20], // lastName | ||
|
|
||
| new ArrayList<>(),// transactions (initially empty, will be populated later) | ||
| (String) row[11], // severety | ||
| (String) row[12], // state | ||
| (Integer) row[13],// userId | ||
| (Boolean) row[14],// deleted | ||
| (String) row[15],// createdBy | ||
| (Timestamp) row[16], // createdDate | ||
| (Timestamp) row[17], // lastModDate | ||
| (Boolean) row[18], // isCompleted | ||
| (String) row[21], // gender | ||
| (String) row[22], // district | ||
| (Long) row[23], // beneficiaryID | ||
| (String) row[24], // age | ||
| (Boolean) row[25], // retryNeeded | ||
| (Integer) row[26] // callCounter | ||
| //(String) row[22] // lastCall | ||
| ); |
There was a problem hiding this comment.
π οΈ Refactor suggestion
Add null checks and use constants for array indices.
The direct array index access without null checks is error-prone. Consider:
- Adding null checks for each field
- Using constants for array indices
- Adding documentation for the array structure
+ // Constants for array indices
+ private static final int COMPLAINT_ID_INDEX = 0;
+ private static final int SUBJECT_INDEX = 1;
+ // ... add other indices
for (Object[] row : worklistData) {
+ // Add null checks
+ if (row == null || row.length < 27) {
+ logger.warn("Invalid row data received");
+ continue;
+ }
GrievanceWorklistDTO grievance = new GrievanceWorklistDTO(
- (String) row[0],
+ (String) row[COMPLAINT_ID_INDEX],
// ... continue with other fieldsπ 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.
| for (Object[] row : worklistData) { | |
| GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( | |
| (String) row[0], // complaintID | |
| (String) row[1], // subjectOfComplaint | |
| (String) row[2], // complaint | |
| (Long) row[3], // beneficiaryRegID | |
| (Integer) row[4],// providerServiceMapID | |
| (String) row[5], // primaryNumber | |
| (String) row[19], // firstName | |
| (String) row[20], // lastName | |
| new ArrayList<>(),// transactions (initially empty, will be populated later) | |
| (String) row[11], // severety | |
| (String) row[12], // state | |
| (Integer) row[13],// userId | |
| (Boolean) row[14],// deleted | |
| (String) row[15],// createdBy | |
| (Timestamp) row[16], // createdDate | |
| (Timestamp) row[17], // lastModDate | |
| (Boolean) row[18], // isCompleted | |
| (String) row[21], // gender | |
| (String) row[22], // district | |
| (Long) row[23], // beneficiaryID | |
| (String) row[24], // age | |
| (Boolean) row[25], // retryNeeded | |
| (Integer) row[26] // callCounter | |
| //(String) row[22] // lastCall | |
| ); | |
| // Constants for array indices | |
| private static final int COMPLAINT_ID_INDEX = 0; | |
| private static final int SUBJECT_OF_COMPLAINT_INDEX = 1; | |
| private static final int COMPLAINT_INDEX = 2; | |
| private static final int BENEFICIARY_REG_ID_INDEX = 3; | |
| private static final int PROVIDER_SERVICE_MAP_ID_INDEX = 4; | |
| private static final int PRIMARY_NUMBER_INDEX = 5; | |
| // ... add additional constant definitions for all needed indices ... | |
| private static final int SEVERETY_INDEX = 11; | |
| private static final int STATE_INDEX = 12; | |
| private static final int USER_ID_INDEX = 13; | |
| private static final int DELETED_INDEX = 14; | |
| private static final int CREATED_BY_INDEX = 15; | |
| private static final int CREATED_DATE_INDEX = 16; | |
| private static final int LAST_MOD_DATE_INDEX = 17; | |
| private static final int IS_COMPLETED_INDEX = 18; | |
| private static final int FIRST_NAME_INDEX = 19; | |
| private static final int LAST_NAME_INDEX = 20; | |
| private static final int GENDER_INDEX = 21; | |
| private static final int DISTRICT_INDEX = 22; | |
| private static final int BENEFICIARY_ID_INDEX = 23; | |
| private static final int AGE_INDEX = 24; | |
| private static final int RETRY_NEEDED_INDEX = 25; | |
| private static final int CALL_COUNTER_INDEX = 26; | |
| for (Object[] row : worklistData) { | |
| // Add null checks for row validity | |
| if (row == null || row.length < 27) { | |
| logger.warn("Invalid row data received"); | |
| continue; | |
| } | |
| GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( | |
| (String) row[COMPLAINT_ID_INDEX], // complaintID | |
| (String) row[SUBJECT_OF_COMPLAINT_INDEX], // subjectOfComplaint | |
| (String) row[COMPLAINT_INDEX], // complaint | |
| (Long) row[BENEFICIARY_REG_ID_INDEX], // beneficiaryRegID | |
| (Integer) row[PROVIDER_SERVICE_MAP_ID_INDEX], // providerServiceMapID | |
| (String) row[PRIMARY_NUMBER_INDEX], // primaryNumber | |
| (String) row[FIRST_NAME_INDEX], // firstName | |
| (String) row[LAST_NAME_INDEX], // lastName | |
| new ArrayList<>(), // transactions (initially empty, will be populated later) | |
| (String) row[SEVERETY_INDEX], // severety | |
| (String) row[STATE_INDEX], // state | |
| (Integer) row[USER_ID_INDEX], // userId | |
| (Boolean) row[DELETED_INDEX], // deleted | |
| (String) row[CREATED_BY_INDEX], // createdBy | |
| (Timestamp) row[CREATED_DATE_INDEX], // createdDate | |
| (Timestamp) row[LAST_MOD_DATE_INDEX], // lastModDate | |
| (Boolean) row[IS_COMPLETED_INDEX], // isCompleted | |
| (String) row[GENDER_INDEX], // gender | |
| (String) row[DISTRICT_INDEX], // district | |
| (Long) row[BENEFICIARY_ID_INDEX], // beneficiaryID | |
| (String) row[AGE_INDEX], // age | |
| (Boolean) row[RETRY_NEEDED_INDEX], // retryNeeded | |
| (Integer) row[CALL_COUNTER_INDEX] // callCounter | |
| //(String) row[22] // lastCall | |
| ); | |
| // ... rest of the processing ... | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Great! Thank you for implementing the suggested improvements. This will make the code more maintainable and robust.
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: 4
β»οΈ Duplicate comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
262-288: π οΈ Refactor suggestionUse constants for array indices to improve maintainability.
The hardcoded array indices make the code error-prone and difficult to maintain.
+ // Constants for array indices + private static final class GrievanceIndices { + static final int COMPLAINT_ID = 0; + static final int SUBJECT = 1; + static final int COMPLAINT = 2; + static final int BENEFICIARY_REG_ID = 3; + // ... add other indices + } GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( - (String) row[0], // complaintID - (String) row[1], // subjectOfComplaint + (String) row[GrievanceIndices.COMPLAINT_ID], + (String) row[GrievanceIndices.SUBJECT], // ... continue with other fields
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java(1 hunks)src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java(1 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java(2 hunks)src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(3 hunks)
π§ Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/iemr/common/repository/grievance/GrievanceOutboundRepository.java
- src/main/java/com/iemr/common/service/grievance/GrievanceHandlingService.java
- src/main/java/com/iemr/common/dto/grivance/GrievanceTransactionDTO.java
β° Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
π Additional comments (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
3-45: LGTM! Clean dependency injection setup.The imports are well-organized, and the constructor properly injects the new repository dependency.
| // Fetch grievance worklist data using @Procedure annotation | ||
| List<Object[]> worklistData = grievanceOutboundRepo.getGrievanceWorklistData(getGrievanceWorklistRequest.getProviderServiceMapID(), getGrievanceWorklistRequest.getUserId()); | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
Add error handling for database operation.
The stored procedure call should be wrapped in a try-catch block to handle potential database errors.
// Fetch grievance worklist data using @Procedure annotation
+ List<Object[]> worklistData;
+ try {
worklistData = grievanceOutboundRepo.getGrievanceWorklistData(
getGrievanceWorklistRequest.getProviderServiceMapID(),
getGrievanceWorklistRequest.getUserId()
);
+ if (worklistData == null || worklistData.isEmpty()) {
+ logger.info("No grievance data found for the given criteria");
+ return new ArrayList<>();
+ }
+ } catch (Exception e) {
+ logger.error("Failed to fetch grievance data: {}", e.getMessage());
+ throw new Exception("Failed to retrieve 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.
| // Fetch grievance worklist data using @Procedure annotation | |
| List<Object[]> worklistData = grievanceOutboundRepo.getGrievanceWorklistData(getGrievanceWorklistRequest.getProviderServiceMapID(), getGrievanceWorklistRequest.getUserId()); | |
| // Fetch grievance worklist data using @Procedure annotation | |
| List<Object[]> worklistData; | |
| try { | |
| worklistData = grievanceOutboundRepo.getGrievanceWorklistData( | |
| getGrievanceWorklistRequest.getProviderServiceMapID(), | |
| getGrievanceWorklistRequest.getUserId() | |
| ); | |
| if (worklistData == null || worklistData.isEmpty()) { | |
| logger.info("No grievance data found for the given criteria"); | |
| return new ArrayList<>(); | |
| } | |
| } catch (Exception e) { | |
| logger.error("Failed to fetch grievance data: {}", e.getMessage()); | |
| throw new Exception("Failed to retrieve grievance data", e); | |
| } |
There was a problem hiding this comment.
@srishtigrp78 Thank you for implementing the error handling! This will make the code more robust and maintainable.
(ο½βΒ΄)γ"
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| GrievanceTransactionDTO transaction = new GrievanceTransactionDTO( | ||
|
|
||
| (String) row[22], // fileName | ||
| (String) row[6], // fileType | ||
| (String) row[7], // redressed | ||
| (Timestamp) row[8], // createdAt | ||
| (Timestamp) row[9], // updatedAt | ||
| (String) row[10] // comment | ||
| ); | ||
|
|
||
| grievance.getTransactions().add(transaction); // Add the transaction to the grievance's list |
There was a problem hiding this comment.
Fix incorrect field mapping in transaction DTO.
The district field is incorrectly mapped to fileName in the transaction DTO.
GrievanceTransactionDTO transaction = new GrievanceTransactionDTO(
- (String) row[22], // fileName
+ (String) row[6], // fileName
(String) row[6], // fileType
(String) row[7], // redressed
(Timestamp) row[8], // createdAt
(Timestamp) row[9], // updatedAt
(String) row[10] // comment
);Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@srishtigrp78 Thanks for fixing the field mapping!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
π§Ή Nitpick comments (2)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
274-307: Consider extracting mapping logic to a separate method.The mapping of array values to DTO fields is quite long and could be moved to a dedicated method for better readability and maintainability.
+ private GrievanceWorklistDTO mapToGrievanceDTO(Object[] row) { + return new GrievanceWorklistDTO( + (String) row[0], + (String) row[1], + // ... rest of the mapping + ); + } for (Object[] row : worklistData) { if (row == null || row.length < 27) { logger.warn("invalid row data received"); continue; } - GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( - // ... current mapping - ); + GrievanceWorklistDTO grievance = mapToGrievanceDTO(row);
309-321: Consider extracting transaction mapping to a separate method.Similar to the grievance mapping, the transaction mapping could be moved to a dedicated method.
+ private GrievanceTransactionDTO mapToTransactionDTO(Object[] row) { + return new GrievanceTransactionDTO( + (String) row[6], + (String) row[7], + // ... rest of the mapping + ); + } - GrievanceTransactionDTO transaction = new GrievanceTransactionDTO( - // ... current mapping - ); + GrievanceTransactionDTO transaction = mapToTransactionDTO(row);
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(3 hunks)
π§° Additional context used
π§ Learnings (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#161
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:256-261
Timestamp: 2025-02-21T05:55:17.989Z
Learning: Type safety checks for array values from stored procedure 'Pr_Grievanceworklist' are not required as the data types are guaranteed by the procedure contract.
β° Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
π Additional comments (3)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (3)
3-3: LGTM! Clean import organization and dependency injection.The imports are well-organized, and the constructor-based dependency injection follows best practices.
Also applies to: 19-25, 27-27, 31-31, 39-39, 42-44
246-252: LGTM! Robust input validation.The method includes thorough input validation for the request string and required fields.
256-272: LGTM! Comprehensive error handling.The implementation includes proper error handling with:
- Null checks for required fields
- Try-catch block for database operations
- Appropriate logging
- Empty result handling
|
|
||
| (String) row[6], // fileName | ||
| (String) row[6], // fileType | ||
| (String) row[7], // redressed |
There was a problem hiding this comment.
Fix duplicate file value assignment in transaction DTO.
The same value row[6] is used for both fileName and fileType.
- (String) row[6], // fileName
- (String) row[6], // fileType
+ (String) row[6], // fileName
+ (String) row[7], // fileTypeCommittable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
added one more field file name so had to fix row no
There was a problem hiding this comment.
@srishtigrp78 Thanks for fixing the row numbers! The implementation now correctly maps the fields.
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: 0
π§Ή Nitpick comments (2)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (2)
275-280: Consider using a constant for the expected row length.The magic number
28in the row length validation could be extracted to a constant for better maintainability.+ private static final int EXPECTED_ROW_LENGTH = 28; for (Object[] row : worklistData) { - if (row == null || row.length < 28) + if (row == null || row.length < EXPECTED_ROW_LENGTH)
281-307: Consider using a builder pattern for complex object construction.The
GrievanceWorklistDTOconstructor has many parameters which makes it hard to maintain. Consider using a builder pattern to improve readability and maintainability.- GrievanceWorklistDTO grievance = new GrievanceWorklistDTO( - (String) row[0], // complaintID - // ... other parameters - (Integer) row[27] // callCounter - ); + GrievanceWorklistDTO grievance = GrievanceWorklistDTO.builder() + .complaintID((String) row[0]) + // ... other parameters + .callCounter((Integer) row[27]) + .build();
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java(3 hunks)
π§° Additional context used
π§ Learnings (1)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (1)
Learnt from: srishtigrp78
PR: PSMRI/Common-API#161
File: src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java:256-261
Timestamp: 2025-02-21T05:55:17.989Z
Learning: Type safety checks for array values from stored procedure 'Pr_Grievanceworklist' are not required as the data types are guaranteed by the procedure contract.
β° Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
π Additional comments (4)
src/main/java/com/iemr/common/service/grievance/GrievanceHandlingServiceImpl.java (4)
38-45: LGTM! Constructor dependency injection is properly implemented.The new repository dependency is correctly injected through constructor injection, following best practices for dependency management.
246-272: LGTM! Input validation and error handling are well implemented.The method includes:
- Null/empty request validation
- Required field validation
- Database error handling with proper logging
310-318: LGTM! Transaction mapping is correctly implemented.The transaction data is properly mapped from the result set to the DTO.
320-324: Consider validating transaction data before adding to list.While the transaction is added to the list without validation, it might be worth checking if any essential transaction fields are null before adding it.
+ if (transaction.getCreatedAt() != null) { // or any other essential field grievance.getTransactions().add(transaction); + } else { + logger.warn("Skipping transaction with null created date for complaint: {}", grievance.getComplaintID()); + }
AMM-1145 | API to fetch grievance outbound worklist (PSMRI#161)
π Description
JIRA ID: AMM-1145
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
API to fetch grievance outbound worklist
β 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
Bug Fixes