bug 1710 and 1739: availability for item-level holds

Fixed rules for determining whether an item is
available for an item-level hold request.  Create a
new function in C4::Reserves, IsAvailableForItemRequest(),
that checks whether an item is potentially available for
an item-level hold request.

An item is considered available if:

* it is not lost AND,
* it is not marked not for loan AND,
* it is not withdrawn AND,
* it is not damaged (unless the AllowHoldsOnDamagedItems syspref is ON), AND
* it is not on loan (unless the AllowOnShelfHolds syspref is ON)

Preventing a hold request on withdrawn items is bug 1739, as is
the new preference on whether to allow holds on damaged items.

Removing the condition that an item cannot be requested if
it has already been requested by another patron is the topic of bug 1710.

Note that this patch does not change the behavior where if
independent branches is on and the canreservefromotherbranches
syspref is off, a staff operator is prevented from placing
an item-level hold request on an item from a different branch.

Signed-off-by: Joshua Ferraro <jmf@liblime.com>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 7b9345a..ae07e4e 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -107,6 +107,8 @@ BEGIN {
         
         &CheckReserves
         &CancelReserve
+
+        &IsAvailableForItemLevelRequest
     );
 }    
 
@@ -1075,6 +1077,81 @@ sub GetReserveInfo {
 
 }
 
+=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 
+* it is not marked as not for loan
+
+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} 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}); 
+    }
+}
+
 =item _FixPriority
 
 &_FixPriority($biblio,$borrowernumber,$rank);

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 9653a58..df074d5 100755 (executable)
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -58,7 +58,6 @@ my $itemtypes = GetItemTypes();
 
 # get biblionumber.....
 my $biblionumber = $query->param('biblionumber');
-my $bibdata;
 if (! $biblionumber) {
        $template->param(message=>1,no_biblionumber=>1);
        &get_out($query, $cookie, $template->output);
@@ -169,7 +168,6 @@ foreach my $itm (@items) {
     push @duedates, { date_due => format_date( $itm->{'date_due'} ) }
       if defined $itm->{'date_due'};
     $itm->{ $itm->{'publictype'} } = 1;
-       warn $itm->{'notforloan'};
     my $fee = GetReserveFee( undef, $borrowernumber, $itm->{'biblionumber'},
         'a', ( $itm->{'biblioitemnumber'} ) );
     $fee = sprintf "%.02f", $fee;
@@ -412,16 +410,7 @@ foreach my $biblioitemnumber (@biblioitemnumbers) {
         # If there is no loan, return and transfer, we show a checkbox.
         $item->{notforloan} = $item->{notforloan} || 0;
 
-       # FIXME: every library will define this differently
-        # An item is available only if:
-        if (
-            not defined $reservedate    # not reserved yet
-            and $issues->{'date_due'} eq ''         # not currently on loan
-            and not $item->{itemlost}   # not lost
-            and not $item->{notforloan} # not forbidden to loan
-            and $transfertwhen eq ''    # not currently on transfert
-          )
-        {
+        if (IsAvailableForItemLevelRequest($itemnumber)) {
             $item->{available} = 1;
         }
 

diff --git a/reserve/request.pl b/reserve/request.pl
index 51abfbd..fd35b9b 100755 (executable)
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -341,19 +341,9 @@ foreach my $biblioitemnumber (@biblioitemnumbers) {
         }
     }
 
-    # FIXME: every library will define this differently
-        # An item is available only if:
-        if (
-            not defined $reservedate    # not reserved yet
-            and $issues->{'date_due'} eq ''         # not currently on loan
-            and not $item->{itemlost}   # not lost
-            and not $item->{notforloan} # not forbidden to loan
-        and not $item->{cantreserve}
-            and $transfertwhen eq ''    # not currently on transfert
-          )
-        {
-            $item->{available} = 1;
-        }
+    if (IsAvailableForItemLevelRequest($itemnumber) and not $item->{cantreserve}) {
+        $item->{available} = 1;
+    }
 
     # FIXME: move this to a pm
     my $sth2 = $dbh->prepare("SELECT * FROM reserves WHERE borrowernumber=? AND itemnumber=? AND found='W'");