From e580d73b044544770661ae5883eac1e8c5da1434 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Tue, 7 Feb 2017 17:51:18 +0200 Subject: [PATCH] Bug 7534: Validate pickup location in CanBook/ItemBeReserved This patch adds $branchcode_to parameter to CanBookBeReserved and CanItemBeReserved. It represents the pickup location for the hold. To test: 1. prove t/db_dependent/Holds.t --- C4/Reserves.pm | 24 +++++++++++++++++++----- t/db_dependent/Holds.t | 26 +++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index c078af1..0b914fb 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,13 @@ 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 =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 +589,18 @@ sub CanItemBeReserved { } } + if ($branchcode_to) { + my $destination = Koha::Libraries->find({ + branchcode => $branchcode_to, + }); + unless ($destination) { + return 'libraryNotFound'; + } + unless ($destination->pickup_location) { + return 'libraryNotPickupLocation'; + } + } + return 'OK'; } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 61b9e6e..cf2c1d4 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 61; +use Test::More tests => 62; use MARC::Record; use C4::Biblio; use C4::Items; @@ -16,6 +16,8 @@ use C4::Calendar; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Holds; +use Koha::Items; +use Koha::Libraries; BEGIN { use FindBin; @@ -485,7 +487,29 @@ 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); + my $patron = $builder->build({ source => 'Borrower' })->{ borrowernumber }; + + t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1); + t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); + $library->pickup_location('1')->store; + is(CanItemBeReserved($patron, $item->itemnumber, $branch_to), + 'OK', 'Library is a pickup location'); + $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