From 1b0ce2517f0826ea92094be54da38d7dfd1fe547 Mon Sep 17 00:00:00 2001 From: Srdjan Jankovic Date: Tue, 20 Sep 2011 15:56:17 +1200 Subject: [PATCH] bug_2830: Remove reserve when checking out if the borrower is not the first one in the reserve queue Content-Type: text/plain; charset="utf-8" To test: Create 4 holds on a bib, for patrons A, B, C, and D, Check in the item to mark hold as waiting for patron A Check out the item to patron B -> reserve for patron B should be removed Check in the item to mark hold as waiting for patron A Check out the item to Patron A, hold should complete normally Check in the item to mark hold as waiting for patron C Check out the item to patron D -> reserve for patron D should be removed. Check in the item to mark hold as waiting for patron C Check out the item to patron C, hold should complete normally Check in the item -> there should be no more reserves. We also tested: Created 4 holds on a bib with two items, for patrons A, B, C, and D All worked as expected. One comment, if a reserve that already has a waiting item is filled by an item with a different barcode, the original reserve is deleted. Cool. It might be nice to alert the librarian that there might be an item on the hold shelf. i.e. "I just deleted a reserve that was in waiting status, go get it off the hold shelf and send me to someone else or reshelve it" That's probably a different ENH bug though. :) Signed-off-by: Liz Rea http://bugs.koha-community.org/show_bug.cgi?id=2380 --- C4/Circulation.pm | 34 +----------------- C4/Reserves.pm | 58 ++++++++++++++++++++++++++++-- t/db_dependent/lib/KohaTest/Reserves.pm | 1 + 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 520c116..5865d91 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -984,38 +984,8 @@ sub AddIssue { } # See if the item is on reserve. - my ( $restype, $res ) = - C4::Reserves::CheckReserves( $item->{'itemnumber'} ); - if ($restype) { - my $resbor = $res->{'borrowernumber'}; - if ( $resbor eq $borrower->{'borrowernumber'} ) { - # The item is reserved by the current patron - ModReserveFill($res); - } - elsif ( $restype eq "Waiting" ) { - # warn "Waiting"; - # The item is on reserve and waiting, but has been - # reserved by some other patron. - } - elsif ( $restype eq "Reserved" ) { - # warn "Reserved"; - # The item is reserved by someone else. - if ($cancelreserve) { # cancel reserves on this item - CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'}); - } - } - if ($cancelreserve) { - CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'}); - } - else { - # set waiting reserve to first in reserve queue as book isn't waiting now - ModReserve(1, - $res->{'biblionumber'}, - $res->{'borrowernumber'}, - $res->{'branchcode'} - ); - } - } + MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve ); + # Starting process for transfer job (checking transfert and validate it if we have one) my ($datesent) = GetTransfers($item->{'itemnumber'}); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8373840..469b719 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -109,6 +109,7 @@ BEGIN { &ModReserveStatus &ModReserveCancelAll &ModReserveMinusPriority + &MoveReserve &CheckReserves &CanBookBeReserved @@ -731,8 +732,8 @@ sub GetReserveStatus { =head2 CheckReserves - ($status, $reserve) = &CheckReserves($itemnumber); - ($status, $reserve) = &CheckReserves(undef, $barcode); + ($status, $reserve, $all_reserves) = &CheckReserves($itemnumber); + ($status, $reserve, $all_reserves) = &CheckReserves(undef, $barcode); Find a book in the reserves. @@ -800,7 +801,7 @@ sub CheckReserves { my $priority = 10000000; foreach my $res (@reserves) { if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) { - return ( "Waiting", $res ); # Found it + return ( "Waiting", $res, \@reserves ); # Found it } else { # See if this item is more important than what we've got so far if ( $res->{'priority'} && $res->{'priority'} < $priority ) { @@ -821,7 +822,7 @@ sub CheckReserves { # We return the most important (i.e. next) reservation. if ($highest) { $highest->{'itemnumber'} = $item; - return ( "Reserved", $highest ); + return ( "Reserved", $highest, \@reserves ); } else { return ( 0, 0 ); @@ -1794,6 +1795,55 @@ sub _ShiftPriorityByDateAndPriority { return $new_priority; # so the caller knows what priority they wind up receiving } + +=head2 MoveReserve + + MoveReserve( $itemnumber, $borrowernumber, $cancelreserve ) + +Use when checking out an item to handle reserves +If $cancelreserve boolean is set to true, it will remove existing reserve + +=cut + +sub MoveReserve { + my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_; + + my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber ); + return unless $res; + + my $biblionumber = $res->{biblionumber}; + my $biblioitemnumber = $res->{biblioitemnumber}; + + if ($res->{borrowernumber} == $borrowernumber) { + ModReserveFill($res); + } + else { + # warn "Reserved"; + # The item is reserved by someone else. + # Find this item in the reserves + + my $borr_res; + foreach (@$all_reserves) { + $_->{'borrowernumber'} == $borrowernumber or next; + $_->{'biblionumber'} == $biblionumber or next; + + $borr_res = $_; + last; + } + + if ( $borr_res ) { + # The item is reserved by the current patron + ModReserveFill($borr_res); + } + + if ($cancelreserve) { # cancel reserves on this item + CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'}); + CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'}); + } + } +} + + =head1 AUTHOR Koha Development Team diff --git a/t/db_dependent/lib/KohaTest/Reserves.pm b/t/db_dependent/lib/KohaTest/Reserves.pm index 5317029..8b05dd0 100644 --- a/t/db_dependent/lib/KohaTest/Reserves.pm +++ b/t/db_dependent/lib/KohaTest/Reserves.pm @@ -29,6 +29,7 @@ sub methods : Test( 1 ) { ModReserveAffect ModReserveCancelAll ModReserveMinusPriority + MoveReserve GetReserveInfo _FixPriority _Findgroupreserve -- 1.7.2.5