Bug 27208 - Add a configurable time delay feature to hold notice templates
Summary: Add a configurable time delay feature to hold notice templates
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Björn Nylén
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-11 16:02 UTC by Barbara Johnson
Modified: 2023-11-02 21:18 UTC (History)
8 users (show)

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


Attachments
Bug 27208: Add a user specified delay before sending pickup notices (19.09 KB, patch)
2021-12-10 10:08 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Atomicupdate (1.20 KB, patch)
2021-12-10 10:09 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Schema changes (806 bytes, patch)
2021-12-10 10:10 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Unit tests (3.00 KB, patch)
2021-12-10 10:11 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Add a user specified delay before sending pickup notices. (19.09 KB, patch)
2021-12-16 10:17 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Atomicupdate (1.19 KB, patch)
2021-12-16 10:18 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Schema changes (835 bytes, patch)
2021-12-16 10:18 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Unit tests (3.03 KB, patch)
2021-12-16 10:18 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Add the ability to delay pickup notices. (23.41 KB, patch)
2022-01-02 16:08 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Atomicupdate (1.68 KB, patch)
2022-01-02 16:08 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Schema changes (835 bytes, patch)
2022-01-02 16:08 UTC, Björn Nylén
Details | Diff | Splinter Review
Bug 27208: Unit tests (7.46 KB, patch)
2022-01-02 16:09 UTC, Björn Nylén
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barbara Johnson 2020-12-11 16:02:24 UTC
It would be helpful to have a configurable time delay feature added to the notice templates.  In our case, items with holds are immediately trapped, and the hold notice gets sent, when they get returned through our sorter.  However, we are quarantining materials for 72 hours so we don’t want the hold pickup notice sent until the items come out of quarantine.  Other possible uses for a configurable time delay might be when one library in a system is quarantining items but another is not.  In a consortium, with a mix of school and public libraries, they might want to send notices at different times.  Other libraries have mentioned their patrons arrive at the library because they got their pickup notice but staff hasn’t actually had time to process the holds.  This would be especially helpful with hold notices but could also apply to other notices.
Comment 1 Rebecca Coert 2021-04-13 22:08:29 UTC
+1 This would be very useful for all types of notices.  Setting a "send time" for each notice type would make this fully customizable for a multitude of reasons.
Comment 2 Björn Nylén 2021-12-10 10:08:51 UTC
Created attachment 128387 [details] [review]
Bug 27208: Add a user specified delay before sending pickup notices
Comment 3 Björn Nylén 2021-12-10 10:09:37 UTC
Created attachment 128388 [details] [review]
Bug 27208: Atomicupdate
Comment 4 Björn Nylén 2021-12-10 10:10:52 UTC
Created attachment 128389 [details] [review]
Bug 27208: Schema changes
Comment 5 Björn Nylén 2021-12-10 10:11:45 UTC
Created attachment 128390 [details] [review]
Bug 27208: Unit tests
Comment 6 Björn Nylén 2021-12-10 10:26:13 UTC
Proposing this solution. Th patch adds a new column to the message_queue, delay_until, that contains a time that the notice should be delayed until. 
Om the returns page there are a new circulation setting, Delay pickup notice until, where the librarian can set a time the notice will be delyed until.
On the patron account notice log, there will be a note indicating the delayed messages.(pending, delayed until [datetime])

For us the main usage in our stacks where books are processed as they are picked but they might not be available for the patron until a few hours later in the day.

The change to the message queue should be backwards compatible as the delay_until will be set to null unless a time is provided and those messages will be processed as before. 

To test:

1. Apply patch and run updatedatabase.pl to add new column to message_queue.
2. Check in an item with a hold to produce a pickup notice.
3. Select a time for pickup notice delay. Note the warning about the delay.
4. Check in another item with hold to produce a pickup notice.
5. Check the patron notice logs, the one with a delay should read "pending, delayed until ...".
6. Run process_message_queue.pl before and after the selected time above.
The delayed message should not be sent until after the set time.
Comment 7 Katrin Fischer 2021-12-12 14:59:28 UTC
Hi Björn, 

thx for your patches! Some first things I noticed:

1) QA test tools
There are some problems noticed by the QA test tools. It's a good habit to run them yourself before submitting patches to catch these kind of things:

 FAIL	circ/returns.pl
   FAIL	  critic
		# Variables::ProhibitConditionalDeclarations: Got 1 violation(s). 

 FAIL	installer/data/mysql/atomicupdate/bug_27208-add_delay_util_to_message_queue.pl
   FAIL	  file permissions
		File must have the exec flag
   FAIL	  valid
		Useless use of anonymous hash ({}) in void context 

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
   FAIL	  forbidden patterns
		forbidden pattern: console.log (line 1167)

2) Authorship of patches

The first 2 patches don't have your authorship:

Author: root <root@9b7442205204>

3) Functionality

It looks like this will allow for manually setting a time until the notice will be delayed on the checkin page. I am not sure if this is what Barbara had in mind here as it won't work for self checkins using the web based checkin or the RFID based ones. - And it also might be quite easy to get things confused if you have multiple circulation desks.

