batchRebuildBiblioTables.pl doesn't crash anymore when GetMarcBiblio fails.
[koha.git] / C4 / Reserves.pm
index 9d0061c..19f483f 100644 (file)
@@ -1,6 +1,3 @@
-# -*- tab-width: 8 -*-
-# NOTE: This file uses standard 8-character tabs
-
 package C4::Reserves;
 
 # Copyright 2000-2002 Katipo Communications
@@ -22,22 +19,25 @@ package C4::Reserves;
 # Koha; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
 # Suite 330, Boston, MA  02111-1307 USA
 
-# $Id$
 
 use strict;
-require Exporter;
+# use warnings;  # FIXME: someday
 use C4::Context;
 use C4::Biblio;
+use C4::Dates qw/format_date format_date_in_iso/;
+use C4::Members;
+use C4::Items;
 use C4::Search;
 use C4::Circulation;
 use C4::Accounts;
 
-our ($VERSION,@ISA,@EXPORT,@EXPORT_OK,%EXPORT_TAGS);
-
-my $library_name = C4::Context->preference("LibraryName");
+# for _koha_notify_reserve
+use C4::Members::Messaging;
+use C4::Letters;
+use C4::Branch qw( GetBranchDetail );
+use List::MoreUtils qw( firstidx any );
 
-# set the version for version checking
-$VERSION = do { my @v = '$Revision$' =~ /\d+/g; shift(@v) . "." . join( "_", map { sprintf "%03d", $_ } @v ); };
+use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 =head1 NAME
 
@@ -57,6 +57,7 @@ C4::Reserves - Koha functions for dealing with reservation.
              =0      : then the reserve is being dealed
   - found : NULL       : means the patron requested the 1st available, and we haven't choosen the item
             W(aiting)  : the reserve has an itemnumber affected, and is on the way
+            T(ransfet) : the reserve has an itemnumber affected, and is beeing transfered to pickup branch
             F(inished) : the reserve has been completed, and is done
   - itemnumber : empty : the reserve is still unaffected to an item
                  filled: the reserve is attached to an item
@@ -83,37 +84,46 @@ C4::Reserves - Koha functions for dealing with reservation.
 
 =cut
 
-@ISA = qw(Exporter);
-
-@EXPORT = qw(
-  &AddReserve
-  
-  &GetReservesFromItemnumber
-  &GetReservesFromBiblionumber
-  &GetReservesFromBorrowernumber
-  &GetReservesForBranch
-  &GetReservesToBranch
-  &GetReserveCount
-  &GetReserveFee
-  &GetReservesForBranch
-  &GetReservesToBranch
-  &GetOtherReserves
+BEGIN {
+    # set the version for version checking
+    $VERSION = 3.01;
+       require Exporter;
+    @ISA = qw(Exporter);
+    @EXPORT = qw(
+        &AddReserve
   
-  &ModReserveFill
-  &ModReserveAffect
-  &ModReserve
-  &ModReserveStatus
-  &ModReserveCancelAll
-  &ModReserveMinusPriority
-
-  &CheckReserves
-  &CancelReserve
-);
+        &GetPendingReserves
+        &GetReservesFromItemnumber
+        &GetReservesFromBiblionumber
+        &GetReservesFromBorrowernumber
+        &GetReservesForBranch
+        &GetReservesToBranch
+        &GetReserveCount
+        &GetReserveFee
+               &GetReserveInfo
+        &GetReserveStatus
+        
+        &GetOtherReserves
+        
+        &ModReserveFill
+        &ModReserveAffect
+        &ModReserve
+        &ModReserveStatus
+        &ModReserveCancelAll
+        &ModReserveMinusPriority
+        
+        &CheckReserves
+        &CanBookBeReserved
+        &CanItemBeReserved
+        &CancelReserve
 
+        &IsAvailableForItemLevelRequest
+    );
+}    
 
 =item AddReserve
 
-    AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$notes,$title,$checkitem,$found)
+    AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$notes,$title,$checkitem,$found, $from)
 
 =cut
 
