Bug 24644

Summary:	Hold not marked filled if item set to positive not for loan value
Product:	Koha	Reporter:	Andrew Fuerste-Henry <andrew>
Component:	Hold requests	Assignee:	Bugs List <koha-bugs>
Status:	NEW ---	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	andreas.jonsson, gmcharlt, jonathan.druart, nick, patrick.robitaille
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13559
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 24644: Just a try

Description Andrew Fuerste-Henry 2020-02-12 16:53:57 UTC

Confirmed in master and 19.05. To test:
1: make sure AllowNotForLoanOverride is set to allow
2: place a hold on an item with notforloan=0
3: check item in, confirm hold now shows Waiting
4: edit item, set notforloan to any positive value
5: check item out, performing override to allow checkout
6: confirm hold still shows Waiting

Not a situation one expects to see often, but confusing when it pops up.

Comment 1 Jonathan Druart 2020-02-17 14:28:56 UTC

I have tracked this down a bit. The problem is that MoveReserve (called by AddIssue) calls CheckReserves:

1863     my ( $restype, $res, undef ) = CheckReserves( $itemnumber, undef, $lookahead );                 
1864     return unless $res;

Here $res is not defined because of:

 792     # if item is not for loan it cannot be reserved either.....
 793     # except where items.notforloan < 0 :  This indicates the item is holdable.
 794     return if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;

I have no idea how to fix this and making sure I am not introducing a regression somewhere else.

Comment 2 Jonathan Druart 2020-02-17 14:37:28 UTC

Created attachment 99110 [details] [review]
Bug 24644: Just a try

That does not work, t/db_dependent/Reserves.t is failing

Comment 3 Jonathan Druart 2020-02-17 14:38:30 UTC

(In reply to Jonathan Druart from comment #2)
> Created attachment 99110 [details] [review] [review]
> Bug 24644: Just a try
> 
> That does not work, t/db_dependent/Reserves.t is failing

This patch does fix the reported issue however.

Comment 4 Katrin Fischer 2022-06-25 14:58:38 UTC

Items with positive NFL values are not holdable by definition. I think this is not something that can/should just be changed.

The deeper problem is how we deal with 'enforced' holds (AllowHoldPolicyOverride), as they can cause all types of issues contradicting the hold rules.

For item level holds, maybe the suggestion on bug 13559 could be an idea.