Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold
Summary: Allow automatically canceled expired waiting holds to fill the next hold
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
: 10827 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-06-08 15:24 UTC by Kyle M Hall
Modified: 2019-08-15 20:46 UTC (History)
19 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 14364 - Allow automatically canceled expired waiting holds to fill the next hold (33.05 KB, patch)
2015-06-08 16:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (33.11 KB, patch)
2015-07-30 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (32.66 KB, patch)
2015-11-25 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364: (QA followup) (1.33 KB, patch)
2016-03-30 22:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (36.65 KB, patch)
2016-03-30 22:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14364: (QA followup) (1.33 KB, patch)
2016-04-03 13:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (36.81 KB, patch)
2016-06-13 12:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364: (QA followup) (1.33 KB, patch)
2016-06-13 12:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364 [QA Followup] - Add unit tests (4.76 KB, patch)
2016-06-13 12:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (17.84 KB, patch)
2018-02-02 13:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold (17.25 KB, patch)
2018-10-05 14:54 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-06-08 15:24:26 UTC
Right now, if a library automatically cancels expired waiting holds, a librarian must still re-checkin an item to trap the next available hold for that item. It would be better if the next hold was automatically trapped and the librarians receive an email notification so they can make any changes to the item if need be ( hold area, hold slip in item, etc ).
Comment 1 Kyle M Hall 2015-06-08 16:10:51 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-07-30 15:35:07 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-08-05 08:29:02 UTC
I would like to get another signoff on this one please.
Comment 4 Christopher Brannon 2015-09-22 14:42:51 UTC
I'm not sure how many libraries would like this workflow.  Having the system to make changes to the item and notify a patron before the staff have a chance to handle it is not a common practice.  I haven't had a chance to test it yet, but wouldn't the staff grabbing the item and checking it in to generate a slip cause it to send out another notification?

Having a hold notify a patron before the item is prepped can introduce some issues.  For example, what if the person comes in before the staff have read the e-mail and processed the book.  How do other staff find the item?  We often have patrons come to the desk literally minutes after the notification goes out, and the patron happens to be in the library.  This produces a kink in the workflow and has the potential to frustrate both patron and staff.

What would be preferred is the following (at least at our library):

1) Waiting hold expires or is cancelled.
2) Staff alerted and item status is changed so as not to show that it is available, but the hold is over.
3) Staff grab books and check them in.
4) Koha sees that they have expired or cancelled holds and asks if you would like to move the item on to the next hold (if one exists), or renew the hold.
5) A new slip is generated in either case, and the item is moved on or back on the hold shelf.

This is just my opinion, but I think my suggested workflow is more acceptable.

I'm not changing the status on the enhancement to in discussion, since this is an option and not a fundamental change, but I'm not sure how many folks will go for this.

Christopher
Comment 5 HB-NEKLS 2015-09-22 16:45:03 UTC
As a multi-library consortia, this feature would be unusable for us, especially using a single email field in the syspref -- it would be much better to use the email address in the branches table and email the holdingbranch email address of the item when a hold awaiting pickup is canceled. 

Additionally, I agree with Christopher's suggested workflow, having seen patrons come in for holds awaiting pickup before the items even make it to the circ desk.
Comment 6 Kyle M Hall 2015-09-24 15:30:53 UTC
(In reply to Christopher Brannon from comment #4)
> I'm not sure how many libraries would like this workflow.  Having the system
> to make changes to the item and notify a patron before the staff have a
> chance to handle it is not a common practice.  I haven't had a chance to
> test it yet, but wouldn't the staff grabbing the item and checking it in to
> generate a slip cause it to send out another notification?

Since the hold is already waiting at that point, another notification would not be sent.

> Having a hold notify a patron before the item is prepped can introduce some
> issues.  For example, what if the person comes in before the staff have read
> the e-mail and processed the book.  How do other staff find the item?  We
> often have patrons come to the desk literally minutes after the notification
> goes out, and the patron happens to be in the library.  This produces a kink
> in the workflow and has the potential to frustrate both patron and staff.
> 
> What would be preferred is the following (at least at our library):
> 
> 1) Waiting hold expires or is cancelled.
> 2) Staff alerted and item status is changed so as not to show that it is
> available, but the hold is over.
> 3) Staff grab books and check them in.
> 4) Koha sees that they have expired or cancelled holds and asks if you would
> like to move the item on to the next hold (if one exists), or renew the hold.
> 5) A new slip is generated in either case, and the item is moved on or back
> on the hold shelf.
> 
> This is just my opinion, but I think my suggested workflow is more
> acceptable.

That is essentially Koha's existing holds workflow. The only difference would be to make item's as unavailable if the item would fill the next hold. It's not insurmountable, but would definitely be a separate feater that could live along side of this one ( in an either/or fashion ).

