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 ).
Created attachment 39998 [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
Created attachment 41236 [details] [review] [SIGNED-OFF] 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 Signed-off-by: Gene Miller <genem@takomaparkmd.gov>
I would like to get another signoff on this one please.
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
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.
(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 ).
(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 ; )
See comment #3 - I agree as there is some discussion here, another sign-off would be good.
simple notice conflicts, less simple Reserves.pm conflict
Created attachment 45149 [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 Signed-off-by: Gene Miller <genem@takomaparkmd.gov>
This solution fixes bug#10827 "List of holds does not update correctly". If you agree I will set it as RESOLVED.
(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.
Thanks Katrin. I will do it in that way.
*** Bug 10827 has been marked as a duplicate of this bug. ***
Still waiting for another signoff.
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.
(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.
Created attachment 49699 [details] [review] Bug 14364: (QA followup) - Remove use of C4:Dates - Fix misnamed variable
Created attachment 49700 [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 Signed-off-by: Gene Miller <genem@takomaparkmd.gov>
Fixing sequence of attached patches so they apply cleanly with git-bz.
Created attachment 49822 [details] [review] Bug 14364: (QA followup) - Remove use of C4:Dates - Fix misnamed variable
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.
Created attachment 52310 [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 Signed-off-by: Gene Miller <genem@takomaparkmd.gov>
Created attachment 52311 [details] [review] Bug 14364: (QA followup) - Remove use of C4:Dates - Fix misnamed variable
Created attachment 52312 [details] [review] Bug 14364 [QA Followup] - Add unit tests
(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...
Sorry, patch doesn't apply. I'll test ASAP after rebased patches are attached.
Created attachment 71148 [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
This is a long waiting enhancement for many libraries. It would be nice to continue with it... Is it possible?
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
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.
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
Any chance to have it in 19.05?
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.
Many thanks Katrin. What a pity...
Any chance to go on with this?
We'd need Kyle to fix the problems noted by Liz now to get this moving again.
Created attachment 103297 [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
This patch is cranky about sample_notices.sql. I think it needs to get adapted to the new yaml format for that file?
Created attachment 105015 [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
Applying: Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold Using index info to reconstruct a base tree... M C4/Reserves.pm M Koha/Hold.pm M installer/data/mysql/pl-PL/mandatory/sample_notices.sql A installer/data/mysql/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/pl-PL/mandatory/sample_notices.sql CONFLICT (content): Merge conflict in installer/data/mysql/pl-PL/mandatory/sample_notices.sql Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging Koha/Hold.pm Auto-merging C4/Reserves.pm CONFLICT (content): Merge conflict in C4/Reserves.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14364---Allow-automatically-canceled-expired-w-4l6wf_.patch patch doesn't apply
Created attachment 115427 [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
Created attachment 115466 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>
I am sorry, this no longer applies: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold Using index info to reconstruct a base tree... M C4/Reserves.pm M Koha/Hold.pm M installer/data/mysql/mandatory/sysprefs.sql M installer/data/mysql/pl-PL/mandatory/sample_notices.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/pl-PL/mandatory/sample_notices.sql CONFLICT (content): Merge conflict in installer/data/mysql/pl-PL/mandatory/sample_notices.sql Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging Koha/Hold.pm CONFLICT (content): Merge conflict in Koha/Hold.pm Auto-merging C4/Reserves.pm CONFLICT (content): Merge conflict in C4/Reserves.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14364---Allow-automatically-canceled-expired-w-834arw.patch
Created attachment 120494 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>
Rebased, but I think we need an update here - the current patches rely on _koha_notify_hold to send the message to the library. That routine is not called when the hold is transferred, I think we need a unique subroutine for that message that is called whenever the hold is changed, whether transferred or waiting
(In reply to Nick Clemens from comment #46) > Rebased, but I think we need an update here - the current patches rely on > _koha_notify_hold to send the message to the library. That routine is not > called when the hold is transferred, I think we need a unique subroutine for > that message that is called whenever the hold is changed, whether > transferred or waiting Thx Nick, also for noticing this!
Created attachment 120941 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>
Created attachment 120942 [details] [review] Bug 14364: (QA follow-up) Generate message for transfers as well
Hi Kyle, 1) For the From address, please use the $library->inbound_email_address routine: + my $email = + C4::Context->preference('ExpireReservesAutoFillEmail') + || $library->{branchemail} + || C4::Context->preference('KohaAdminEmailAddress'); It will take the Reply-To addresses into account and make the behaviour match with other notices sent to the library. 2) Duplicated code It looks like the code for sending the HOLD_CHANGED email is duplicated in sub _koha_notify_reserve and _koha_notify_hold_changed multiple times? (also reserve should always be hold) 3) Improve sample email I believe the copynumber is not helpful, we should include the itemcallnumber instead. Please fix capitalization "Canceled Hold Available for Different Patron". 4) Pref descriptions Please link ExpireReservesAutoFillEmail :) Please adjust description for the change in 1). "Send email notification of the new hold filled with a canceled item to" Does canceled item work here? Maybe "hold filled with an expired hold item" instead? 5) Sample notices missing You've only added the new notice to nb-NO, but we need it in various other files as well. See xt/sample_notices.t. It gives a bit confusing advice right now as it thinks the notice has to be deleted, but it has to be added to installer/data/mysql/en/mandatory/sample_notices.yml first and then the test can be rerun.
Created attachment 128757 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>
Created attachment 128758 [details] [review] Bug 14364: (QA follow-up) Generate message for transfers as well
Created attachment 128759 [details] [review] Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs This patch removes a duplicated stanza left form moving routine Changes the routines to use inbound_library_address Improves the display if the system preferences Updates the update file Moves smaple notice
I am sorry, but patches no longer apply, could you please rebase? Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold 128757 - Bug 14364: Allow automatically canceled expired waiting holds to fill the next hold 128758 - Bug 14364: (QA follow-up) Generate message for transfers as well 128759 - Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 14364: Allow automatically canceled expired waiting holds to fill the next hold Using index info to reconstruct a base tree... M C4/Reserves.pm M Koha/Hold.pm M installer/data/mysql/mandatory/sysprefs.sql M koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging Koha/Hold.pm Auto-merging C4/Reserves.pm Applying: Bug 14364: (QA follow-up) Generate message for transfers as well Applying: Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs error: sha1 information is lacking or useless (C4/Reserves.pm). error: could not build fake ancestor Patch failed at 0001 Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-14364-follow-up-Cleanup-duplicate-code-and-adj-QGOiaL.patch
Created attachment 132019 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com>
Created attachment 132020 [details] [review] Bug 14364: (QA follow-up) Generate message for transfers as well
Created attachment 132021 [details] [review] Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs This patch removes a duplicated stanza left form moving routine Changes the routines to use inbound_library_address Improves the display if the system preferences Updates the update file Moves smaple notice
1) Capitalization It's all good, but in the database update statement: Canceled Hold Available for Different Patron', '0', 'Canceled Hold Available for Different Patron', 'The patron picking up <<biblio.title>> (<<items.barcode>>) has changed to <<borrowers.firstname>> <<borrowers.surname>> (<<borrowers.cardnumber 2) Spelling Should this be reply-to address? + - . If no address is defined here, the email will be sent to the library's reply to address. 2) Tests are failing :( prove t/db_dependent/Holds/ExpireReservesAutoFill.t t/db_dependent/Holds/ExpireReservesAutoFill.t .. 1/3 # No tests run! t/db_dependent/Holds/ExpireReservesAutoFill.t .. 2/3 # Failed test 'No tests run for subtest "Test automatically canceled expired waiting holds to fill the next hold, without a transfer"' # at t/db_dependent/Holds/ExpireReservesAutoFill.t line 131. Undefined subroutine &main::CancelExpiredReserves called at t/db_dependent/Holds/ExpireReservesAutoFill.t line 101. # Looks like your test exited with 11 just after 2. t/db_dependent/Holds/ExpireReservesAutoFill.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 2/3 subtests Test Summary Report ------------------- t/db_dependent/Holds/ExpireReservesAutoFill.t (Wstat: 2816 Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 11 Parse errors: Bad plan. You planned 3 tests but ran 2. Files=1, Tests=2, 3 wallclock secs ( 0.03 usr 0.00 sys + 2.66 cusr 0.30 csys = 2.99 CPU) Result: FAIL
Created attachment 133812 [details] [review] Bug 14364: Fix spelling and tests Moved the installer to pl file Adjusted capitalization / reply-to Fix tests by importing needed fuction and adjusting a wrong call
Created attachment 133923 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133924 [details] [review] Bug 14364: (QA follow-up) Generate message for transfers as well Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133925 [details] [review] Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs This patch removes a duplicated stanza left form moving routine Changes the routines to use inbound_library_address Improves the display if the system preferences Updates the update file Moves smaple notice Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133926 [details] [review] Bug 14364: Fix spelling and tests Moved the installer to pl file Adjusted capitalization / reply-to Fix tests by importing needed fuction and adjusting a wrong call Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133927 [details] [review] Bug 14364: (QA follow-up) Fix capitalization Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 133928 [details] [review] Bug 14364: (QA follow-up) Add email validation to ExpireReservesAutoFillEmail Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Please rebase.
Created attachment 137927 [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 Signed-off-by: Victoria Faafia <vfaafia29@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137928 [details] [review] Bug 14364: (QA follow-up) Generate message for transfers as well Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137929 [details] [review] Bug 14364: (follow-up) Cleanup duplicate code and adjust notices and prefs This patch removes a duplicated stanza left form moving routine Changes the routines to use inbound_library_address Improves the display if the system preferences Updates the update file Moves smaple notice Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137930 [details] [review] Bug 14364: Fix spelling and tests Moved the installer to pl file Adjusted capitalization / reply-to Fix tests by importing needed fuction and adjusting a wrong call Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137931 [details] [review] Bug 14364: (QA follow-up) Fix capitalization Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 137932 [details] [review] Bug 14364: (QA follow-up) Add email validation to ExpireReservesAutoFillEmail Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 22.11. Nice work everyone, thanks!
Enhancement will not be backported to 22.05.x series
> Changes the routines to use inbound_library_address Ouch :). Now our all customers have all the rest messages sent from $library->from_email_address; _except_ 'HOLD's: $library->inbound_email_address This slightly made a mess. Katrin/Nick: do we expect that "inbound_email_address" will be soon refactored to everywhere use?
(In reply to Andrew Nugged from comment #75) > > Changes the routines to use inbound_library_address > > Ouch :). Now our all customers have all the rest messages sent from > > $library->from_email_address; > > _except_ 'HOLD's: > > $library->inbound_email_address > > This slightly made a mess. > > Katrin/Nick: do we expect that "inbound_email_address" will be soon > refactored to everywhere use? I think the change to the 'from' is actually a mistake. Koha/Library.pm 171 NOTE: This is the address to use for 'reply_to' or 'to' fields; It should not usually be 172 used as the 'from' address for emails as it may lead to mail being caught by spam filters.
(In reply to Nick Clemens from comment #76) > (In reply to Andrew Nugged from comment #75) > > > Changes the routines to use inbound_library_address > > > > Ouch :). Now our all customers have all the rest messages sent from > > > > $library->from_email_address; > > > > _except_ 'HOLD's: > > > > $library->inbound_email_address > > > > This slightly made a mess. > > > > Katrin/Nick: do we expect that "inbound_email_address" will be soon > > refactored to everywhere use? > > I think the change to the 'from' is actually a mistake. > > Koha/Library.pm > 171 NOTE: This is the address to use for 'reply_to' or 'to' fields; It > should not usually be > 172 used as the 'from' address for emails as it may lead to mail being > caught by spam filters. still valid?
(In reply to Katrin Fischer from comment #77) > (In reply to Nick Clemens from comment #76) > > (In reply to Andrew Nugged from comment #75) > > > > Changes the routines to use inbound_library_address > > > > > > Ouch :). Now our all customers have all the rest messages sent from > > > > > > $library->from_email_address; > > > > > > _except_ 'HOLD's: > > > > > > $library->inbound_email_address > > > > > > This slightly made a mess. > > > > > > Katrin/Nick: do we expect that "inbound_email_address" will be soon > > > refactored to everywhere use? > > > > I think the change to the 'from' is actually a mistake. > > > > Koha/Library.pm > > 171 NOTE: This is the address to use for 'reply_to' or 'to' fields; It > > should not usually be > > 172 used as the 'from' address for emails as it may lead to mail being > > caught by spam filters. > > still valid? See bug 32455