From 1630cade3517cec315dcb0d106898ace96ad0191 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 18 Sep 2014 07:44:36 -0400 Subject: [PATCH] Bug 11634 [QA Followup 3] - Found holds should be considered unavailable --- C4/Circulation.pm | 3 ++- C4/Reserves.pm | 31 +++++++++++++++++++++++++++++-- t/db_dependent/Circulation.t | 29 +++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 7d957fb..26da5e5 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2674,7 +2674,8 @@ sub CanBookBeRenewed { foreach my $b (@borrowernumbers) { foreach my $i (@itemnumbers) { if ( IsAvailableForItemLevelRequest($i) - && CanItemBeReserved( $b, $i ) ) + && CanItemBeReserved( $b, $i ) + && !IsItemOnHoldAndFound($i) ) { push( @reservable, $i ); } diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a6704ba..8701cbe 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -119,7 +119,7 @@ BEGIN { &CheckReserves &CanBookBeReserved - &CanItemBeReserved + &CanItemBeReserved &CancelReserve &CancelExpiredReserves @@ -135,6 +135,8 @@ BEGIN { &SuspendAll &GetReservesControlBranch + + IsItemOnHoldAndFound ); @EXPORT_OK = qw( MergeHolds ); } @@ -204,6 +206,7 @@ sub AddReserve { $const, $priority, $notes, $checkitem, $found, $waitingdate, $expdate ); + my $reserve_id = $sth->{mysql_insertid}; # Send e-mail to librarian if syspref is active if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){ @@ -247,7 +250,7 @@ sub AddReserve { $sth->execute($borrowernumber, $biblionumber, $resdate, $_); } - return; # FIXME: why not have a useful return value? + return $reserve_id; } =head2 GetReserve @@ -2336,6 +2339,30 @@ sub CalculatePriority { return @row ? $row[0]+1 : 1; } +=head2 IsItemOnHoldAndFound + + my $bool = IsItemFoundHold( $itemnumber ); + + Returns true if the item is currently on hold + and that hold has a non-null found status ( W, T, etc. ) + +=cut + +sub IsItemOnHoldAndFound { + my ($itemnumber) = @_; + + my $rs = Koha::Database->new()->schema()->resultset('Reserve'); + + my $found = $rs->count( + { + itemnumber => $itemnumber, + found => { '!=' => undef } + } + ); + + return $found; +} + =head1 AUTHOR Koha Development Team diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index a284c4f..8aa2dd3 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -26,7 +26,7 @@ use C4::Reserves; use Koha::DateUtils; use Koha::Database; -use Test::More tests => 57; +use Test::More tests => 59; BEGIN { use_ok('C4::Circulation'); @@ -233,7 +233,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); $biblionumber ); - # Create 2 borrowers + # Create borrowers my %renewing_borrower_data = ( firstname => 'John', surname => 'Renewal', @@ -248,8 +248,16 @@ C4::Context->dbh->do("DELETE FROM accountlines"); branchcode => $branch, ); + my %hold_waiting_borrower_data = ( + firstname => 'Kyle', + surname => 'Reservation', + categorycode => 'S', + branchcode => $branch, + ); + my $renewing_borrowernumber = AddMember(%renewing_borrower_data); my $reserving_borrowernumber = AddMember(%reserving_borrower_data); + my $hold_waiting_borrowernumber = AddMember(%hold_waiting_borrower_data); my $renewing_borrower = GetMember( borrowernumber => $renewing_borrowernumber ); @@ -282,11 +290,28 @@ C4::Context->dbh->do("DELETE FROM accountlines"); $title, $checkitem, $found ); + # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds C4::Context->set_preference('AllowRenewalIfOtherItemsAvailable', 1 ); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber2); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); + # Now let's add a waiting hold on the 3rd item, it's no longer available tp check out by just anyone, so we should no longer + # be able to renew these items + my $hold = Koha::Database->new()->schema()->resultset('Reserve')->create( + { + borrowernumber => $hold_waiting_borrowernumber, + biblionumber => $biblionumber, + itemnumber => $itemnumber3, + branchcode => $branch, + priority => 0, + found => 'W' + } + ); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber); + is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); + ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber2); + is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); C4::Context->set_preference('AllowRenewalIfOtherItemsAvailable', 0 ); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber); -- 1.7.2.5