Bug 15986 - Add a script for sending hold waiting reminder notices
Summary: Add a script for sending hold waiting reminder notices
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-04 20:33 UTC by Nick Clemens
Modified: 2020-03-02 14:57 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15986 - Add a script for sending hold waiting reminder notices (30.00 KB, patch)
2016-07-12 17:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15986 - Add a script for sending hold waiting reminder notices (29.60 KB, patch)
2016-07-13 00:21 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15986 - Add a script for sending hold waiting reminder notices (30.64 KB, patch)
2016-07-21 16:40 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15986 - Add a script for sending hold waiting reminder notices (30.70 KB, patch)
2016-09-20 16:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15986: Add a script for sending hold waiting reminder notices (21.28 KB, patch)
2019-07-25 13:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15986: Add a script for sending hold waiting reminder notices (22.22 KB, patch)
2019-07-25 14:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15986: Add a script for sending hold waiting reminder notices (22.21 KB, patch)
2020-03-02 14:21 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2016-03-04 20:33:46 UTC
Write a script that will take two parameters:

a) Length of time hold has been waiting ( in days )

b) Letter code of notice to be emailed

2) Script will take these two parameters and send the specified notice for each hold a patron has.
Comment 1 Nick Clemens 2016-07-12 17:28:42 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-07-13 00:21:13 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2016-07-18 03:24:51 UTC
Comment on attachment 53354 [details] [review]
Bug 15986 - Add a script for sending hold waiting reminder notices

Review of attachment 53354 [details] [review]:
-----------------------------------------------------------------

::: misc/cronjobs/holds_reminder.pl
@@ +1,4 @@
> +#!/usr/bin/perl
> +
> +# Copyright 2008 Liblime
> +# Copyright 2010 BibLibre

Can we please fix the copyright notices, this is a new script right?
Comment 4 Chris Cormack 2016-07-18 03:26:34 UTC
Just a general comment, we have a setting where borrowers can opt in/out of holds filled notices.
Should this script respect that preference, IE if they don't get a hold filled notice will they be happy getting one saying it's waiting. Annoying patrons is something libraries generally like to avoid :)

It should be easy enough to use the check we have for holds filled in circulation I hope
Comment 5 Nick Clemens 2016-07-21 16:40:42 UTC
Created attachment 53594 [details] [review]
Bug 15986 - Add a script for sending hold waiting reminder notices

This patch adds a script for sending holds reminder notice to patrons.
Essentially it is a modification of the overdue_notices.pl script

To test:
1 - Create a hold reminder notice (Koha module: Holds)
2 - Set some waiting holds for patrons
3 - Run the script with various options and verify correct data is
    generated
4 - Verify that with 'optout' switch set only patrons who have email
hold filled notices specified will recieve a reminder

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)
Comment 6 Nick Clemens 2016-07-21 16:47:17 UTC
(In reply to Chris Cormack from comment #4)
> Just a general comment, we have a setting where borrowers can opt in/out of
> holds filled notices.
> Should this script respect that preference, IE if they don't get a hold
> filled notice will they be happy getting one saying it's waiting. Annoying
> patrons is something libraries generally like to avoid :)

Good point, I added it as a switch, some libraries charge fines for holds not picked up etc, so they may want to force the notification, I figure this way makes it flexible
Comment 7 Chris Cormack 2016-07-21 21:31:25 UTC
Yep sounds good, I'll give it a test when I'm back at work
Comment 8 Kyle M Hall 2016-09-20 16:19:01 UTC
Created attachment 55723 [details] [review]
[SIGNED-OFF] Bug 15986 - Add a script for sending hold waiting reminder notices

This patch adds a script for sending holds reminder notice to patrons.
Essentially it is a modification of the overdue_notices.pl script

To test:
1 - Create a hold reminder notice (Koha module: Holds)
2 - Set some waiting holds for patrons
3 - Run the script with various options and verify correct data is
    generated
