|
Description
Lari Strand
2024-01-04 10:45:45 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)" Created attachment 187112 [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 i 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 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 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 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. 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> 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? (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. 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. (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. 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> 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 Nice work everyone! Pushed to main for 25.11 |