> I'm not changing the status on the enhancement to in discussion, since this
> is an option and not a fundamental change, but I'm not sure how many folks
> will go for this.

Input is always appreciated. Indeed this is an entirely optional workflow that I'm sure will work great for some libraries ( like the one that requested the feature ) and not at all for others ( like you and Heather ).
Comment 7 Kyle M Hall 2015-09-24 15:33:45 UTC
(In reply to Heather Braum from comment #5)
> As a multi-library consortia, this feature would be unusable for us,
> especially using a single email field in the syspref -- it would be much
> better to use the email address in the branches table and email the
> holdingbranch email address of the item when a hold awaiting pickup is
> canceled. 

The new email pref is entirely optional. If it is not filled in, the feature defaults to the branch email, and if that is not filled in, the koha admin email address.

> Additionally, I agree with Christopher's suggested workflow, having seen
> patrons come in for holds awaiting pickup before the items even make it to
> the circ desk.

Please see my notes on that above! I don't think it's an insurmountable issue even without additional features. I imagine the patron would walk up, say "You've got book X on hold for me", and then the librarian would grab the book, check it in, and see that it is indeed waiting for them. At that point the librarian could check out the book as usual. If I'm missing anything please let me know. It's been a while since I've worked the circ desk ; )
Comment 8 Katrin Fischer 2015-10-02 08:59:01 UTC
See comment #3 - I agree as there is some discussion here, another sign-off would be good.
Comment 9 Nick Clemens 2015-11-17 22:02:30 UTC
simple notice conflicts, less simple Reserves.pm conflict
Comment 10 Kyle M Hall 2015-11-25 16:03:57 UTC Comment hidden (obsolete)
Comment 11 Claudio 2015-11-25 17:41:00 UTC
This solution fixes bug#10827 "List of holds does not update correctly". If you agree I will set it as RESOLVED.
Comment 12 Katrin Fischer 2015-11-25 17:43:10 UTC
(In reply to bondiurbano from comment #11)
> This solution fixes bug#10827 "List of holds does not update correctly". If
> you agree I will set it as RESOLVED.

Maybe better to mark it as a duplicate. I wouldn't set it resolved until a solution has made it into master.
Comment 13 Claudio 2015-11-25 17:45:49 UTC
Thanks Katrin. I will do it in that way.
Comment 14 Claudio 2015-11-25 17:48:23 UTC
*** Bug 10827 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Druart 2015-12-02 10:27:39 UTC
Still waiting for another signoff.
Comment 16 Nick Clemens 2015-12-02 23:16:21 UTC
The holds transfer as expected, and the patron gets the email but I am not getting the admin email.

I also got the error below when running misc/cronjobs/holds/cancel_expired_holds.pl:

DBD::mysql::st execute failed: Duplicate entry '6' for key 'PRIMARY' [for Statement "
            INSERT INTO old_reserves
            SELECT * FROM reserves
            WHERE  reserve_id = ?
        " with ParamValues: 0='6'] at C4/Reserves.pm line 1080.
Can't locate object method "new" via package "C4::Dates" (perhaps you forgot to load "C4::Dates"?) at C4/Reserves.pm line 2073.
Comment 17 Héctor Eduardo Castro Avalos 2015-12-03 04:11:16 UTC
(In reply to Nick Clemens from comment #16)
> The holds transfer as expected, and the patron gets the email but I am not
> getting the admin email.
> 
> I also got the error below when running
> misc/cronjobs/holds/cancel_expired_holds.pl:
> 
> DBD::mysql::st execute failed: Duplicate entry '6' for key 'PRIMARY' [for
> Statement "
>             INSERT INTO old_reserves
>             SELECT * FROM reserves
>             WHERE  reserve_id = ?
>         " with ParamValues: 0='6'] at C4/Reserves.pm line 1080.
> Can't locate object method "new" via package "C4::Dates" (perhaps you forgot
> to load "C4::Dates"?) at C4/Reserves.pm line 2073.

You're right Nick, patch Fail QA since Marc has been removed C4::Dates from system.

Kyle, please remove C4::Dates and use Koha::DateUtils instead.

substitute  => { today => C4::Dates->new()->output() }, in line 2077

I add here Marc since he removed the C4::Dates from Koha.
Comment 18 Nick Clemens 2016-03-30 22:02:59 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens 2016-03-30 22:18:06 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2016-04-03 13:14:32 UTC
Fixing sequence of attached patches so they apply cleanly with git-bz.
Comment 21 Katrin Fischer 2016-04-03 13:15:23 UTC Comment hidden (obsolete)
Comment 22 Katrin Fischer 2016-04-03 13:40:04 UTC
I think this could be useful to some libraries as it is, but I agree, there are some catches. Some future variation could be: Cancel the old hold, inform the library, but do not send the information to the next patron yet (this would allow the library to take care of the hold, slip, etc.) before the patron appears at the circulation desk.

Some notes from the initial code review:

1) The new notice template includes
+Copy: <<items.copynumber>>
but doesn't include the callnumber. I think it should be amended - without the callnumber it's not possible to identify the item in question.
Also there are some capitalization issues:
Canceled Hold Available for Different Patron

2) From the system preferene it's not clear that this feature is tied to the automatic cancellation:
ExpireReservesAutoFill
yes: "Do"
no: "Don't"
automatically fill the next hold using the item. If enabled, an email notification will be sent to either the email address defined in 

3) The tests for for ModReserveAffect have been adapted, but there are no new  tests the change itself. There are also no changes to the tests for the changed:
CancelExpiredReserves
CancelReserve

Failing for 3) - please take care of adding tests for this new functionality relfecting the changes in the routines.
Comment 23 Kyle M Hall 2016-06-13 12:41:27 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2016-06-13 12:41:36 UTC Comment hidden (obsolete)
Comment 25 Kyle M Hall 2016-06-13 12:41:40 UTC Comment hidden (obsolete)
Comment 26 Jonathan Druart 2016-07-02 12:44:22 UTC
(In reply to Jonathan Druart from comment #3)
> I would like to get another signoff on this one please.

(In reply to Katrin Fischer from comment #8)
> See comment #3 - I agree as there is some discussion here, another sign-off
> would be good.

(In reply to Jonathan Druart from comment #15)
> Still waiting for another signoff.

And... still waiting...
Comment 27 Owen Leonard 2017-04-11 17:19:40 UTC
Sorry, patch doesn't apply. I'll test ASAP after rebased patches are attached.
Comment 28 Kyle M Hall 2018-02-02 13:53:48 UTC Comment hidden (obsolete)
Comment 29 Claudio 2018-10-03 15:11:36 UTC
This is a long waiting enhancement for many libraries. It would be nice to continue with it... Is it possible?
Comment 30 Kyle M Hall 2018-10-05 14:54:36 UTC
Created attachment 80126 [details] [review]
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold

Right now, if a library automatically cancels expired waiting holds, a
librarian must still re-checkin an item to trap the next available hold
for that item. It would be better if the next hold was automatically
trapped and the librarians receive an email notification so they can
make any changes to the item if need be ( hold area, hold slip in item,
etc ).

Test Plan:
 1) Apply this patch
 2) Run updatedatabase.pl
 3) Create a record with one item
 4) Place two holds on that record
 5) Check in the item and set it to waiting for the first patron
 6) Set ReservesMaxPickUpDelay to 1
 7) Enable ExpireReservesMaxPickUpDelay
 8) Enable ExpireReservesAutoFill
 9) Set an email address in ExpireReservesAutoFillEmail
