From e580d73b044544770661ae5883eac1e8c5da1434 Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@jns.fi>
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