Bug 8996 - In result page items with negative notforloan are available
Summary: In result page items with negative notforloan are available
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-31 15:41 UTC by Fridolin SOMERS
Modified: 2013-05-23 06:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (1.05 KB, patch)
2012-10-31 16:00 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8996: In result page items with negative notforloan are available (1.62 KB, patch)
2012-10-31 16:31 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2012-10-31 15:41:50 UTC

    
Comment 1 Fridolin SOMERS 2012-10-31 15:44:12 UTC
On intranet result page, items with notforloan < 0 are available.
But this negative value is used when item is on order.
It should be unavailable.
Comment 2 Fridolin SOMERS 2012-10-31 16:00:48 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2012-10-31 16:31:52 UTC
Created attachment 13147 [details] [review]
[SIGNED-OFF] Bug 8996: In result page items with negative notforloan are available

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

--- Before ---

4 items, 2 available:

    Athens [YA Taylor] (2)
    Circulating

2 unavailable:

    Athens [YA Taylor] Download (1)
    Circulating
    The Plains [YA Taylor] Staff Collection (1)
    Circulating

--- After ---

4 items, 1 available:

    Athens [YA Taylor] (1)
    Circulating

3 unavailable:

    Athens [YA Taylor] Ordered (1)
    Circulating
    Athens [YA Taylor] Download (1)
    Circulating
    The Plains [YA Taylor] Staff Collection (1)
    Circulating
Comment 4 Marcel de Rooy 2012-11-07 13:16:36 UTC
QA Comment:
I would agree with your second adjustment on line 1840 (removing the >0 condition, including negative notforloan).
I am not sure about your first adjustment on line 1802 (changing ==-1 to <0 ).
As I understand, a negative notforloan value does not immediately imply that the item is on order. Historically, the value of -1 means on order, although someone could change or remove the authvalue. (In that case he would be hindered by the hardcoded -1 tests in code..) But where do we say that -2 means on order too? 

Please clarify or correct me where my conclusion seems wrong. Changing status to reflect need of clarification.
Comment 5 Fridolin SOMERS 2012-11-08 07:19:48 UTC
> But where do we say that -2 means on order too? 
I would say yes, I found several places in actual code indicating that all negative notforloan values mean "On order" :

XSLT.pm line 247 :
    if ( $item->{notforloan} < 0) {
        $status = "On order";
    } 

Reserves.pm line 820 :
    # if item is not for loan it cannot be reserved either.....
    #    execpt where items.notforloan < 0 :  This indicates the item is holdable. 
    return ( '' ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;

opac-details.pl line 523 :
    $norequests = 0
       if ( (not $itm->{'wthdrawn'} )
         && (not $itm->{'itemlost'} )
         && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'} )
		 && (not $itemtypes->{$itm->{'itype'}}->{notforloan} )
         && ($itm->{'itemnumber'} ) );

The Koha configurations I sow use several negative values to indicate the several levels of acquisition.

Feel free to ask more.
Comment 6 Marcel de Rooy 2012-11-08 07:34:40 UTC
(In reply to comment #5)
> I would say yes, I found several places in actual code indicating that all
> negative notforloan values mean "On order" :

Well, enough to convince me. Will send a mail to Nicole asking if this is already documented somewhere too.
Status to Passed QA.
Comment 7 Jared Camins-Esakov 2012-11-10 02:30:47 UTC
This patch has been pushed to master.
Comment 8 Chris Cormack 2012-11-11 06:00:34 UTC
Pushed to 3.8.x, will be in 3.8.7