Bug 12656

Summary: Allow a CANCELLATION_REASON to be specified on the cancel_expired_holds.pl job
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CirculationAssignee: Martin Renvoize <martin.renvoize>
Status: Pushed to master --- QA Contact: Nick Clemens <nick>
Severity: enhancement    
Priority: P5 - low CC: andrew, arm, barbara.johnson, black23, cslone, fiona.borthwick, george, gmcharlt, janet.mcgowan, jdemuth, jonathan.druart, jzairo, katrin.fischer, kyle.m.hall, kyle, lisetteslatah, lucas, m.de.rooy, marjorie.barry-vila, martin.renvoize, nick, tmisilo, viktor.sarge
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20453
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
These patches add a --reason flag to the cancel expired reserves cronjob. If a reason is provided the cronjob will trigger a HOLD_CANCELLATION notice for the patron if one is configured. The reason will be available in the notice template to allow custom messages to be sent
Version(s) released in:
20.11.00
Bug Depends on: 25534    
Bug Blocks: 27103    
Attachments: Bug 12656: Allow --reason to be passed to cancel_expired_holds
Bug 12656: Allow --reason to be passed to cancel_expired_holds
Attachment to Bug 12656 - Send a notice to patrons when their hold was cancelled because it was not picked up
Bug 12656: Allow --reason to be passed to cancel_expired_holds

