31540 – Holds reminder cronjob should consider expiration date of holds, and not send notices if hold expired

Bug 31540 - Holds reminder cronjob should consider expiration date of holds, and not send notices if hold expired

Summary: Holds reminder cronjob should consider expiration date of holds, and not send...

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Hold requests (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low normal
Assignee:	Nick Clemens (kidclamp)
QA Contact:	Katrin Fischer

URL:
Keywords:

Depends on:
Blocks:

Reported:	2022-09-08 20:04 UTC by Nick Clemens (kidclamp)
Modified:	2023-12-28 20:43 UTC (History)
CC List:	4 users (show)

See Also:
Change sponsored?:	---
Patch complexity:	Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:	22.11.00, 22.05.08, 21.11.15
Circulation function:

Attachments
Bug 31540: Exclude expired holds from the reminder job (1.45 KB, patch) 2022-09-12 14:39 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 31540: Exclude expired holds from the reminder job (1.50 KB, patch) 2022-09-13 19:50 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 31540: Exclude expired holds from the reminder job (1.56 KB, patch) 2022-09-26 21:50 UTC, Katrin Fischer	Details \| Diff \| Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Nick Clemens (kidclamp) 2022-09-08 20:04:04 UTC

Comment 1 Nick Clemens (kidclamp) 2022-09-12 14:39:12 UTC

Created attachment 140461 [details] [review]
Bug 31540: Exclude expired holds from the reminder job

Some libraries don't want to auto-cancel holds, but we should not remind
a patron about a hold which has expired.

To test:
1 - Place a hold for a patron
2 - Set it waiting
3 - Run the holds reminder script in the future
    perl misc/cronjobs/holds/holds_reminder.pl -day 1 --date '2023-09-12' -v
4 - Note the holds would be reminded
5 - Set expirationdate for the hold less than today
    UPDATE reserves SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY);
6 - Run the remidner cron again
7 - No holds trigger!

Comment 2 David Nind 2022-09-13 19:50:02 UTC

Created attachment 140605 [details] [review]
Bug 31540: Exclude expired holds from the reminder job

Some libraries don't want to auto-cancel holds, but we should not remind
a patron about a hold which has expired.

To test:
1 - Place a hold for a patron
2 - Set it waiting
3 - Run the holds reminder script in the future
    perl misc/cronjobs/holds/holds_reminder.pl -day 1 --date '2023-09-12' -v
4 - Note the holds would be reminded
5 - Set expirationdate for the hold less than today
    UPDATE reserves SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY);
6 - Run the remidner cron again
7 - No holds trigger!

Signed-off-by: David Nind <david@davidnind.com>

Comment 3 Katrin Fischer 2022-09-26 21:50:09 UTC

Created attachment 141014 [details] [review]
Bug 31540: Exclude expired holds from the reminder job

Some libraries don't want to auto-cancel holds, but we should not remind
a patron about a hold which has expired.

To test:
1 - Place a hold for a patron
2 - Set it waiting
3 - Run the holds reminder script in the future
    perl misc/cronjobs/holds/holds_reminder.pl -day 1 --date '2023-09-12' -v
4 - Note the holds would be reminded
5 - Set expirationdate for the hold less than today
    UPDATE reserves SET expirationdate = DATE_SUB(CURDATE(), INTERVAL 1 DAY);
6 - Run the remidner cron again
7 - No holds trigger!

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Comment 4 Katrin Fischer 2022-09-26 21:50:46 UTC

Nick, you'll have to send me cookies for the next time I have to fix your Assignee!

Comment 5 Tomás Cohen Arazi (tcohen) 2022-10-03 11:50:29 UTC

Pushed to master for 22.11.

Nice work everyone, thanks!

Comment 6 Lucas Gass (lukeg) 2022-12-05 15:35:58 UTC

Backported to 22.05.x for upcoming 22.05.08 release

Comment 7 Arthur Suzuki 2022-12-14 09:43:44 UTC

applied to 21.11 for 21.11.15

Comment 8 Wainui Witika-Park 2022-12-20 01:37:37 UTC

Backported to 21.05.x for 21.05.22

Comment 9 Wainui Witika-Park 2022-12-23 00:21:09 UTC

Not releasing in 21.05.22

Comment 10 Wainui Witika-Park 2023-03-15 00:45:37 UTC

(In reply to wainuiwitikapark from comment #9)
> Not releasing in 21.05.22

Reverted from 21.05 branch