[Patches] [PATCH] [Signed off] bug_2830: Remove reserve when checking out if the borrower is not the first one in the reserve queue
koha-patchbot at kohaaloha.com
koha-patchbot at kohaaloha.com
Fri Nov 18 06:00:03 NZDT 2011
From: Srdjan Jankovic <srdjan at catalyst.net.nz>
Date: Thu, 13 Oct 2011 16:57:41 +1300
Subject: [PATCH] [Signed off] 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.
Signed-off-by: Liz Rea <wizzyrea at gmail.com>
---
C4/Circulation.pm | 42 ++-----------------
C4/Items.pm | 2 +-
C4/Reserves.pm | 67 ++++++++++++++++++++++++++-----
C4/Search.pm | 4 +-
C4/XSLT.pm | 2 +-
circ/circulation.pl | 2 +-
opac/opac-user.pl | 2 +-
t/db_dependent/Reserves.t | 6 +-
t/db_dependent/lib/KohaTest/Reserves.pm | 1 +
9 files changed, 72 insertions(+), 56 deletions(-)
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 9f81773..12f0898 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -319,7 +319,7 @@ sub transferbook {
# find reserves.....
# That'll save a database query.
- my ( $resfound, $resrec ) =
+ my ( $resfound, $resrec, undef ) =
CheckReserves( $itemnumber );
if ( $resfound and not $ignoreRs ) {
$resrec->{'ResFound'} = $resfound;
@@ -869,7 +869,7 @@ sub CanBookBeIssued {
}
# See if the item is on reserve.
- my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+ my ( $restype, $res, undef ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
if ($restype) {
my $resbor = $res->{'borrowernumber'};
my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );
@@ -983,39 +983,7 @@ 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'});
@@ -1636,7 +1604,7 @@ sub AddReturn {
# find reserves.....
# if we don't have a reserve with the status W, we launch the Checkreserves routine
- my ($resfound, $resrec) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+ my ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
if ($resfound) {
$resrec->{'ResFound'} = $resfound;
$messages->{'ResFound'} = $resrec;
@@ -2199,7 +2167,7 @@ sub CanBookBeRenewed {
$error="too_many";
}
- my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
+ my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
if ($resfound) {
$renewokay = 0;
$error="on_reserve"
diff --git a/C4/Items.pm b/C4/Items.pm
index 0b91b65..6429251 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1228,7 +1228,7 @@ sub GetItemsInfo {
$serial = 1;
}
if ( $datedue eq '' ) {
- my ( $restype, $reserves ) =
+ my ( $restype, $reserves, undef ) =
C4::Reserves::CheckReserves( $data->{'itemnumber'} );
# Previous conditional check with if ($restype) is not needed because a true
# result for one item will result in subsequent items defaulting to this true
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 4af8a85..3259a68 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -109,6 +109,7 @@ BEGIN {
&ModReserveStatus
&ModReserveCancelAll
&ModReserveMinusPriority
+ &MoveReserve
&CheckReserves
&CanBookBeReserved
@@ -523,7 +524,7 @@ sub GetOtherReserves {
my ($itemnumber) = @_;
my $messages;
my $nextreservinfo;
- my ( $restype, $checkreserves ) = CheckReserves($itemnumber);
+ my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber);
if ($checkreserves) {
my $iteminfo = GetItem($itemnumber);
if ( $iteminfo->{'holdingbranch'} ne $checkreserves->{'branchcode'} ) {
@@ -738,8 +739,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.
@@ -804,11 +805,11 @@ sub CheckReserves {
# note: we get the itemnumber because we might have started w/ just the barcode. Now we know for sure we have it.
my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber ) = $sth->fetchrow_array;
- return ( 0, 0 ) unless $itemnumber; # bail if we got nothing.
+ return ( '' ) unless $itemnumber; # bail if we got nothing.
# if item is not for loan it cannot be reserved either.....
# execpt where items.notforloan < 0 : This indicates the item is holdable.
- return ( 0, 0 ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
+ return ( '' ) if ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
# Find this item in the reserves
my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber );
@@ -822,7 +823,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 ) {
@@ -843,11 +844,10 @@ sub CheckReserves {
# We return the most important (i.e. next) reservation.
if ($highest) {
$highest->{'itemnumber'} = $item;
- return ( "Reserved", $highest );
- }
- else {
- return ( 0, 0 );
+ return ( "Reserved", $highest, \@reserves );
}
+
+ return ( '' );
}
=head2 CancelExpiredReserves
@@ -1816,6 +1816,53 @@ 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'});
+ }
+ }
+}
+
=head2 MergeHolds
MergeHolds($dbh,$to_biblio, $from_biblio);
diff --git a/C4/Search.pm b/C4/Search.pm
index 98db887..428cb26 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -1673,7 +1673,7 @@ sub searchResults {
my ($transfertfrom, $transfertto);
# is item on the reserve shelf?
- my $reservestatus = 0;
+ my $reservestatus = '';
my $reserveitem;
unless ($item->{wthdrawn}
@@ -1695,7 +1695,7 @@ sub searchResults {
# should map transit status to record indexed in Zebra.
#
($transfertwhen, $transfertfrom, $transfertto) = C4::Circulation::GetTransfers($item->{itemnumber});
- ($reservestatus, $reserveitem) = C4::Reserves::CheckReserves($item->{itemnumber});
+ ($reservestatus, $reserveitem, undef) = C4::Reserves::CheckReserves($item->{itemnumber});
}
# item is withdrawn, lost, damaged, not for loan, reserved or in transit
diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index 8bc4000..39071d5 100755
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -190,7 +190,7 @@ sub buildKohaItemsNamespace {
my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->{itemnumber});
- my ( $reservestatus, $reserveitem ) = C4::Reserves::CheckReserves($item->{itemnumber});
+ my ( $reservestatus, $reserveitem, undef ) = C4::Reserves::CheckReserves($item->{itemnumber});
if ( $itemtypes->{ $item->{itype} }->{notforloan} || $item->{notforloan} || $item->{onloan} || $item->{wthdrawn} || $item->{itemlost} || $item->{damaged} ||
(defined $transfertwhen && $transfertwhen ne '') || $item->{itemnotforloan} || (defined $reservestatus && $reservestatus eq "Waiting") ){
diff --git a/circ/circulation.pl b/circ/circulation.pl
index efb87da..5c239ce 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -430,7 +430,7 @@ sub build_issue_data {
$it->{'borrowernumber'},$it->{'itemnumber'}
);
$it->{"renew_error_${can_renew_error}"} = 1 if defined $can_renew_error;
- my ( $restype, $reserves ) = CheckReserves( $it->{'itemnumber'} );
+ my ( $restype, $reserves, undef ) = CheckReserves( $it->{'itemnumber'} );
$it->{'can_renew'} = $can_renew;
$it->{'can_confirm'} = !$can_renew && !$restype;
$it->{'renew_error'} = $restype;
diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 03c9e81..6952a85 100755
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -139,7 +139,7 @@ my $canrenew = 0;
if ($issues){
foreach my $issue ( sort { $b->{'date_due'} cmp $a->{'date_due'} } @$issues ) {
# check for reserves
- my ( $restype, $res ) = CheckReserves( $issue->{'itemnumber'} );
+ my ( $restype, $res, undef ) = CheckReserves( $issue->{'itemnumber'} );
if ( $restype ) {
$issue->{'reserved'} = 1;
}
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 177febb..ca4d42e 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -51,12 +51,12 @@ AddReserve($branch, $borrowernumber, $biblionumber,
$constraint, $bibitems, $priority, $notes,
$title, $checkitem, $found);
-my ($status, $reserve) = CheckReserves($itemnumber, $barcode);
+my ($status, $reserve, $all_reserves) = CheckReserves($itemnumber, $barcode);
ok($status eq "Reserved", "CheckReserves Test 1");
-($status, $reserve) = CheckReserves($itemnumber);
+($status, $reserve, $all_reserves) = CheckReserves($itemnumber);
ok($status eq "Reserved", "CheckReserves Test 2");
-($status, $reserve) = CheckReserves(undef, $barcode);
+($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode);
ok($status eq "Reserved", "CheckReserves Test 3");
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
More information about the Patches
mailing list