I wonder if it would not be easier/better to make this a configuration setting, but probably a little harder to do. I am thinking circulation rules, since you might want to differentiate for different branches, but possibly a global setting could also be a good starting point.

What do others think?
Comment 8 Björn Nylén 2021-12-14 10:50:52 UTC
Thx for pointing out the qa-tools, will correct the patches.

For the functionality, we feel that it's neccessary for staff to be able to set the delay depending on situation, even within the same library. 

Controling it through circ-rules might be an issue depending on the whether rules are set by holdig or home branch. The processing is obviously done the holding branch but circ rules may be set according the the homebranch?

One solution may be to have a syspref eg. PickupNoticeDelayMode that can be set to off|fixed|staff that controls how a delay is set, and a PickupNoticeDelayDuration pref to set the fixed time. Should be fairly straightforward to implement, a fixed duration could be calculated in ModReserveAffect to capture both staff and selfcheck machines.
Comment 9 Björn Nylén 2021-12-16 10:17:59 UTC
Created attachment 128587 [details] [review]
Bug 27208: Add a user specified delay before sending pickup notices.

This bug adds the ability to delay sending a notice until a set timestamp.
It also adds a user selectable time on the returns page to delay sending of
the pickup notice. The notice delay will also be visible in the patron notice log.

To test:

1. Apply patch and run updatedatabase.pl to add new column to message_queue.
2. Check in an item with a hold to produce a pickup notice.
3. Select a time for pickup notice delay. Note the warning about the delay.
4. Check in another item with hold to produce a pickup notice.
5. Check the patron notice logs, the one with a delay should read "pending, delayed until ...".
6. Run process_message_queue.pl before and after the selected time above.
The delayed message should not be sent until after the set time.
Comment 10 Björn Nylén 2021-12-16 10:18:03 UTC
Created attachment 128588 [details] [review]
Bug 27208: Atomicupdate
Comment 11 Björn Nylén 2021-12-16 10:18:07 UTC
Created attachment 128589 [details] [review]
Bug 27208: Schema changes
Comment 12 Björn Nylén 2021-12-16 10:18:10 UTC
Created attachment 128590 [details] [review]
Bug 27208: Unit tests

This commit adds unit test for changes in C4/Letters.pm
Comment 13 Björn Nylén 2021-12-16 10:21:20 UTC
Updated patches that passed the qa-tools. Set back to Needs signoff and awaiting any comments on the functionality.
Comment 14 Barbara Johnson 2021-12-16 16:35:02 UTC
We use an automated materials handling system.  As items are returned through the sorter holds are trapped, sorted, checked in and a hold pickup slip is generated.  This all happens via SIP outside of Koha. So for our library we would be looking for an automated setting to control the delay rather than having to re-checkin the item in Koha and manually select a notice delay for each item.  

I like the idea of a system pref such as the suggested PickupNoticeDelayMode and the PickupNoticeDelayDuration pref to set the the amount of delay.
Comment 15 Björn Nylén 2022-01-02 16:08:48 UTC
Created attachment 128948 [details] [review]
Bug 27208: Add the ability to delay pickup notices.

This bug adds the ability to delay sending a pickup notice. This can be done in two
ways, either by a fixed configurable delay of all pickup notices or a time selectable
by staff at checkin.  This is controlled by two new system preferences,
PickupNoticeDelayMode and PickupNoticeDelayDuration. A delayed notice will also be
visible in the patron notice log.

To test:

1. Apply patch and run updatedatabase.pl to add new column to message_queue.
2. Check in items with holds with different delay modes ('no delay','fixed delay','selectable by staff').
3. Check the patron notice logs to check notice delay. Delayed notices have status 'pending, delayed until...'
4. Expected behaviour is:
   'No delay': No notices are delayed.
   'Fixed delay': All pickup notices are delayed by the no of hours set in PickupNoticeDelayDuration.
   'Delay selectable by staff': Pickup notice is delayed until the time set in UI. If no delay is set,
                                notice will not be delayed. Also note the warning at the checkin page
                                when a delay is set.
5. Run process_message_queue.pl before and after the delay times above. Notices without delay are sent
   immediatly, delayed notices are only sent after the set time.
6. Tests should pass.
Comment 16 Björn Nylén 2022-01-02 16:08:52 UTC
Created attachment 128949 [details] [review]
Bug 27208: Atomicupdate
Comment 17 Björn Nylén 2022-01-02 16:08:57 UTC
Created attachment 128950 [details] [review]
Bug 27208: Schema changes
Comment 18 Björn Nylén 2022-01-02 16:09:01 UTC
Created attachment 128951 [details] [review]
Bug 27208: Unit tests

This commit adds unit test for changes in C4/Letters.pm and C4/Reserves.pm
Comment 19 Björn Nylén 2022-01-02 16:11:13 UTC
Updated patch. Behavior is now controlled by PickupNoticeDelayMode and allows either a fixed delay or one set by the librarian at checkin.
Comment 20 David Cook 2022-01-03 23:17:31 UTC
(This is just a side note but I wonder if another way of doing this would be implementing a TTL mechanism.)
Comment 21 Lucas Gass 2023-11-02 21:18:01 UTC
Patchset does not apply cleanly to master.