Skip to content

Week not changes but day of call changes caused BUG #881

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.

Sign up for GitHub

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

Open
Kantas2601 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Week not changes but day of call changes caused BUG #881

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion kilkari/src/main/java/org/motechproject/nms/kilkari/service/impl/CsrServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.motechproject.nms.kilkari.service.CsrVerifierService;
import org.motechproject.nms.kilkari.service.SubscriptionService;
import org.motechproject.nms.kilkari.utils.KilkariConstants;
import org.motechproject.nms.props.domain.DayOfTheWeek;
import org.motechproject.nms.props.domain.FinalCallStatus;
import org.motechproject.nms.props.domain.StatusCode;
import org.motechproject.nms.props.domain.WhatsAppOptInStatusCode;
Expand Down Expand Up @@ -322,10 +323,13 @@ public void processCallSummaryRecord(MotechEvent event) { //NOPMD NcssMethodCoun

case FAILED:
String weekId = getWeekIdForSubscription(subscription.getStartDate());
DayOfTheWeek dayOfTheWeek = subscription.getFirstMessageDayOfWeek();
DayOfTheWeek tomorrow = DayOfTheWeek.today().nextDay();

//If there was a DOB/LMP update during RCH import, number of weeks into subscription would have changed.
//No need to reschedule this call. Exception for w1, because regardless of which week the subscription starts in, user
//always gets w1 message initially
if(!csrDto.getWeekId().equals("w1_1")&&!weekId.equals(csrDto.getWeekId())){
if ((!csrDto.getWeekId().equals("w1_1") && !weekId.equals(csrDto.getWeekId())) || dayOfTheWeek.equals(tomorrow)){
if(callRetry!=null){
Comment on lines +326 to 333
Copy link

@coderabbitai coderabbitai bot Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Possible NullPointerException – guard against a missing firstMessageDayOfWeek value

subscription.getFirstMessageDayOfWeek() can legally be null for older or partially-migrated records.
Dereferencing it in dayOfTheWeek.equals(tomorrow) will crash the whole CSR batch and leave the retry row undeleted, re-introducing the very duplication this PR is fixing.

Diff suggestion:

- DayOfTheWeek dayOfTheWeek = subscription.getFirstMessageDayOfWeek();
- DayOfTheWeek tomorrow = DayOfTheWeek.today().nextDay();
+ DayOfTheWeek firstMessageDay = subscription.getFirstMessageDayOfWeek();
+ DayOfTheWeek tomorrow        = DayOfTheWeek.today().nextDay();
- if ((!csrDto.getWeekId().equals("w1_1") && !weekId.equals(csrDto.getWeekId())) || dayOfTheWeek.equals(tomorrow)){
+ if ((!csrDto.getWeekId().equals("w1_1") && !weekId.equals(csrDto.getWeekId()))
+         || (firstMessageDay != null && firstMessageDay.equals(tomorrow))) {

Without the null-check the nightly job will throw and fail for every record that lacks the field.

📝 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
DayOfTheWeek dayOfTheWeek = subscription.getFirstMessageDayOfWeek();
DayOfTheWeek tomorrow = DayOfTheWeek.today().nextDay();
//If there was a DOB/LMP update during RCH import, number of weeks into subscription would have changed.
//No need to reschedule this call. Exception for w1, because regardless of which week the subscription starts in, user
//always gets w1 message initially
if(!csrDto.getWeekId().equals("w1_1")&&!weekId.equals(csrDto.getWeekId())){
if ((!csrDto.getWeekId().equals("w1_1") && !weekId.equals(csrDto.getWeekId())) || dayOfTheWeek.equals(tomorrow)){
if(callRetry!=null){
DayOfTheWeek firstMessageDay = subscription.getFirstMessageDayOfWeek();
DayOfTheWeek tomorrow = DayOfTheWeek.today().nextDay();
//If there was a DOB/LMP update during RCH import, number of weeks into subscription would have changed.
//No need to reschedule this call. Exception for w1, because regardless of which week the subscription starts in, user
//always gets w1 message initially
if ((!csrDto.getWeekId().equals("w1_1") && !weekId.equals(csrDto.getWeekId()))
|| (firstMessageDay != null && firstMessageDay.equals(tomorrow))) {
if(callRetry!=null){
🤖 Prompt for AI Agents 
In
kilkari/src/main/java/org/motechproject/nms/kilkari/service/impl/CsrServiceImpl.java
around lines 326 to 333, the code dereferences
subscription.getFirstMessageDayOfWeek() without checking for null, which can
cause a NullPointerException for older or partially migrated records. To fix
this, add a null check for dayOfTheWeek before calling
dayOfTheWeek.equals(tomorrow), ensuring the condition only evaluates if
dayOfTheWeek is not null to prevent the batch job from crashing.

callRetryDataService.delete(callRetry);
}
Expand Down