Open
Conversation
Test/incentive leprosy
…d_PB Incentive for Completing treatment of PB (Paucibacillary) leprosy pat…
Fix/incentive upload file
incentive upload files
incentive upload files
add record id
add record id
fix method name
…entive make changes in response
…incentive_changes_3.8.0 # Conflicts: # src/main/java/com/iemr/flw/service/MaaMeetingService.java # src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request updates the codebase to version 3.8.0, renames the M_User domain entity to User, refactors repository queries to remove date-range filtering, expands DTOs with new fields, and significantly enhances the incentive processing system with new service implementations and multi-branched update logic. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/iemr/flw/service/impl/SammelanServiceImpl.java (1)
39-39:⚠️ Potential issue | 🟠 MajorThread safety issue: instance field
recordshared across requests.The
SammelanRecord recordinstance field can be overwritten by concurrent requests in a Spring singleton service, leading to race conditions. Use a local variable instead.🔧 Proposed fix
- private SammelanRecord record;Then in
submitSammelan, the variable is already declared locally on line 65, so just remove the instance field declaration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/SammelanServiceImpl.java` at line 39, The instance field `record` on SammelanServiceImpl introduces a thread-safety bug because the singleton service can be shared across requests; remove the class-level declaration `private SammelanRecord record;` and rely on the existing local `record` variable already declared inside the `submitSammelan` method so each request uses its own stack-local SammelanRecord instead of a shared field.src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java (1)
36-36:⚠️ Potential issue | 🟡 MinorFix incorrect logger class reference.
The logger is initialized with
ChildCareServiceImpl.classinstead of the current class. This will cause misleading log output.🐛 Proposed fix
- private final Logger logger = LoggerFactory.getLogger(ChildCareServiceImpl.class); + private final Logger logger = LoggerFactory.getLogger(IncentiveServiceImpl.class);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java` at line 36, The logger in IncentiveServiceImpl is created with the wrong class reference; update the Logger initialization (variable logger using LoggerFactory.getLogger(...)) to use IncentiveServiceImpl.class instead of ChildCareServiceImpl.class so log entries correctly identify the current class (adjust the declaration in IncentiveServiceImpl to call LoggerFactory.getLogger(IncentiveServiceImpl.class)).
🟡 Minor comments (3)
src/main/java/com/iemr/flw/service/impl/DeliveryOutcomeServiceImpl.java-111-112 (1)
111-112:⚠️ Potential issue | 🟡 MinorMissing null check:
usermay be null if userId not found.If
getUserNamedByUserIdreturnsnullfor an unknown userId, the query will proceed with a null parameter, potentially returning unexpected results or empty data without proper feedback.🛡️ Proposed null check
String user = userRepo.getUserNamedByUserId(dto.getAshaId()); + if (user == null) { + logger.warn("No user found for ashaId: {}", dto.getAshaId()); + return List.of(); + } List<DeliveryOutcome> deliveryOutcomeList = deliveryOutcomeRepo.getDeliveryOutcomeByAshaId(user,dto.getFromDate(),dto.getToDate());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/DeliveryOutcomeServiceImpl.java` around lines 111 - 112, In DeliveryOutcomeServiceImpl, userRepo.getUserNamedByUserId(dto.getAshaId()) can return null; add a null-check after calling userRepo.getUserNamedByUserId (referencing getUserNamedByUserId and dto.getAshaId) and handle it before calling deliveryOutcomeRepo.getDeliveryOutcomeByAshaId(user,...). If user is null, either log and throw a clear exception (e.g., IllegalArgumentException or a custom NotFound exception) or return an empty List<DeliveryOutcome> so the downstream query is not invoked with a null parameter; ensure the chosen behavior is consistent with the service's error handling and update the method return accordingly.src/main/java/com/iemr/flw/dto/iemr/DeliveryOutcomeDTO.java-13-14 (1)
13-14:⚠️ Potential issue | 🟡 MinorDate format inconsistent with project standard.
The pattern
"MMM d yyyy h:mm:ss a"differs from the project's standardized format"MMM dd, yyyy h:mm:ss a":
- Single
dvsdd(missing leading zero for single-digit days)- Missing comma after the year
This inconsistency applies to all four
@JsonFormatannotations in this file (lines 13, 26, 31, 34).🔧 Proposed fix
- `@JsonFormat`(pattern = "MMM d yyyy h:mm:ss a") + `@JsonFormat`(pattern = "MMM dd, yyyy h:mm:ss a") private Timestamp dateOfDelivery;Apply the same fix to
dateOfDischarge,createdDate, andupdatedDate.Based on learnings: "The standardized date format across the FLW-API codebase is 'MMM dd, yyyy h:mm:ss a'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/dto/iemr/DeliveryOutcomeDTO.java` around lines 13 - 14, Update the `@JsonFormat` annotations in DeliveryOutcomeDTO to use the project-standard pattern "MMM dd, yyyy h:mm:ss a" so dates include leading zeroes and the comma; specifically change the pattern on the fields dateOfDelivery, dateOfDischarge, createdDate, and updatedDate in class DeliveryOutcomeDTO to "MMM dd, yyyy h:mm:ss a".src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java-230-256 (1)
230-256:⚠️ Potential issue | 🟡 MinorInconsistent return values make error handling difficult.
The method returns different values for different failure scenarios:
nullfor missing activity/pending activity (Lines 231, 243, 250, 255)"Invalid request"/"Invalid incentive name"for input validation (Lines 209, 217)e.getMessage()for exceptions (Line 384)"Incentive update successfully"for successConsider using a consistent response pattern or throwing exceptions for error cases to help callers distinguish between different failure modes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java` around lines 230 - 256, The method in IncentiveServiceImpl uses inconsistent return values (null, strings like "Invalid request"/"Invalid incentive name", exception messages, and success text) which complicates error handling; replace the multiple null/string returns in the block that checks incentiveActivity, optionalPendingActivity (incentivePendingActivityRepository.findByMincentiveIdAndActivityId), existingActivity, and pendingActivityDTO.getImages() with a consistent error handling strategy—either throw well-typed exceptions (e.g., InvalidRequestException / NotFoundException) at each failure point or return a standardized response DTO (e.g., OperationResult with status, message, and optional data); update the method's signature accordingly and ensure callers of this method handle the new exception types or the standardized response instead of relying on null/string values.
🧹 Nitpick comments (23)
src/main/java/com/iemr/flw/repo/iemr/PNCVisitRepo.java (1)
9-9: Remove unused import.The
java.sql.Timestampimport is no longer used after removing the date parameters fromgetPNCForPW.🧹 Proposed fix
-import java.sql.Timestamp;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/repo/iemr/PNCVisitRepo.java` at line 9, Remove the unused import of java.sql.Timestamp from the PNCVisitRepo file: open the class PNCVisitRepo and delete the import line "import java.sql.Timestamp;" (this import is no longer needed after removing date parameters from getPNCForPW), then recompile to ensure no other references to Timestamp remain.src/main/java/com/iemr/flw/service/impl/NotificationSchedulerService.java (2)
30-36: Consider removing stale commented-out code.This commented block references the old
M_Usertype which has been renamed. If this code is no longer needed, consider removing it entirely to improve maintainability. If it may be needed later, update it to useUserto avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/NotificationSchedulerService.java` around lines 30 - 36, The commented-out block in NotificationSchedulerService references the outdated M_User type and legacy request handling (lines with M_User, cookieUtil, employeeMasterInter, maternalHealthService.sendAncDueTomorrowNotifications); remove this stale commented code entirely if it's no longer needed, or if you intend to keep it, update references to the current User type and modern request retrieval so the comment reflects valid, compileable usage (e.g., replace M_User with User and ensure cookieUtil / employeeMasterInter usage matches current APIs); commit the cleaned-up NotificationSchedulerService to avoid confusion.
58-61: Variable naming could be more consistent; however, the code is functionally correct.The variable
m_userfollows the old naming convention from the previousM_Usertype. Since the type is nowUser, renaming touserwould improve consistency throughout the codebase. ThegetUserID()method is properly generated by Lombok's@Dataannotation on theUserclass and works correctly.♻️ Optional naming cleanup
- for(User m_user: employeeMasterInter.getAllUsers()){ - childCareService.getTomorrowImmunizationReminders(m_user.getUserID()); + for(User user : employeeMasterInter.getAllUsers()){ + childCareService.getTomorrowImmunizationReminders(user.getUserID()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/NotificationSchedulerService.java` around lines 58 - 61, The loop variable uses an outdated name `m_user` even though the type is now `User`; rename `m_user` to `user` in the loop that iterates `employeeMasterInter.getAllUsers()` and update its usages (e.g., the call to `childCareService.getTomorrowImmunizationReminders(m_user.getUserID())`) to use `user.getUserID()` so naming is consistent with the `User` type and code style.src/main/java/com/iemr/flw/domain/iemr/ScreeningKalaAzar.java (1)
106-107: Inconsistent column naming convention.The column
beneficiary_statusIduses camelCase while the rest of the file (including the newly updated columns) follows snake_case convention. Consider renaming this tobeneficiary_status_idfor consistency across the entity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/domain/iemr/ScreeningKalaAzar.java` around lines 106 - 107, The `@Column` annotation on the ScreeningKalaAzar entity field beneficiaryStatusId uses inconsistent camelCase "beneficiary_statusId"; update the `@Column`(name = "...") value to the snake_case "beneficiary_status_id" to match the file's convention and ensure ORM mapping consistency for the ScreeningKalaAzar class and any queries or DTOs that reference beneficiaryStatusId.src/main/java/com/iemr/flw/dto/iemr/CdrDTO.java (1)
3-3: Unused import:jakarta.persistence.Columnis not used in this DTO.This import appears to be unnecessary since no
@Columnannotations are present in this class.🧹 Proposed cleanup
package com.iemr.flw.dto.iemr; -import jakarta.persistence.Column; import lombok.Data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/dto/iemr/CdrDTO.java` at line 3, The import jakarta.persistence.Column in CdrDTO is unused and should be removed; open the CdrDTO class and delete the unused import statement (the one importing jakarta.persistence.Column) so the file no longer contains an unnecessary import and compiles/linters no longer flag it.src/main/java/com/iemr/flw/repo/iemr/IncentiveRecordRepo.java (2)
27-38: Consider renaming for clarity: this method uses date range, not exact date match.The method name
findRecordByActivityIdCreatedDateBenIdis overloaded three times with different semantics. This version usesBETWEENfor a date range, while others use exact match. Consider a distinct name likefindRecordByActivityIdCreatedDateRangeBenIdto avoid confusion.♻️ Proposed rename for clarity
- IncentiveActivityRecord findRecordByActivityIdCreatedDateBenId( + IncentiveActivityRecord findRecordByActivityIdCreatedDateRangeBenId( `@Param`("id") Long id, `@Param`("startDate") Timestamp startDate, `@Param`("endDate") Timestamp endDate, `@Param`("benId") Long benId, `@Param`("ashaId") Integer ashaId );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/repo/iemr/IncentiveRecordRepo.java` around lines 27 - 38, The repository method name findRecordByActivityIdCreatedDateBenId is ambiguous because it uses a BETWEEN date range; rename it to findRecordByActivityIdCreatedDateRangeBenId (or similar) in the IncentiveRecordRepo interface, update the method declaration signature there and any callers/tests that reference the old name, and run a compile/test to ensure all references are updated and Spring Data still picks up the `@Query` on the renamed method.
20-21: Redundant method:findById(Long id)is already provided byJpaRepository.
JpaRepository<IncentiveActivityRecord, Long>already inheritsfindById(Long id)fromCrudRepository. This explicit declaration is unnecessary unless you intend to override behavior.🧹 Proposed cleanup
- Optional<IncentiveActivityRecord> findById(Long id); -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/repo/iemr/IncentiveRecordRepo.java` around lines 20 - 21, The explicit declaration of findById(Long id) in the IncentiveRecordRepo interface is redundant because JpaRepository<IncentiveActivityRecord, Long> already provides it; remove the findById(Long id) method declaration from the IncentiveRecordRepo interface (or, if you intended custom behavior, implement a custom repository method instead and do not shadow the inherited findById). Ensure the interface relies on the inherited JpaRepository methods and delete the Optional<IncentiveActivityRecord> findById(Long id); line to clean up the repo.src/main/java/com/iemr/flw/service/impl/DeliveryOutcomeServiceImpl.java (1)
118-118: Use parameterized logging instead of string concatenation.String concatenation in logger calls is evaluated even when the log level is disabled, causing unnecessary overhead.
♻️ Proposed fix
- logger.error("DeliveryOutcome Exception:"+e.getMessage()); + logger.error("DeliveryOutcome Exception: {}", e.getMessage(), e);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/DeliveryOutcomeServiceImpl.java` at line 118, The logger call in DeliveryOutcomeServiceImpl currently concatenates the exception message (logger.error("DeliveryOutcome Exception:"+e.getMessage())), which forces string allocation even when logging is disabled; change it to use parameterized logging by supplying a message template and passing e (and/or e.getMessage()) as parameters so the logging framework can avoid building the string when the level is disabled and still capture the Throwable for the stack trace; update the invocation that uses the logger variable in the DeliveryOutcomeServiceImpl class where the local exception variable e is available.src/main/java/com/iemr/flw/service/impl/UpdateIncentivePendindDocService.java (3)
42-44: Redundant null check on newly instantiated object.The
incentivePendingActivityobject is instantiated on line 37 and can never be null at line 42. This check is dead code.♻️ Proposed fix
public void updatePendingActivity(Integer userId, Long recordId, Long activityId, Long mIncentiveId) { IncentivePendingActivity incentivePendingActivity = new IncentivePendingActivity(); incentivePendingActivity.setActivityId(activityId); incentivePendingActivity.setRecordId(recordId); incentivePendingActivity.setUserId(userId); incentivePendingActivity.setMincentiveId(mIncentiveId); - if (incentivePendingActivity != null) { - incentivePendingActivityRepository.save(incentivePendingActivity); - } - + incentivePendingActivityRepository.save(incentivePendingActivity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/UpdateIncentivePendindDocService.java` around lines 42 - 44, The null check around incentivePendingActivity is redundant because the object is instantiated earlier in the method; remove the if (incentivePendingActivity != null) { ... } wrapper and directly call incentivePendingActivityRepository.save(incentivePendingActivity) (in UpdateIncentivePendindDocService) to simplify the code and eliminate dead code.
12-12: Typo in class name: "Pendind" should be "Pending".The class name
UpdateIncentivePendindDocServicecontains a typo. Consider renaming toUpdateIncentivePendingDocServicefor clarity and maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/UpdateIncentivePendindDocService.java` at line 12, Rename the class UpdateIncentivePendindDocService to UpdateIncentivePendingDocService everywhere: update the class declaration, the file name, any constructors, logger/static references, Spring component/bean names or annotations referencing the old class, and all imports/usages (including tests) to the new identifier so compilation and dependency injection continue to work.
13-17: Unused autowired dependencies.
incentivesRepoanduserRepoare autowired but never used in this class. Remove them to reduce unnecessary coupling and improve clarity.♻️ Proposed fix
`@Service` public class UpdateIncentivePendindDocService { - `@Autowired` - private IncentivesRepo incentivesRepo; - - `@Autowired` - private UserServiceRoleRepo userRepo; - `@Autowired` private IncentiveRecordRepo recordRepo; `@Autowired` private IncentivePendingActivityRepository incentivePendingActivityRepository;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/UpdateIncentivePendindDocService.java` around lines 13 - 17, Remove the unused `@Autowired` fields in UpdateIncentivePendindDocService: delete the private IncentivesRepo incentivesRepo and private UserServiceRoleRepo userRepo declarations (and their `@Autowired` annotations) since they are never referenced; after removing them, also remove any now-unused import statements for IncentivesRepo and UserServiceRoleRepo to keep the class clean.src/main/java/com/iemr/flw/domain/iemr/User.java (1)
138-146: Remove stale TODO comments in constructors.The TODO comments "Auto-generated constructor stub" are IDE-generated placeholders that should be removed for cleaner code.
♻️ Proposed fix
public User() { - // TODO Auto-generated constructor stub } public User(Integer userID, String userName) { - // TODO Auto-generated constructor stub this.userID = userID; this.userName = userName; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/domain/iemr/User.java` around lines 138 - 146, Remove the stale "TODO Auto-generated constructor stub" comments from the two User constructors; specifically edit the no-arg constructor public User() and the two-arg constructor public User(Integer userID, String userName) to delete those placeholder comments so the constructors contain only their intended initialization (or remain empty) without IDE TODO clutter.src/main/java/com/iemr/flw/controller/IncentiveController.java (1)
119-124: Inconsistent error codes within the same controller.Error codes changed from 5000 to 500 for this endpoint, but other endpoints in this controller (lines 43-48, 70-75, 96-101) still use 5000. Consider aligning error codes for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/controller/IncentiveController.java` around lines 119 - 124, In IncentiveController, several response.setError calls in the shown method use HTTP code 500 while other endpoints in the same controller use 5000; update the response.setError invocations in this method (the branches that currently call response.setError(500, "...") and the catch block response.setError(500, "...")) to use 5000 so the error codes match the rest of the controller; keep the existing logger.error(...) unchanged except ensure any message text remains the same while the response.setError numeric argument is changed to 5000.src/main/java/com/iemr/flw/service/impl/SammelanServiceImpl.java (1)
151-152: Logger does not capture exception stack trace.Using
logger.info("...: ", e)with a trailing comma but no placeholder{}will not log the exception properly. Uselogger.errorwith proper formatting for exceptions.♻️ Proposed fix
} catch (Exception e) { - logger.info("SammelanRecord save Record: ", e); + logger.error("SammelanRecord save Record: {}", e.getMessage(), e); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/SammelanServiceImpl.java` around lines 151 - 152, In SammelanServiceImpl's catch block where you currently call logger.info("SammelanRecord save Record: ", e), replace that call to use logger.error and pass the exception as the throwable parameter so the full stack trace is emitted (e.g., change the logging invocation in the catch block that handles the save flow to use logger.error("SammelanRecord save Record", e)). Ensure you update the logging level from info to error and pass the exception object as the second argument so the logger prints the stack trace.src/main/java/com/iemr/flw/dto/iemr/TBScreeningDTO.java (1)
40-40: Consider correctingsympotomaticbefore this DTO contract solidifies.If clients can be migrated, prefer
symptomatic; otherwise keep backward compatibility with an explicit JSON alias/property mapping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/dto/iemr/TBScreeningDTO.java` at line 40, The DTO field sympotomatic in TBScreeningDTO is misspelled; either rename it to symptomatic (update the private field, its getter/setter, and all usages) if clients can migrate, or preserve backward compatibility by keeping the corrected field name but annotating it with Jackson JSON mapping (e.g., `@JsonProperty` or `@JsonAlias` referencing "sympotomatic") so incoming/outgoing JSON still supports the old name; update tests and any serialization/deserialization sites that reference sympotomatic accordingly.src/main/java/com/iemr/flw/service/UwinSessionService.java (1)
11-11: Keep service contract DTO-oriented for boundary consistency.Returning
UwinSessionhere leaks persistence shape while the rest of the interface is DTO-based. Prefer an update response DTO.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/UwinSessionService.java` at line 11, The service method updateSession in UwinSessionService currently returns the persistence entity UwinSession which leaks internal model; change its signature to return a DTO (e.g., UwinSessionResponseDTO or UpdateUwinSessionResponseDTO) and update all implementations/tests to map the persisted UwinSession to that DTO before returning; specifically modify the interface method declaration updateSession(UwinSessionRequestDTO req, Long recordId, Long activityId) to return the chosen DTO type, update import and mapper usage in classes implementing UwinSessionService, and adjust callers to consume the DTO instead of the entity.src/main/java/com/iemr/flw/service/IncentiveLogicService.java (1)
4-4: Remove unused@Serviceimport.This is an interface, so the
@Serviceannotation import is unused. The annotation belongs on the implementation class (IncentiveLogicImpl), not the interface.🔧 Proposed fix
package com.iemr.flw.service; import com.iemr.flw.domain.iemr.IncentiveActivityRecord; -import org.springframework.stereotype.Service; import java.util.Date;Based on learnings: "In the PSMRI/FLW-API project, the Service annotation should be placed on implementation classes rather than interfaces."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/IncentiveLogicService.java` at line 4, Remove the unused import of org.springframework.stereotype.Service from the IncentiveLogicService interface; the `@Service` annotation belongs on the implementation class, so ensure IncentiveLogicImpl is annotated with `@Service` (or already has it) and then delete the import line from IncentiveLogicService to avoid unused imports and clarify that the interface has no Spring stereotype.src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java (2)
63-70: Consider autowiring interfaces instead of implementations.Autowiring concrete implementations (
ChildCareServiceImpl,DeathReportsServiceImpl) tightly couples this class. If corresponding interfaces exist (e.g.,ChildCareService,DeathReportsService), prefer injecting those for better testability and flexibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java` around lines 63 - 70, In IncentiveServiceImpl the fields childCareService and deathReportsService are autowired to concrete classes (ChildCareServiceImpl, DeathReportsServiceImpl); change those field types to their corresponding interfaces (e.g., ChildCareService and DeathReportsService), update the `@Autowired` declarations to use the interface types, adjust imports, and ensure the Spring beans implement those interfaces (so injection still works and tests can mock the interfaces).
326-328: Redundant null checks.
pendingActivityDTOis already checked at Line 208, andincentiveNameis assigned at Line 215 (would have returned if invalid). The null checks here are redundant.♻️ Simplify condition
- if (pendingActivityDTO != null - && incentiveName != null - && IncentiveName.CHILD_DEATH_REPORTING.name().equals(incentiveName.name())) { + if (IncentiveName.CHILD_DEATH_REPORTING.name().equals(incentiveName.name())) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.java` around lines 326 - 328, The if condition in IncentiveServiceImpl redundantly checks pendingActivityDTO and incentiveName even though pendingActivityDTO is validated earlier and incentiveName is assigned/validated; simplify the condition by removing those null checks and directly check the enum value (e.g., replace "if (pendingActivityDTO != null && incentiveName != null && IncentiveName.CHILD_DEATH_REPORTING.name().equals(incentiveName.name()))" with a direct enum comparison such as "if (incentiveName == IncentiveName.CHILD_DEATH_REPORTING)" to both simplify and avoid unnecessary name() calls).src/main/java/com/iemr/flw/service/impl/ChildCareServiceImpl.java (1)
939-948: Remove tautological null check.The
if (incentivePendingActivity != null)check at Line 945 is always true because the object was just created at Line 940.♻️ Proposed fix
private void updatePendingActivity(Integer userId, Long recordId, Long activityId, Long mIncentiveId) { IncentivePendingActivity incentivePendingActivity = new IncentivePendingActivity(); incentivePendingActivity.setActivityId(activityId); incentivePendingActivity.setRecordId(recordId); incentivePendingActivity.setUserId(userId); incentivePendingActivity.setMincentiveId(mIncentiveId); - if (incentivePendingActivity != null) { - incentivePendingActivityRepository.save(incentivePendingActivity); - } + incentivePendingActivityRepository.save(incentivePendingActivity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/ChildCareServiceImpl.java` around lines 939 - 948, The null check in updatePendingActivity is tautological because IncentivePendingActivity incentivePendingActivity is instantiated just above; remove the if (incentivePendingActivity != null) guard and always call incentivePendingActivityRepository.save(incentivePendingActivity) after setting activityId, recordId, userId and mincentiveId to simplify the method and avoid dead code.src/main/java/com/iemr/flw/service/MaaMeetingService.java (2)
197-207: Remove tautological null check.The
if (incentivePendingActivity != null)check at Line 203 is always true because the object was just created at Line 198.♻️ Proposed fix
private void updatePendingActivity(Integer userId, Long recordId, Long activityId, Long mIncentiveId) { IncentivePendingActivity incentivePendingActivity = new IncentivePendingActivity(); incentivePendingActivity.setActivityId(activityId); incentivePendingActivity.setRecordId(recordId); incentivePendingActivity.setUserId(userId); incentivePendingActivity.setMincentiveId(mIncentiveId); - if (incentivePendingActivity != null) { - incentivePendingActivityRepository.save(incentivePendingActivity); - } + incentivePendingActivityRepository.save(incentivePendingActivity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/MaaMeetingService.java` around lines 197 - 207, The null check in updatePendingActivity is tautological because a new IncentivePendingActivity is instantiated just before it; remove the if (incentivePendingActivity != null) block and call incentivePendingActivityRepository.save(incentivePendingActivity) directly after setting activityId, recordId, userId, and mincentiveId to simplify the method and avoid dead code.
114-149: Consider extracting common update logic to reduce duplication.
updateMeetingFromFileUploadduplicates most of the field-update logic fromupdateMeeting(Lines 84-106). Consider extracting the common logic to a private helper method.♻️ Example refactor
private void applyUpdatesFromRequest(MaaMeeting meeting, MaaMeetingRequestDTO req) throws JsonProcessingException { if (req.getMeetingDate() != null) meeting.setMeetingDate(req.getMeetingDate()); if (req.getPlace() != null) meeting.setPlace(req.getPlace()); if (req.getParticipants() != null) meeting.setParticipants(req.getParticipants()); if (req.getAshaId() != null) meeting.setAshaId(req.getAshaId()); if (req.getCreatedBY() != null) meeting.setCreatedBy(req.getCreatedBY()); if (req.getMeetingImages() != null && req.getMeetingImages().length > 0) { List<String> base64Images = Arrays.stream(req.getMeetingImages()) .filter(file -> file != null && !file.isEmpty()) .map(this::convertToBase64) .collect(Collectors.toList()); meeting.setMeetingImagesJson(objectMapper.writeValueAsString(base64Images)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/MaaMeetingService.java` around lines 114 - 149, The updateMeetingFromFileUpload method duplicates field-update logic from updateMeeting—extract the common updates into a private helper (e.g., private void applyUpdatesFromRequest(MaaMeeting meeting, MaaMeetingRequestDTO req) throws JsonProcessingException) that sets meetingDate, place, participants, ashaId, createdBy and handles meetingImages -> meeting.setMeetingImagesJson(objectMapper.writeValueAsString(...)) using convertToBase64; update both updateMeeting and updateMeetingFromFileUpload to call applyUpdatesFromRequest, preserve the JsonProcessingException in the signature, and keep the existing checkAndUpdateIncentive(incentiveRecordId) call when meetingImagesJson is present before repository.save(existingMeeting).src/main/java/com/iemr/flw/service/impl/DiseaseControlServiceImpl.java (1)
329-344: Redundant nestedisConfirmedcheck.This block is already inside
if (screeningLeprosy.getIsConfirmed())at Line 295, so the check at Line 329 is always true. Consider removing the redundant condition or restructuring the logic.♻️ Proposed simplification
} - if (screeningLeprosy.getIsConfirmed()) { - IncentiveActivityRecord incentiveActivityRecord = - incentiveLogicService.incentiveForIdentificationLeprosy( - screeningLeprosy.getBenId(), - screeningLeprosy.getTreatmentStartDate(), - screeningLeprosy.getTreatmentEndDate(), - diseaseControlDTO.getUserId()); - - if (incentiveActivityRecord != null) { - logger.info("Incentive processed for Screening Leprosy successfully. RecordId={}", - incentiveActivityRecord.getId()); - } else { - logger.info("Incentive not created"); - } - - } + // Identification incentive for confirmed cases + IncentiveActivityRecord identificationRecord = + incentiveLogicService.incentiveForIdentificationLeprosy( + screeningLeprosy.getBenId(), + screeningLeprosy.getTreatmentStartDate(), + screeningLeprosy.getTreatmentEndDate(), + diseaseControlDTO.getUserId()); + + if (identificationRecord != null) { + logger.info("Incentive processed for Screening Leprosy successfully. RecordId={}", + identificationRecord.getId()); + } else { + logger.info("Incentive not created"); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/flw/service/impl/DiseaseControlServiceImpl.java` around lines 329 - 344, Inside DiseaseControlServiceImpl remove the redundant nested check of screeningLeprosy.getIsConfirmed() around the incentive creation: the inner if block that calls incentiveLogicService.incentiveForIdentificationLeprosy(...) and logs the result is already guarded by an outer if (screeningLeprosy.getIsConfirmed()), so delete the inner condition and keep its body (the call to incentiveForIdentificationLeprosy and subsequent logging) so the incentive logic executes once when the outer condition is true.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
pom.xmlsrc/main/java/com/iemr/flw/controller/AshaProfileController.javasrc/main/java/com/iemr/flw/controller/DeathReportsController.javasrc/main/java/com/iemr/flw/controller/IncentiveController.javasrc/main/java/com/iemr/flw/controller/MaternalHealthController.javasrc/main/java/com/iemr/flw/domain/iemr/DeliveryOutcome.javasrc/main/java/com/iemr/flw/domain/iemr/IncentivePendingActivity.javasrc/main/java/com/iemr/flw/domain/iemr/ScreeningKalaAzar.javasrc/main/java/com/iemr/flw/domain/iemr/User.javasrc/main/java/com/iemr/flw/dto/iemr/CdrDTO.javasrc/main/java/com/iemr/flw/dto/iemr/DeliveryOutcomeDTO.javasrc/main/java/com/iemr/flw/dto/iemr/MdsrDTO.javasrc/main/java/com/iemr/flw/dto/iemr/PendingActivityDTO.javasrc/main/java/com/iemr/flw/dto/iemr/TBScreeningDTO.javasrc/main/java/com/iemr/flw/masterEnum/GroupName.javasrc/main/java/com/iemr/flw/masterEnum/IncentiveName.javasrc/main/java/com/iemr/flw/repo/identity/BeneficiaryRepo.javasrc/main/java/com/iemr/flw/repo/iemr/CdrRepo.javasrc/main/java/com/iemr/flw/repo/iemr/DeliveryOutcomeRepo.javasrc/main/java/com/iemr/flw/repo/iemr/EmployeeMasterRepo.javasrc/main/java/com/iemr/flw/repo/iemr/IncentivePendingActivityRepository.javasrc/main/java/com/iemr/flw/repo/iemr/IncentiveRecordRepo.javasrc/main/java/com/iemr/flw/repo/iemr/MdsrRepo.javasrc/main/java/com/iemr/flw/repo/iemr/PNCVisitRepo.javasrc/main/java/com/iemr/flw/repo/iemr/TBScreeningRepo.javasrc/main/java/com/iemr/flw/service/EmployeeMasterInter.javasrc/main/java/com/iemr/flw/service/IncentiveLogicService.javasrc/main/java/com/iemr/flw/service/MaaMeetingService.javasrc/main/java/com/iemr/flw/service/UwinSessionService.javasrc/main/java/com/iemr/flw/service/impl/AshaProfileImpl.javasrc/main/java/com/iemr/flw/service/impl/BeneficiaryServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/ChildCareServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/DeathReportsServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/DeliveryOutcomeServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/DiseaseControlServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/EmployeeMasterImpl.javasrc/main/java/com/iemr/flw/service/impl/IncentiveLogicImpl.javasrc/main/java/com/iemr/flw/service/impl/IncentiveServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/MaternalHealthServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/NotificationSchedulerService.javasrc/main/java/com/iemr/flw/service/impl/SammelanServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/SmsSchedulerService.javasrc/main/java/com/iemr/flw/service/impl/TBScreeningServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/UpdateIncentivePendindDocService.javasrc/main/java/com/iemr/flw/service/impl/UwinSessionServiceImpl.javasrc/main/java/com/iemr/flw/service/impl/VillageLevelFormServiceImpl.javasrc/main/java/com/iemr/flw/utils/JwtAuthenticationUtil.javasrc/main/java/com/iemr/flw/utils/redis/RedisConfig.java
💤 Files with no reviewable changes (2)
- src/main/java/com/iemr/flw/controller/AshaProfileController.java
- src/main/java/com/iemr/flw/domain/iemr/DeliveryOutcome.java
Comment on lines
+33
to
+40
| @Query("SELECT t FROM RMNCHMBeneficiaryaddress t " + | ||
| "WHERE t.createdDate BETWEEN :fromDate AND :toDate " + | ||
| "AND t.createdBy = :userName") | ||
| Page<RMNCHMBeneficiaryaddress> getBenDataWithinDates( | ||
| @Param("userName") String userName, | ||
| @Param("fromDate") Timestamp fromDate, | ||
| @Param("toDate") Timestamp toDate, | ||
| Pageable pageable); |
There was a problem hiding this comment.
Date-range semantics changed; same-day records can be dropped.
Line 34 now compares full timestamps (BETWEEN :fromDate AND :toDate) instead of date-only values. If toDate is passed as YYYY-MM-DD 00:00:00, rows later on that same day are excluded. This is a behavior regression from the prior date-truncated query.
💡 Suggested fix (preserve date-only filtering in JPQL)
- `@Query`("SELECT t FROM RMNCHMBeneficiaryaddress t " +
- "WHERE t.createdDate BETWEEN :fromDate AND :toDate " +
- "AND t.createdBy = :userName")
+ `@Query`("SELECT t FROM RMNCHMBeneficiaryaddress t " +
+ "WHERE FUNCTION('DATE', t.createdDate) BETWEEN FUNCTION('DATE', :fromDate) AND FUNCTION('DATE', :toDate) " +
+ "AND t.createdBy = :userName")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/repo/identity/BeneficiaryRepo.java` around lines
33 - 40, The JPQL BETWEEN currently compares full timestamps in the query on
RMNCHMBeneficiaryaddress (method getBenDataWithinDates), causing rows later the
same day to be dropped when toDate is midnight; fix by using date-only
comparison or an exclusive end bound: either change the JPQL to compare the date
portion (e.g. use function('date', t.createdDate) BETWEEN :fromDate AND :toDate
and accept LocalDate params) or rewrite the predicate to use inclusive start and
exclusive end (t.createdDate >= :fromDate AND t.createdDate < :toDatePlusOne)
and add a toDatePlusOne parameter computed by adding one day to the provided
toDate; update the method signature/parameters accordingly.
| @Query(" SELECT c FROM CDR c WHERE c.createdBy = :userId and c.createdDate >= :fromDate and c.createdDate <= :toDate") | ||
| List<CDR> getAllCdrByBenId(@Param("userId") String userId, | ||
| @Param("fromDate") Timestamp fromDate, @Param("toDate") Timestamp toDate); | ||
| List<CDR>findByCreatedBy(String userName); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for old method references and new method usage
echo "=== Searching for old method 'getAllCdrByBenId' ==="
rg -n 'getAllCdrByBenId' --type java
echo "=== Searching for new method 'findByCreatedBy' ==="
rg -n 'findByCreatedBy' --type java
echo "=== Searching for CdrRepo usage (excluding CdrRepo.java itself) ==="
rg -n 'CdrRepo' --type java --glob '!CdrRepo.java'
echo "=== Checking imports of CdrRepo ==="
rg -n 'import.*CdrRepo' --type javaRepository: PSMRI/FLW-API
Length of output: 1079
🏁 Script executed:
#!/bin/bash
# Check the context of findByCreatedBy usage in DeathReportsServiceImpl
echo "=== Context around line 187 in DeathReportsServiceImpl ==="
sed -n '180,195p' src/main/java/com/iemr/flw/service/impl/DeathReportsServiceImpl.java
echo "=== Broader context to understand the method flow ==="
sed -n '170,220p' src/main/java/com/iemr/flw/service/impl/DeathReportsServiceImpl.java
echo "=== Check if old method had any documentation ==="
rg -B5 -A10 'getAllCdrByBenId' --type javaRepository: PSMRI/FLW-API
Length of output: 2324
Address unbounded result set risk and verify filtering logic change is intentional.
This is a breaking change, and callers have been updated. However, the issue is more significant than initially stated:
-
Filtering logic fundamentally changed: The method previously filtered by beneficiary ID + date range (
getAllCdrByBenId); it now filters only by user who created the record (findByCreatedBy). This changes the semantics of what data is retrieved. -
Unbounded result set: With no date bounds or pagination applied,
findByCreatedBycould return all CDR records created by a user, potentially causing performance degradation or memory issues for users with large record volumes. -
Intentional refactoring: This pattern is consistently applied across multiple repositories (MdsrRepo, DeliveryOutcomeRepo, MdaFormSubmissionRepository), suggesting a deliberate design decision.
Verify that the change in filtering logic (from beneficiary-based + date range to user-based only) aligns with actual requirements, and consider adding pagination or result limits to prevent unbounded queries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/repo/iemr/CdrRepo.java` at line 17, The new
repository method findByCreatedBy(String userName) changed semantics from the
previous beneficiary+date filtering (getAllCdrByBenId) and can return an
unbounded result set; confirm intended behavior and either restore the original
beneficiary + date-range signature (e.g., reintroduce getAllCdrByBenId with
benId, startDate, endDate) or make findByCreatedBy bounded by adding pagination
or explicit date params (e.g., change to Page<CDR> findByCreatedBy(String
userName, Pageable pageable) or add startDate/endDate args), then update all
callers to use the corrected method and ensure tests reflect the intended filter
semantics.
| @Query(" SELECT m FROM MDSR m WHERE m.createdBy = :userId and m.createdDate >= :fromDate and m.createdDate <= :toDate") | ||
| List<MDSR> getAllMdsrByAshaId(@Param("userId") String userId, | ||
| @Param("fromDate") Timestamp fromDate, @Param("toDate") Timestamp toDate); | ||
| List<MDSR> findByCreatedBy(String userName); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, let's find the repository interface definition
echo "=== Repository Interface Definition ==="
rg -nP --type=java '\binterface\s+MdsrRepo\b' -A 10
echo -e "\n=== Method Signatures in MdsrRepo ==="
rg -nP --type=java '\bfindByCreatedBy\s*\(' --type=java
echo -e "\n=== Call Sites for findByCreatedBy ==="
rg -nP --type=java -C 3 'mdsrRepo\.findByCreatedBy\s*\('
echo -e "\n=== Searching for any Pageable imports and usage patterns ==="
rg -nP --type=java 'import.*Pageable|org\.springframework\.data\.domain'
echo -e "\n=== Checking for date-based filtering near findByCreatedBy calls ==="
rg -nP --type=java -C 5 'findByCreatedBy\s*\('Repository: PSMRI/FLW-API
Length of output: 11665
Add pagination to unbounded findByCreatedBy() fetch
Line 17 returns all records for a user without bounds, which loads entire result set into memory and can cause memory spikes and slow response times for users with large historical datasets. This is especially problematic for death report queries that should be time-bound.
At the call site (DeathReportsServiceImpl:206), the results are converted directly with no downstream filtering or pagination applied. Add a Pageable parameter to bound the query at the repository level, consistent with pagination patterns used elsewhere in the codebase.
Suggested fix
+import org.springframework.data.domain.Pageable;
import java.util.List;
@@
- List<MDSR> findByCreatedBy(String userName);
+ List<MDSR> findByCreatedBy(String userName, Pageable pageable);📝 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.
Suggested change
| List<MDSR> findByCreatedBy(String userName); | |
| import org.springframework.data.domain.Pageable; | |
| import java.util.List; | |
| // ... other code ... | |
| List<MDSR> findByCreatedBy(String userName, Pageable pageable); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/repo/iemr/MdsrRepo.java` at line 17, The
repository method findByCreatedBy currently returns an unbounded List and must
be changed to support paging: update the MdsrRepo signature to accept a Pageable
(e.g., change to Page<MDSR> findByCreatedBy(String userName, Pageable pageable))
and update the call site in DeathReportsServiceImpl (around line 206) to pass a
Pageable (or construct one from request parameters) and to handle the returned
Page (use page.getContent(), totalElements, etc. as needed) so results are
bounded and consistent with existing pagination patterns.
Comment on lines
+15
to
+16
| @Query(value = "SELECT pnc FROM PNCVisit pnc WHERE pnc.createdBy = :userId and pnc.isActive = true") | ||
| List<PNCVisit> getPNCForPW(@Param("userId") String userId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -A 5 'getPNCForPW' --type javaRepository: PSMRI/FLW-API
Length of output: 1224
🏁 Script executed:
#!/bin/bash
# Get context around the usage in MaternalHealthServiceImpl
rg -n -B 10 -A 10 'getPNCForPW' src/main/java/com/iemr/flw/service/impl/MaternalHealthServiceImpl.java | head -50
# Also check if there's a controller method calling this service method
rg -n 'getPNCForPW|getAllPNCVisits|getPNC' --type java src/main/java/com/iemr/flw/controller/ 2>/dev/null || echo "No controller references found"Repository: PSMRI/FLW-API
Length of output: 1196
🏁 Script executed:
#!/bin/bash
# Get full controller method implementation
rg -n -A 20 'public String getPNCVisitDetails' src/main/java/com/iemr/flw/controller/MaternalHealthController.java
echo "---"
# Find GetBenRequestHandler definition
rg -n -A 10 'class GetBenRequestHandler' --type javaRepository: PSMRI/FLW-API
Length of output: 2280
Implement date-range filtering and pagination to prevent unbounded result sets.
The query returns all active PNC visits for a user without date bounds. More critically, the GetBenRequestHandler DTO provides fromDate, toDate, and pageNo fields, but the service method ignores these parameters entirely, discarding the caller's intent to filter and paginate results. This could lead to significant performance degradation and memory pressure if users accumulate many historical records.
Apply date filtering in the repository query or service layer using the DTO's fromDate and toDate fields, and implement pagination using pageNo to keep result sets bounded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/repo/iemr/PNCVisitRepo.java` around lines 15 - 16,
The repository method getPNCForPW currently returns all active PNCVisit rows for
a user without applying the GetBenRequestHandler.fromDate/toDate or pagination
(pageNo); change the repo to accept date range and pagination (e.g., add
parameters for fromDate and toDate and a Pageable or return Page<PNCVisit>)
and/or replace the `@Query` with a derived query such as
findByCreatedByAndIsActiveAndVisitDateBetween or a JPQL query that filters by
:fromDate and :toDate; then update the service method that calls getPNCForPW to
pass GetBenRequestHandler.fromDate, toDate and a PageRequest built from pageNo
so results are bounded and filtered by date before returning to callers.
Comment on lines
+18
to
+19
| @Query(value = "SELECT tbs FROM TBScreening tbs WHERE tbs.userId = :userId") | ||
| List<TBScreening> getByUserId(@Param("userId") Integer userId); |
There was a problem hiding this comment.
User-only TB query is now unbounded and may return full history.
This change removes time-window scoping and can significantly increase data volume and latency for long-tenure users.
Suggested bounded query restoration
- `@Query`(value = "SELECT tbs FROM TBScreening tbs WHERE tbs.userId = :userId")
- List<TBScreening> getByUserId(`@Param`("userId") Integer userId);
+ `@Query`(value = "SELECT tbs FROM TBScreening tbs WHERE tbs.userId = :userId AND tbs.visitDate BETWEEN :fromDate AND :toDate")
+ List<TBScreening> getByUserId(`@Param`("userId") Integer userId,
+ `@Param`("fromDate") Timestamp fromDate,
+ `@Param`("toDate") Timestamp toDate);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Query(value = "SELECT tbs FROM TBScreening tbs WHERE tbs.userId = :userId") | |
| List<TBScreening> getByUserId(@Param("userId") Integer userId); | |
| `@Query`(value = "SELECT tbs FROM TBScreening tbs WHERE tbs.userId = :userId AND tbs.visitDate BETWEEN :fromDate AND :toDate") | |
| List<TBScreening> getByUserId(`@Param`("userId") Integer userId, | |
| `@Param`("fromDate") Timestamp fromDate, | |
| `@Param`("toDate") Timestamp toDate); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/repo/iemr/TBScreeningRepo.java` around lines 18 -
19, The repository method getByUserId in TBScreeningRepo now returns an
unbounded history; change the Query to restrict results by a time window: add
parameters (e.g., `@Param`("start"), `@Param`("end")) to the method signature and
modify the JPQL to include a date filter such as "AND tbs.screeningDate BETWEEN
:start AND :end" (or use the entity's appropriate timestamp field like createdAt
if different), and update callers to pass the start/end dates so queries are
bounded to the desired time range.
Comment on lines
+69
to
+72
| }catch (Exception e){ | ||
| logger.error("TBScreening Exception:"+e.getMessage()); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Do not swallow fetch exceptions by returning null.
This path hides real failures and can misclassify errors downstream. Log the full exception and propagate/translate it explicitly.
Suggested error-handling fix
- }catch (Exception e){
- logger.error("TBScreening Exception:"+e.getMessage());
- }
- return null;
+ } catch (Exception e) {
+ logger.error("TBScreening exception while fetching records", e);
+ throw new RuntimeException("Failed to fetch TB screening records", 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.
Suggested change
| }catch (Exception e){ | |
| logger.error("TBScreening Exception:"+e.getMessage()); | |
| } | |
| return null; | |
| } catch (Exception e) { | |
| logger.error("TBScreening exception while fetching records", e); | |
| throw new RuntimeException("Failed to fetch TB screening records", e); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/service/impl/TBScreeningServiceImpl.java` around
lines 69 - 72, The catch block in TBScreeningServiceImpl currently swallows
exceptions and returns null; replace this by logging the full exception (use
logger.error with the Throwable, e.g., logger.error("TBScreening Exception", e))
and then propagate the error instead of returning null—either rethrow the same
exception or wrap it in an appropriate unchecked/checked application exception
(e.g., throw new RuntimeException("Failed to fetch TB screening", e)) so callers
can distinguish failure from an empty result.
Comment on lines
+117
to
+120
| // Update fields if present in request | ||
| if (req.getPlace() != null) session.setPlace(req.getPlace()); | ||
| if (req.getParticipants() != null) session.setParticipants(req.getParticipants()); | ||
| if (req.getDate() != null) session.setDate(req.getDate()); |
There was a problem hiding this comment.
Apply the same validation rules in updateSession as saveSession.
updateSession currently accepts future dates/backdated dates and out-of-range participants, while saveSession rejects them.
Suggested fix
+ private void validateSessionUpdate(UwinSessionRequestDTO req) {
+ Timestamp now = Timestamp.from(Instant.now());
+ if (req.getDate() != null) {
+ if (req.getDate().after(now)) {
+ throw new RuntimeException("Date cannot be in future");
+ }
+ if (req.getDate().toLocalDateTime().isBefore(LocalDateTime.now().minusMonths(2))) {
+ throw new RuntimeException("Backdate not allowed beyond 2 months");
+ }
+ }
+ if (req.getParticipants() != null && (req.getParticipants() < 0 || req.getParticipants() > 999)) {
+ throw new RuntimeException("Participants count must be between 0–999");
+ }
+ }
+
public UwinSession updateSession(UwinSessionRequestDTO req, Long recordId, Long activityId) throws Exception {
+ validateSessionUpdate(req);
Optional<UwinSession> optionalUwinSession = repo.findById(recordId);
...
}📝 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.
Suggested change
| // Update fields if present in request | |
| if (req.getPlace() != null) session.setPlace(req.getPlace()); | |
| if (req.getParticipants() != null) session.setParticipants(req.getParticipants()); | |
| if (req.getDate() != null) session.setDate(req.getDate()); | |
| private void validateSessionUpdate(UwinSessionRequestDTO req) { | |
| Timestamp now = Timestamp.from(Instant.now()); | |
| if (req.getDate() != null) { | |
| if (req.getDate().after(now)) { | |
| throw new RuntimeException("Date cannot be in future"); | |
| } | |
| if (req.getDate().toLocalDateTime().isBefore(LocalDateTime.now().minusMonths(2))) { | |
| throw new RuntimeException("Backdate not allowed beyond 2 months"); | |
| } | |
| } | |
| if (req.getParticipants() != null && (req.getParticipants() < 0 || req.getParticipants() > 999)) { | |
| throw new RuntimeException("Participants count must be between 0–999"); | |
| } | |
| } | |
| public UwinSession updateSession(UwinSessionRequestDTO req, Long recordId, Long activityId) throws Exception { | |
| validateSessionUpdate(req); | |
| Optional<UwinSession> optionalUwinSession = repo.findById(recordId); | |
| // ... existing method logic ... | |
| // Update fields if present in request | |
| if (req.getPlace() != null) session.setPlace(req.getPlace()); | |
| if (req.getParticipants() != null) session.setParticipants(req.getParticipants()); | |
| if (req.getDate() != null) session.setDate(req.getDate()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/service/impl/UwinSessionServiceImpl.java` around
lines 117 - 120, The updateSession method currently skips the date and
participants validation that saveSession enforces; replicate the same
validations from saveSession in updateSession (or extract them into a shared
validateSessionRequest(RequestType) helper and call it from both) so that
updateSession rejects future dates, backdated/out-of-range dates and
participants outside the allowed range before applying session.setDate(...) or
session.setParticipants(...); ensure you reuse the same checks and exception
types/messages used in saveSession (or call the shared validator) so behavior
and error responses remain consistent.
Comment on lines
+191
to
+193
| if(session.getAttachmentsJson()!=null){ | ||
| record.setIsEligible(true); | ||
| recordRepo.save(record); |
There was a problem hiding this comment.
Eligibility should require actual attachments, not just non-null JSON.
attachmentsJson can be "[]", which still marks records eligible in these checks.
Suggested fix
+ private boolean hasRealAttachments(String attachmentsJson) {
+ return attachmentsJson != null && !"[]".equals(attachmentsJson);
+ }
...
- if(session.getAttachmentsJson()!=null){
+ if (hasRealAttachments(session.getAttachmentsJson())) {
record.setIsEligible(true);
...
- if(session.getAttachmentsJson()!=null){
+ if (hasRealAttachments(session.getAttachmentsJson())) {
record.setIsEligible(true);Also applies to: 222-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/service/impl/UwinSessionServiceImpl.java` around
lines 191 - 193, The current eligibility check marks records eligible if
session.getAttachmentsJson() is non-null even when it is an empty array string
like "[]"; update the logic where record.setIsEligible(true) and
recordRepo.save(record) are called (both occurrences using
session.getAttachmentsJson()) to parse or inspect the attachmentsJson value and
only set eligibility when it contains at least one attachment (e.g., check that
the JSON is not empty array or length>2, or parse into a collection and verify
size>0) before calling record.setIsEligible(true) and saving via
recordRepo.save(record).
Comment on lines
226
to
228
| updateIncentivePendindDocService.updatePendingActivity(session.getAshaId(),session.getId(),record.getId(),incentiveActivityCH.getId()); | ||
| } | ||
| recordRepo.save(record); |
There was a problem hiding this comment.
Save the record before using its ID in pending-doc linkage.
Line 226 passes record.getId() before persistence, so pending-doc updates can receive a null recordId.
Suggested fix
- if(session.getAttachmentsJson()!=null){
- record.setIsEligible(true);
- }else {
- record.setIsEligible(false);
- updateIncentivePendindDocService.updatePendingActivity(session.getAshaId(),session.getId(),record.getId(),incentiveActivityCH.getId());
- }
- recordRepo.save(record);
+ if (session.getAttachmentsJson() != null) {
+ record.setIsEligible(true);
+ recordRepo.save(record);
+ } else {
+ record.setIsEligible(false);
+ IncentiveActivityRecord saved = recordRepo.save(record);
+ updateIncentivePendindDocService.updatePendingActivity(
+ session.getAshaId(), session.getId(), saved.getId(), incentiveActivityCH.getId());
+ }📝 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.
Suggested change
| updateIncentivePendindDocService.updatePendingActivity(session.getAshaId(),session.getId(),record.getId(),incentiveActivityCH.getId()); | |
| } | |
| recordRepo.save(record); | |
| if (session.getAttachmentsJson() != null) { | |
| record.setIsEligible(true); | |
| recordRepo.save(record); | |
| } else { | |
| record.setIsEligible(false); | |
| IncentiveActivityRecord saved = recordRepo.save(record); | |
| updateIncentivePendindDocService.updatePendingActivity( | |
| session.getAshaId(), session.getId(), saved.getId(), incentiveActivityCH.getId()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/service/impl/UwinSessionServiceImpl.java` around
lines 226 - 228, The call to
updateIncentivePendindDocService.updatePendingActivity uses record.getId()
before the record is persisted, which can pass a null ID; change the order in
UwinSessionServiceImpl so you call recordRepo.save(record) first (or capture the
returned savedRecord from recordRepo.save and use savedRecord.getId()), then
call updateIncentivePendindDocService.updatePendingActivity(session.getAshaId(),
session.getId(), savedRecord.getId(), incentiveActivityCH.getId()) to ensure a
non-null persisted recordId is passed.
Comment on lines
+153
to
+158
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy"); | ||
|
|
||
| LocalDate localDate = LocalDate.parse(vhncForm.getVhncDate(), formatter); | ||
|
|
||
| Date date = Date.from(localDate.atStartOfDay(ZoneId.systemDefault()).toInstant()); | ||
| incentiveLogicService.incentiveForAttendingVhsnc(0L,date,date,Math.toIntExact(vhncForm.getUserId())); |
There was a problem hiding this comment.
Make form save + incentive creation atomic.
These blocks run after persistence; if parsing or incentive logic throws, you can end up with saved form data but missing incentive record.
Suggested hardening
+import org.springframework.transaction.annotation.Transactional;
...
- private Boolean saveVhncFormData(VhncFormDTO vhncFormDTO, Integer userID) {
+ `@Transactional`
+ private Boolean saveVhncFormData(VhncFormDTO vhncFormDTO, Integer userID) {
VhncForm vhncForm = new VhncForm();
...
- vhncFormRepo.save(vhncForm);
- DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy");
- LocalDate localDate = LocalDate.parse(vhncForm.getVhncDate(), formatter);
- Date date = Date.from(localDate.atStartOfDay(ZoneId.systemDefault()).toInstant());
+ Date date = parseFormDate(vhncForm.getVhncDate());
+ vhncFormRepo.save(vhncForm);
incentiveLogicService.incentiveForAttendingVhsnc(0L, date, date, Math.toIntExact(vhncForm.getUserId()));
return true;
}
+
+ private Date parseFormDate(String formDate) {
+ DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy");
+ LocalDate localDate = LocalDate.parse(formDate, formatter);
+ return Date.from(localDate.atStartOfDay(ZoneId.systemDefault()).toInstant());
+ }Also applies to: 178-183, 245-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/flw/service/impl/VillageLevelFormServiceImpl.java`
around lines 153 - 158, The form save and subsequent incentive creation
(incentiveLogicService.incentiveForAttendingVhsnc) must be made atomic: ensure
the persistence of vhncForm and the Date parsing/LocalDate.parse and the call to
incentiveLogicService occur inside a single transactional boundary (e.g.,
annotate the enclosing service method in VillageLevelFormServiceImpl with
`@Transactional` or use TransactionTemplate) so that any parsing or incentive
logic exceptions roll back the saved form; apply the same transactional change
to the other blocks that call incentiveForAttendingVhsnc (the blocks around
LocalDate.parse/use of DateTimeFormatter at the locations flagged 178-183 and
245-250) so both save and incentive creation succeed or both are rolled back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Chores