Bug 35700 - Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays
Summary: Holds reminder cronjob --triggered switch does not work as intended if the da...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Lisette Scheer
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-04 10:45 UTC by Lari Strand
Modified: 2025-10-09 21:01 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This bugfix adds a check to the hold reminders cronjob so the job will skip if today is a holiday when the --holiday flag is used. This will prevent the notice from repeating reminders that would send on a usually closed day again on the next open day.
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays (4.98 KB, patch)
2025-09-30 16:17 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays (2.33 KB, patch)
2025-09-30 16:26 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays (4.83 KB, patch)
2025-09-30 16:36 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays (3.95 KB, patch)
2025-09-30 16:39 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays (3.99 KB, patch)
2025-10-03 21:25 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 35700: (QA follow-up) fix POD and $today (1.95 KB, patch)
2025-10-06 16:17 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 35700: Holds reminder cronjob - Fix --triggered switch if the day to send notice hits concurrent holidays (3.65 KB, patch)
2025-10-07 08:46 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Strand 2024-01-04 10:45:45 UTC
If the calendar has concurrent holidays, when holds_reminder.pl cronjob is being ran daily with the --triggered switch and the day the notice should be sent lands on a holiday which is repeated the following days, the cronjob sends holds reminders until the holiday period ends.

To reproduce:

Reserve an item, receive it at a branch where the reserve was made to.
Move the waitingdate and reservedate of the reserve to x days before the reserve was made in the database. 
Add holidays (single non-repeated days) to the calendar so that the first holiday lands on a date when the hold reminder should be sent.
Set up the cronjob to run so that the --days trigger matches the first added holiday using also the --triggered switch:

holds_reminder.pl -v --days x --triggered --holidays -c

for example. Check that the x:th day from the reserve's waitingdate is a holiday, and the date has following days set as holidays as well.

Notice that on the following day/days of the first sent reminder, the message is sent again. Same happens the following holidays.

It seems the notice is sent also if there is a repeated holiday following the first single day holiday the message was sent (like if saturdays and sundays were repeated holidays).

I guess this could be reproduced easier by moving your system date forward and running the cronscript again. I couldn't reproduce the same bug when moving back the waitingdate of the reserve to simulate a day had passed since the first message for some reason. Notice that modifying the holidays use a cache, so just making changes to the database is not enough for the cronscript to notice the changes made.
Comment 1 Esther Melander 2024-04-16 16:36:07 UTC
I have seen this behavior recently on a site, but it was occurring on a single holiday. The library is only closed on Sundays. The hold reminder notice was hitting and sending on the closed Sunday and sending a duplicate notice on the next open day, Monday. The calendars for the impacted branches had Sundays set as a holiday.

This is the cron configuration:

echo "running holds_reminder.pl at $(date +%Y%m%d":"%H":"%M":"%S)"                                                                                                                                                 
$KOHA_CRON_PATH/holds/holds_reminder.pl -holidays -t -days 3 --confirm                                                                                                                                             
echo "done running holds_reminder.pl at $(date +%Y%m%d":"%H":"%M":"%S)"
Comment 2 Lisette Scheer 2025-09-30 16:17:29 UTC Comment hidden (obsolete)
Comment 3 Lisette Scheer 2025-09-30 16:26:49 UTC
Created attachment 187113 [details] [review]
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays

If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.

To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron

11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off
Comment 4 Lisette Scheer 2025-09-30 16:36:07 UTC
Created attachment 187114 [details] [review]
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays

If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.

To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron

11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off
Comment 5 Lisette Scheer 2025-09-30 16:39:00 UTC
Created attachment 187115 [details] [review]
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays

If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.

To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron

11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off
Comment 6 Andrew Fuerste-Henry 2025-09-30 20:56:19 UTC
I believe your test plan wants to use "-days 4" for testing, rather than "-days 3" if you're setting the item to have gone to Waiting on 9/25/25 and want the first reminder to go out 9/29 in the absence of holidays.

Additionally, if 9/29 and 9/30 are both holidays, then with the patch we should expect a notice to be generated on 10/1 (repeat of step 9, rather than repeat of step 7).

My testing has been successful based on the above assumptions.
Comment 7 Andrew Fuerste-Henry 2025-10-03 21:25:22 UTC
Created attachment 187426 [details] [review]
Bug 35700: Holds reminder cronjob --triggered switch does not work as intended if the day to send notice hits concurrent holidays