@@ -121,7 +131,7 @@ sub AddReserve {
     my (
         $branch,    $borrowernumber, $biblionumber,
         $constraint, $bibitems,  $priority,       $notes,
-        $title,      $checkitem, $found
+        $title,      $checkitem, $found, $from
     ) = @_;
     my $fee =
           GetReserveFee($borrowernumber, $biblionumber, $constraint,
@@ -151,7 +161,6 @@ sub AddReserve {
         my $usth = $dbh->prepare($query);
         $usth->execute( $borrowernumber, $nextacctno, $fee,
             "Reserve Charge - $title", $fee );
-        $usth->finish;
     }
 
     #if ($const eq 'a'){
@@ -169,42 +178,148 @@ sub AddReserve {
         $const,          $priority,     $notes,   $checkitem,
         $found,          $waitingdate
     );
-    $sth->finish;
+
+    # Send e-mail to librarian if syspref is active
+    if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
+        my $borrower = GetMemberDetails($borrowernumber);
+        my $biblio   = GetBiblioData($biblionumber);
+       my $lettertype = ($from eq "intranet") ? "STAFFHOLDPLACED" : "HOLDPLACED";
+        my $letter = C4::Letters::getletter( 'reserves', $lettertype);
+        my $admin_email_address = C4::Context->preference('KohaAdminEmailAddress');
+
+        my %keys = (%$borrower, %$biblio);
+        foreach my $key (keys %keys) {
+            my $replacefield = "<<$key>>";
+            $letter->{content} =~ s/$replacefield/$keys{$key}/g;
+            $letter->{title} =~ s/$replacefield/$keys{$key}/g;
+        }
+        
+        C4::Letters::EnqueueLetter(
+                            {   letter                 => $letter,
+                                borrowernumber         => $borrowernumber,
+                                message_transport_type => 'email',
+                                from_address           => $admin_email_address,
+                                to_address           => $admin_email_address,
+                            }
+                        );
+        
+
+    }
+
 
     #}
-    if ( ( $const eq "o" ) || ( $const eq "e" ) ) {
-        my $numitems = @$bibitems;
-        my $i        = 0;
-        while ( $i < $numitems ) {
-            my $biblioitem = @$bibitems[$i];
-            my $query      = qq/
-          INSERT INTO reserveconstraints
-              (borrowernumber,biblionumber,reservedate,biblioitemnumber)
-          VALUES
+    ($const eq "o" || $const eq "e") or return;   # FIXME: why not have a useful return value?
+    $query = qq/
+        INSERT INTO reserveconstraints
+            (borrowernumber,biblionumber,reservedate,biblioitemnumber)
+        VALUES
             (?,?,?,?)
-      /;
-            my $sth = $dbh->prepare("");
-            $sth->execute( $borrowernumber, $biblionumber, $resdate,
-                $biblioitem );
-            $sth->finish;
-            $i++;
+    /;
+    $sth = $dbh->prepare($query);    # keep prepare outside the loop!
+    foreach (@$bibitems) {
+        $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
+    }
+        
+    return;     # FIXME: why not have a useful return value?
+}
+
+
+
+=item GetPendingReserves
+
+=cut
+
+sub GetPendingReserves {
+    my ($filters, $startindex, $results) = @_;
+
+    $startindex = "0" if not $startindex;
+
+    my @query_params;
+    my $indepbranch  = C4::Context->preference('IndependantBranches') ? C4::Context->userenv->{'branch'} : undef;
+    my $dbh          = C4::Context->dbh;
+    
+    my $query = "SELECT DISTINCT(biblionumber) AS biblionumber 
+                 FROM reserves 
+                 LEFT JOIN biblio USING(biblionumber)
+                 WHERE reserves.found IS NULL ";
+    
+    if ($indepbranch){
+           $query .= " AND branchcode = ? ";
+        push @query_params, $indepbranch;
+    }
+    
+    my $sth = $dbh->prepare($query);
+    $sth->execute(@query_params);
+    
+    my %reserves;
+    
+    while ( my $reserve = $sth->fetchrow_hashref ) {
+        my $line;
+        unless( $line = $reserves{$reserve->{biblionumber}} ){
+            $line      = {};
+            my $biblio = GetBiblioData($reserve->{biblionumber});
+            my @items  = GetItemsInfo($reserve->{biblionumber});
+                    
+            $line->{title}           = $biblio->{title};
+            foreach my $item (@items){
+                next if ( ($indepbranch && $indepbranch ne $item->{holdingbranch}) 
+                          or $item->{onloan} 
+                          or $item->{notforloan} 
+                          or $item->{itemlost} 
+                          or $item->{count_reserves} eq "Waiting" or $item->{count_reserves} eq "Transit");
+                $line->{count}++;
+                $line->{holdingbranches}->{$item->{holdingbranch}} = 1;
+                $line->{callnumbers}->{$item->{itemcallnumber}} = 1;
+                $line->{locations}->{$item->{location}} = 1;
+                $line->{itemtypes}->{$item->{itemtype}} = 1;
+            }
         }
+        $line->{reservecount}++;
+        $reserves{$reserve->{biblionumber}} = $line if($line->{count});
     }
-    return;
+    
+    my @reserves;
+    foreach my $rkey (keys %reserves){
+        my $line = $reserves{$rkey};
+        $line->{biblionumber} = $rkey;
+        
+        foreach my $datatype (qw/holdingbranches callnumbers locations itemtypes/){
+            my @newdatas = ();
+            foreach my $data (keys %{$line->{$datatype}}){
+                push @newdatas, { 'value' => $data}
+            }
+            $line->{$datatype} = \@newdatas;
+        }
+        my $filtered = 1;
+        foreach my $key (keys %$filters){
+            my $value = $filters->{$key};
+            $filtered = 0 if not (any { $_->{value} =~ /^$value$/ } @{$line->{$key}}) and $value;
+        }
+        push @reserves, $line if $filtered; # if (any { $_->{value} =~ /^FOSPC$/ } @{$line->{holdingbranches}});
+    }
+    
+    my $count = scalar @reserves;
+    my $endindex = ($count > $startindex + $results) ? $startindex + $results : $count;
+    
+    if($count){
+        @reserves = @reserves[$startindex..$endindex];
+    }
+    
+    
+    return ($count, \@reserves);
 }
 
 =item GetReservesFromBiblionumber
 
-@borrowerreserv=&GetReserves($biblionumber,$itemnumber,$borrowernumber);
+($count, $title_reserves) = &GetReserves($biblionumber);
 
-this function get the list of reservation for an C<$biblionumber>, C<$itemnumber> or C<$borrowernumber>
-given on input arg. 
-Only 1 argument has to be passed.
+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>.
 
 =cut
 
 sub GetReservesFromBiblionumber {
-    my ( $biblionumber, $itemnumber, $borrowernumber ) = @_;
+    my ($biblionumber) = shift or return (0, []);
     my $dbh   = C4::Context->dbh;
 
     # Find the desired items in the reserves
@@ -217,11 +332,10 @@ sub GetReservesFromBiblionumber {
                 reservedate,
                 constrainttype,
                 found,
-                itemnumber
+                itemnumber,
+                reservenotes
         FROM     reserves
-        WHERE     cancellationdate IS NULL
-        AND    (found <> \'F\' OR found IS NULL)
-        AND biblionumber = ?
+        WHERE biblionumber = ?
         ORDER BY priority";
     my $sth = $dbh->prepare($query);
     $sth->execute($biblionumber);
@@ -229,39 +343,34 @@ sub GetReservesFromBiblionumber {
     my $i = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
 
-        # FIXME - What is this if-statement doing? How do constraints work?
-        if ( $data->{constrainttype} eq 'o' ) {
+        # FIXME - What is this doing? How do constraints work?
+        if ($data->{constrainttype} eq 'o') {
             $query = '
                 SELECT biblioitemnumber
-                FROM reserveconstraints
-                WHERE biblionumber   = ?
-                    AND borrowernumber = ?
-                AND reservedate    = ?
+                FROM  reserveconstraints
+                WHERE  biblionumber   = ?
+                AND   borrowernumber = ?
+                AND   reservedate    = ?
             ';
             my $csth = $dbh->prepare($query);
-            $csth->execute( $data->{biblionumber}, $data->{borrowernumber},
-                $data->{reservedate}, );
-
+            $csth->execute($data->{biblionumber}, $data->{borrowernumber}, $data->{reservedate});
             my @bibitemno;
             while ( my $bibitemnos = $csth->fetchrow_array ) {
-                push( @bibitemno, $bibitemnos );
+                push( @bibitemno, $bibitemnos );    # FIXME: inefficient: use fetchall_arrayref
             }
-            my $count = @bibitemno;
-
+            my $count = scalar @bibitemno;
+    
             # if we have two or more different specific itemtypes
             # reserved by same person on same day
             my $bdata;
             if ( $count > 1 ) {
-                $bdata = GetBiblioItemData( $bibitemno[$i] );
-                $i++;
+                $bdata = GetBiblioItemData( $bibitemno[$i] );   # FIXME: This doesn't make sense.
+                $i++; #  $i can increase each pass, but the next @bibitemno might be smaller?
             }
             else {
-
                 # Look up the book we just found.
                 $bdata = GetBiblioItemData( $bibitemno[0] );
             }
-            $csth->finish;
-
             # Add the results of this latest search to the current
             # results.
             # FIXME - An 'each' would probably be more efficient.
@@ -271,7 +380,6 @@ sub GetReservesFromBiblionumber {
         }
         push @results, $data;
     }
-    $sth->finish;
     return ( $#results + 1, \@results );
 }
 
@@ -290,8 +398,6 @@ sub GetReservesFromItemnumber {
     SELECT reservedate,borrowernumber,branchcode
     FROM   reserves
     WHERE  itemnumber=?
-        AND  cancellationdate IS NULL
-        AND  (found <> 'F' OR found IS NULL)
     ";
     my $sth_res = $dbh->prepare($query);
     $sth_res->execute($itemnumber);
@@ -316,7 +422,6 @@ sub GetReservesFromBorrowernumber {
             SELECT *
             FROM   reserves
             WHERE  borrowernumber=?
-                AND  cancellationdate IS NULL
                 AND found =?
             ORDER BY reservedate
         ");
@@ -326,8 +431,6 @@ sub GetReservesFromBorrowernumber {
             SELECT *
             FROM   reserves
             WHERE  borrowernumber=?
-                AND  cancellationdate IS NULL
-                AND (found != 'F' or found is null)
             ORDER BY reservedate
         ");
         $sth->execute($borrowernumber);
@@ -336,6 +439,89 @@ sub GetReservesFromBorrowernumber {
     return @$data;
 }
 #-------------------------------------------------------------------------------------
+=item CanBookBeReserved
+
+$error = &CanBookBeReserved($borrowernumber, $biblionumber)
+
+=cut
+
+sub CanBookBeReserved{
+    my ($borrowernumber, $biblionumber) = @_;
+
+    my $dbh           = C4::Context->dbh;
+    my $biblio        = GetBiblioData($biblionumber);
+    my $borrower      = C4::Members::GetMember(borrowernumber=>$borrowernumber);
+    my $controlbranch = C4::Context->preference('ReservesControlBranch');
+    my $itype         = C4::Context->preference('item-level_itypes');
+    my $reservesrights= C4::Context->preference('maxreserves');
+    my $reservescount = 0;
+    
+    # we retrieve the user rights
+    my @args;
+    my $branchcode;
+    
+    
+    if($controlbranch eq "ItemHomeLibrary"){
+        $branchcode = '*';
+    }elsif($controlbranch eq "PatronLibrary"){
+        $branchcode = $borrower->{branchcode};
+    }
+
+    $reservescount = GetReserveCount($borrowernumber);
+
+    if($reservescount < $reservesrights){
+        return 1;
+    }else{
+        return 0;
+    }
+    
+}
+
+=item CanItemBeReserved
+
+$error = &CanItemBeReserved($borrowernumber, $itemnumber)
+
+this function return 1 if an item can be issued by this borrower.
+
+=cut
+
+sub CanItemBeReserved{
+    my ($borrowernumber, $itemnumber) = @_;
+    
+    my $dbh             = C4::Context->dbh;
+            
+    my $controlbranch   = C4::Context->preference('ReservesControlBranch') || "ItemHomeLibrary";
+    my $itype           = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
+    my $allowedreserves = C4::Context->preference('maxreserves');
+    
+    # we retrieve borrowers and items informations #
+    my $item     = C4::Items::GetItem($itemnumber);
+    my $borrower = C4::Members::GetMember($borrowernumber, 'borrowernumber');     
+
+    my $branchcode   = "*";
+    my $branchfield  = "reserves.branchcode";
+    
+    if( $controlbranch eq "ItemHomeLibrary" ){
+        $branchcode = $item->{homebranch};
+    }elsif( $controlbranch eq "PatronLibrary" ){
+        $branchcode = $borrower->{branchcode};
+    }
+    
+    # we retrieve user rights on this itemtype and branchcode
+    my $issuingrule = C4::Circulation::GetIssuingRule($borrower->{categorycode}, $item->{$itype}, $branchcode);
+    
+    # we retrieve count
+    
+    my $reservecount = GetReserveCount($borrowernumber);
+
+    # we check if it's ok or not
+    if(( $reservecount < $allowedreserves ) and $issuingrule->{maxissueqty} ){
+        return 1;
+    }else{
+        return 0;
+    }
+}
+#-------------------------------------------------------------------------------------
 
 =item GetReserveCount
 
@@ -354,14 +540,10 @@ sub GetReserveCount {
         SELECT COUNT(*) AS counter
         FROM reserves
           WHERE borrowernumber = ?
-          AND cancellationdate IS NULL
-          AND (found != \'F\' OR found IS NULL)
     ';
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $row = $sth->fetchrow_hashref;
-    $sth->finish;
-
     return $row->{counter};
 }
 
@@ -390,7 +572,7 @@ sub GetOtherReserves {
             );
 
             #launch the subroutine dotransfer
-            C4::Circulation::ModItemTransfer(
+            C4::Items::ModItemTransfer(
                 $itemnumber,
                 $iteminfo->{'holdingbranch'},
                 $checkreserves->{'branchcode'}
@@ -493,8 +675,7 @@ sub GetReserveFee {
             while ( my $itdata = $sth2->fetchrow_hashref ) {
                 my $sth3 = $dbh->prepare(
                     "SELECT * FROM issues
-                       WHERE itemnumber = ?
-                         AND returndate IS NULL"
+                       WHERE itemnumber = ?"
                 );
                 $sth3->execute( $itdata->{'itemnumber'} );
                 if ( my $isdata = $sth3->fetchrow_hashref ) {
@@ -533,9 +714,8 @@ sub GetReservesToBranch {
     my $sth = $dbh->prepare(
         "SELECT borrowernumber,reservedate,itemnumber,timestamp
          FROM reserves 
-         WHERE priority='0' AND cancellationdate is null  
-           AND branchcode=?
-           AND found IS NULL "
+         WHERE priority='0' 
+           AND branchcode=?"
     );
     $sth->execute( $frombranch );
     my @transreserv;
@@ -544,7 +724,6 @@ sub GetReservesToBranch {
         $transreserv[$i] = $data;
         $i++;
     }
-    $sth->finish;
     return (@transreserv);
 }
 
@@ -557,25 +736,42 @@ sub GetReservesToBranch {
 sub GetReservesForBranch {
     my ($frombranch) = @_;
     my $dbh          = C4::Context->dbh;
-    my $sth          = $dbh->prepare( "
-        SELECT borrowernumber,reservedate,itemnumber,waitingdate
+       my $query        = "SELECT borrowernumber,reservedate,itemnumber,waitingdate
         FROM   reserves 
         WHERE   priority='0'
-            AND cancellationdate IS NULL 
-            AND found='W' 
-            AND branchcode=?
-        ORDER BY waitingdate" );
-    $sth->execute($frombranch);
+            AND found='W' ";
+    if ($frombranch){
+        $query .= " AND branchcode=? ";
+       }
+    $query .= "ORDER BY waitingdate" ;
+    my $sth = $dbh->prepare($query);
+    if ($frombranch){
+               $sth->execute($frombranch);
+       }
+    else {
+               $sth->execute();
+       }
     my @transreserv;
     my $i = 0;
     while ( my $data = $sth->fetchrow_hashref ) {
         $transreserv[$i] = $data;
         $i++;
     }
-    $sth->finish;
     return (@transreserv);
 }
 
+sub GetReserveStatus {
+    my ($itemnumber) = @_;
+    
+    my $dbh = C4::Context->dbh;
+    
+    my $itemstatus = $dbh->prepare("SELECT found FROM reserves WHERE itemnumber = ?");
+    
+    $itemstatus->execute($itemnumber);
+    my ($found) = $itemstatus->fetchrow_array;
+    return $found;
+}
+
 =item CheckReserves
 
   ($status, $reserve) = &CheckReserves($itemnumber);
@@ -610,7 +806,7 @@ sub CheckReserves {
         my $qitem = $dbh->quote($item);
         # Look up the item by itemnumber
         my $query = "
-            SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan
+            SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan
             FROM   items
             LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
             LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
@@ -622,7 +818,7 @@ sub CheckReserves {
         my $qbc = $dbh->quote($barcode);
         # Look up the item by barcode
         my $query = "
-            SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan
+            SELECT items.biblionumber, items.biblioitemnumber, itemtypes.notforloan, items.notforloan AS itemnotforloan
             FROM   items
             LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
             LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype
@@ -635,15 +831,15 @@ sub CheckReserves {
         # FIXME - This function uses $item later on. Ought to set it here.
     }
     $sth->execute;
-    my ( $biblio, $bibitem, $notforloan ) = $sth->fetchrow_array;
+    my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item ) = $sth->fetchrow_array;
     $sth->finish;
-
     # if item is not for loan it cannot be reserved either.....
-    return ( 0, 0 ) if $notforloan;
+    #    execption to notforloan is where items.notforloan < 0 :  This indicates the item is holdable. 
+    return ( 0, 0 ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
 
     # get the reserves...
     # Find this item in the reserves
-    my @reserves = _Findgroupreserve( $bibitem, $biblio );
+    my @reserves = _Findgroupreserve( $bibitem, $biblio, $item );
     my $count    = scalar @reserves;
 
     # $priority and $highest are used to find the most important item
@@ -656,10 +852,13 @@ sub CheckReserves {
         foreach my $res (@reserves) {
             # FIXME - $item might be undefined or empty: the caller
             # might be searching by barcode.
-            if ( $res->{'itemnumber'} == $item ) {
+            if ( $res->{'itemnumber'} == $item && $res->{'priority'} == 0) {
                 # Found it
                 return ( "Waiting", $res );
             }
+            elsif( $res->{'itemnumber'} == $item && $res->{'found'} eq 'T' ){
+                return ( "Transit", $res );
+            }
             else {
                 # See if this item is more important than what we've got
                 # so far.
@@ -705,7 +904,7 @@ priorities of the other people who are waiting on the book.
 sub CancelReserve {
     my ( $biblio, $item, $borr ) = @_;
     my $dbh = C4::Context->dbh;
-        if ( ( $item and $borr ) and ( not $biblio ) ) {
+        if ( $item and $borr ) {
         # removing a waiting reserve record....
         # update the database...
         my $query = "
@@ -719,8 +918,23 @@ sub CancelReserve {
         my $sth = $dbh->prepare($query);
         $sth->execute( $item, $borr );
         $sth->finish;
+        $query = "
+            INSERT INTO old_reserves
+            SELECT * FROM reserves
+            WHERE  itemnumber       = ?
+             AND   borrowernumber   = ?
+        ";
+        $sth = $dbh->prepare($query);
+        $sth->execute( $item, $borr );
+        $query = "
+            DELETE FROM reserves
+            WHERE  itemnumber       = ?
+             AND   borrowernumber   = ?
+        ";
+        $sth = $dbh->prepare($query);
+        $sth->execute( $item, $borr );
     }
-    if ( ( $biblio and $borr ) and ( not $item ) ) {
+    else {
         # removing a reserve record....
         # get the prioritiy on this record....
         my $priority;
@@ -730,7 +944,6 @@ sub CancelReserve {
               AND borrowernumber = ?
               AND cancellationdate IS NULL
               AND itemnumber IS NULL
-              AND (found <> 'F' OR found IS NULL)
         /;
         my $sth = $dbh->prepare($query);
         $sth->execute( $biblio, $borr );
@@ -743,8 +956,6 @@ sub CancelReserve {
                    priority         = 0
             WHERE  biblionumber     = ?
               AND  borrowernumber   = ?
-              AND cancellationdate IS NULL
-              AND (found <> 'F' or found IS NULL)
         /;
 
         # update the database, removing the record...
@@ -752,6 +963,23 @@ sub CancelReserve {
         $sth->execute( $biblio, $borr );
         $sth->finish;
 
+        $query = qq/
+            INSERT INTO old_reserves
+            SELECT * FROM reserves
+            WHERE  biblionumber     = ?
+              AND  borrowernumber   = ?
+        /;
+        $sth = $dbh->prepare($query);
+        $sth->execute( $biblio, $borr );
+
+        $query = qq/
+            DELETE FROM reserves
+            WHERE  biblionumber     = ?
+              AND  borrowernumber   = ?
+        /;
+        $sth = $dbh->prepare($query);
+        $sth->execute( $biblio, $borr );
+
         # now fix the priority on the others....
         _FixPriority( $priority, $biblio );
     }
@@ -759,7 +987,36 @@ sub CancelReserve {
 
 =item ModReserve
 
-&ModReserve($rank,$biblio,$borrower,$branch)
+=over 4
+
+ModReserve($rank, $biblio, $borrower, $branch[, $itemnumber])
+
+=back
+
+Change a hold request's priority or cancel it.
+
+C<$rank> specifies the effect of the change.  If C<$rank>
+is 'W' or 'n', nothing happens.  This corresponds to leaving a
+request alone when changing its priority in the holds queue
+for a bib.
+
+If C<$rank> is 'del', the hold request is cancelled.
+
+If C<$rank> is an integer greater than zero, the priority of
+the request is set to that value.  Since priority != 0 means
+that the item is not waiting on the hold shelf, setting the 
+priority to a non-zero value also sets the request's found
+status and waiting date to NULL. 
+
+The optional C<$itemnumber> parameter is used only when
+C<$rank> is a non-zero integer; if supplied, the itemnumber 
+of the hold request is set accordingly; if omitted, the itemnumber
+is cleared.
+
+FIXME: Note that the forgoing can have the effect of causing
+item-level hold requests to turn into title-level requests.  This
+will be fixed once reserves has separate columns for requested
+itemnumber and supplying itemnumber.
 
 =cut
 
@@ -775,21 +1032,33 @@ sub ModReserve {
             SET    cancellationdate=now()
             WHERE  biblionumber   = ?
              AND   borrowernumber = ?
-             AND   cancellationdate is NULL
-             AND   (found <> 'F' or found is NULL)
         /;
         my $sth = $dbh->prepare($query);
         $sth->execute( $biblio, $borrower );
         $sth->finish;
+        $query = qq/
+            INSERT INTO old_reserves
+            SELECT *
+            FROM   reserves 
+            WHERE  biblionumber   = ?
+             AND   borrowernumber = ?
+        /;
+        $sth = $dbh->prepare($query);
+        $sth->execute( $biblio, $borrower );
+        $query = qq/
+            DELETE FROM reserves 
+            WHERE  biblionumber   = ?
+             AND   borrowernumber = ?
+        /;
+        $sth = $dbh->prepare($query);
+        $sth->execute( $biblio, $borrower );
         
     }
-    else {
+    elsif ($rank =~ /^\d+/ and $rank > 0) {
         my $query = qq/
-        UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL
+        UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL
             WHERE biblionumber   = ?
              AND borrowernumber = ?
-             AND cancellationdate is NULL
-             AND (found <> 'F' or found is NULL)
         /;
         my $sth = $dbh->prepare($query);
         $sth->execute( $rank, $branch,$itemnumber, $biblio, $borrower);
@@ -842,6 +1111,23 @@ sub ModReserveFill {
     $sth->execute( $biblionumber, $resdate, $borrowernumber );
     $sth->finish;
 
+    # move to old_reserves
+    $query = "INSERT INTO old_reserves
+                 SELECT * FROM reserves
+                 WHERE  biblionumber     = ?
+                    AND reservedate      = ?
+                    AND borrowernumber   = ?
+                ";
+    $sth = $dbh->prepare($query);
+    $sth->execute( $biblionumber, $resdate, $borrowernumber );
+    $query = "DELETE FROM reserves
+                 WHERE  biblionumber     = ?
+                    AND reservedate      = ?
+                    AND borrowernumber   = ?
+                ";
+    $sth = $dbh->prepare($query);
+    $sth->execute( $biblionumber, $resdate, $borrowernumber );
+    
     # now fix the priority on the others (if the priority wasn't
     # already sorted!)....
     unless ( $priority == 0 ) {
@@ -874,7 +1160,6 @@ sub ModReserveStatus {
     ";
     my $sth_set = $dbh->prepare($query);
     $sth_set->execute( $newstatus, $itemnumber );
-    $sth_set->finish;
 }
 
 =item ModReserveAffect
@@ -905,12 +1190,11 @@ sub ModReserveAffect {
     if ($transferToDo) {
     $query = "
         UPDATE reserves
-        SET    priority = 0,
-               itemnumber = ?
+        SET    priority   = 0,
+               itemnumber = ?,
+               found      = 'T'
         WHERE borrowernumber = ?
           AND biblionumber = ?
-          AND reserves.cancellationdate IS NULL
-          AND (reserves.found <> 'F' OR reserves.found IS NULL)
     ";
     }
     else {
@@ -923,13 +1207,14 @@ sub ModReserveAffect {
                 itemnumber = ?
         WHERE borrowernumber = ?
           AND biblionumber = ?
-          AND reserves.cancellationdate IS NULL
-          AND (reserves.found <> 'F' OR reserves.found IS NULL)
     ";
     }
     $sth = $dbh->prepare($query);
     $sth->execute( $itemnumber, $borrowernumber,$biblionumber);
     $sth->finish;
+    
+    _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo );
+
     return;
 }
 
@@ -971,25 +1256,125 @@ sub ModReserveMinusPriority {
     my $query = "
         UPDATE reserves
         SET    priority = 0 , itemnumber = ? 
-        WHERE  cancellationdate IS NULL 
-          AND  borrowernumber=?
+        WHERE  borrowernumber=?
           AND  biblionumber=?
     ";
     my $sth_upd = $dbh->prepare($query);
     $sth_upd->execute( $itemnumber, $borrowernumber, $biblionumber );
-    $sth_upd->finish;
     # second step update all others reservs
-    $query = "
-            UPDATE reserves
-            SET    priority = priority-1
-            WHERE  biblionumber = ?
-            AND priority > 0
-            AND cancellationdate IS NULL
-    ";
-    $sth_upd = $dbh->prepare($query);
-    $sth_upd->execute( $biblionumber );
-    $sth_upd->finish;
-    $sth_upd->finish;
+    _FixPriority($biblionumber, $borrowernumber, '0');
+}
+
+=item GetReserveInfo
+
+&GetReserveInfo($borrowernumber,$biblionumber);
+
+ Get item and borrower details for a current hold.
+ Current implementation this query should have a single result.
+=cut
+
+sub GetReserveInfo {
+       my ( $borrowernumber, $biblionumber ) = @_;
+    my $dbh = C4::Context->dbh;
+       my $strsth="SELECT reservedate, reservenotes, reserves.borrowernumber,
+                               reserves.biblionumber, reserves.branchcode,
+                               notificationdate, reminderdate, priority, found,
+                               firstname, surname, phone, 
+                               email, address, address2,
+                               cardnumber, city, zipcode,
+                               biblio.title, biblio.author,
+                               items.holdingbranch, items.itemcallnumber, items.itemnumber, 
+                               barcode, notes
+                       FROM reserves left join items 
+                               ON items.itemnumber=reserves.itemnumber , 
+                               borrowers, biblio 
+                       WHERE 
+                               reserves.borrowernumber=?  &&
+                               reserves.biblionumber=? && 
+                               reserves.borrowernumber=borrowers.borrowernumber && 
+                               reserves.biblionumber=biblio.biblionumber ";
+       my $sth = $dbh->prepare($strsth); 
+       $sth->execute($borrowernumber,$biblionumber);
+
+       my $data = $sth->fetchrow_hashref;
+       return $data;
+
+}
+
+=item IsAvailableForItemLevelRequest
+
+=over 4
+
+my $is_available = IsAvailableForItemLevelRequest($itemnumber);
+
+=back
+
+Checks whether a given item record is available for an
+item-level hold request.  An item is available if
+
+* it is not lost AND 
+* it is not damaged AND 
+* it is not withdrawn AND 
+* does not have a not for loan value > 0
+
+Whether or not the item is currently on loan is 
+also checked - if the AllowOnShelfHolds system preference
+is ON, an item can be requested even if it is currently
+on loan to somebody else.  If the system preference
+is OFF, an item that is currently checked out cannot
+be the target of an item-level hold request.
+
+Note that IsAvailableForItemLevelRequest() does not
+check if the staff operator is authorized to place
+a request on the item - in particular,
+this routine does not check IndependantBranches
+and canreservefromotherbranches.
+
+=cut
+
+sub IsAvailableForItemLevelRequest {
+    my $itemnumber = shift;
+   
+    my $item = GetItem($itemnumber);
+
+    # must check the notforloan setting of the itemtype
+    # FIXME - a lot of places in the code do this
+    #         or something similar - need to be
+    #         consolidated
+    my $dbh = C4::Context->dbh;
+    my $notforloan_query;
+    if (C4::Context->preference('item-level_itypes')) {
+        $notforloan_query = "SELECT itemtypes.notforloan
+                             FROM items
+                             JOIN itemtypes ON (itemtypes.itemtype = items.itype)
+                             WHERE itemnumber = ?";
+    } else {
+        $notforloan_query = "SELECT itemtypes.notforloan
+                             FROM items
+                             JOIN biblioitems USING (biblioitemnumber)
+                             JOIN itemtypes USING (itemtype)
+                             WHERE itemnumber = ?";
+    }
+    my $sth = $dbh->prepare($notforloan_query);
+    $sth->execute($itemnumber);
+    my $notforloan_per_itemtype = 0;
+    if (my ($notforloan) = $sth->fetchrow_array) {
+        $notforloan_per_itemtype = 1 if $notforloan;
+    }
+
+    my $available_per_item = 1;
+    $available_per_item = 0 if $item->{itemlost} or
+                               ( $item->{notforloan} > 0 ) or
+                               ($item->{damaged} and not C4::Context->preference('AllowHoldsOnDamagedItems')) or
+                               $item->{wthdrawn} or
+                               $notforloan_per_itemtype;
+
+    
+    if (C4::Context->preference('AllowOnShelfHolds')) {
+        return $available_per_item;
+    } else {
+        return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W")); 
+    }
 }
 
 =item _FixPriority
@@ -1019,7 +1404,6 @@ sub _FixPriority {
             SET    priority = 0
             WHERE biblionumber = ?
               AND borrowernumber = ?
-              AND cancellationdate IS NULL
               AND found ='W'
         /;
         my $sth = $dbh->prepare($query);
@@ -1031,13 +1415,13 @@ sub _FixPriority {
     # get whats left
 # FIXME adding a new security in returned elements for changing priority,
 # now, we don't care anymore any reservations with itemnumber linked (suppose a waiting reserve)
+       # This is wrong a waiting reserve has W set
+       # The assumption that having an itemnumber set means waiting is wrong and should be corrected any place it occurs
     my $query = qq/
         SELECT borrowernumber, reservedate, constrainttype
         FROM   reserves
         WHERE  biblionumber   = ?
-          AND  cancellationdate IS NULL
-          AND  itemnumber IS NULL
-          AND  ((found <> 'F' and found <> 'W') or found is NULL)
+          AND  ((found <> 'W') or found is NULL)
         ORDER BY priority ASC
     /;
     my $sth = $dbh->prepare($query);
@@ -1087,14 +1471,13 @@ sub _FixPriority {
 
 =item _Findgroupreserve
 
-  @results = &_Findgroupreserve($biblioitemnumber, $biblionumber);
+  @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber);
 
-****** FIXME ******
-I don't know what this does, because I don't understand how reserve
-constraints work. I think the idea is that you reserve a particular
-biblio, and the constraint allows you to restrict it to a given
-biblioitem (e.g., if you want to borrow the audio book edition of "The
-Prophet", rather than the first available publication).
+Looks for an item-specific match first, then for a title-level match, returning the
+first match found.  If neither, then we look for a 3rd kind of match based on
+reserve constraints.
+
+TODO: add more explanation about reserve constraints
 
 C<&_Findgroupreserve> returns :
 C<@results> is an array of references-to-hash whose keys are mostly
@@ -1104,38 +1487,154 @@ C<biblioitemnumber>.
 =cut
 
 sub _Findgroupreserve {
-    my ( $bibitem, $biblio ) = @_;
+    my ( $bibitem, $biblio, $itemnumber ) = @_;
     my $dbh   = C4::Context->dbh;
+
+    # check for exact targetted match
+       # This select is valid for both item_level and biblio_level
+    my $item_level_target_query = qq/
+        SELECT reserves.biblionumber        AS biblionumber,
+               reserves.borrowernumber      AS borrowernumber,
+               reserves.reservedate         AS reservedate,
+               reserves.branchcode          AS branchcode,
+               reserves.cancellationdate    AS cancellationdate,
+               reserves.found               AS found,
+               reserves.reservenotes        AS reservenotes,
+               reserves.priority            AS priority,
+               reserves.timestamp           AS timestamp,
+               biblioitems.biblioitemnumber AS biblioitemnumber,
+               reserves.itemnumber          AS itemnumber
+        FROM reserves
+        JOIN biblioitems USING (biblionumber)
+        JOIN hold_fill_targets USING (biblionumber, borrowernumber, itemnumber)
+        WHERE found IS NULL
+        AND priority > 0
+        AND hold_fill_targets.itemnumber = ?
+
+    /;
+    my $sth = $dbh->prepare($item_level_target_query);
+    $sth->execute($itemnumber);
+       my $data = $sth->fetchall_arrayref({});
+    return @$data if (@$data);
+
+    # check for title-level targetted match
+    my $title_level_target_query = qq/
+        SELECT reserves.biblionumber        AS biblionumber,
+               reserves.borrowernumber      AS borrowernumber,
+               reserves.reservedate         AS reservedate,
+               reserves.branchcode          AS branchcode,
+               reserves.cancellationdate    AS cancellationdate,
+               reserves.found               AS found,
+               reserves.reservenotes        AS reservenotes,
+               reserves.priority            AS priority,
+               reserves.timestamp           AS timestamp,
+               biblioitems.biblioitemnumber AS biblioitemnumber,
+               reserves.itemnumber          AS itemnumber
+        FROM reserves
+        JOIN biblioitems USING (biblionumber)
+        JOIN hold_fill_targets USING (biblionumber, borrowernumber)
+        WHERE found IS NULL
+        AND priority > 0
+        AND item_level_request = 0
+        AND hold_fill_targets.itemnumber = ?
+    /;
+    $sth = $dbh->prepare($title_level_target_query);
+    $sth->execute($itemnumber);
+    $data = $sth->fetchall_arrayref({});
+    return @$data if (@$data);
+    
     my $query = qq/
-        SELECT reserves.biblionumber AS biblionumber,
-               reserves.borrowernumber AS borrowernumber,
-               reserves.reservedate AS reservedate,
-               reserves.branchcode AS branchcode,
-               reserves.cancellationdate AS cancellationdate,
-               reserves.found AS found,
-               reserves.reservenotes AS reservenotes,
-               reserves.priority AS priority,
-               reserves.timestamp AS timestamp,
+        SELECT reserves.biblionumber               AS biblionumber,
+               reserves.borrowernumber             AS borrowernumber,
+               reserves.reservedate                AS reservedate,
+               reserves.branchcode                 AS branchcode,
+               reserves.cancellationdate           AS cancellationdate,
+               reserves.found                      AS found,
+               reserves.reservenotes               AS reservenotes,
+               reserves.priority                   AS priority,
+               reserves.timestamp                  AS timestamp,
                reserveconstraints.biblioitemnumber AS biblioitemnumber,
-               reserves.itemnumber AS itemnumber
+               reserves.itemnumber                 AS itemnumber
         FROM reserves
           LEFT JOIN reserveconstraints ON reserves.biblionumber = reserveconstraints.biblionumber
         WHERE reserves.biblionumber = ?
           AND ( ( reserveconstraints.biblioitemnumber = ?
           AND reserves.borrowernumber = reserveconstraints.borrowernumber
-          AND reserves.reservedate    =reserveconstraints.reservedate )
+          AND reserves.reservedate    = reserveconstraints.reservedate )
           OR  reserves.constrainttype='a' )
-          AND reserves.cancellationdate is NULL
-          AND (reserves.found <> 'F' or reserves.found is NULL)
+          AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?)
     /;
-    my $sth = $dbh->prepare($query);
-    $sth->execute( $biblio, $bibitem );
-    my @results;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+    $sth = $dbh->prepare($query);
+    $sth->execute( $biblio, $bibitem, $itemnumber );
+    $data = $sth->fetchall_arrayref({});
+    return @$data if (@$data);
+       return undef;
+}
+
+=item _koha_notify_reserve
+
+=over 4
+
+_koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber );
+
+=back
+
+Sends a notification to the patron that their hold has been filled (through
+ModReserveAffect, _not_ ModReserveFill)
+
+=cut
+
+sub _koha_notify_reserve {
+    my ($itemnumber, $borrowernumber, $biblionumber) = @_;
+
+    my $dbh = C4::Context->dbh;
+    my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'Hold Filled' } );
+
+    return if ( !defined( $messagingprefs->{'letter_code'} ) );
+
+    my $sth = $dbh->prepare("
+        SELECT *
+        FROM   reserves
+        WHERE  borrowernumber = ?
+            AND biblionumber = ?
+    ");
+    $sth->execute( $borrowernumber, $biblionumber );
+    my $reserve = $sth->fetchrow_hashref;
+    my $branch_details = GetBranchDetail( $reserve->{'branchcode'} );
+
+    my $admin_email_address = $branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress');
+
+    my $letter = getletter( 'reserves', $messagingprefs->{'letter_code'} );
+
+    C4::Letters::parseletter( $letter, 'branches', $reserve->{'branchcode'} );
+    C4::Letters::parseletter( $letter, 'borrowers', $reserve->{'borrowernumber'} );
+    C4::Letters::parseletter( $letter, 'biblio', $reserve->{'biblionumber'} );
+    C4::Letters::parseletter( $letter, 'reserves', $reserve->{'borrowernumber'}, $reserve->{'biblionumber'} );
+
+    if ( $reserve->{'itemnumber'} ) {
+        C4::Letters::parseletter( $letter, 'items', $reserve->{'itemnumber'} );
+    }
+    $letter->{'content'} =~ s/<<[a-z0-9_]+\.[a-z0-9]+>>//g; #remove any stragglers
+
+    if ( -1 !=  firstidx { $_ eq 'email' } @{$messagingprefs->{transports}} ) {
+        # aka, 'email' in ->{'transports'}
+        C4::Letters::EnqueueLetter(
+            {   letter                 => $letter,
+                borrowernumber         => $borrowernumber,
+                message_transport_type => 'email',
+                from_address           => $admin_email_address,
+            }
+        );
+    }
+
+    if ( -1 != firstidx { $_ eq 'sms' } @{$messagingprefs->{transports}} ) {
+        C4::Letters::EnqueueLetter(
+            {   letter                 => $letter,
+                borrowernumber         => $borrowernumber,
+                message_transport_type => 'sms',
+            }
+        );
     }
-    $sth->finish;
-    return @results;
 }
 
 =back
@@ -1146,3 +1645,4 @@ Koha Developement team <info@koha.org>
 
 =cut
 
+1;