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 <axelle.clarisse@univ-amu.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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' };
 }
 

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;