From 545c314a33c660878ddced95064d59eed6a9755a Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Tue, 7 Feb 2017 17:51:18 +0200 Subject: [PATCH] Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved This patch adds $branchcode_to parameter to CanBookBeReserved and CanItemBeReserved. It represents the pickup location for the hold. This patch checks if the library is configured to be a pickup location (see Bug 7534), and also if the item can be transferred into the given library (see Bug 18072). To test: 1. prove t/db_dependent/Holds.t --- C4/Reserves.pm | 28 +++++++++++++++++++++++----- t/db_dependent/Holds.t | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index c078af1..c60e2db 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -418,7 +418,7 @@ sub GetReservesFromBorrowernumber { =head2 CanBookBeReserved - $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber) + $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber, $branchcode) if ($canReserve eq 'OK') { #We can reserve this Item! } See CanItemBeReserved() for possible return values. @@ -426,7 +426,7 @@ See CanItemBeReserved() for possible return values. =cut sub CanBookBeReserved{ - my ($borrowernumber, $biblionumber) = @_; + my ($borrowernumber, $biblionumber, $branchcode) = @_; my $items = GetItemnumbersForBiblio($biblionumber); #get items linked via host records @@ -437,7 +437,7 @@ sub CanBookBeReserved{ my $canReserve; foreach my $item (@$items) { - $canReserve = CanItemBeReserved( $borrowernumber, $item ); + $canReserve = CanItemBeReserved( $borrowernumber, $item, $branchcode ); return 'OK' if $canReserve eq 'OK'; } return $canReserve; @@ -445,7 +445,7 @@ sub CanBookBeReserved{ =head2 CanItemBeReserved - $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber) + $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode) if ($canReserve eq 'OK') { #We can reserve this Item! } @RETURNS OK, if the Item can be reserved. @@ -454,11 +454,14 @@ sub CanBookBeReserved{ cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK. tooManyReserves, if the borrower has exceeded his maximum reserve amount. notReservable, if holds on this item are not allowed + libraryNotFound if given branchcode is not an existing library + libraryNotPickupLocation if given branchcode is not configured to be a pickup location + cannotBeTransferred if branch transfer limit applies on given item and branchcode =cut sub CanItemBeReserved { - my ( $borrowernumber, $itemnumber ) = @_; + my ( $borrowernumber, $itemnumber, $branchcode_to ) = @_; my $dbh = C4::Context->dbh; my $ruleitemtype; # itemtype of the matching issuing rule @@ -587,6 +590,21 @@ sub CanItemBeReserved { } } + if ($branchcode_to) { + my $destination = Koha::Libraries->find({ + branchcode => $branchcode_to, + }); + unless ($destination) { + return 'libraryNotFound'; + } + unless ($destination->pickup_location) { + return 'libraryNotPickupLocation'; + } + unless ($item->can_be_transferred({ to => $destination->branchcode })) { + return 'cannotBeTransferred'; + } + } + return 'OK'; } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 61b9e6e..e74dcf2 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -16,6 +16,9 @@ use C4::Calendar; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Holds; +use Koha::Item::Transfer::Limits; +use Koha::Items; +use Koha::Libraries; BEGIN { use FindBin; @@ -485,7 +488,36 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); +subtest 'Pickup location availability tests' => sub { + plan tests => 3; + my ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('ONLY1'); + my ( $item_bibnum, $item_bibitemnum, $itemnumber ) + = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum ); + my $item = Koha::Items->find($itemnumber); + my $branch_to = $builder->build({ source => 'Branch' })->{ branchcode }; + my $library = Koha::Libraries->find($branch_to); + $library->pickup_location('1')->store; + my $patron = $builder->build({ source => 'Borrower' })->{ borrowernumber }; + + t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); + t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); + + my $limit = Koha::Item::Transfer::Limit->new({ + fromBranch => $item->holdingbranch, + toBranch => $branch_to, + itemtype => $item->effective_itemtype, + })->store; + is(CanItemBeReserved($patron, $item->itemnumber, $branch_to), + 'cannotBeTransferred', 'Item cannot be transferred'); + $limit->delete; + $library->pickup_location('0')->store; + is(CanItemBeReserved($patron, $item->itemnumber, $branch_to), + 'libraryNotPickupLocation', 'Library is not a pickup location'); + is(CanItemBeReserved($patron, $item->itemnumber, 'nonexistent'), + 'libraryNotFound', 'Cannot set unknown library as pickup location'); +}; +my ( $bibnum, $title, $bibitemnum ); # Helper method to set up a Biblio. sub create_helper_biblio { my $itemtype = shift; -- 1.9.1