10) Modify the holds waitingdate to be in the past
11) Run misc/cronjobs/holds/cancel_expired_holds.pl
12) Note the hold is now waiting for the next patron
12) Note a waiting hold notification email was sent to that patron
13) Note a hold changed notification email was sent to the library
Comment 31 Genevieve Beaudry 2019-02-15 21:04:20 UTC
at step 11, I get the following error:

Use of inherited AUTOLOAD for non-method Koha::Hold::ModReserveAffect() is deprecated. This will be fatal in Perl 5.28 at /inlibro/git/koha-master-dev-inlibro/Koha/Hold.pm line 385.
Can't locate object method "_columns" via package "177333" (perhaps you forgot to load "177333"?) at /inlibro/git/koha-master-dev-inlibro/Koha/Object.pm line 443.
Comment 32 Liz Rea 2019-03-20 14:49:30 UTC
I just tested this, and it doesn't fail in this way any more, but the feature doesn't work either - please have another look!

Cheers,
Liz
Comment 33 Claudio 2019-05-11 21:50:14 UTC
Any chance to have it in 19.05?
Comment 34 Katrin Fischer 2019-05-11 22:21:18 UTC
Hi Claudio, we have reached 'feature freeze' - this means no new features will be pushed for 19.05 apart from a select few that the RM has marked with rel_19_05_candidate. See here:
http://lists.koha-community.org/pipermail/koha-devel/2019-April/045179.html
So chances are rather slim right now.
Comment 35 Claudio 2019-05-11 22:45:58 UTC
Many thanks Katrin. What a pity...
Comment 36 Claudio 2019-08-13 10:39:32 UTC
Any chance to go on with this?
Comment 37 Katrin Fischer 2019-08-15 20:46:22 UTC
We'd need Kyle to fix the problems noted by Liz now to get this moving again.