-
Notifications
You must be signed in to change notification settings - Fork 15
Changes incentive loigc for CG in VillageLevel #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes incentive loigc for CG in VillageLevel #196
Conversation
📝 WalkthroughWalkthroughThree new incentive calculation methods were added to IncentiveLogicService interface and implemented in IncentiveLogicImpl, each processing incentives conditionally for CG state. VillageLevelFormServiceImpl was refactored to invoke these new methods with date conversions instead of previous incentive triggering calls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/iemr/flw/service/impl/IncentiveLogicImpl.java`:
- Around line 135-226: The meeting incentive methods (incentiveForVhndMeeting,
incentiveForClusterMeeting, incentiveForAttendingVhsnc) currently call the
deduping save path that keys only on (activityId, createdDate, benId) which
causes cross-user suppression when benId==0; update the code path so that when
benId == 0 you call the repository/save overload that includes userId (the same
approach used in checkAndAddIncentives) — i.e., modify processIncentive (or the
place that ultimately calls saveIncentive) to detect benId==0 and use the
saveIncentive/save repository method variant that takes userId so duplicates are
scoped per user.
In `@src/main/java/com/iemr/flw/service/impl/VillageLevelFormServiceImpl.java`:
- Around line 153-158: Guard the LocalDate.parse call for vhncForm.getVhncDate()
by first checking for null/blank and then parsing inside a try/catch so
malformed or missing dates do not throw and abort the save; if parsing fails,
log a warning (or collect a validation message) and skip calling
incentiveLogicService.incentiveForAttendingVhsnc(…) instead of letting the
exception propagate. Apply the same pattern to the PHC and VHND parsing blocks
(the other parse sites referenced at the 178-183 and 245-250 ranges) so all date
parsing is defensive and incentive calls are only made for successfully parsed
dates.
| @Override | ||
| public IncentiveActivityRecord incentiveForVhndMeeting(Long benId, Date treatmentStartDate, Date treatmentEndDate, Integer userId) { | ||
| try { | ||
| Integer stateCode = userService.getUserDetail(userId).getStateId(); | ||
|
|
||
| if (stateCode == null) { | ||
| logger.warn("State code is null for user: {}", userId); | ||
| return null; | ||
| } | ||
|
|
||
| String activityName = "VHND_PARTICIPATION"; | ||
|
|
||
| if (stateCode.equals(StateCode.CG.getStateCode())) { | ||
| return processIncentive( | ||
| activityName, | ||
| GroupName.ACTIVITY.getDisplayName(), | ||
| benId, | ||
| treatmentStartDate, | ||
| treatmentEndDate, | ||
| userId); | ||
| } | ||
|
|
||
| // state not supported | ||
| logger.info("No incentive mapping for stateCode: {}", stateCode); | ||
| return null; | ||
|
|
||
| } catch (Exception e) { | ||
| logger.error("Check VHND_PARTICIPATION Incentive Exception: ", e); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public IncentiveActivityRecord incentiveForClusterMeeting(Long benId, Date treatmentStartDate, Date treatmentEndDate, Integer userId) { | ||
| try { | ||
| Integer stateCode = userService.getUserDetail(userId).getStateId(); | ||
|
|
||
| if (stateCode == null) { | ||
| logger.warn("State code is null for user: {}", userId); | ||
| return null; | ||
| } | ||
|
|
||
| String activityName = "CLUSTER_MEETING"; | ||
|
|
||
| if (stateCode.equals(StateCode.CG.getStateCode())) { | ||
| return processIncentive( | ||
| activityName, | ||
| GroupName.ACTIVITY.getDisplayName(), | ||
| benId, | ||
| treatmentStartDate, | ||
| treatmentEndDate, | ||
| userId); | ||
| } | ||
|
|
||
| // state not supported | ||
| logger.info("No incentive mapping for stateCode: {}", stateCode); | ||
| return null; | ||
|
|
||
| } catch (Exception e) { | ||
| logger.error("Check CLUSTER_MEETING Incentive Exception: ", e); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public IncentiveActivityRecord incentiveForAttendingVhsnc(Long benId, Date treatmentStartDate, Date treatmentEndDate, Integer userId) { | ||
| try { | ||
| Integer stateCode = userService.getUserDetail(userId).getStateId(); | ||
|
|
||
| if (stateCode == null) { | ||
| logger.warn("State code is null for user: {}", userId); | ||
| return null; | ||
| } | ||
|
|
||
| String activityName = "VHSNC_MEETING"; | ||
|
|
||
| if (stateCode.equals(StateCode.CG.getStateCode())) { | ||
| return processIncentive( | ||
| activityName, | ||
| GroupName.ACTIVITY.getDisplayName(), | ||
| benId, | ||
| treatmentStartDate, | ||
| treatmentEndDate, | ||
| userId); | ||
| } | ||
|
|
||
| // state not supported | ||
| logger.info("No incentive mapping for stateCode: {}", stateCode); | ||
| return null; | ||
|
|
||
| } catch (Exception e) { | ||
| logger.error("Check VHSNC_MEETING Incentive Exception: ", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent cross-user duplicate suppression when benId = 0.
These new meeting incentives are invoked with benId = 0 from VillageLevelFormServiceImpl. saveIncentive dedupes only on (activityId, createdDate, benId), so the first record for a date will block incentives for other users on that same date. This is a functional regression compared to the previous flow that also keyed on userId.
Consider using the repo overload that includes userId when benId == 0 (as already done in checkAndAddIncentives), so duplicates are scoped per user.
✅ Suggested fix in saveIncentive
- IncentiveActivityRecord existing = incentiveRecordRepo
- .findRecordByActivityIdCreatedDateBenId(
- activity.getId(), startTimestamp, benId);
+ IncentiveActivityRecord existing;
+ if (Long.valueOf(0L).equals(benId)) {
+ existing = incentiveRecordRepo
+ .findRecordByActivityIdCreatedDateBenId(
+ activity.getId(), startTimestamp, benId, userId);
+ } else {
+ existing = incentiveRecordRepo
+ .findRecordByActivityIdCreatedDateBenId(
+ activity.getId(), startTimestamp, benId);
+ }🤖 Prompt for AI Agents
In `@src/main/java/com/iemr/flw/service/impl/IncentiveLogicImpl.java` around lines
135 - 226, The meeting incentive methods (incentiveForVhndMeeting,
incentiveForClusterMeeting, incentiveForAttendingVhsnc) currently call the
deduping save path that keys only on (activityId, createdDate, benId) which
causes cross-user suppression when benId==0; update the code path so that when
benId == 0 you call the repository/save overload that includes userId (the same
approach used in checkAndAddIncentives) — i.e., modify processIncentive (or the
place that ultimately calls saveIncentive) to detect benId==0 and use the
saveIncentive/save repository method variant that takes userId so duplicates are
scoped per user.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard date parsing to avoid runtime failures on malformed dates.
LocalDate.parse(...) will throw if the stored date is null/blank or not in dd-MM-yyyy format. Because the form is already saved at this point, an exception can bubble up and leave partial writes while the API fails. Consider validating/handling parse failures and skipping incentive creation (or surfacing a validation error) instead of throwing.
💡 Example guard (apply same pattern to PHC/VHND blocks)
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()));
+ try {
+ 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()));
+ } catch (DateTimeParseException ex) {
+ logger.warn("Invalid VHNC date '{}'; skipping incentive", vhncForm.getVhncDate(), ex);
+ }+import java.time.format.DateTimeParseException;Also applies to: 178-183, 245-250
🤖 Prompt for AI Agents
In `@src/main/java/com/iemr/flw/service/impl/VillageLevelFormServiceImpl.java`
around lines 153 - 158, Guard the LocalDate.parse call for
vhncForm.getVhncDate() by first checking for null/blank and then parsing inside
a try/catch so malformed or missing dates do not throw and abort the save; if
parsing fails, log a warning (or collect a validation message) and skip calling
incentiveLogicService.incentiveForAttendingVhsnc(…) instead of letting the
exception propagate. Apply the same pattern to the PHC and VHND parsing blocks
(the other parse sites referenced at the 178-183 and 245-250 ranges) so all date
parsing is defensive and incentive calls are only made for successfully parsed
dates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/flw/service/impl/IncentiveLogicImpl.java (1)
96-133:⚠️ Potential issue | 🟠 MajorLine 103: Incorrect activity name for multibacillary method—copy-paste bug.
The
incentiveForLeprosyMultibacillaryConfirmedmethod uses"NLEP_PB_TREATMENT"(paucibacillary) instead of a distinct activity name for multibacillary cases. Line 130's error log also references the wrong constant. This appears to be a copy-paste error from the paucibacillary method. Both PB and MB cases will be tracked under the same activity, breaking incentive tracking for multibacillary cases. Update line 103 to use the correct activity name (likely"NLEP_MB_TREATMENT"or similar) and fix the error log on line 130.
🧹 Nitpick comments (2)
src/main/java/com/iemr/flw/service/impl/VillageLevelFormServiceImpl.java (1)
245-250: Extract duplicated date-conversion-and-incentive-call into a helper.The date-parsing + incentive-call pattern is repeated verbatim in
saveVhncFormData(lines 153–158),savePhcForm(lines 178–183), andsaveVhndFormData(lines 245–250). A small private helper would eliminate the duplication and make it easier to add the defensive try/catch in one place.♻️ Suggested helper
private void triggerIncentive(String dateStr, Integer userId, java.util.function.BiConsumer<Date, Integer> incentiveCall) { try { DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy"); LocalDate localDate = LocalDate.parse(dateStr, formatter); Date date = Date.from(localDate.atStartOfDay(ZoneId.systemDefault()).toInstant()); incentiveCall.accept(date, userId); } catch (Exception ex) { logger.warn("Invalid date '{}'; skipping incentive for user {}", dateStr, userId, ex); } }Usage:
// In saveVhncFormData: triggerIncentive(vhncForm.getVhncDate(), Math.toIntExact(vhncForm.getUserId()), (date, uid) -> incentiveLogicService.incentiveForAttendingVhsnc(0L, date, date, uid));src/main/java/com/iemr/flw/service/impl/IncentiveLogicImpl.java (1)
135-229: Consider extracting the repeated method body into a shared helper.All three new methods (and the two leprosy methods) follow an identical pattern: fetch state code → null-check → match state → call
processIncentive→ catch/log. The only varying inputs are the activity name, the set of supported states, and the group name. A single parameterized private method would eliminate ~90 lines of duplication.♻️ Sketch
private IncentiveActivityRecord incentiveForActivity( String activityName, String groupName, StateCode requiredState, Long benId, Date start, Date end, Integer userId) { try { Integer stateCode = userService.getUserDetail(userId).getStateId(); if (stateCode == null) { logger.warn("State code is null for user: {}", userId); return null; } if (stateCode.equals(requiredState.getStateCode())) { return processIncentive(activityName, groupName, benId, start, end, userId); } logger.info("No incentive mapping for stateCode: {}", stateCode); return null; } catch (Exception e) { logger.error("Check {} Incentive Exception: ", activityName, e); return null; } }Each public method then becomes a one-liner delegation.
📋 Description
JIRA ID: AMM-2092,AMM-2093,AMM-2094
*Incentive for Participation in Village Health and Nutrition Day (VHND)
*Incentive for organizing and attending monthly meetings of Village Health, Sanitation, and Nutrition Committee (VHSNC).
✅ 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
Refactor