From f4ac05739f99844b4fcc18bd7b510d6c3c64b522 Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Mon, 15 Oct 2012 14:06:04 +0200
Subject: [PATCH] Bug 8918: Calculate hold priority in AddReserve
Content-Type: text/plain; charset=utf-8

Priority was calculated outside of this sub, in separate places.
Priority was not calculated when using ILS-DI.

This patch factorize code by putting the priority calculation code into
AddReserve.

Test plan:
1/ Place multiple holds in staff interface and check the priority is
   incremented for each hold.
2/ Do the same in OPAC.
3/ Place multiple holds using ILS-DI HoldTitle service:
   /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER
   and check the priority is incremented for each hold.
4/ Do the same using HoldItem service:
   /cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&item_id=ITEMNUMBER
---
 C4/ILSDI/Services.pm                               |   16 +-----
 C4/Reserves.pm                                     |   50 ++++++++++++++++++-
 .../en/modules/admin/preferences/circulation.pref  |    2 +-
 .../en/modules/admin/preferences/web_services.pref |    3 +-
 opac/opac-reserve.pl                               |   14 +-----
 reserve/placerequest.pl                            |   21 ++-------
 6 files changed, 59 insertions(+), 47 deletions(-)

diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
index 9a86d2e..76c4825 100644
--- a/C4/ILSDI/Services.pm
+++ b/C4/ILSDI/Services.pm
@@ -624,7 +624,7 @@ sub HoldTitle {
 
     # Add the reserve
     #          $branch, $borrowernumber, $biblionumber, $constraint, $bibitems,  $priority, $notes, $title, $checkitem,  $found
-    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, 0, undef, $title, undef, undef );
+    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, undef, undef, $title, undef, undef );
 
     # Hashref building
     my $out;
@@ -686,9 +686,8 @@ sub HoldItem {
     my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber );
     return { code => 'NotHoldable' } unless $canbookbereserved and $canitembereserved;
 
-    my $branch;
-
     # Pickup branch management
+    my $branch;
     if ( $cgi->param('pickup_location') ) {
         $branch = $cgi->param('pickup_location');
         my $branches = GetBranches();
@@ -697,18 +696,9 @@ sub HoldItem {
         $branch = $$borrower{branchcode};
     }
 
-    my $rank;
-    my $found;
-
-    # Get rank and found
-    $rank = '0' unless C4::Context->preference('ReservesNeedReturns');
-    if ( $item->{'holdingbranch'} eq $branch ) {
-        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
-    }
-
     # Add the reserve
     # $branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found
-    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $rank, '', '', '', $title, $itemnumber, $found );
+    AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, undef, '', '', '', $title, $itemnumber );
 
     # Hashref building
     my $out;
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index f066404..f06d0aa 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -134,7 +134,10 @@ BEGIN {
 
         &GetReservesControlBranch
     );
-    @EXPORT_OK = qw( MergeHolds );
+    @EXPORT_OK = qw(
+        MergeHolds
+        GetReserveNextRank
+    );
 }    
 
 =head2 AddReserve
@@ -161,9 +164,29 @@ sub AddReserve {
     } else {
         undef $expdate; # make reserves.expirationdate default to null rather than '0000-00-00'
     }
+
+    # Calculate priority if not given
+    if (not defined $priority) {
+        if ($checkitem and not C4::Context->preference('ReservesNeedReturns')) {
+            $priority = 0;
+        } else {
+            $priority = GetReserveNextRank($biblionumber);
+        }
+    }
+
+    # Calculate found if not given
+    if ($priority == 0 and not defined $found) {
+        my $item = GetItem($checkitem);
+        if ($item->{holdingbranch} eq $branch) {
+            $found = 'W';
+        } else {
+            $found = 'T';
+        }
+    }
+
     if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) {
-	# Make room in reserves for this before those of a later reserve date
-	$priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority );
+        # Make room in reserves for this before those of a later reserve date
+        $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority );
     }
     my $waitingdate;
 
@@ -2236,6 +2259,27 @@ sub GetReservesControlBranch {
     return $branchcode;
 }
 
