Bug 8918: Fix reserve priority in ILS-DI
[koha.git] / C4 / Reserves.pm
index 78e9a5b..03a5c30 100644 (file)
@@ -252,6 +252,8 @@ sub AddReserve {
 
     $res = GetReserve( $reserve_id );
 
+    Return the current reserve.
+
 =cut
 
 sub GetReserve {
@@ -261,25 +263,39 @@ sub GetReserve {
     my $query = "SELECT * FROM reserves WHERE reserve_id = ?";
     my $sth = $dbh->prepare( $query );
     $sth->execute( $reserve_id );
-    my $res = $sth->fetchrow_hashref();
-    return $res;
+    return $sth->fetchrow_hashref();
 }
 
 =head2 GetReservesFromBiblionumber
 
-  ($count, $title_reserves) = GetReservesFromBiblionumber($biblionumber);
+  my $reserves = GetReservesFromBiblionumber({
+    biblionumber => $biblionumber,
+    [ itemnumber => $itemnumber, ]
+    [ all_dates => 1|0 ]
+  });
+
+This function gets the list of reservations for one C<$biblionumber>,
+returning an arrayref pointing to the reserves for C<$biblionumber>.
 
-This function gets the list of reservations for one C<$biblionumber>, returning a count
-of the reserves and an arrayref pointing to the reserves for C<$biblionumber>.
+By default, only reserves whose start date falls before the current
+time are returned.  To return all reserves, including future ones,
+the C<all_dates> parameter can be included and set to a true value.
+
+If the C<itemnumber> parameter is supplied, reserves must be targeted
+to that item or not targeted to any item at all; otherwise, they
+are excluded from the list.
 
 =cut
 
 sub GetReservesFromBiblionumber {
-    my ($biblionumber) = shift or return (0, []);
-    my ($all_dates) = shift;
+    my ( $params ) = @_;
+    my $biblionumber = $params->{biblionumber} or return [];
+    my $itemnumber = $params->{itemnumber};
+    my $all_dates = $params->{all_dates} // 0;
     my $dbh   = C4::Context->dbh;
 
     # Find the desired items in the reserves
+    my @params;
     my $query = "
         SELECT  reserve_id,
                 branchcode,
@@ -298,12 +314,17 @@ sub GetReservesFromBiblionumber {
                 suspend_until
         FROM     reserves
         WHERE biblionumber = ? ";
+    push( @params, $biblionumber );
     unless ( $all_dates ) {
-        $query .= "AND reservedate <= CURRENT_DATE()";
+        $query .= " AND reservedate <= CAST(NOW() AS DATE) ";
+    }
+    if ( $itemnumber ) {
+        $query .= " AND ( itemnumber IS NULL OR itemnumber = ? )";
+        push( @params, $itemnumber );
     }
     $query .= "ORDER BY priority";
     my $sth = $dbh->prepare($query);
-    $sth->execute($biblionumber);
+    $sth->execute( @params );
     my @results;
     my $i = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
@@ -345,32 +366,33 @@ sub GetReservesFromBiblionumber {
         }
         push @results, $data;
     }
-    return ( $#results + 1, \@results );
+    return \@results;
 }
 
 =head2 GetReservesFromItemnumber
 
- ( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = GetReservesFromItemnumber($itemnumber);
+ ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $waitingdate ) = GetReservesFromItemnumber($itemnumber);
+
+Get the first reserve for a specific item number (based on priority). Returns the abovementioned values for that reserve.
 
-TODO :: Description here
+The routine does not look at future reserves (read: item level holds), but DOES include future waits (a confirmed future hold).
 
 =cut
 
 sub GetReservesFromItemnumber {
-    my ( $itemnumber, $all_dates ) = @_;
+    my ( $itemnumber ) = @_;
     my $dbh   = C4::Context->dbh;
     my $query = "
-    SELECT reservedate,borrowernumber,branchcode,reserve_id
+    SELECT reservedate,borrowernumber,branchcode,reserve_id,waitingdate
     FROM   reserves
-    WHERE  itemnumber=?
+    WHERE  itemnumber=? AND ( reservedate <= CAST(now() AS date) OR
+           waitingdate IS NOT NULL )
+    ORDER BY priority
     ";
-    unless ( $all_dates ) {
-       $query .= " AND reservedate <= CURRENT_DATE()";
-    }
     my $sth_res = $dbh->prepare($query);
     $sth_res->execute($itemnumber);
-    my ( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = $sth_res->fetchrow_array;
-    return ( $reservedate, $borrowernumber, $branchcode, $reserve_id );
+    my ( $reservedate, $borrowernumber,$branchcode, $reserve_id, $wait ) = $sth_res->fetchrow_array;
+    return ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $wait );
 }
 
 =head2 GetReservesFromBorrowernumber
@@ -625,7 +647,6 @@ sub GetReserveFee {
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $data = $sth->fetchrow_hashref;
-    $sth->finish();
     my $fee      = $data->{'reservefee'};
     my $cntitems = @- > $bibitems;
 
@@ -666,7 +687,6 @@ sub GetReserveFee {
                 }
             }
         }
-        $sth1->finish;
         my $cntitemsfound = @biblioitems;
         my $issues        = 0;
         my $x             = 0;
@@ -802,10 +822,11 @@ sub GetReserveStatus {
     if(defined $found) {
         return 'Waiting'  if $found eq 'W' and $priority == 0;
         return 'Finished' if $found eq 'F';
-        return 'Reserved' if $priority > 0;
     }
-    return '';
-    #empty string here will remove need for checking undef, or less log lines
+
+    return 'Reserved' if $priority > 0;
+
+    return ''; # empty string here will remove need for checking undef, or less log lines
 }
 
 =head2 CheckReserves
@@ -1002,6 +1023,8 @@ sub CancelReserve {
 
     my $dbh = C4::Context->dbh;
 
+    my $reserve = GetReserve( $reserve_id );
+
     my $query = "
         UPDATE reserves
         SET    cancellationdate = now(),
@@ -1011,7 +1034,6 @@ sub CancelReserve {
     ";
     my $sth = $dbh->prepare($query);
     $sth->execute( $reserve_id );
-    $sth->finish;
 
     $query = "
         INSERT INTO old_reserves
@@ -1029,7 +1051,7 @@ sub CancelReserve {
     $sth->execute( $reserve_id );
 
     # now fix the priority on the others....
-    _FixPriority( $reserve_id );
+    _FixPriority({ biblionumber => $reserve->{biblionumber} });
 }
 
 =head2 ModReserve
@@ -1087,29 +1109,7 @@ sub ModReserve {
 
     my $dbh = C4::Context->dbh;
     if ( $rank eq "del" ) {
-        my $query = "
-            UPDATE reserves
-            SET    cancellationdate=now()
-            WHERE  reserve_id = ?
-        ";
-        my $sth = $dbh->prepare($query);
-        $sth->execute( $reserve_id );
-        $sth->finish;
-        $query = "
-            INSERT INTO old_reserves
-            SELECT *
-            FROM   reserves 
-            WHERE  reserve_id = ?
-        ";
-        $sth = $dbh->prepare($query);
-        $sth->execute( $reserve_id );
-        $query = "
-            DELETE FROM reserves 
-            WHERE  reserve_id = ?
-        ";
-        $sth = $dbh->prepare($query);
-        $sth->execute( $reserve_id );
-        
+        CancelReserve({ reserve_id => $reserve_id });
     }
     elsif ($rank =~ /^\d+/ and $rank > 0) {
         my $query = "
@@ -1118,7 +1118,6 @@ sub ModReserve {
         ";
         my $sth = $dbh->prepare($query);
         $sth->execute( $rank, $branchcode, $itemnumber, $reserve_id );
-        $sth->finish;
 
         if ( defined( $suspend_until ) ) {
             if ( $suspend_until ) {
@@ -1129,7 +1128,7 @@ sub ModReserve {
             }
         }
 
-        _FixPriority( $reserve_id, $rank );
+        _FixPriority({ reserve_id => $reserve_id, rank =>$rank });
     }
 }
 
@@ -1164,7 +1163,6 @@ sub ModReserveFill {
     my $sth = $dbh->prepare($query);
     $sth->execute( $biblionumber, $borrowernumber, $resdate );
     ($priority) = $sth->fetchrow_array;
-    $sth->finish;
 
     # update the database...
     $query = "UPDATE reserves
@@ -1176,7 +1174,6 @@ sub ModReserveFill {
                 ";
     $sth = $dbh->prepare($query);
     $sth->execute( $biblionumber, $resdate, $borrowernumber );
-    $sth->finish;
 
     # move to old_reserves
     $query = "INSERT INTO old_reserves
@@ -1198,7 +1195,7 @@ sub ModReserveFill {
     # now fix the priority on the others (if the priority wasn't
     # already sorted!)....
     unless ( $priority == 0 ) {
-        _FixPriority( $reserve_id );
+        _FixPriority({ reserve_id => $reserve_id });
     }
 }
 
@@ -1256,7 +1253,12 @@ sub ModReserveAffect {
 
     # get request - need to find out if item is already
     # waiting in order to not send duplicate hold filled notifications
-    my $request = GetReserveInfo($borrowernumber, $biblionumber);
+    my $reserve_id = GetReserveId({
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+    });
+    return unless defined $reserve_id;
+    my $request = GetReserveInfo($reserve_id);
     my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0;
 
     # If we affect a reserve that has to be transfered, don't set to Waiting
@@ -1336,8 +1338,8 @@ sub ModReserveMinusPriority {
     ";
     my $sth_upd = $dbh->prepare($query);
     $sth_upd->execute( $itemnumber, $reserve_id );
-    # second step update all others reservs
-    _FixPriority( $reserve_id, '0');
+    # second step update all others reserves
+    _FixPriority({ reserve_id => $reserve_id, rank => '0' });
 }
 
 =head2 GetReserveInfo
@@ -1490,15 +1492,15 @@ sub AlterPriority {
 
       my $priority = $reserve->{'priority'};
       $priority = $where eq 'up' ? $priority - 1 : $priority + 1;
-      _FixPriority( $reserve_id, $priority )
+      _FixPriority({ reserve_id => $reserve_id, rank => $priority })
 
     } elsif ( $where eq 'top' ) {
 
-      _FixPriority( $reserve_id, '1' )
+      _FixPriority({ reserve_id => $reserve_id, rank => '1' })
 
     } elsif ( $where eq 'bottom' ) {
 
-      _FixPriority( $reserve_id, '999999' )
+      _FixPriority({ reserve_id => $reserve_id, rank => '999999' });
 
     }
 }
@@ -1518,9 +1520,8 @@ sub ToggleLowestPriority {
 
     my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?");
     $sth->execute( $reserve_id );
-    $sth->finish;
     
-    _FixPriority( $reserve_id, '999999' );
+    _FixPriority({ reserve_id => $reserve_id, rank => '999999' });
 }
 
 =head2 ToggleSuspend
@@ -1536,7 +1537,7 @@ be cleared when it is unsuspended.
 sub ToggleSuspend {
     my ( $reserve_id, $suspend_until ) = @_;
 
-    $suspend_until = output_pref( dt_from_string( $suspend_until ), 'iso' ) if ( $suspend_until );
+    $suspend_until = output_pref({ dt => dt_from_string( $suspend_until ), dateformat => 'iso' }) if ( $suspend_until );
 
     my $do_until = ( $suspend_until ) ? '?' : 'NULL';
 
@@ -1553,7 +1554,6 @@ sub ToggleSuspend {
     push( @params, $reserve_id );
 
     $sth->execute( @params );
-    $sth->finish;
 }
 
 =head2 SuspendAll
@@ -1609,28 +1609,57 @@ sub SuspendAll {
     $dbh = C4::Context->dbh;
     $sth = $dbh->prepare( $query );
     $sth->execute( @query_params );
-    $sth->finish;
 }
 
 
 =head2 _FixPriority
 
-  &_FixPriority( $reserve_id, $rank, $ignoreSetLowestRank);
+  _FixPriority({
+    reserve_id => $reserve_id,
+    [rank => $rank,]
+    [ignoreSetLowestRank => $ignoreSetLowestRank]
+  });
+
+  or
+
+  _FixPriority({ biblionumber => $biblionumber});
+
+This routine adjusts the priority of a hold request and holds
+on the same bib.
+
+In the first form, where a reserve_id is passed, the priority of the
+hold is set to supplied rank, and other holds for that bib are adjusted
+accordingly.  If the rank is "del", the hold is cancelled.  If no rank
+is supplied, all of the holds on that bib have their priority adjusted
+as if the second form had been used.
+
+In the second form, where a biblionumber is passed, the holds on that
+bib (that are not captured) are sorted in order of increasing priority,
+then have reserves.priority set so that the first non-captured hold
+has its priority set to 1, the second non-captured hold has its priority
+set to 2, and so forth.
 
-Only used internally (so don't export it)
-Changed how this functions works #
-Now just gets an array of reserves in the rank order and updates them with
-the array index (+1 as array starts from 0)
-and if $rank is supplied will splice item from the array and splice it back in again
-in new priority rank
+In both cases, holds that have the lowestPriority flag on are have their
+priority adjusted to ensure that they remain at the end of the line.
 
-=cut 
+Note that the ignoreSetLowestRank parameter is meant to be used only
+when _FixPriority calls itself.
+
+=cut
 
 sub _FixPriority {
-    my ( $reserve_id, $rank, $ignoreSetLowestRank ) = @_;
+    my ( $params ) = @_;
+    my $reserve_id = $params->{reserve_id};
+    my $rank = $params->{rank} // '';
+    my $ignoreSetLowestRank = $params->{ignoreSetLowestRank};
+    my $biblionumber = $params->{biblionumber};
+
     my $dbh = C4::Context->dbh;
 
-    my $res = GetReserve( $reserve_id );
+    unless ( $biblionumber ) {
+        my $res = GetReserve( $reserve_id );
+        $biblionumber = $res->{biblionumber};
+    }
 
     if ( $rank eq "del" ) {
          CancelReserve({ reserve_id => $reserve_id });
@@ -1658,7 +1687,7 @@ sub _FixPriority {
         ORDER BY priority ASC
     ";
     my $sth = $dbh->prepare($query);
-    $sth->execute( $res->{'biblionumber'} );
+    $sth->execute( $biblionumber );
     while ( my $line = $sth->fetchrow_hashref ) {
         push( @priority,     $line );
     }
@@ -1693,7 +1722,6 @@ sub _FixPriority {
             $j + 1,
             $priority[$j]->{'reserve_id'}
         );
-        $sth->finish;
     }
     
     $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" );
@@ -1701,7 +1729,11 @@ sub _FixPriority {
     
     unless ( $ignoreSetLowestRank ) {
       while ( my $res = $sth->fetchrow_hashref() ) {
-        _FixPriority( $res->{'reserve_id'}, '999999', 1 );
+        _FixPriority({
+            reserve_id => $res->{'reserve_id'},
+            rank => '999999',
+            ignoreSetLowestRank => 1
+        });
       }
     }
 }
@@ -2171,8 +2203,11 @@ sub ReserveSlip {
 
 #   return unless ( C4::Context->boolean_preference('printreserveslips') );
 
-    my $reserve = GetReserveInfo($borrowernumber,$biblionumber )
-      or return;
+    my $reserve_id = GetReserveId({
+        biblionumber => $biblionumber,
+        borrowernumber => $borrowernumber
+    }) or return;
+    my $reserve = GetReserveInfo($reserve_id) or return;
 
     return  C4::Letters::GetPreparedLetter (
         module => 'circulation',
@@ -2214,6 +2249,42 @@ sub GetReservesControlBranch {
     return $branchcode;
 }
 
+=head2 CalculatePriority
+
+    my $p = CalculatePriority($biblionumber, $resdate);
+
+Calculate priority for a new reserve on biblionumber.
+The reserve date parameter is optional. Plays a role if the preference
+AllowHoldDateInFuture is set.
+After calculation of this priority, it is recommended to call
+_ShiftPriorityByDateAndPriority. Note that this is currently done in
+AddReserves.
+
+=cut
+
+sub  CalculatePriority {
+    my ( $biblionumber, $resdate ) = @_;
+
+    my $sql = qq{
+        SELECT COUNT(*) FROM reserves
+        WHERE biblionumber=? AND priority>0 AND
+            (found IS NULL or found='')
+    };
+        #skip found==W or found==T (waiting or transit holds)
+    if( $resdate ) {
+        $sql.= ' AND ( reservedate<=? )';
+    }
+    else {
+        $sql.= ' AND ( reservedate < NOW() )';
+    }
+    my $dbh = C4::Context->dbh();
+    my @row= $dbh->selectrow_array( $sql, undef, $resdate?
+        ($biblionumber, $resdate): ($biblionumber) );
+
+    return @row? $row[0]+1: 1;
+        #if @row does not contain anything, something went wrong..
+}
+
 =head1 AUTHOR
 
 Koha Development Team <http://koha-community.org/>