If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.

To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron

11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off

Signed-off-by: Katie Bliss <KEBliss@dmpl.org>
Comment 8 Jonathan Druart 2025-10-06 10:48:55 UTC
I have some questions:

1. 
-  [ -date <YYYY-MM-DD> ]
Why?

2.
+        my $today    = $date_to_run;
[...]
+        next if $calendar->is_holiday($today);
[...]
+            $waiting_date = $calendar->addDays( $date_to_run, $duration );    #Add negative of days

Why are you using a new "today" variable? Also note that $today is not today's date and I think is misleading (and it's why we named it date_to_run)

3. Shouldn't we actually use Koha::Calendar::next_open_days here?
Comment 9 Lisette Scheer 2025-10-06 14:59:29 UTC
(In reply to Jonathan Druart from comment #8)
> I have some questions:
> 
> 1. 
> -  [ -date <YYYY-MM-DD> ]
> Why?

Accident, I had originally been adding an additional flag and accidentally deleted this 

2. Also a hangover from my original overcomplication

I'm uploading a qa followup to fix these. 

> 3. Shouldn't we actually use Koha::Calendar::next_open_days here?

Instead of next if $calendar->is_holiday($date_to_run); or instead of something else? 
addDays already uses next_open_days for calculating the date, we just need to not run it on days there's a holiday to prevent duplicates.
Comment 10 Lisette Scheer 2025-10-06 16:17:37 UTC
Created attachment 187482 [details] [review]
Bug 35700: (QA follow-up) fix POD and $today

This patch replaces the inncorrectly removed POD note and replaces $today with the more correct $date_to_run for checking if we should be skipping sending the message on the day.
Comment 11 Jonathan Druart 2025-10-07 08:44:55 UTC
(In reply to Lisette Scheer from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > 3. Shouldn't we actually use Koha::Calendar::next_open_days here?
> 
> Instead of next if $calendar->is_holiday($date_to_run); or instead of
> something else? 
> addDays already uses next_open_days for calculating the date, we just need
> to not run it on days there's a holiday to prevent duplicates.

Yes, I think you are right.
Comment 12 Jonathan Druart 2025-10-07 08:46:36 UTC
Created attachment 187505 [details] [review]
Bug 35700: Holds reminder cronjob - Fix --triggered switch if the day to send notice hits concurrent holidays

If the holds reminder cronjob would send a reminder on a holiday,when holidays are taken into account, it also will send it on the subsequent holidays and the first open day.
This bugfix adds a check to the cron when --holidays is set to skip sending the notices for that library if today is a holiday.

To Test:
Before applying:
1. Create a hold for a patron (I used patron 23529000035676 and 39999000000252)
2. Ensure your patron has an email address set and the hold reminder email selected.
3. Set the hold to waiting by checking it and confirming the hold.
4. In Tools->Calendar add a holiday to yesterday and a seperate holiday to today.
5. On the command line (the date should be set to 5 days ago for my commands to work and any dates adjusted accordingly)
koha-mysql kohadev
update reserves set waitingdate="2025-09-25"
exit
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-09-29
6. Observe that 1 notice would have sent to your patron
7. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays
8. Observe that 1 notice would have sent to your patron
9. On the command line
perl misc/cronjobs/holds/holds_reminder.pl -days 3 -t -v -holidays --date 2025-10-01
10. Observe that 1 notice would have sent to your patron

11. Apply patches
12. Repeat the last command from step 5 and step 7
13. Observe that no notice should be sent for your patron
14. Repeat step 7
15. Observe that a notice should be sent for your patron.
16. Run
perl misc/cronjobs/holds/holds_reminder.pl -days 5 -t -v
17. Confirm the notice should be sent for your patron
18. Sign off

Signed-off-by: Katie Bliss <KEBliss@dmpl.org>

Bug 35700: (QA follow-up) fix POD and $today

This patch replaces the inncorrectly removed POD note and replaces $today with the more correct $date_to_run for checking if we should be skipping sending the message on the day.

Amended-by: Jonathan Druart
Squashed and adjusted commit title

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2025-10-07 08:47:49 UTC
Please don't forget to adjust the commit's title. You must tell what the patch does, not what the bug is.

https://wiki.koha-community.org/wiki/Commit_messages#Examples
Comment 14 Lucas Gass (lukeg) 2025-10-09 21:01:45 UTC
Nice work everyone!

Pushed to main for 25.11