+=head2 GetReserveNextRank
+
+    my $next_rank = GetReserveNextRank($biblionumber);
+
+Calculate the next available reseve rank for a biblionumber.
+
+=cut
+
+sub GetReserveNextRank {
+    my ($biblionumber) = @_;
+
+    my ( $count, $reserves ) = GetReservesFromBiblionumber($biblionumber,1);
+    foreach my $res (@$reserves) {
+        $count-- if (defined $res->{found} and $res->{found} eq 'W');
+    }
+    my $rank = $count + 1;
+
+    return $rank;
+}
+
+
 =head1 AUTHOR
 
 Koha Development Team <http://koha-community.org/>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
index 33ae4c9..ae9d815 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ -436,7 +436,7 @@ Circulation:
               choices:
                   yes: "Don't automatically"
                   no: Automatically
-            - mark holds as found and waiting when a hold is placed specifically on them and they are already checked in.
+            - mark a hold as found and waiting when a hold is placed on a specific item and that item is already checked in.
         -
             - Patrons can only have
             - pref: maxreserves
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
index 7244675..7a4c910 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref
@@ -35,9 +35,10 @@ Web services:
                 no: Disable
             - ILS-DI services for OPAC users
         -
+            - Allow IP addresses
             - pref: ILS-DI:AuthorizedIPs
               class: Text
-            - allowed IPs to use the ILS-DI services
+            - to use the ILS-DI services (when enabled). Separate the IP addresses with commas and without spaces. Leave the field blank to allow any IP address.
     Reporting:
         -
             - Only return
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index df55c19..307b36c 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -223,7 +223,6 @@ if ( $query->param('place_reserve') ) {
         }
 
         my $biblioData = $biblioDataHash{$biblioNum};
-        my $found;
 
         # Check for user supplied reserve date
         my $startdate;
@@ -235,17 +234,8 @@ if ( $query->param('place_reserve') ) {
 
         my $expiration_date = $query->param("expiration_date_$biblioNum");
 
-      # If a specific item was selected and the pickup branch is the same as the
-      # holdingbranch, force the value $rank and $found.
-        my $rank = $biblioData->{rank};
         if ( $itemNum ne '' ) {
             $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum );
-            $rank = '0' unless C4::Context->preference('ReservesNeedReturns');
-            my $item = GetItem($itemNum);
-            if ( $item->{'holdingbranch'} eq $branch ) {
-                $found = 'W'
-                  unless C4::Context->preference('ReservesNeedReturns');
-            }
         }
         else {
             $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum );
@@ -266,10 +256,10 @@ if ( $query->param('place_reserve') ) {
             AddReserve(
                 $branch,      $borrowernumber,
                 $biblioNum,   'a',
-                [$biblioNum], $rank,
+                [$biblioNum], undef,
                 $startdate,   $expiration_date,
                 $notes,       $biblioData->{title},
-                $itemNum,     $found
+                $itemNum,     undef
             );
             ++$reserve_cnt;
         }
diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl
index 3fe459c..bd1ce13 100755
--- a/reserve/placerequest.pl
+++ b/reserve/placerequest.pl
@@ -68,19 +68,6 @@ foreach my $bibnum (@biblionumbers) {
     $bibinfos{$bibnum} = \%bibinfo;
 }
 
-my $found;
-
-# if we have an item selectionned, and the pickup branch is the same as the holdingbranch
-# of the document, we force the value $rank and $found .
-if ($checkitem ne ''){
-    $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns');
-    my $item = $checkitem;
-    $item = GetItem($item);
-    if ( $item->{'holdingbranch'} eq $branch ){
-        $found = 'W' unless C4::Context->preference('ReservesNeedReturns');
-    }
-}
-
 if ($type eq 'str8' && $borrower){
 
     foreach my $biblionumber (keys %bibinfos) {
@@ -110,17 +97,17 @@ if ($type eq 'str8' && $borrower){
         if ($multi_hold) {
             my $bibinfo = $bibinfos{$biblionumber};
             AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber],
-                       $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found);
+                       $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem, undef);
         } else {
             if ($input->param('request') eq 'any'){
                 # place a request on 1st available
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found);
+                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef);
             } elsif ($reqbib[0] ne ''){
                 # FIXME : elsif probably never reached, (see top of the script)
                 # place a request on a given item
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found);
+                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef);
             } else {
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found);
+                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef);
             }
         }
     }
-- 
1.7.7.6