Bug 4812

Summary: Reserves for a specific copy of a book say book is available even though it it still checked out to someone else
Product: Koha Reporter: Kyle M Hall <kyle.m.hall>
Component: Hold requestsAssignee: Kyle M Hall <kyle.m.hall>
Status: Patch doesn't apply --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: PATCH-Sent (DO NOT USE) CC: emily.lamancusa, fridolin.somers, kyle, magnus, martin.renvoize, maryse.simard, pongtawat, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20423
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Intranet Holds Bugfix
Bugfix for both intranet & opac holds.
Screenshot of holds list after patch was applied
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out

Description Kyle M Hall 2010-05-25 15:38:28 UTC
When you place a reserve on a specific copy of a book a note is immediately put on the card of the person for whom the book is being reserved that says the book is available even though it is still checked out to someone else.

The code is in placerequest.pl

# if we have an item selectionned, and the pickup branch is the same as the holdingbranch
# of the document, we force the value $rank and $found .
if ($checkitem ne ''){
    $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns');
    my $item = $checkitem;
    $item = GetItem($item);
    if ( $item->{'holdingbranch'} eq $branch ){
        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
    }
}

This code does not bother to check to see if an item is actually on the shelf, it sets it to 'W' reguardless.
Comment 1 Galen Charlton 2010-05-25 21:54:04 UTC
Same problem may effect item-level requests placed via C4/ILSDI/Services.pm and opac/opac-reserve.pl.
Comment 2 Kyle M Hall 2010-05-27 13:03:52 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2010-05-27 13:10:03 UTC
Created attachment 2173 [details] [review]
Bugfix for both intranet & opac holds.
Comment 4 Katrin Fischer 2011-03-19 15:54:12 UTC
I was able to reproduce the problem.

I tried testing the patch:
1) activate sys pref Reserveneedsreturn
2) checkout item (patron 1 from library A, item from library A)
3) item level hold for patron 2 (also from library A)
4) item shows as pending in OPAC -ok
5) item shows as "Item being transferred to" title's holds page (?)

I am not sure if 5) is a problem with my configuration. Can someone else test?
Comment 5 Owen Leonard 2011-03-29 14:48:15 UTC
Created attachment 3500 [details]
Screenshot of holds list after patch was applied

I was able to reproduce this bug with ReservesNeedReturns set to "Automatically mark holds as found and waiting..." After applying the patch and following the same procedure cait describes I saw the attached screen. An incomplete "Item being transferred to" under "Pickup library" and "No barcode" under details, although the hold was for a specific copy.
Comment 6 Jared Camins-Esakov 2013-01-02 01:43:09 UTC
Is this still an issue?
Comment 7 Fridolin Somers 2013-10-31 14:49:42 UTC
We have this problem for one of your libraries.
It is a real bug for me, not an enhancement.

By the way, I think the mean of ReservesNeedReturns is wrong, it generates several bugs when inactive : 
- this bug
- you can place a waiting reserve when there is already a waiting reserve
- you do not have messages present is return page

In my opinion, reserves module is too weak to repair, this syspref should be removed.
Comment 8 Pongtawat 2014-08-12 09:34:19 UTC
I found this is still a problem in Koha 3.12

Step to reproduce:

1) activate sys pref ReservesNeedReturns
2) patron 1 checkout item A
3) patron 2 place an item-specific hold for item A to be deliver at its current location
4) item A shows as "Checked out to 1" and "Waiting for 2" at the same time. (Intranet)

The code appears to still be the same as in Kyle M Hall's original comment. If ReservesNeedReturns is off, it will just mark item as waiting for pick up.

Moreover, I found that this preference descriptive text quite confusing: to turn ReservesNeedReturns on, I have to choose "Don't automatically mark holds ...". I would expect something like "Do" to mean on and "Don't" to mean off. Maybe just my silliness anyway :P
Comment 9 Marc VĂ©ron 2016-08-22 08:23:58 UTC
Is this bug still valid?
Comment 10 Kyle M Hall (khall) 2018-03-15 14:01:40 UTC
Created attachment 72960 [details] [review]
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out

When you place a reserve on a specific copy of a book a note is immediately put
on the card of the person for whom the book is being reserved that says the
book is available even though it is still checked out to someone else.

The code is in placerequest.pl & opac-reserve.pl

holdingbranch
if ($checkitem ne ''){
    $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns');
    my $item = $checkitem;
    $item = GetItem($item);
    if ( $item->{'holdingbranch'} eq $branch ){
        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
    }
}

This code does not bother to check to see if an item is actually on the shelf,
it sets it to 'W' reguardless.
Comment 11 Kyle M Hall (khall) 2018-03-15 14:06:07 UTC
Created attachment 72961 [details] [review]
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out

When you place a reserve on a specific copy of a book a note is immediately put
on the card of the person for whom the book is being reserved that says the
book is available even though it is still checked out to someone else.

The code is in placerequest.pl & opac-reserve.pl

holdingbranch
if ($checkitem ne ''){
    $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns');
    my $item = $checkitem;
    $item = GetItem($item);
    if ( $item->{'holdingbranch'} eq $branch ){
        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
    }
}

This code does not bother to check to see if an item is actually on the shelf,
it sets it to 'W' reguardless.
Comment 12 Kyle M Hall (khall) 2018-03-15 14:06:49 UTC
Created attachment 72962 [details] [review]
Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out

When you place a reserve on a specific copy of a book a note is immediately put
on the card of the person for whom the book is being reserved that says the
book is available even though it is still checked out to someone else.

The code is in placerequest.pl & opac-reserve.pl

holdingbranch
if ($checkitem ne ''){
    $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns');
    my $item = $checkitem;
    $item = GetItem($item);
    if ( $item->{'holdingbranch'} eq $branch ){
        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
    }
}

This code does not bother to check to see if an item is actually on the shelf,
it sets it to 'W' reguardless.
Comment 13 Maryse Simard 2018-08-03 20:44:26 UTC
Patch doesn't apply. It seems to be due to the fact that the ReservesNeedReturns logic was moved from placerequest.pl & opac-reserve.pl to Reserves.pm by bug 20724.

Applying: Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out
.git/rebase-apply/patch:32: trailing whitespace.
                    $rank  = '0'; 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	opac/opac-reserve.pl
M	reserve/placerequest.pl
.git/rebase-apply/patch:32: trailing whitespace.
                    $rank  = '0'; 
warning: 1 line applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging reserve/placerequest.pl
CONFLICT (content): Merge conflict in reserve/placerequest.pl
Auto-merging opac/opac-reserve.pl
CONFLICT (content): Merge conflict in opac/opac-reserve.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 4812: Reserves for a specific copy of a book say book is available even though it it still checked out
Comment 14 Emily Lamancusa (emlam) 2024-01-11 21:51:58 UTC
Is this still valid?