batchRebuildBiblioTables.pl doesn't crash anymore when GetMarcBiblio fails.
[koha.git] / C4 / Reserves.pm
index cc20485..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
@@ -24,16 +21,23 @@ package C4::Reserves;
 
 
 use strict;
+# 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;
 
-use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
+# for _koha_notify_reserve
+use C4::Members::Messaging;
+use C4::Letters;
+use C4::Branch qw( GetBranchDetail );
+use List::MoreUtils qw( firstidx any );
 
-my $library_name = C4::Context->preference("LibraryName");
+use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
 
 =head1 NAME
 
@@ -53,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
@@ -87,6 +92,7 @@ BEGIN {
     @EXPORT = qw(
         &AddReserve
   
+        &GetPendingReserves
         &GetReservesFromItemnumber
         &GetReservesFromBiblionumber
         &GetReservesFromBorrowernumber
@@ -95,7 +101,8 @@ BEGIN {
         &GetReserveCount
         &GetReserveFee
                &GetReserveInfo
-    
+        &GetReserveStatus
+        
         &GetOtherReserves
         
         &ModReserveFill
@@ -106,6 +113,8 @@ BEGIN {
         &ModReserveMinusPriority
         
         &CheckReserves
+        &CanBookBeReserved
+        &CanItemBeReserved
         &CancelReserve
 
         &IsAvailableForItemLevelRequest
@@ -114,7 +123,7 @@ BEGIN {
 
 =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
 
@@ -122,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,
@@ -152,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'){
@@ -170,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
@@ -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 );
 }
 
@@ -331,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
 
@@ -353,8 +544,6 @@ sub GetReserveCount {
     my $sth = $dbh->prepare($query);
     $sth->execute($borrowernumber);
     my $row = $sth->fetchrow_hashref;
-    $sth->finish;
-
     return $row->{counter};
 }
 
@@ -383,7 +572,7 @@ sub GetOtherReserves {
             );
 
             #launch the subroutine dotransfer
-            C4::Circulation::ModItemTransfer(
+            C4::Items::ModItemTransfer(
                 $itemnumber,
                 $iteminfo->{'holdingbranch'},
                 $checkreserves->{'branchcode'}
@@ -535,7 +724,6 @@ sub GetReservesToBranch {
         $transreserv[$i] = $data;
         $i++;
     }
-    $sth->finish;
     return (@transreserv);
 }
 
@@ -569,10 +757,21 @@ sub GetReservesForBranch {
         $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);
@@ -657,6 +856,9 @@ sub CheckReserves {
                 # 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.
@@ -785,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
 
@@ -823,9 +1054,9 @@ sub ModReserve {
         $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 = ?
         /;
@@ -929,7 +1160,6 @@ sub ModReserveStatus {
     ";
     my $sth_set = $dbh->prepare($query);
     $sth_set->execute( $newstatus, $itemnumber );
-    $sth_set->finish;
 }
 
 =item ModReserveAffect
@@ -960,8 +1190,9 @@ sub ModReserveAffect {
     if ($transferToDo) {
     $query = "
         UPDATE reserves
-        SET    priority = 0,
-               itemnumber = ?
+        SET    priority   = 0,
+               itemnumber = ?,
+               found      = 'T'
         WHERE borrowernumber = ?
           AND biblionumber = ?
     ";
@@ -981,6 +1212,9 @@ sub ModReserveAffect {
     $sth = $dbh->prepare($query);
     $sth->execute( $itemnumber, $borrowernumber,$biblionumber);
     $sth->finish;
+    
+    _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo );
+
     return;
 }
 
@@ -1027,18 +1261,8 @@ sub ModReserveMinusPriority {
     ";
     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
-    ";
-    $sth_upd = $dbh->prepare($query);
-    $sth_upd->execute( $biblionumber );
-    $sth_upd->finish;
-    $sth_upd->finish;
+    _FixPriority($biblionumber, $borrowernumber, '0');
 }
 
 =item GetReserveInfo
@@ -1091,7 +1315,7 @@ item-level hold request.  An item is available if
 * it is not lost AND 
 * it is not damaged AND 
 * it is not withdrawn AND 
-* it is not marked as not for loan
+* 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
@@ -1140,15 +1364,16 @@ sub IsAvailableForItemLevelRequest {
 
     my $available_per_item = 1;
     $available_per_item = 0 if $item->{itemlost} or
-                               $item->{notforloan} 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}); 
+        return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W")); 
     }
 }
 
@@ -1248,12 +1473,11 @@ sub _FixPriority {
 
   @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
@@ -1265,35 +1489,152 @@ C<biblioitemnumber>.
 sub _Findgroupreserve {
     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.itemnumber IS NULL OR reserves.itemnumber = ?)
     /;
-    my $sth = $dbh->prepare($query);
+    $sth = $dbh->prepare($query);
     $sth->execute( $biblio, $bibitem, $itemnumber );
-    my @results;
-    while ( my $data = $sth->fetchrow_hashref ) {
-        push( @results, $data );
+    $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