4 - Verify that with 'optout' switch set only patrons who have email
hold filled notices specified will recieve a reminder

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)

Signed-off-by: Robert <rhilliker@hotchkiss.org>
Comment 9 Katrin Fischer 2016-11-01 23:02:59 UTC
I love the amount of documentation for this script!

I didn't get to test this yet, but while reading through it I wondered:
There is a lot of work done in order to allow exporting in different formats. I was wondering if it might be better to leave the job of creating a printable version to gather_print_notices.pl working towards a more unified workflow for all the printed letters.
Comment 10 Marcel de Rooy 2016-11-02 15:36:55 UTC
I like the feature, but would love to see less (new) code in cron scripts and more code in our modules. With advantages of testing etc.
Comment 11 Sally 2017-01-16 14:13:59 UTC
This is a really exciting feature.

I know it's been sent back to discussion from being signed off - what's the next step for moving it forward?  We think this would make a fantastic addition to Koha.
Comment 12 Chris Cormack 2017-01-17 04:09:38 UTC
(In reply to Sally Healey from comment #11)
> This is a really exciting feature.
> 
> I know it's been sent back to discussion from being signed off - what's the
> next step for moving it forward?  We think this would make a fantastic
> addition to Koha.

To move it forward it needs comment 10 addressed, and a lot of the code moved out of the .pl file into a module, with unit tests written. 
This shouldn't be a big job, but will mean way less chance of breakage/maintenance in the future.
As well as a signoff from outside of the sponsoring library.

I think comment 9 could be a future enhancement. But mostly comment 10 (moving the code to a module and writing tests) and an independent sign off are what are need
Comment 13 Katrin Fischer 2017-01-17 23:38:25 UTC
I think if we do #9 later it will introduce a change in workflow - if we did it from the beginning, we could safe people from having to change their setups. But I am not blocking this, just explaining my point of view maybe also for future enh.
Comment 14 Nick Clemens 2019-07-25 13:58:31 UTC
Created attachment 91767 [details] [review]
Bug 15986: Add a script for sending hold waiting reminder notices

This patch adds a script for sending holds reminder notice to patrons.

We add a 'send_notice' routine to Koha::Patrons - this will either send using the patron's
email prefs, or allow forcing of a single method via the cron

To test:
 1 - Create an email hold reminder notice for a single library (Koha module: Holds, code HOLDREMINDER, branch: CPL)
 2 - Set some waiting holds today for patrons at CPL, ensure those patrons have 'email' as the transport for hold filled notices
 3 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL
 4 - You should see the patrons here would have received emails
 5 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL
 6 - You should see the emails that were sent
 7 - Check the patron notices tab to confirm
 8 - Note a ptron with two holds waiting receives only one notice
 9 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
10 - No notices are sent
11 - Adjust the waiting date for the holds:
    UPDATE reserves SET waitingdate=DATE_SUB(CURDATE(), INTERVAL 3 DAYS) WHERE waitingdate = CURDATE();
12 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
13 - Confirm the holds are now reminded
14 - Set yesterday as a holiday for CPL
15 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 3
16 - Notices should not be sent
17 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 4
18 - Notices should be sent again
19 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4
20 - Should get feedback that notice was not found for other libraries
21 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
22 - Notice is not found
23 - Add the notice for sms
24 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
25 - The notice should be sent
26 - Check patrons messaging tab to confirm
27 - prove -v t/db_dependent/Koha/Patrons.t

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)
Comment 15 Nick Clemens 2019-07-25 14:00:10 UTC
(In reply to Chris Cormack from comment #12)
> I think comment 9 could be a future enhancement. But mostly comment 10
> (moving the code to a module and writing tests) and an independent sign off
> are what are need

Code for sending moved to Koha::Patron::send_notice
Comment 16 Nick Clemens 2019-07-25 14:25:52 UTC
(In reply to Katrin Fischer from comment #13)
> I think if we do #9 later it will introduce a change in workflow - if we did
> it from the beginning, we could safe people from having to change their
> setups. But I am not blocking this, just explaining my point of view maybe
> also for future enh.

Removed the option to print out notices, now fallback to print if no email/sms
Comment 17 Nick Clemens 2019-07-25 14:40:24 UTC
Created attachment 91768 [details] [review]
Bug 15986: Add a script for sending hold waiting reminder notices

This patch adds a script for sending holds reminder notice to patrons.

We add a 'send_notice' routine to Koha::Patrons - this will either send using the patron's
email prefs, or allow forcing of a single method via the cron

To test:
 1 - Create an email hold reminder notice for a single library (Koha module: Holds, code HOLDREMINDER, branch: CPL)
 2 - Set some waiting holds today for patrons at CPL, ensure those patrons have 'email' as the transport for hold filled notices
 3 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL
 4 - You should see the patrons here would have received emails
 5 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL
 6 - You should see the emails that were sent
 7 - Check the patron notices tab to confirm
 8 - Note a ptron with two holds waiting receives only one notice
 9 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
10 - No notices are sent
11 - Adjust the waiting date for the holds:
    UPDATE reserves SET waitingdate=DATE_SUB(CURDATE(), INTERVAL 3 DAYS) WHERE waitingdate = CURDATE();
12 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
13 - Confirm the holds are now reminded
14 - Set yesterday as a holiday for CPL
15 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 3
16 - Notices should not be sent
17 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 4
18 - Notices should be sent again
19 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4
20 - Should get feedback that notice was not found for other libraries
21 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
22 - Notice is not found
23 - Add the notice for sms
24 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
25 - The notice should be sent
26 - Check patrons messaging tab to confirm
27 - prove -v t/db_dependent/Koha/Patrons.t

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)
Comment 18 David Nind 2020-02-17 08:23:15 UTC
Patch no longer applies
Comment 19 Nick Clemens 2020-03-02 14:21:19 UTC
Created attachment 99919 [details] [review]
Bug 15986: Add a script for sending hold waiting reminder notices

This patch adds a script for sending holds reminder notice to patrons.

We add a 'send_notice' routine to Koha::Patrons - this will either send using the patron's
email prefs, or allow forcing of a single method via the cron

To test:
 1 - Create an email hold reminder notice for a single library (Koha module: Holds, code HOLDREMINDER, branch: CPL)
 2 - Set some waiting holds today for patrons at CPL, ensure those patrons have 'email' as the transport for hold filled notices
 3 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL
 4 - You should see the patrons here would have received emails
 5 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL
 6 - You should see the emails that were sent
 7 - Check the patron notices tab to confirm
 8 - Note a ptron with two holds waiting receives only one notice
 9 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
10 - No notices are sent
11 - Adjust the waiting date for the holds:
    UPDATE reserves SET waitingdate=DATE_SUB(CURDATE(), INTERVAL 3 DAYS) WHERE waitingdate = CURDATE();
12 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -li CPL -days 3
13 - Confirm the holds are now reminded
14 - Set yesterday as a holiday for CPL
15 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 3
16 - Notices should not be sent
17 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -li CPL -holidays -days 4
18 - Notices should be sent again
19 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4
20 - Should get feedback that notice was not found for other libraries
21 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
22 - Notice is not found
23 - Add the notice for sms
24 - perl misc/cronjobs/holds_reminder.pl -v -lettercode HOLDREMINDER -n -holidays -days 4 -mtt sms
25 - The notice should be sent
26 - Check patrons messaging tab to confirm
27 - prove -v t/db_dependent/Koha/Patrons.t

Sponsored by: The Hotchkiss School (http://www.hotchkiss.org/)
Comment 20 Jonathan Druart 2020-03-02 14:57:04 UTC
Is the test_mode parameter of the method really useful?