Bug 11512 - Only allow to override maximum number of holds from staff as other overrides would never be filled
Summary: Only allow to override maximum number of holds from staff as other overrides ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
: 8859 14021 (view as bug list)
Depends on:
Blocks: 21495
  Show dependency treegraph
 
Reported: 2014-01-09 20:15 UTC by Kyle M Hall
Modified: 2019-10-14 19:56 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11512 - Forced holds that violate issuing rules will never be filled (1.46 KB, patch)
2017-10-26 11:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11512 - Forced holds that violate issuing rules will never be filled (1.46 KB, patch)
2017-10-26 11:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11512 - Forced holds that violate issuing rules will never be filled (1.52 KB, patch)
2018-03-12 14:16 UTC, Jesse Maseto
Details | Diff | Splinter Review
Bug 11512: Update syspref description (1.09 KB, patch)
2018-03-23 11:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11512: Update syspref description (1.09 KB, patch)
2018-03-23 11:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11512 - Forced holds that violate issuing rules will never be filled (1.61 KB, patch)
2018-04-06 06:27 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11512: Update syspref description (1.17 KB, patch)
2018-04-06 06:27 UTC, Marcel de Rooy
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 2014-01-09 20:15:10 UTC
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.

I think the only way to resolve this is to add a flag to the reserves table to alert the system that despite the fact that this hold should usually be skipped over, in this case, it should not.
Comment 1 Katrin Fischer 2015-06-07 00:28:18 UTC
*** Bug 8859 has been marked as a duplicate of this bug. ***
Comment 2 Katrin Fischer 2015-06-09 01:26:02 UTC
*** Bug 14021 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2015-07-22 10:07:54 UTC
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.
Comment 4 Kyle M Hall 2017-10-26 11:42:08 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2017-10-26 11:43:00 UTC Comment hidden (obsolete)
Comment 6 Jesse Maseto 2018-03-12 14:16:26 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2018-03-23 09:02:16 UTC
QA: Looking here
Comment 8 Marcel de Rooy 2018-03-23 09:12:06 UTC
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.
Comment 9 Kyle M Hall 2018-03-23 11:29:02 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2018-03-23 11:32:27 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2018-04-06 06:27:33 UTC
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>
Comment 12 Marcel de Rooy 2018-04-06 06:27:39 UTC
Created attachment 73735 [details] [review]
Bug 11512: Update syspref description

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Jonathan Druart 2018-04-06 17:53:16 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 14 Stefan Berndtsson 2018-05-25 11:04:06 UTC
(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.
Comment 15 Katrin Fischer 2018-10-05 08:36:04 UTC
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.
Comment 16 Sarah Cornell 2018-11-10 14:29:41 UTC
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.