From e3a76c9d954996866e88fa4c636213067106ce13 Mon Sep 17 00:00:00 2001
From: Alex Arnaud <alex.arnaud@biblibre.com>
Date: Wed, 6 May 2015 14:58:20 +0200
Subject: [PATCH] Add maxreserves and alreadyreserved checks when placing hold
 via ILSDI

http://bugs.koha-community.org/show_bug.cgi?id=14155

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
---
 C4/ILSDI/Services.pm            |   12 +++++--
 C4/Reserves.pm                  |   50 ++++++++++++++++++++++++++
 reserve/request.pl              |   17 +++------
 t/db_dependent/ILSDI_Services.t |   75 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 139 insertions(+), 15 deletions(-)

diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
index e69c070..1a45e36 100644
--- a/C4/ILSDI/Services.pm
+++ b/C4/ILSDI/Services.pm
@@ -26,7 +26,7 @@ use C4::Circulation;
 use C4::Branch;
 use C4::Accounts;
 use C4::Biblio;
-use C4::Reserves qw(AddReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest);
+use C4::Reserves qw(AddReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest AlreadyReserved ReachedMaxReserves);
 use C4::Context;
 use C4::AuthoritiesMarc;
 use XML::Simple;
@@ -605,9 +605,13 @@ sub HoldTitle {
     my $biblionumber = $cgi->param('bib_id');
     my $biblio = GetBiblio( $biblionumber );
     return { code => 'RecordNotFound' } unless $$biblio{biblionumber};
-    
+
     my $title = $$biblio{title};
 
+    return { code => 'NotHoldable' } if ReachedMaxReserves($borrowernumber);
+
+    return { code => 'NotHoldable' } if AlreadyReserved($biblionumber, $borrowernumber);
+
     # Check if the biblio can be reserved
     return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber ) eq 'OK';
 
