https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37651 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #27 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #25)
The description "But it does not interfere with issuing, renewing or transferring items" is not the most lucky one. I might have added it myself long ago ;)
Note also that we are talking about a feature that say over 90% of the libraries is not using. (HEA: 1040 AllowHoldDateinFuture 728 OPACAllowHoldDateInFuture )
10% is not an insignificant amount of libraries imo
Lets look at the processes mentioned. (When writing this patch, I reviewed all occurrences of current_holds btw.)
Checkout AddIssue already calls MoveReserve and that does check ConfirmFutureHolds. So this is an "existing interference". The description was wrong.
MoveReserve in this context is checking for future holds, but only to fill the current patrons future holds - or cancel or remove waiting status from an existing hold - it does not block the checkout in any way - and it looks like this won't change after the patch. So the description seems right to me
Renew CanBookBeRenewed calls current_holds. This is a "new interference". If a library allows future holds and wants to confirm them within a few days specified by ConfirmFutureHolds, currently this is ignored by CanBookBeRenewed. So the book is renewed although there is a hold for it on tomorrow. Not sure if we should write code for that? Probably most do not want to renew here.
I don't know that this is true - a patron who has a back might be allowed to renew as it is in their possession. I think future holds confirmation is meant to capture a book that is returned - forcing others to return it earlier is more akin to what bookings and recalls do. This is a significant change in behavior I think, and should, at least, be added as a test case for CanBookBeRenewed In my review, these are the effects: C4::Circulation CanBookBeRenewed (will now be no) C4/ILSDI/Services GetAvailability (will now be on hold) C4/Reserves AddReserve (will now not be set to waiting if there is a future hold) Koha/Item first_hold (could now return a future hold over a non-future hold, this seems okay to me, but I am not sure of all contexts of first_hold) I think below will just add the new holds to display without behaviour change: Koha/Acquisition/Order current_item_level_holds C4/SIP/ILS/Item new acqui/parcel.pl circ/transferstoreceive.pl opac/opac-reserve.pl reserve/request.pl I won't block this one, but can leave to the RM to decide - I would like to see tests added to CanBookBeRenewed and GetAvailability for QA -- You are receiving this mail because: You are watching all bug changes.