Description
Kyle M Hall (khall)
2014-01-09 20:15:10 UTC
*** Bug 8859 has been marked as a duplicate of this bug. *** *** Bug 14021 has been marked as a duplicate of this bug. *** I have another report from a library about this - it's really annoying that we offer to override holds, but then they just never get filled... hard to explain. Created attachment 68594 [details] [review] Bug 11512 - Forced holds that violate issuing rules will never be filled Currently in Koha, if you choose to force a hold from the staff side that would contravened the current issuing rules, that hold will never be filled, as it is always skipped over by CheckReserves. This patch disallows overrideing except for tooManyReserves which are the only overridden holds that will be trapped. Test Plan: 1) Apply this patch 2) Attempt to override hold placement, only placements where the patron has too many holds already should be allowed Created attachment 68595 [details] [review] Bug 11512 - Forced holds that violate issuing rules will never be filled Currently in Koha, if you choose to force a hold from the staff side that would contravened the current issuing rules, that hold will never be filled, as it is always skipped over by CheckReserves. This patch disallows overrideing except for tooManyReserves which are the only overridden holds that will be trapped. Test Plan: 1) Apply this patch 2) Attempt to override hold placement, only placements where the patron has too many holds already should be allowed Created attachment 72689 [details] [review] Bug 11512 - Forced holds that violate issuing rules will never be filled Currently in Koha, if you choose to force a hold from the staff side that would contravened the current issuing rules, that hold will never be filled, as it is always skipped over by CheckReserves. This patch disallows overrideing except for tooManyReserves which are the only overridden holds that will be trapped. Test Plan: 1) Apply this patch 2) Attempt to override hold placement, only placements where the patron has too many holds already should be allowed Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com> QA: Looking here It seems that we are restricting AllowHoldPolicyOverride here at the only code location where it is really relevant. So this pref really does not do what it implicitly advertises. I would suggest to rename the pref or at least add a disclaimer at the preference form and help page etc. before passing QA on it. Created attachment 73172 [details] [review] Bug 11512: Update syspref description Created attachment 73173 [details] [review] Bug 11512: Update syspref description Created attachment 73734 [details] [review] Bug 11512 - Forced holds that violate issuing rules will never be filled Currently in Koha, if you choose to force a hold from the staff side that would contravened the current issuing rules, that hold will never be filled, as it is always skipped over by CheckReserves. This patch disallows overrideing except for tooManyReserves which are the only overridden holds that will be trapped. Test Plan: 1) Apply this patch 2) Attempt to override hold placement, only placements where the patron has too many holds already should be allowed Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 73735 [details] [review] Bug 11512: Update syspref description Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 18.05, thanks to everybody involved! (In reply to Marcel de Rooy from comment #11) > Currently in Koha, if you choose to force a hold from the staff side that > would contravened the current issuing rules, that hold will never be filled, > as it is always skipped over by CheckReserves. > > This patch disallows overrideing except for tooManyReserves which are the > only overridden holds that will be trapped. From what I can see in the code, this isn't actually true. CheckReserves does not include neither LOST nor WITHDRAWN in its checks. In fact, we use the LOST part here. When longoverdue.pl marks a book as LOST, our staff adds a reserve to that item, so that they will be notified if it is returned. With the code prior to this patch, this works perfectly fine since we have the AllowHoldPolicyOverride set and returning a LOST item fills the hold. We've also used a similiar thing with withdrawn, but that's not as critical, so I won't argue against that one. The assumption that this is always skipped is not valid, unless I've missed something. We can patch things temporarily, but some sort of option for better override selectability would probably be useful here. Hi Stefan, this is a good use case - the lost items. If this is a regression we should fix it. Could you open a separate bug and link to this? It will give the problem better visibility. I have another use case, Katrin. We need to place holds on bibs that only have lost items on them because we intend to replace the item. The patron request/hold is usually what triggers that purchase. In some cases the hold will need to be trigered by that "lost" item being returned, but in many cases it will be triggered by an entirely new item that will definitely be eligible for the hold. This just reiterates the need to be able to override. |