From ce411c186dd2a4e35a3b1855b86283522cc7bfd3 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 Signed-off-by: Koha Team AMU Signed-off-by: Nick Clemens --- C4/Reserves.pm | 24 +++++++++++++++++++----- t/db_dependent/Holds.t | 27 ++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8e8bea3349..d7b9c9a1af 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -265,7 +265,7 @@ sub AddReserve { =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. @@ -273,7 +273,7 @@ See CanItemBeReserved() for possible return values. =cut sub CanBookBeReserved{ - my ($borrowernumber, $biblionumber) = @_; + my ($borrowernumber, $biblionumber, $branchcode) = @_; my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber"); #get items linked via host records @@ -284,7 +284,7 @@ sub CanBookBeReserved{ my $canReserve; foreach my $itemnumber (@itemnumbers) { - $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber ); + $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber, $branchcode ); return $canReserve if $canReserve->{status} eq 'OK'; } return $canReserve; @@ -292,7 +292,7 @@ sub CanBookBeReserved{ =head2 CanItemBeReserved - $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber) + $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber, $branchcode) if ($canReserve->{status} eq 'OK') { #We can reserve this Item! } @RETURNS { status => OK }, if the Item can be reserved. @@ -301,11 +301,13 @@ sub CanBookBeReserved{ { status => cannotReserveFromOtherBranches }, if syspref 'canreservefromotherbranches' is OK. { status => tooManyReserves, limit => $limit }, if the borrower has exceeded their maximum reserve amount. { status => notReservable }, if holds on this item are not allowed + { status => libraryNotFound }, if given branchcode is not an existing library + { status => 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 @@ -458,6 +460,18 @@ sub CanItemBeReserved { } } + if ($branchcode_to) { + my $destination = Koha::Libraries->find({ + branchcode => $branchcode_to, + }); + unless ($destination) { + return { status => 'libraryNotFound' }; + } + unless ($destination->pickup_location) { + return { status => 'libraryNotPickupLocation' }; + } + } + return { status => 'OK' }; } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 27151ae84f..2350bb6e80 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 => 55; +use Test::More tests => 62; use MARC::Record; use Koha::Patrons; use C4::Items; @@ -19,6 +19,8 @@ use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); use Koha::Biblios; use Koha::Holds; +use Koha::Items; +use Koha::Libraries; BEGIN { use FindBin; @@ -479,6 +481,29 @@ subtest 'Test max_holds per library/patron category' => sub { is( $ret->{status}, 'OK', 'Patron can place hold with branch/category rule of 5, category rule of 5' ); }; +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'); +}; + # Helper method to set up a Biblio. sub create_helper_biblio { my $itemtype = shift; -- 2.39.5