Bug 12656 - Allow a CANCELLATION_REASON to be specified on the cancel_expired_holds.pl job
Summary: Allow a CANCELLATION_REASON to be specified on the cancel_expired_holds.pl job
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Nick Clemens
URL:
Keywords:
Depends on: 25534
Blocks: 27103
  Show dependency treegraph
 
Reported: 2014-07-24 18:01 UTC by Nicole C. Engard
Modified: 2020-11-27 10:41 UTC (History)
23 users (show)

See Also:
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


Attachments
Bug 12656: Allow --reason to be passed to cancel_expired_holds (3.14 KB, patch)
2020-10-22 15:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12656: Allow --reason to be passed to cancel_expired_holds (4.81 KB, patch)
2020-10-22 15:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Attachment to Bug 12656 - Send a notice to patrons when their hold was cancelled because it was not picked up (4.88 KB, patch)
2020-10-24 00:18 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 12656: Allow --reason to be passed to cancel_expired_holds (5.34 KB, patch)
2020-10-24 01:10 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 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