@@ -684,6 +688,10 @@ sub HoldItem {
     # If the biblio does not match the item, return an error code
     return { code => 'RecordNotFound' } if $$item{biblionumber} ne $$biblio{biblionumber};
 
+    return { code => 'NotHoldable' } if ReachedMaxReserves($borrowernumber);
+
+    return { code => 'NotHoldable' } if AlreadyReserved($biblionumber, $borrowernumber);
+
     # Check for item disponibility
     my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber );
     my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber );
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 4b10ff9..1ea152e 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -126,6 +126,9 @@ BEGIN {
         &CancelReserve
         &CancelExpiredReserves
 
+        &AlreadyReserved
+        &ReachedMaxReserves
+
         &AutoUnsuspendReserves
 
         &IsAvailableForItemLevelRequest
@@ -1017,6 +1020,53 @@ sub CancelExpiredReserves {
 
 }
 
+=head2 AlreadyReserved
+
+    $alreadyreserved = AlreadyReserved($biblionumber, $borrowernumber);
+
+    return 1 if borrower already reserved this biblio.
+
+=cut
+
+sub AlreadyReserved {
+    my ($biblionumber, $borrowernumber) = @_;
+
+    return 0 unless $biblionumber && $borrowernumber;
+
+    my $reserves = GetReservesFromBiblionumber({ biblionumber => $biblionumber, all_dates => 1 });
+
+    return 0 unless scalar( @$reserves );
+
+    foreach my $res (@$reserves) {
+        return 1 if $borrowernumber eq $res->{borrowernumber};
+    }
+
+    return 0;
+}
+
+=head2 ReachedMaxReserves
+
+    $hasmaxreserves = ReachedMaxReserves($borrowernumber);
+
+    return 1 if borrower reached maxreserves.
+
+=cut
+
+sub ReachedMaxReserves {
+    my ($borrowernumber, $number_wanted) = @_;
+    $number_wanted ||= 1;
+
+    my $number_reserves = GetReserveCount( $borrowernumber );
+
+    return 0 unless C4::Context->preference('maxreserves');
+
+    if ( $number_reserves + $number_wanted >= C4::Context->preference('maxreserves') ) {
+        return 1;
+    }
+
+    return 0;
+}
+
 =head2 AutoUnsuspendReserves
 
   AutoUnsuspendReserves();
diff --git a/reserve/request.pl b/reserve/request.pl
index 80cf0d6..dacdff3 100755
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -151,9 +151,7 @@ if ($borrowernumber_hold && !$action) {
     my $new_reserves_count = scalar( @biblionumbers );
 
     my $maxreserves = C4::Context->preference('maxreserves');
-    if ( $maxreserves
-        && ( $reserves_count + $new_reserves_count > $maxreserves ) )
-    {
+    if ( ReachedMaxReserves($borrowerinfo->{'borrowernumber'}, $new_reserves_count) ) {
         my $new_reserves_allowed =
             $maxreserves - $reserves_count > 0
           ? $maxreserves - $reserves_count
@@ -243,23 +241,18 @@ foreach my $biblionumber (@biblionumbers) {
     my $reserves = GetReservesFromBiblionumber({ biblionumber => $biblionumber, all_dates => 1 });
     my $count = scalar( @$reserves );
     my $totalcount = $count;
-    my $holds_count = 0;
     my $alreadyreserved = 0;
 
     foreach my $res (@$reserves) {
         if ( defined $res->{found} ) { # found can be 'W' or 'T'
             $count--;
         }
-
-        if ( defined $borrowerinfo && defined($borrowerinfo->{borrowernumber}) && ($borrowerinfo->{borrowernumber} eq $res->{borrowernumber}) ) {
-            $holds_count++;
-        }
     }
 
-    if ( $holds_count ) {
-            $alreadyreserved = 1;
-            $biblioloopiter{warn} = 1;
-            $biblioloopiter{alreadyres} = 1;
+    if ( AlreadyReserved($biblionumber, $borrowerinfo->{borrowernumber}) ) {
+        $alreadyreserved = 1;
+        $biblioloopiter{warn} = 1;
+        $biblioloopiter{alreadyres} = 1;
     }
 
     $template->param(
diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t
index 9aee73e..473c826 100644
--- a/t/db_dependent/ILSDI_Services.t
+++ b/t/db_dependent/ILSDI_Services.t
@@ -4,9 +4,13 @@ use Modern::Perl;
 
 use C4::Members qw/AddMember GetMember GetBorrowercategory/;
 use C4::Branch;
+use C4::Biblio;
+use C4::Items;
+use C4::Reserves;
 use CGI qw ( -utf8 );
+use t::lib::Mocks;
 
-use Test::More tests => 15;
+use Test::More tests => 23;
 
 BEGIN {
     use_ok('C4::ILSDI::Services');
@@ -18,6 +22,9 @@ my $dbh = C4::Context->dbh;
 $dbh->{AutoCommit} = 0;
 $dbh->{RaiseError} = 1;
 
+my $maxallowedreserves = 3;
+t::lib::Mocks::mock_preference('maxreserves', $maxallowedreserves);
+
 # Create patron
 my %data = (
     firstname =>  'my firstname',
@@ -29,6 +36,16 @@ my %data = (
     password => 'ilsdi-password',
 );
 
+my %data2 = (
+    firstname =>  'Joe',
+    surname => 'John',
+    categorycode => 'UT',
+    branchcode => 'UT',
+    cardnumber => '99999',
+    userid => 'jjohn',
+    password => 'ilsdi-password',
+);
+
 # Crate patron category
 unless ( GetBorrowercategory('UT') ) {
     $dbh->do("INSERT INTO categories
@@ -44,8 +61,28 @@ unless ( GetBranchDetail('UT') ) {
 
 
 my $borrowernumber = AddMember(%data);
+my $borrowernumber2 = AddMember(%data2);
 my $borrower = GetMember( borrowernumber => $borrowernumber );
 
+# Create biblio and item
+my %item_branch_infos = (
+    homebranch    => 'UT',
+    holdingbranch => 'UT',
+);
+
+my $record = MARC::Record->new;
+my $title = MARC::Field->new('200','','','a' => 'Hello world.');
+$record->append_fields($title);
+
+my ($biblionumber) = AddBiblio($record, '');
+my ($biblionumber2) = AddBiblio(MARC::Record->new, '');
+my ($biblionumber3) = AddBiblio(MARC::Record->new, '');
+my ($biblionumber4) = AddBiblio(MARC::Record->new, '');
+my $itemnumber = AddItem({ barcode => '0101', %item_branch_infos }, $biblionumber);
+my $itemnumber2 = AddItem({ barcode => '0201', %item_branch_infos }, $biblionumber2);
+my $itemnumber3 = AddItem({ barcode => '0301', %item_branch_infos }, $biblionumber3);
+my $itemnumber4 = AddItem({ barcode => '0401', %item_branch_infos }, $biblionumber4);
+
 { # AuthenticatePatron test
 
     my $query = new CGI;
@@ -90,3 +127,39 @@ my $borrower = GetMember( borrowernumber => $borrowernumber );
 
 }
 
+{ # Add reserves tests
+
+    my $query = new CGI;
+    $query->param('patron_id',$borrowernumber);
+    $query->param('bib_id', $biblionumber);
+    $query->param('request_location','127.0.0.1');
+    my $reply = C4::ILSDI::Services::HoldTitle($query);
+    is($reply->{'title'}, 'Hello world.');
+    is($reply->{'pickup_location'}, 'Unit test library', 'Hold title');
+
+    $reply = C4::ILSDI::Services::HoldTitle($query);
+    is($reply->{'code'}, 'NotHoldable', 'Hold same title again - ILSDI');
+
+    $query->param('bib_id', $biblionumber2);
+    $reply = C4::ILSDI::Services::HoldTitle($query);
+    is($reply->{'pickup_location'}, 'Unit test library', 'Hold a second title - ILSDI');
+
+    $query->param('bib_id', $biblionumber3);
+    $reply = C4::ILSDI::Services::HoldTitle($query);
+    is($reply->{'pickup_location'}, 'Unit test library', 'Hold a third title - ILSDI');
+
+    $query->param('bib_id', $biblionumber4);
+    $reply = C4::ILSDI::Services::HoldTitle($query);
+    is($reply->{'code'}, 'NotHoldable', 'Hold a fourth title - ILSDI');
+
+    $query->param('bib_id', $biblionumber4);
+    $query->param('item_id', $itemnumber4);
+    $reply = C4::ILSDI::Services::HoldItem($query);
+    is($reply->{'code'}, 'NotHoldable', 'Hold a fourth item - ILSDI');
+
+    $query->param('bib_id', $biblionumber);
+    $query->param('patron_id',$borrowernumber2);
+    $query->param('item_id', $itemnumber);
+    $reply = C4::ILSDI::Services::HoldItem($query);
+    is($reply->{'pickup_location'}, 'Unit test library', 'Joe John hold an item - ILSDI');
+}
-- 
1.7.10.4