Description Nicole C. Engard 2014-07-24 18:01:23 UTC
It would be nice to add a notice that tells patrons when their hold was cancelled because it wasn't picked up in time - not a notice for call cancels, just the one that was done due to a late pickup.
Comment 1 Agnes Rivers-Moore 2014-10-01 19:18:03 UTC
I agree. I am disappointed that Koha leaves no trace for staff or borrower when a hold is cancelled. 
Perhaps there could be a preference choice whether to send a notice for all cancelled holds, or just for those cancelled after waiting too long for pickup.
Having that notice show up in the borrower messages tab would be really useful!
Agnes
Comment 2 Marcel de Rooy 2015-06-25 10:23:13 UTC
(In reply to Agnes Rivers-Moore from comment #1)
> I agree. I am disappointed that Koha leaves no trace for staff or borrower
> when a hold is cancelled. 
> Perhaps there could be a preference choice whether to send a notice for all
> cancelled holds, or just for those cancelled after waiting too long for
> pickup.

The notice for not-picked-up-in-time would be good. But note that the user already got a notice about the waiting item. (It only does not tell us - by default - when the wait will expire. We added a paragraph to that notice about expiry within 7 days.)

I am even more interested (just like Agnes) in a notice when staff cancels a hold manually. Perhaps if there were some standard reasons for cancelling a hold (Not found/, it would be great too if a notice was sent that included the reason the staff user selected when cancelling the hold.
If Patron Messaging Preferences would include Hold cancelled, the user could enable/disable this notice (requires enhanced messaging).

Will change the title of this report a bit.
Comment 3 Tom Misilo 2015-06-25 10:26:24 UTC
I think it would be great to be able to cancel hold's silently as well. 

For example if a patron places a hold on an e-book (which at least for us), cannot actually be picked up in the library. Or if someone places a hold on a Course Reserve item, even though we do not allow hold's on course reserves.
Comment 4 Marcel de Rooy 2018-11-12 12:10:52 UTC
Any news here?
Comment 5 Agnes Rivers-Moore 2019-04-23 22:45:32 UTC
The 'holds history' function allows staff to see a borrower's cancelled holds. Tha helps, but is not a complete answer. Still would like the borrower notification for cancelled due to late pickup. Also would like a notice with 'cancelled for x reason' options, that staff could choose to send or not send at the time of cancellation.
Comment 6 Katrin Fischer 2019-04-24 05:28:11 UTC
(In reply to Agnes Rivers-Moore from comment #5)
> The 'holds history' function allows staff to see a borrower's cancelled
> holds. Tha helps, but is not a complete answer. Still would like the
> borrower notification for cancelled due to late pickup. Also would like a
> notice with 'cancelled for x reason' options, that staff could choose to
> send or not send at the time of cancellation.

+1!
Comment 7 Jessie Zairo 2019-08-01 19:13:47 UTC
I am also interested in a notice when staff cancels a hold manually. I agree, maybe an authorized value or standard reasons for cancelling a hold (lost / missing / withdrawn). The notice could included the reason the hold was cancelled.

I like the idea of Patron Messaging Preferences including Hold cancelled.
Comment 8 Andrew Fuerste-Henry 2020-04-29 18:14:55 UTC
BTW, bug 20453 is nearly a duplicate of this one. It adds a notice for when a hold is cancelled, but only when it's cancelled manually.
Comment 9 Martin Renvoize 2020-10-02 12:52:10 UTC
How does this relate to bug 25534 ?
Comment 10 Katrin Fischer 2020-10-04 18:27:54 UTC
The other bug is about a cancelled hold - this is about holds the patrons haven't picked up during the time they have been waiting at the library. I think this would be a special case maybe?
Comment 11 Katrin Fischer 2020-10-04 18:31:15 UTC
(In reply to Katrin Fischer from comment #10)
> The other bug is about a cancelled hold - this is about holds the patrons
> haven't picked up during the time they have been waiting at the library. I
> think this would be a special case maybe?

Maybe we could just add the option with the cancellation reason to waitingreserves.pl too and then libraries could define "not picked up" as one of the reasons.
Comment 12 Martin Renvoize 2020-10-22 15:27:54 UTC
Created attachment 112199 [details] [review]
Bug 12656: Allow --reason to be passed to cancel_expired_holds

This patch adds the --reason option to cancel_expired_holds which allows
the library to optionally set a reason for cancellation when running the
cronjob. This will prompt the HOLD_CANCELLED notice to be sent to the
patron.
Comment 13 Martin Renvoize 2020-10-22 15:42:00 UTC
Created attachment 112200 [details] [review]
Bug 12656: Allow --reason to be passed to cancel_expired_holds

This patch adds the --reason option to cancel_expired_holds which allows
the library to optionally set a reason for cancellation when running the
cronjob. This will prompt the HOLD_CANCELLED notice to be sent to the
patron.

To test:
1/ Ensure the unit tests continue to pass after the patch
   (t/db_dependent/Reserves/CancelExpiredReserves.t)
Comment 14 Katrin Fischer 2020-10-22 20:49:37 UTC
E-Mails, cron and unit tests cannot be tested on a sandbox, so removing the Academy keyword.
Comment 15 Lisette Scheer 2020-10-24 00:18:09 UTC
Created attachment 112337 [details] [review]
Attachment to Bug 12656 - Send a notice to patrons when their hold was cancelled because it was not picked up

Bug 12656: Allow --reason to be passed to cancel_expired_holds

This patch adds the --reason option to cancel_expired_holds which allows
the library to optionally set a reason for cancellation when running the
cronjob. This will prompt the HOLD_CANCELLED notice to be sent to the
patron.

To test:
1/ Ensure the unit tests continue to pass after the patch
   (t/db_dependent/Reserves/CancelExpiredReserves.t)

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>
Comment 16 Nick Clemens 2020-10-24 01:10:36 UTC
Created attachment 112350 [details] [review]
Bug 12656: Allow --reason to be passed to cancel_expired_holds

This patch adds the --reason option to cancel_expired_holds which allows
the library to optionally set a reason for cancellation when running the
cronjob. This will prompt the HOLD_CANCELLED notice to be sent to the
patron.

To test:
1/ Ensure the unit tests continue to pass after the patch
   (t/db_dependent/Reserves/CancelExpiredReserves.t)

Also:
1 - Add an expired hold for a patron:
    INSERT INTO RESERVES (borrowernumber, biblionumber, expirationdate, found,branchcode,itemnumber) VALUES (5,5,'2020-01-01','W','CPL',983);
2 - Set ExpireReservesMaxPickUpDelay to Allow
3 - Run the cronjob:
    perl misc/cronjobs/hold/cancel_expired_holds.pl --reason EXPIRED
4 - Visit the patron's notices tab
5 - Confirm they have been sent a cancellation notice

Signed-off-by: Lisette Scheer <lisettes@latahlibrary.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Nick Clemens 2020-10-24 01:12:47 UTC
I was unsure at first about not being able to add a reason without sending the notice, however, it seems like you just won't add a reason if you don't want to send.

You can also force the reason and prevent sending by wrapping the notice in a conditional:
[% UNLESS hold.cancellation_reason == 'INSANITY' %]
...
[% END %]
Comment 18 Martin Renvoize 2020-10-24 07:33:39 UTC
(In reply to Nick Clemens from comment #17)
> I was unsure at first about not being able to add a reason without sending
> the notice, however, it seems like you just won't add a reason if you don't
> want to send.
> 
> You can also force the reason and prevent sending by wrapping the notice in
> a conditional:
> [% UNLESS hold.cancellation_reason == 'INSANITY' %]
> ...
> [% END %]

Totally agree, at first o felt it a bit odd you couldn't add a reason without sending the notice but I came to the same conclusion regarding doing it at the TT template conditional level. I think if we wanted a nicer way to allow addition of a reason without triggering the notice we should do that as its own bug as we're just using the logic introduced in the dependancy bug here.

Thanks for the testing and QA everyone. 😀
Comment 19 Jonathan Druart 2020-11-04 12:02:11 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 20 Lucas Gass 2020-11-16 21:14:17 UTC
enhancement will not be backported to 20.05.x