Attachment 15648 Details for Bug 5786 – bug_5786, bug_5787, bug_5788: Holds rewrite

[patch] bug_5786, bug_5787, bug_5788: Holds rewrite

bug5786-bug5787-bug5788-Holds-rewrite.patch (text/plain), 76.99 KB, created by Srdjan Jankovic on 2013-02-25 03:49:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Srdjan Jankovic

Created: 2013-02-25 03:49:22 UTC

Size: 76.99 KB

patch

obsolete

>From 375b8ec44da3a8e6ba82a6548dcf7b069f4c3c6b Mon Sep 17 00:00:00 2001
>From: Srdjan <srdjan@catalyst.net.nz>
>Date: Mon, 25 Feb 2013 16:43:00 +1300
>Subject: [PATCH] bug_5786, bug_5787, bug_5788: Holds rewrite
>
>bug_5786: moved AllowOnShelfHolds to issuingrules
>bug_5787: moved OPACItemHolds to issuingrules
>bug_5788: Added reservesmaxpickupdelay to issuingrules
>
>C4::Reserves:
>* Added OnShelfHoldsAllowed() to check issuingrules
>* Added OPACItemHoldsAllowed() to check issuingrules
>* IsAvailableForItemLevelRequest() changed interface, now takes
>  $item_record,$borrower_record; calls OnShelfHoldsAllowed()
>
>opac/opac-reserve.pl and opac/opac-search.pl:
>* rewrote hold allowed rule to use OPACItemHoldsAllowed()
>* also use OnShelfHoldsAllowed() through
>* IsAvailableForItemLevelRequest()
>
>templates:
>* Removed AllowOnShelfHolds and OPACItemHolds global flags, they now
>  only have meaning per item type
>
>Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
>
>I have tested this patch left, right and upside down for the last
>several months. All tests have passed.
>---
> C4/Auth.pm                                         |    1 -
> C4/ILSDI/Services.pm                               |    2 +-
> C4/Items.pm                                        |    2 +
> C4/Reserves.pm                                     |  340 ++++++++++++--------
> C4/VirtualShelves/Page.pm                          |    1 -
> admin/smart-rules.pl                               |   19 +-
> admin/systempreferences.pl                         |    2 -
> .../mysql/it-IT/necessari/system_preferences.sql   |    1 -
> installer/data/mysql/kohastructure.sql             |    3 +
> installer/data/mysql/sysprefs.sql                  |    2 -
> installer/data/mysql/updatedatabase.pl             |   36 ++-
> installer/html-template-to-template-toolkit.pl     |    2 +-
> .../en/modules/admin/preferences/circulation.pref  |    6 -
> .../prog/en/modules/admin/preferences/opac.pref    |    6 -
> .../prog/en/modules/admin/smart-rules.tt           |   94 +++---
> .../prog/en/includes/opac-detail-sidebar.inc       |    8 +-
> .../opac-tmpl/prog/en/modules/opac-reserve.tt      |  168 +++++-----
> .../prog/en/modules/opac-results-grouped.tt        |   18 +-
> .../opac-tmpl/prog/en/modules/opac-results.tt      |    6 +-
> .../opac-tmpl/prog/en/modules/opac-shelves.tt      |    2 +-
> opac/opac-ISBDdetail.pl                            |    9 +-
> opac/opac-MARCdetail.pl                            |    8 +-
> opac/opac-detail.pl                                |    2 -
> opac/opac-reserve.pl                               |   44 +--
> opac/opac-search.pl                                |   19 +-
> reserve/request.pl                                 |    2 +-
> 26 files changed, 438 insertions(+), 365 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index efe1200..b36caa7 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -402,7 +402,6 @@ sub get_template_and_user {
>             OPACAmazonCoverImages     => C4::Context->preference("OPACAmazonCoverImages"),
>             OPACFRBRizeEditions       => C4::Context->preference("OPACFRBRizeEditions"),
>             OpacHighlightedWords       => C4::Context->preference("OpacHighlightedWords"),
>-            OPACItemHolds             => C4::Context->preference("OPACItemHolds"),
>             OPACShelfBrowser          => "". C4::Context->preference("OPACShelfBrowser"),
>             OpacShowRecentComments    => C4::Context->preference("OpacShowRecentComments"),
>             OPACURLOpenInNewWindow    => "" . C4::Context->preference("OPACURLOpenInNewWindow"),
>diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
>index 68cbdb3..0735acf 100644
>--- a/C4/ILSDI/Services.pm
>+++ b/C4/ILSDI/Services.pm
>@@ -495,7 +495,7 @@ sub GetServices {
>     my $canbookbereserved = CanBookBeReserved( $borrower, $biblionumber );
>     if ($canbookbereserved) {
>         push @availablefor, 'title level hold';
>-        my $canitembereserved = IsAvailableForItemLevelRequest($itemnumber);
>+        my $canitembereserved = IsAvailableForItemLevelRequest($item, $borrower);
>         if ($canitembereserved) {
>             push @availablefor, 'item level hold';
>         }
>diff --git a/C4/Items.pm b/C4/Items.pm
>index dbd06b6..481bb1c 100644
>--- a/C4/Items.pm
>+++ b/C4/Items.pm
>@@ -164,6 +164,8 @@ sub GetItem {
>         ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array();
>     }
> 	#if we don't have an items.itype, use biblioitems.itemtype.
>+    # FIXME this should respect the itypes systempreference
>+    # if (C4::Context->preference('item-level_itypes')) {
> 	if( ! $data->{'itype'} ) {
> 		my $sth = $dbh->prepare("SELECT itemtype FROM biblioitems  WHERE biblionumber = ?");
> 		$sth->execute($data->{'biblionumber'});
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index a380bf0..1206286 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -68,13 +68,13 @@ This modules provides somes functions to deal with reservations.
>   The complete workflow is :
>   ==== 1st use case ====
>   patron request a document, 1st available :                      P >0, F=NULL, I=NULL
>-  a library having it run "transfertodo", and clic on the list    
>+  a library having it run "transfertodo", and clic on the list
>          if there is no transfer to do, the reserve waiting
>-         patron can pick it up                                    P =0, F=W,    I=filled 
>+         patron can pick it up                                    P =0, F=W,    I=filled
>          if there is a transfer to do, write in branchtransfer    P =0, F=T,    I=filled
>            The pickup library recieve the book, it check in       P =0, F=W,    I=filled
>   The patron borrow the book                                      P =0, F=F,    I=filled
>-  
>+
>   ==== 2nd use case ====
>   patron requests a document, a given item,
>     If pickup is holding branch                                   P =0, F=W,   I=filled
>@@ -93,7 +93,7 @@ BEGIN {
>     @ISA = qw(Exporter);
>     @EXPORT = qw(
>         &AddReserve
>-  
>+
>         &GetReservesFromItemnumber
>         &GetReservesFromBiblionumber
>         &GetReservesFromBorrowernumber
>@@ -103,9 +103,9 @@ BEGIN {
>         &GetReserveFee
>         &GetReserveInfo
>         &GetReserveStatus
>-        
>+
>         &GetOtherReserves
>-        
>+
>         &ModReserveFill
>         &ModReserveAffect
>         &ModReserve
>@@ -113,7 +113,7 @@ BEGIN {
>         &ModReserveCancelAll
>         &ModReserveMinusPriority
>         &MoveReserve
>-        
>+
>         &CheckReserves
>         &CanBookBeReserved
> 	&CanItemBeReserved
>@@ -123,7 +123,9 @@ BEGIN {
>         &AutoUnsuspendReserves
> 
>         &IsAvailableForItemLevelRequest
>-        
>+
>+        &OPACItemHoldsAllowed
>+
>         &AlterPriority
>         &ToggleLowestPriority
> 
>@@ -132,7 +134,7 @@ BEGIN {
>         &SuspendAll
>     );
>     @EXPORT_OK = qw( MergeHolds );
>-}    
>+}
> 
> =head2 AddReserve
> 
>@@ -240,7 +242,7 @@ sub AddReserve {
>     foreach (@$bibitems) {
>         $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
>     }
>-        
>+
>     return;     # FIXME: why not have a useful return value?
> }
> 
>@@ -302,7 +304,7 @@ sub GetReservesFromBiblionumber {
>                 push( @bibitemno, $bibitemnos );    # FIXME: inefficient: use fetchall_arrayref
>             }
>             my $count = scalar @bibitemno;
>-    
>+
>             # if we have two or more different specific itemtypes
>             # reserved by same person on same day
>             my $bdata;
>@@ -417,30 +419,30 @@ This function return 1 if an item can be issued by this borrower.
> 
> sub CanItemBeReserved{
>     my ($borrowernumber, $itemnumber) = @_;
>-    
>+
>     my $dbh             = C4::Context->dbh;
>     my $allowedreserves = 0;
>-            
>+
>     my $controlbranch = C4::Context->preference('ReservesControlBranch');
>     my $itype         = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype";
> 
>     # we retrieve borrowers and items informations #
>     my $item     = GetItem($itemnumber);
>-    my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);     
>-    
>+    my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);
>+
>     # we retrieve user rights on this itemtype and branchcode
>-    my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed 
>-                             FROM issuingrules 
>-                             WHERE (categorycode in (?,'*') ) 
>-                             AND (itemtype IN (?,'*')) 
>-                             AND (branchcode IN (?,'*')) 
>-                             ORDER BY 
>-                               categorycode DESC, 
>-                               itemtype     DESC, 
>+    my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed
>+                             FROM issuingrules
>+                             WHERE (categorycode in (?,'*') )
>+                             AND (itemtype IN (?,'*'))
>+                             AND (branchcode IN (?,'*'))
>+                             ORDER BY
>+                               categorycode DESC,
>+                               itemtype     DESC,
>                                branchcode   DESC;"
>                            );
>-                           
>-    my $querycount ="SELECT 
>+
>+    my $querycount ="SELECT
>                             count(*) as count
>                             FROM reserves
>                                 LEFT JOIN items USING (itemnumber)
>@@ -448,13 +450,13 @@ sub CanItemBeReserved{
>                                 LEFT JOIN borrowers USING (borrowernumber)
>                             WHERE borrowernumber = ?
>                                 ";
>-    
>-    
>+
>+
>     my $itemtype     = $item->{$itype};
>     my $categorycode = $borrower->{categorycode};
>     my $branchcode   = "";
>     my $branchfield  = "reserves.branchcode";
>-    
>+
>     if( $controlbranch eq "ItemHomeLibrary" ){
>         $branchfield = "items.homebranch";
>         $branchcode = $item->{homebranch};
>@@ -462,34 +464,33 @@ sub CanItemBeReserved{
>         $branchfield = "borrowers.branchcode";
>         $branchcode = $borrower->{branchcode};
>     }
>-    
>-    # we retrieve rights 
>+
>+    # we retrieve rights
>     $sth->execute($categorycode, $itemtype, $branchcode);
>     if(my $rights = $sth->fetchrow_hashref()){
>         $itemtype        = $rights->{itemtype};
>-        $allowedreserves = $rights->{reservesallowed}; 
>+        $allowedreserves = $rights->{reservesallowed};
>     }else{
>         $itemtype = '*';
>     }
>-    
>+
>     # we retrieve count
>-    
>+
>     $querycount .= "AND $branchfield = ?";
>-    
>+
>     $querycount .= " AND $itype = ?" if ($itemtype ne "*");
>     my $sthcount = $dbh->prepare($querycount);
>-    
>+
>     if($itemtype eq "*"){
>         $sthcount->execute($borrowernumber, $branchcode);
>     }else{
>         $sthcount->execute($borrowernumber, $branchcode, $itemtype);
>     }
>-    
>+
>     my $reservecount = "0";
>     if(my $rowcount = $sthcount->fetchrow_hashref()){
>         $reservecount = $rowcount->{count};
>     }
>-    
>     # we check if it's ok or not
>     if( $reservecount < $allowedreserves ){
>         return 1;
>@@ -688,8 +689,8 @@ sub GetReservesToBranch {
>     my $dbh = C4::Context->dbh;
>     my $sth = $dbh->prepare(
>         "SELECT borrowernumber,reservedate,itemnumber,timestamp
>-         FROM reserves 
>-         WHERE priority='0' 
>+         FROM reserves
>+         WHERE priority='0'
>            AND branchcode=?"
>     );
>     $sth->execute( $frombranch );
>@@ -712,7 +713,7 @@ sub GetReservesForBranch {
>     my ($frombranch) = @_;
>     my $dbh          = C4::Context->dbh;
> 	my $query        = "SELECT borrowernumber,reservedate,itemnumber,waitingdate
>-        FROM   reserves 
>+        FROM   reserves
>         WHERE   priority='0'
>             AND found='W' ";
>     if ($frombranch){
>@@ -737,11 +738,11 @@ sub GetReservesForBranch {
> 
> 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;
>@@ -803,7 +804,7 @@ sub CheckReserves {
>            LEFT JOIN itemtypes   ON biblioitems.itemtype   = itemtypes.itemtype
>         ";
>     }
>-   
>+
>     if ($item) {
>         $sth = $dbh->prepare("$select WHERE itemnumber = ?");
>         $sth->execute($item);
>@@ -818,7 +819,7 @@ sub CheckReserves {
>     return ( '' ) unless $itemnumber; # bail if we got nothing.
> 
>     # if item is not for loan it cannot be reserved either.....
>-    #    execpt where items.notforloan < 0 :  This indicates the item is holdable. 
>+    #    execpt where items.notforloan < 0 :  This indicates the item is holdable.
>     return ( '' ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
> 
>     # Find this item in the reserves
>@@ -873,7 +874,7 @@ sub CancelExpiredReserves {
>     # Cancel reserves that have passed their expiration date.
>     my $dbh = C4::Context->dbh;
>     my $sth = $dbh->prepare( "
>-        SELECT * FROM reserves WHERE DATE(expirationdate) < DATE( CURDATE() ) 
>+        SELECT * FROM reserves WHERE DATE(expirationdate) < DATE( CURDATE() )
>         AND expirationdate IS NOT NULL
>         AND found IS NULL
>     " );
>@@ -882,7 +883,7 @@ sub CancelExpiredReserves {
>     while ( my $res = $sth->fetchrow_hashref() ) {
>         CancelReserve( $res->{'biblionumber'}, '', $res->{'borrowernumber'} );
>     }
>-  
>+
>     # Cancel reserves that have been waiting too long
>     if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) {
>         my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay");
>@@ -1038,12 +1039,12 @@ 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 
>+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. 
>+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 
>+C<$rank> is a non-zero integer; if supplied, the itemnumber
> of the hold request is set accordingly; if omitted, the itemnumber
> is cleared.
> 
>@@ -1073,20 +1074,20 @@ sub ModReserve {
>         $query = qq/
>             INSERT INTO old_reserves
>             SELECT *
>-            FROM   reserves 
>+            FROM   reserves
>             WHERE  biblionumber   = ?
>              AND   borrowernumber = ?
>         /;
>         $sth = $dbh->prepare($query);
>         $sth->execute( $biblio, $borrower );
>         $query = qq/
>-            DELETE FROM reserves 
>+            DELETE FROM reserves
>             WHERE  biblionumber   = ?
>              AND   borrowernumber = ?
>         /;
>         $sth = $dbh->prepare($query);
>         $sth->execute( $biblio, $borrower );
>-        
>+
>     }
>     elsif ($rank =~ /^\d+/ and $rank > 0) {
>         my $query = "
>@@ -1171,7 +1172,7 @@ sub ModReserveFill {
>                 ";
>     $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 ) {
>@@ -1216,7 +1217,7 @@ with the biblionumber & the borrowernumber, we can affect the itemnumber
> to the correct reserve.
> 
> if $transferToDo is not set, then the status is set to "Waiting" as well.
>-otherwise, a transfer is on the way, and the end of the transfer will 
>+otherwise, a transfer is on the way, and the end of the transfer will
> take care of the waiting status
> 
> =cut
>@@ -1297,7 +1298,7 @@ sub ModReserveCancelAll {
> 
>   &ModReserveMinusPriority($itemnumber,$borrowernumber,$biblionumber)
> 
>-Reduce the values of queuded list     
>+Reduce the values of queuded list
> 
> =cut
> 
>@@ -1308,7 +1309,7 @@ sub ModReserveMinusPriority {
>     my $dbh   = C4::Context->dbh;
>     my $query = "
>         UPDATE reserves
>-        SET    priority = 0 , itemnumber = ? 
>+        SET    priority = 0 , itemnumber = ?
>         WHERE  borrowernumber=?
>           AND  biblionumber=?
>     ";
>@@ -1330,42 +1331,42 @@ Current implementation this query should have a single result.
> sub GetReserveInfo {
> 	my ( $borrowernumber, $biblionumber ) = @_;
>     my $dbh = C4::Context->dbh;
>-	my $strsth="SELECT 
>-	               reservedate, 
>-	               reservenotes, 
>+	my $strsth="SELECT
>+	               reservedate,
>+	               reservenotes,
> 	               reserves.borrowernumber,
>-				   reserves.biblionumber, 
>+				   reserves.biblionumber,
> 				   reserves.branchcode,
> 				   reserves.waitingdate,
>-				   notificationdate, 
>-				   reminderdate, 
>-				   priority, 
>+				   notificationdate,
>+				   reminderdate,
>+				   priority,
> 				   found,
>-				   firstname, 
>-				   surname, 
>-				   phone, 
>-				   email, 
>-				   address, 
>+				   firstname,
>+				   surname,
>+				   phone,
>+				   email,
>+				   address,
> 				   address2,
>-				   cardnumber, 
>-				   city, 
>+				   cardnumber,
>+				   city,
> 				   zipcode,
>-				   biblio.title, 
>+				   biblio.title,
> 				   biblio.author,
>-				   items.holdingbranch, 
>-				   items.itemcallnumber, 
>+				   items.holdingbranch,
>+				   items.itemcallnumber,
> 				   items.itemnumber,
>-				   items.location, 
>-				   barcode, 
>+				   items.location,
>+				   barcode,
> 				   notes
>-			FROM reserves 
>-			 LEFT JOIN items USING(itemnumber) 
>+			FROM reserves
>+			 LEFT JOIN items USING(itemnumber)
> 		     LEFT JOIN borrowers USING(borrowernumber)
>-		     LEFT JOIN biblio ON  (reserves.biblionumber=biblio.biblionumber) 
>-			WHERE 
>+		     LEFT JOIN biblio ON  (reserves.biblionumber=biblio.biblionumber)
>+			WHERE
> 				reserves.borrowernumber=?
> 				AND reserves.biblionumber=?";
>-	my $sth = $dbh->prepare($strsth); 
>+	my $sth = $dbh->prepare($strsth);
> 	$sth->execute($borrowernumber,$biblionumber);
> 
> 	my $data = $sth->fetchrow_hashref;
>@@ -1375,22 +1376,18 @@ sub GetReserveInfo {
> 
> =head2 IsAvailableForItemLevelRequest
> 
>-  my $is_available = IsAvailableForItemLevelRequest($itemnumber);
>+  my $is_available = IsAvailableForItemLevelRequest($item_record,$borrower_record);
> 
> 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 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.
>+Need to check the issuingrules onshelfholds column,
>+if this is set items on the shelf can be placed on hold
> 
> Note that IsAvailableForItemLevelRequest() does not
> check if the staff operator is authorized to place
>@@ -1401,50 +1398,82 @@ and canreservefromotherbranches.
> =cut
> 
> sub IsAvailableForItemLevelRequest {
>-    my $itemnumber = shift;
>-   
>-    my $item = GetItem($itemnumber);
>+    my $item = shift;
>+    my $borrower = shift;
> 
>+    my $dbh = C4::Context->dbh;
>     # 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;
>+    my $itype;
>     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 = ?";
>+        # We cant trust GetItem to honour the syspref, so safest to do it ourselves
>+        # When GetItem is fixed, we can remove this
>+        $itype = $item->{itype};
>     }
>-    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;
>+    else {
>+        # XXX This is a bit dodgy. It relies on biblio itemtype column having different name.
>+        # So if we already have a biblioitems join when calling this function,
>+        # we don't need to access the database again
>+        $itype = $item->{itemtype};
>     }
>+    unless ($itype) {
>+        my $query = "SELECT itemtype FROM biblioitems WHERE biblioitemnumber = ? ";
>+        my $sth = $dbh->prepare($query);
>+        $sth->execute($item->{biblioitemnumber});
>+        if (my $data = $sth->fetchrow_hashref()){
>+            $itype = $data->{itemtype};
>+        }
>+    }
>+
>+    my $notforloan_per_itemtype
>+      = $dbh->selectrow_array("SELECT notforloan FROM itemtypes WHERE itemtype = ?",
>+                              undef, $itype);
> 
>-    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;
>+    return 0 if
>+        $notforloan_per_itemtype ||
>+        $item->{itemlost}        ||
>+        $item->{notforloan} > 0  ||
>+        $item->{wthdrawn}        ||
>+        ($item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems'));
> 
> 
>-    if (C4::Context->preference('AllowOnShelfHolds')) {
>-        return $available_per_item;
>+    if (OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch})) {
>+        return 1;
>     } else {
>-        return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W")); 
>+        return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "W";
>     }
> }
> 
>+=head2 OnShelfHoldsAllowed
>+
>+  OnShelfHoldsAllowed($itemtype,$borrowercategory,$branchcode);
>+
>+Checks issuingrules, using the borrowers categorycode, the itemtype, and branchcode to see if onshelf
>+holds are allowed, returns true if so.
>+
>+=cut
>+
>+sub OnShelfHoldsAllowed {
>+    my ($itype,$borrowercategory,$branchcode) = @_;
>+
>+    my $query = "SELECT onshelfholds FROM issuingrules WHERE
>+          (issuingrules.categorycode = ? OR issuingrules.categorycode = '*')
>+        AND
>+          (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
>+        AND
>+          (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')
>+        ORDER BY
>+          issuingrules.categorycode desc,
>+          issuingrules.itemtype desc,
>+          issuingrules.branchcode desc
>+       LIMIT 1";
>+    my $dbh = C4::Context->dbh;
>+    my $onshelfholds = $dbh->selectrow_array($query, undef, $borrowercategory,$itype,$branchcode);
>+    return $onshelfholds;
>+}
>+
> =head2 AlterPriority
> 
>   AlterPriority( $where, $borrowernumber, $biblionumber, $reservedate );
>@@ -1466,8 +1495,8 @@ sub AlterPriority {
>     $sth->finish();
> 
>     if ( $where eq 'up' || $where eq 'down' ) {
>-    
>-      my $priority = $reserve->{'priority'};        
>+
>+      my $priority = $reserve->{'priority'};
>       $priority = $where eq 'up' ? $priority - 1 : $priority + 1;
>       _FixPriority( $biblionumber, $borrowernumber, $priority )
> 
>@@ -1505,7 +1534,7 @@ sub ToggleLowestPriority {
>         $borrowernumber,
>     );
>     $sth->finish;
>-    
>+
>     _FixPriority( $biblionumber, $borrowernumber, '999999' );
> }
> 
>@@ -1612,7 +1641,7 @@ 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
> 
>-=cut 
>+=cut
> 
> sub _FixPriority {
>     my ( $biblio, $borrowernumber, $rank, $ignoreSetLowestRank ) = @_;
>@@ -1691,10 +1720,10 @@ sub _FixPriority {
>         );
>         $sth->finish;
>     }
>-    
>+
>     $sth = $dbh->prepare( "SELECT borrowernumber FROM reserves WHERE lowestPriority = 1 ORDER BY priority" );
>     $sth->execute();
>-    
>+
>     unless ( $ignoreSetLowestRank ) {
>       while ( my $res = $sth->fetchrow_hashref() ) {
>         _FixPriority( $biblio, $res->{'borrowernumber'}, '999999', 1 );
>@@ -1754,7 +1783,7 @@ sub _Findgroupreserve {
>         push( @results, $data );
>     }
>     return @results if @results;
>-    
>+
>     # check for title-level targetted match
>     my $title_level_target_query = qq/
>         SELECT reserves.biblionumber        AS biblionumber,
>@@ -1833,7 +1862,7 @@ sub _koha_notify_reserve {
> 
>     my $dbh = C4::Context->dbh;
>     my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber);
>-    
>+
>     # Try to get the borrower's email address
>     my $to_address;
>     my $which_address = C4::Context->preference('AutoEmailPrimaryAddress');
>@@ -1843,7 +1872,7 @@ sub _koha_notify_reserve {
>     } else {
>         $to_address = $borrower->{$which_address};
>     }
>-    
>+
>     my $letter_code;
>     my $print_mode = 0;
>     my $messagingprefs;
>@@ -1888,7 +1917,7 @@ sub _koha_notify_reserve {
>             borrowernumber => $borrowernumber,
>             message_transport_type => 'print',
>         } );
>-        
>+
>         return;
>     }
> 
>@@ -1967,6 +1996,57 @@ sub _ShiftPriorityByDateAndPriority {
>     return $new_priority;  # so the caller knows what priority they wind up receiving
> }
> 
>+=head2 OPACItemHoldsAllowed
>+
>+  OPACItemHoldsAllowed($item_record,$borrower_record);
>+
>+Checks issuingrules, using the borrowers categorycode, the itemtype, and branchcode to see
>+if specific item holds are allowed, returns true if so.
>+
>+=cut
>+
>+sub OPACItemHoldsAllowed {
>+    my ($item,$borrower) = @_;
>+
>+    my $branchcode = $item->{homebranch} or die "No homebranch";
>+    my $itype;
>+    my $dbh = C4::Context->dbh;
>+    if (C4::Context->preference('item-level_itypes')) {
>+       # We cant trust GetItem to honour the syspref, so safest to do it ourselves
>+       # When GetItem is fixed, we can remove this
>+       $itype = $item->{itype};
>+    }
>+    else {
>+       my $query = "SELECT itemtype FROM biblioitems WHERE biblioitemnumber = ? ";
>+       my $sth = $dbh->prepare($query);
>+       $sth->execute($item->{biblioitemnumber});
>+       if (my $data = $sth->fetchrow_hashref()){
>+           $itype = $data->{itemtype};
>+       }
>+    }
>+
>+    my $query = "SELECT opacitemholds,categorycode,itemtype,branchcode FROM issuingrules WHERE
>+          (issuingrules.categorycode = ? OR issuingrules.categorycode = '*')
>+        AND
>+          (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
>+        AND
>+          (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')
>+        ORDER BY
>+          issuingrules.categorycode desc,
>+          issuingrules.itemtype desc,
>+          issuingrules.branchcode desc
>+       LIMIT 1";
>+    my $sth = $dbh->prepare($query);
>+    $sth->execute($borrower->{categorycode},$itype,$branchcode);
>+    my $data = $sth->fetchrow_hashref;
>+    if ($data->{opacitemholds}){
>+       return 1;
>+    }
>+    else {
>+       return 0;
>+    }
>+}
>+
> =head2 MoveReserve
> 
>   MoveReserve( $itemnumber, $borrowernumber, $cancelreserve )
>diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm
>index 586384d..23e886a 100644
>--- a/C4/VirtualShelves/Page.pm
>+++ b/C4/VirtualShelves/Page.pm
>@@ -237,7 +237,6 @@ sub shelfpage {
>             # explicitly fetch this shelf
>             my ($shelfnumber2,$shelfname,$owner,$category,$sorton) = GetShelf($shelfnumber);
> 
>-            $template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') );
>             if (C4::Context->preference('TagsEnabled')) {
>                 $template->param(TagsEnabled => 1);
>                     foreach (qw(TagsShowOnList TagsInputOnList)) {
>diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
>index c221d71..e24fedb 100755
>--- a/admin/smart-rules.pl
>+++ b/admin/smart-rules.pl
>@@ -101,12 +101,12 @@ elsif ($op eq 'delete-branch-item') {
> # save the values entered
> elsif ($op eq 'add') {
>     my $sth_search = $dbh->prepare('SELECT COUNT(*) AS total FROM issuingrules WHERE branchcode=? AND categorycode=? AND itemtype=?');
>-    my $sth_insert = $dbh->prepare('INSERT INTO issuingrules (branchcode, categorycode, itemtype, maxissueqty, renewalsallowed, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, firstremind, chargeperiod,rentaldiscount, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)');
>-    my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, reservesallowed=?, issuelength=?, lengthunit = ?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, overduefinescap=?  WHERE branchcode=? AND categorycode=? AND itemtype=?");
>+    my $sth_insert = $dbh->prepare('INSERT INTO issuingrules (branchcode, categorycode, itemtype, maxissueqty, renewalsallowed, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, firstremind, chargeperiod,rentaldiscount, onshelfholds, opacitemholds, reservesmaxpickupdelay, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)');
>+    my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, reservesallowed=?, issuelength=?, lengthunit=?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, onshelfholds=?, opacitemholds=?, reservesmaxpickupdelay=?, overduefinescap=? WHERE branchcode=? AND categorycode=? AND itemtype=?");
>     
>     my $br = $branch; # branch
>     my $bor  = $input->param('categorycode'); # borrower category
>-    my $cat  = $input->param('itemtype');     # item type
>+    my $itemtype  = $input->param('itemtype');     # item type
>     my $fine = $input->param('fine');
>     my $finedays     = $input->param('finedays');
>     my $firstremind  = $input->param('firstremind');
>@@ -114,6 +114,7 @@ elsif ($op eq 'add') {
>     my $maxissueqty  = $input->param('maxissueqty');
>     my $renewalsallowed  = $input->param('renewalsallowed');
>     my $reservesallowed  = $input->param('reservesallowed');
>+    my $onshelfholds     = $input->param('onshelfholds') || 0;
>     $maxissueqty =~ s/\s//g;
>     $maxissueqty = undef if $maxissueqty !~ /^\d+/;
>     my $issuelength  = $input->param('issuelength');
>@@ -122,15 +123,17 @@ elsif ($op eq 'add') {
>     $hardduedate = format_date_in_iso($hardduedate);
>     my $hardduedatecompare = $input->param('hardduedatecompare');
>     my $rentaldiscount = $input->param('rentaldiscount');
>+    my $reservesmaxpickupdelay = $input->param('reservesmaxpickupdelay');
>+    my $opacitemholds = $input->param('opacitemholds') || 0;
>     my $overduefinescap = $input->param('overduefinescap') || undef;
>-    $debug and warn "Adding $br, $bor, $cat, $fine, $maxissueqty";
>+    $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty";
> 
>-    $sth_search->execute($br,$bor,$cat);
>+    $sth_search->execute($br,$bor,$itemtype);
>     my $res = $sth_search->fetchrow_hashref();
>     if ($res->{total}) {
>-        $sth_update->execute($fine, $finedays,$firstremind, $chargeperiod, $maxissueqty, $renewalsallowed,$reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount,$overduefinescap, $br,$bor,$cat);
>+        $sth_update->execute($fine, $finedays,$firstremind, $chargeperiod, $maxissueqty, $renewalsallowed,$reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount, $onshelfholds, $opacitemholds, $reservesmaxpickupdelay, $overduefinescap, $br,$bor,$itemtype);
>     } else {
>-        $sth_insert->execute($br,$bor,$cat,$maxissueqty,$renewalsallowed,$reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays,$firstremind,$chargeperiod,$rentaldiscount,$overduefinescap);
>+        $sth_insert->execute($br,$bor,$itemtype,$maxissueqty,$renewalsallowed,$reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays,$firstremind,$chargeperiod,$rentaldiscount,$onshelfholds,$opacitemholds,$reservesmaxpickupdelay,$overduefinescap);
>     }
> } 
> elsif ($op eq "set-branch-defaults") {
>@@ -385,7 +388,7 @@ while (my $row = $sth2->fetchrow_hashref) {
>     $row->{'humancategorycode'} ||= $row->{'categorycode'};
>     $row->{'default_humancategorycode'} = 1 if $row->{'humancategorycode'} eq '*';
>     $row->{'fine'} = sprintf('%.2f', $row->{'fine'});
>-    if ($row->{'hardduedate'} ne '0000-00-00') {
>+    if ($row->{'hardduedate'} && $row->{'hardduedate'} ne '0000-00-00') {
>        $row->{'hardduedate'} = format_date( $row->{'hardduedate'});
>        $row->{'hardduedatebefore'} = 1 if ($row->{'hardduedatecompare'} == -1);
>        $row->{'hardduedateexact'} = 1 if ($row->{'hardduedatecompare'} ==  0);
>diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl
>index af91041..595e364 100755
>--- a/admin/systempreferences.pl
>+++ b/admin/systempreferences.pl
>@@ -186,7 +186,6 @@ $tabsysprefs{HomeOrHoldingBranch}            = "Circulation";
> $tabsysprefs{HomeOrHoldingBranchReturn}      = "Circulation";
> $tabsysprefs{RandomizeHoldsQueueWeight}      = "Circulation";
> $tabsysprefs{StaticHoldsQueueWeight}         = "Circulation";
>-$tabsysprefs{AllowOnShelfHolds}              = "Circulation";
> $tabsysprefs{AllowHoldsOnDamagedItems}       = "Circulation";
> $tabsysprefs{UseBranchTransferLimits}        = "Circulation";
> $tabsysprefs{AllowHoldPolicyOverride}        = "Circulation";
>@@ -367,7 +366,6 @@ $tabsysprefs{suggestion}           = "OPAC";
> $tabsysprefs{OpacTopissue}         = "OPAC";
> $tabsysprefs{OpacBrowser}          = "OPAC";
> $tabsysprefs{OpacRenewalAllowed}   = "OPAC";
>-$tabsysprefs{OPACItemHolds}        = "OPAC";
> $tabsysprefs{OPACGroupResults}     = "OPAC";
> $tabsysprefs{XSLTDetailsDisplay}   = "OPAC";
> $tabsysprefs{XSLTResultsDisplay}   = "OPAC";
>diff --git a/installer/data/mysql/it-IT/necessari/system_preferences.sql b/installer/data/mysql/it-IT/necessari/system_preferences.sql
>index b4ddfa0..7aaa066 100644
>--- a/installer/data/mysql/it-IT/necessari/system_preferences.sql
>+++ b/installer/data/mysql/it-IT/necessari/system_preferences.sql
>@@ -17,7 +17,6 @@
> -- 51 Franklin Street' WHERE variable = ' Fifth Floor' WHERE variable = ' Boston' WHERE variable = ' MA 02110-1301 USA.
> 
> UPDATE systempreferences SET value = 'cataloguing' WHERE variable = 'AcqCreateItem';
>-UPDATE systempreferences SET value = '1' WHERE variable = 'AllowOnShelfHolds';
> UPDATE systempreferences SET value = '1' WHERE variable = 'AllowRenewalLimitOverride';
> UPDATE systempreferences SET value = 'annual' WHERE variable = 'autoBarcode';
> UPDATE systempreferences SET value = 'email' WHERE variable = 'AutoEmailPrimaryAddress';
>diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql
>index ed673da..2e21759 100644
>--- a/installer/data/mysql/kohastructure.sql
>+++ b/installer/data/mysql/kohastructure.sql
>@@ -1019,6 +1019,9 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules
>   `reservesallowed` smallint(6) NOT NULL default "0", -- how many holds are allowed
>   `branchcode` varchar(10) NOT NULL default '', -- the branch this rule is for (branches.branchcode)
>   overduefinescap decimal default NULL, -- the maximum amount of an overdue fine
>+  onshelfholds tinyint(1) NOT NULL default 0, -- allow holds for items that are on shelf
>+  opacitemholds tinyint(1) NOT NULL default "0", -- allow opac users to place specific items on hold
>+  reservesmaxpickupdelay smallint(6) default NULL, -- max pickup delay
>   PRIMARY KEY  (`branchcode`,`categorycode`,`itemtype`),
>   KEY `categorycode` (`categorycode`),
>   KEY `itemtype` (`itemtype`)
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index ca9b6bd..9833c8c 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -187,7 +187,6 @@ INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES
> INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OAI-PMH:archiveID','KOHA-OAI-TEST','OAI-PMH archive identification',NULL,'Free');
> INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OAI-PMH:MaxCount','50','OAI-PMH maximum number of records by answer to ListRecords and ListIdentifiers queries',NULL,'Integer');
> INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OAI-PMH:ConfFile','','If empty, Koha OAI Server operates in normal mode, otherwise it operates in extended mode.',NULL,'File');
>-INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OPACItemHolds','1','Allow OPAC users to place hold on specific items. If OFF, users can only request next available copy.','','YesNo');
> 
> INSERT INTO `systempreferences` (variable, value,options,type, explanation) VALUES ('AddPatronLists','categorycode','categorycode|category_type','Choice','Allow user to choose what list to pick up from when adding patrons');
> INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('ExtendedPatronAttributes','0','Use extended patron IDs and attributes',NULL,'YesNo');
>@@ -220,7 +219,6 @@ INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES
> ('XSLTDetailsDisplay','','','Enable XSL stylesheet control over details page display on intranet','Free'),
> ('XSLTResultsDisplay','','','Enable XSL stylesheet control over results page display on intranet','Free');
> INSERT INTO `systempreferences` (variable,value,options,explanation,type) VALUES('AdvancedSearchTypes','itemtypes','itemtypes|ccode','Select which set of fields comprise the Type limit in the advanced search','Choice');
>-INSERT INTO `systempreferences` (variable,value,options,explanation,type) VALUES('AllowOnShelfHolds', '0', '', 'Allow hold requests to be placed on items that are not on loan', 'YesNo');
> INSERT INTO `systempreferences` (variable,value,options,explanation,type) VALUES('AllowHoldsOnDamagedItems', '1', '', 'Allow hold requests to be placed on damaged items', 'YesNo');
> INSERT INTO `systempreferences` (variable,value,options,explanation,type) VALUES('OpacSuppression', '0', '', 'Turn ON the OPAC Suppression feature, requires further setup, ask your system administrator for details', 'YesNo');
> -- FIXME: add FrameworksLoaded, noOPACUserLogin?
>diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl
>index a44e4e8..cdd55fa 100755
>--- a/installer/data/mysql/updatedatabase.pl
>+++ b/installer/data/mysql/updatedatabase.pl
>@@ -2093,7 +2093,6 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
>     $dbh->do("UPDATE `systempreferences` SET options='10' WHERE variable='globalDueDate'");
>     $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='numSearchResults'");
>     $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='OPACnumSearchResults'");
>-    $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='ReservesMaxPickupDelay'");
>     $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='TransfersMaxDaysWarning'");
>     $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='StaticHoldsQueueWeight'");
>     $dbh->do("UPDATE `systempreferences` SET type='Integer' WHERE variable='holdCancelLength'");
>@@ -6438,6 +6437,41 @@ if ( CheckVersion($DBversion) ) {
> }
> 
> 
>+
>+
>+$DBversion = '3.11.00.XXX';
>+if (C4::Context->preference("Version") < TransformToNum($DBversion)) {
>+    # First create the column
>+    $dbh->do("ALTER TABLE issuingrules ADD onshelfholds tinyint(1) default 0");
>+    # Now update the column
>+    if (C4::Context->preference("AllowOnShelfHolds")){
>+        # Pref is on, set allow for all rules
>+        $dbh->do("UPDATE issuingrules SET onshelfholds=1");
>+    } else {
>+        # If the preference is not set, leave off
>+        $dbh->do("UPDATE issuingrules SET onshelfholds=0");
>+    }
>+    $dbh->do("ALTER TABLE issuingrules MODIFY onshelfholds tinyint(1) default 0 NOT NULL");
>+    # Remove from the systempreferences table
>+    $dbh->do("DELETE FROM systempreferences WHERE variable = 'AllowOnShelfHolds'");
>+
>+    # First create the column
>+    $dbh->do("ALTER TABLE issuingrules ADD opacitemholds tinyint(1) DEFAULT 0");
>+    # Now update the column
>+    if (C4::Context->preference("OPACItemHolds")){
>+       # Pref is on, set allow for all rules
>+       $dbh->do("UPDATE issuingrules SET opacitemholds=1");
>+    }
>+    # If the preference is not set, leave off
>+    # Remove from the systempreferences table
>+    $dbh->do("DELETE FROM systempreferences WHERE variable = 'OPACItemHolds'");
>+
>+    $dbh->do("ALTER TABLE issuingrules ADD reservesmaxpickupdelay smallint(6) DEFAULT NULL");
>+
>+    print "Upgrade to $DBversion done (Move AllowOnShelfHolds to circulation matrix; Move OPACItemHolds system preference to circulation matrix; ReservesMaxPickupDelay circulation rule)\n";
>+    SetVersion ($DBversion);
>+}
>+
> =head1 FUNCTIONS
> 
> =head2 TableExists($table)
>diff --git a/installer/html-template-to-template-toolkit.pl b/installer/html-template-to-template-toolkit.pl
>index e99b195..5f0e0ac 100755
>--- a/installer/html-template-to-template-toolkit.pl
>+++ b/installer/html-template-to-template-toolkit.pl
>@@ -32,7 +32,7 @@ my @globals = ("themelang","JacketImages","OPACAmazonCoverImages","GoogleJackets
> "SyndeticsEnabled", "OpacRenewalAllowed", "item_level_itypes","noItemTypeImages",
> "virtualshelves", "RequestOnOpac", "COinSinOPACResults", "OPACXSLTResultsDisplay",
> "OPACItemsResultsDisplay", "LibraryThingForLibrariesID", "opacuserlogin", "TagsEnabled",
>-"TagsShowOnList", "TagsInputOnList","loggedinusername","AllowOnShelfHolds","opacbookbag",
>+"TagsShowOnList", "TagsInputOnList","loggedinusername","opacbookbag",
> "OPACAmazonEnabled", "SyndeticsCoverImages","using_https");
> 
> # Arguments:
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>index 82c1c02..f7587c4 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -333,12 +333,6 @@ Circulation:
>                   no: "Don't allow"
>             - hold requests to be placed on damaged items.
>         -
>-            - pref: AllowOnShelfHolds
>-              choices:
>-                  yes: Allow
>-                  no: "Don't allow"
>-            - hold requests to be placed on items that are not checked out.
>-        -
>             - pref: AllowHoldDateInFuture
>               choices:
>                   yes: Allow
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>index 6d6672c..10ba63f 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
>@@ -426,12 +426,6 @@ OPAC:
> #              choices:
> #            - If ON, enables subject cloud on OPAC
>         -
>-            - pref: OPACItemHolds
>-              choices:
>-                  yes: Allow
>-                  no: "Don't allow"
>-            - patrons to place holds on specific items in the OPAC. If this is disabled, users can only put a hold on the next available item.
>-        -
>             - pref: OpacRenewalAllowed
>               choices:
>                   yes: Allow
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index 586d72f..16b626e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -76,57 +76,46 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
>                 <th>Suspension in days (day)</th>
>                 <th>Renewals allowed (count)</th>
>                 <th>Holds allowed (count)</th>
>-		<th>Rental discount (%)</th>
>-				<th>&nbsp;</th>
>+                <th>On shelf holds allowed</th>
>+                <th>Allow OPAC users to place holds on items</th>
>+                <th>Holds max pickup delay (day)</th>
>+                <th>Rental discount (%)</th>
>+                <th>&nbsp;</th>
>             </tr>
>-				[% FOREACH rule IN rules %]
>-					[% UNLESS ( loop.odd ) %]
>-					<tr class="highlight">
>-					[% ELSE %]
>-					<tr>
>-					[% END %]
>-							<td>[% IF ( rule.default_humancategorycode ) %]
>-									<em>All</em>
>-								[% ELSE %]
>-									[% rule.humancategorycode %]
>-								[% END %]
>-							</td>
>-							<td>[% IF ( rule.default_humanitemtype ) %]
>-									<em>All</em>
>-								[% ELSE %]
>-									[% rule.humanitemtype %]
>-								[% END %]
>-							</td>
>-							<td>[% IF ( rule.unlimited_maxissueqty ) %]
>-									Unlimited
>-								[% ELSE %]
>-									[% rule.maxissueqty %]
>-								[% END %]
>-							</td>
>-							<td>[% rule.issuelength %]</td>
>-							<td>
>-							    [% rule.lengthunit %]
>-							</td>
>-                                                        <td>[% IF ( rule.hardduedate ) %]
>-                                                               [% IF ( rule.hardduedatebefore ) %]before [% rule.hardduedate %]</td>
>-                                                               [% ELSE %][% IF ( rule.hardduedateexact ) %]on [% rule.hardduedate %]</td>
>-                                                                                 [% ELSE %][% IF ( rule.hardduedateafter ) %]after [% rule.hardduedate %]</td>[% END %]
>-                                                                                 [% END %]
>-                                                               [% END %]
>-                                                            [% ELSE %]None defined[% END %]   
>-							<td>[% rule.fine %]</td>
>-							<td>[% rule.chargeperiod %]</td>
>-							<td>[% rule.firstremind %]</td>
>-                            <td>[% rule.overduefinescap FILTER format("%.2f") %]</td>
>-							<td>[% rule.finedays %]</td>
>-							<td>[% rule.renewalsallowed %]</td>
>-							<td>[% rule.reservesallowed %]</td>
>-							<td>[% rule.rentaldiscount %]</td>
>-							<td>
>-								<a class="button" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype %]&amp;categorycode=[% rule.categorycode %]&amp;branch=[% rule.current_branch %]">Delete</a>
>-							</td>
>-                	</tr>
>-            	[% END %]
>+        [% FOREACH rule IN rules %]
>+            [% IF ( loop.odd ) %] <tr> [% ELSE %] <tr class="highlight"> [% END %]
>+                <td>[% IF ( rule.default_humancategorycode ) %]<em>All</em>[% ELSE %][% rule.humancategorycode %][% END %]</td>
>+                <td>[% IF ( rule.default_humanitemtype ) %]<em>All</em>[% ELSE %][% rule.humanitemtype %][% END %]</td>
>+                <td>[% IF ( rule.unlimited_maxissueqty ) %] Unlimited [% ELSE %][% rule.maxissueqty %][% END %]</td>
>+                <td>[% rule.issuelength %]</td>
>+                <td>[% rule.lengthunit %]</td>
>+                <td>
>+                [% IF ( rule.hardduedate ) %]
>+                    [% IF ( rule.hardduedatebefore ) %]
>+                        before [% rule.hardduedate %]
>+                    [% ELSIF ( rule.hardduedateexact ) %]
>+                        on [% rule.hardduedate %]
>+                    [% ELSIF ( rule.hardduedateafter ) %]
>+                        after [% rule.hardduedate %]
>+                    [% END %]
>+                [% ELSE %]
>+                    None defined
>+                [% END %]   
>+                </td>
>+                <td>[% rule.fine %]</td>
>+                <td>[% rule.chargeperiod %]</td>
>+                <td>[% rule.firstremind %]</td>
>+                <td>[% rule.overduefinescap FILTER format("%.2f") %]</td>
>+                <td>[% rule.finedays %]</td>
>+                <td>[% rule.renewalsallowed %]</td>
>+                <td>[% rule.reservesallowed %]</td>
>+                <td>[% IF rule.onshelfholds %]Yes[% ELSE %]No[% END %]</td>
>+                <td>[% IF rule.opacitemholds %]Yes[% ELSE %]No[% END %]</td>
>+                <td>[% rule.reservesmaxpickupdelay %]</td>
>+                <td>[% rule.rentaldiscount %]</td>
>+                <td> <a class="button" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype %]&amp;categorycode=[% rule.categorycode %]&amp;branch=[% rule.current_branch %]">Delete</a> </td>
>+             </tr>
>+        [% END %]
>                 <tr>
>                     <td>
>                         <select name="categorycode">
>@@ -167,7 +156,10 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde
>                     <td><input name="finedays" size="3" /> </td>
>                     <td><input name="renewalsallowed" size="2" /></td>
>                     <td><input name="reservesallowed" size="2" /></td>
>-		    <td><input name="rentaldiscount" size="2" /></td>
>+                    <td><input type="checkbox" name="onshelfholds" value="1" /></td>
>+                    <td><input type="checkbox" name="opacitemholds" value="1" /></td>
>+                    <td><input name="reservesmaxpickupdelay" size="2" /></td>
>+                    <td><input name="rentaldiscount" size="2" /></td>
>                     <td><input type="hidden" name="branch" value="[% current_branch %]"/><input type="submit" value="Add" class="submit" /></td>
>                 </tr>
>             </table>
>diff --git a/koha-tmpl/opac-tmpl/prog/en/includes/opac-detail-sidebar.inc b/koha-tmpl/opac-tmpl/prog/en/includes/opac-detail-sidebar.inc
>index 0f16f3a..3e79d46 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/includes/opac-detail-sidebar.inc
>+++ b/koha-tmpl/opac-tmpl/prog/en/includes/opac-detail-sidebar.inc
>@@ -2,13 +2,7 @@
>     [% UNLESS ( norequests ) %]
>         [% IF ( opacuserlogin ) %]
>             [% IF ( RequestOnOpac ) %]
>-                [% IF ( AllowOnShelfHolds ) %]
>-                    <li><a class="reserve" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% biblionumber %]">Place hold</a></li>
>-                [% ELSE %]
>-                    [% IF ( ItemsIssued ) %]
>-                        <li><a class="reserve" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% biblionumber %]">Place hold</a></li>
>-                    [% END %]
>-                [% END %]
>+                <li><a class="reserve" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% biblionumber %]">Place hold</a></li>
>             [% END %]
>         [% END %]
>     [% END %]
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt
>index 5788b38..c0d049e 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt
>@@ -210,23 +210,22 @@
>             [% IF ( bad_data ) %]
>               <div id="bad_data" class="dialog alert">ERROR: Internal error: incomplete hold request.</div>
>             [% END %]
>-          [% ELSE %]
>-            [% IF ( none_available ) %]
>+          [% ELSIF ( none_available ) %]
>                 <div id="none_available" class="dialog alert"><strong>Sorry</strong>, none of these items can be placed on hold.
>                 </div>
>-              [% END %]
>           [% END %]
> 
>       [% UNLESS ( message ) %][% UNLESS ( none_available ) %]<h3>Confirm holds for:
>                       [% FOREACH USER_INF IN USER_INFO %]
>                         [% USER_INF.firstname %] [% USER_INF.surname %] ([% USER_INF.cardnumber %])
>                       [% END %]
>-                    </h3>[% END %]
>-	      [% IF (RESERVE_CHARGE) %]
>+                    </h3>
>+          [% IF (RESERVE_CHARGE) %]
> 	      <div class="dialog alert" id="reserve_fee">
> 	        There is a charge of [% RESERVE_CHARGE %] for placing this hold
> 	      </div>
>-	      [% END %]
>+          [% END %]
>+      [% END %][% END %]
> 
>             <form action="/cgi-bin/koha/opac-reserve.pl" method="post" id="hold-request-form">
>             <input type="hidden" name="place_reserve" value="1"/>
>@@ -235,38 +234,47 @@
>             <input type="hidden" name="biblionumbers" id="biblionumbers"/>
>             <input type="hidden" name="selecteditems" id="selections"/>
>             <div id="bigloop">
>+            [% extra_cols = 2 %]
>               <table id="bibitemloop">
>-                [% UNLESS ( none_available ) %]<tr>
>+                <tr>
>+                [% UNLESS ( none_available ) %]
>                   <th>Hold</th>
>+                [% END %]
>                   <th>Title</th>
>-                  [% UNLESS ( item_level_itypes ) %]
>+                [% UNLESS ( item_level_itypes ) %]
>                     <th>Item type</th>
>-                  [% END %]
>+                [% END %]
>+                [% UNLESS ( none_available ) %]
>                   [% IF showholds && showpriority %]
>+                    [% extra_cols = extra_cols + 1 %]
>                   <th>Holds and priority</th>
>                   [% ELSIF showholds %]
>+                    [% extra_cols = extra_cols + 1 %]
>                   <th>Holds</th>
>                   [% ELSIF showpriority %]
>+                    [% extra_cols = extra_cols + 1 %]
>                   <th>Priority</th>
>                   [% END %]
> 		  [% IF ( reserve_in_future ) %]
>+                    [% extra_cols = extra_cols + 1 %]
>         <th>Hold starts on date</th>
> 		  [% END %]
>         <th>Hold not needed after</th>
>-                  [% IF ( OPACItemHolds ) %]
>                     <th id="place_on_hdr" style="display:none">Place on</th>
>-                  [% END %]
>                   [% UNLESS ( singleBranchMode ) %]
> 		    [% IF ( choose_branch ) %]
>+                      [% extra_cols = extra_cols + 1 %]
>                         <th>Pickup location</th>
> 		    [% END %]
>                   [% END %]
>-                </tr>[% ELSE %]<tr><th colspan="5">Title</th></tr>[% END %]
>+                [% END %]
>+                </tr>
> 
>                 [% FOREACH bibitemloo IN bibitemloop %]
>-                  <tr>
>-                      [% IF ( bibitemloo.holdable ) %]
>-                                    <td class="hold">
>+                <tr>
>+                    [% UNLESS none_available %]
>+                    <td class="hold">
>+                        [% IF ( bibitemloo.holdable ) %]
>                       <input class="reserve_mode" name="reserve_mode" type="hidden" value="single"/>
>                       <input class="single_bib" name="single_bib" type="hidden" value="[% bibitemloo.biblionumber %]"/>
>                         <span class="confirmjs_hold" title="[% bibitemloo.biblionumber %]"></span>
>@@ -277,18 +285,21 @@
>                                  value="any" />
>                           <label class="confirm_label" for="[% bibitemloo.checkitem_bib %]">Next available copy</label>
>                         </span>
>-					</td>
>-                      [% ELSE %]
>-                                      [% UNLESS ( none_available ) %]<td class="hold">&nbsp;</td>[% END %]
>-                      [% END %]
>-                    [% IF ( bibitemloo.holdable ) %]<td class="title">[% ELSE %]<td class="title" colspan="5">[% END %]
>+                        [% ELSE %]
>+                        &nbsp;
>+                        [% END %]
>+                    </td>
>+                    [% END %]
>+                    <td class="title">
>                       <a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% bibitemloo.biblionumber %]">[% bibitemloo.title |html %][% IF ( bibitemloo.subtitle ) %] [% FOREACH subtitl IN bibitemloo.subtitle %][% subtitl.subfield %][% END %][% END %]</a>
>                       [% IF ( bibitemloo.author ) %],  by [% bibitemloo.author %][% END %]
> 
>                       [% UNLESS ( bibitemloo.holdable ) %]
>-
>+                          <div class="bibmessage">
>                         [% IF ( bibitemloo.already_reserved ) %]
>-                          <div class="bibmessage">You have already requested this title.</div>
>+                            You have already requested this title.
>+                        [% ELSIF ( bibitemloo.bib_available ) %]
>+                            No available items.
>                         [% ELSE %]
>                           [% UNLESS ( bibitemloo.bib_available ) %]
>                             <div class="bibmessage">No available items.</div>
>@@ -303,16 +314,17 @@
> 
> 
>                         [% END %]
>-
>+                          </div>
>+                      [% END %]
>                     </td>
>-                    [% IF ( bibitemloo.holdable ) %]
>-            
>                         [% UNLESS ( item_level_itypes ) %]
>                         <td class="itype">
>                             [% IF ( bibitemloo.imageurl ) %]<img src="[% bibitemloo.imageurl %]" alt="" />[% END %]
>                             [% bibitemloo.description %]
>                         </td>
>                         [% END %]
>+                    [% IF ( bibitemloo.holdable ) %]
>+            
>                         [% IF showholds || showpriority %]
>                         <td class="priority">
>                         [% IF showpriority %] [% bibitemloo.rank %] [% END %]
>@@ -337,68 +349,36 @@
>         <input name="expiration_date_[% bibitemloo.biblionumber %]" id="to" size="10" readonly="readonly" class="datepickerto" />
>       <p style="margin:.3em 2em;">
>       <a href="#" style="font-size:85%;text-decoration:none;" onclick="document.getElementById('expiration_date_[% bibitemloo.biblionumber %]').value='';return false;">Clear date</a></p>
>-    </td>[% END %]
>+    </td>
> 
>-                    [% IF ( bibitemloo.holdable ) %]
>-		    
>-		    [% IF ( OPACItemHolds ) %]
>+                        <td class="place_on_type" style="display:none">
> 		    
>-                                          <td class="place_on_type" style="display:none">
>-                                            <ul>
>-                                                <li>
>-                                                  [% UNLESS ( bibitemloo.holdable ) %]
>-                                                    <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>-                                                           id="reqany_[% bibitemloo.biblionumber %]"
>-                                                           class="selectany"
>-                                                           value="Any"
>-                                                           disabled="disabled"
>-                                                    />
>-                                                  [% ELSE %]
>-                                                    <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>-                                                           id="reqany_[% bibitemloo.biblionumber %]"
>-                                                           class="selectany"
>-                                                           value="Any"
>-                                                           checked="checked"
>-                                                    />
>-                                                  [% END %]
>-                                                  <label for="reqany_[% bibitemloo.biblionumber %]">Next available copy</label>
>-                                                </li>
>-                                                <li>
>-                                                  [% UNLESS ( bibitemloo.holdable ) %]
>-                                                    <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>-                                                           id="reqspecific_[% bibitemloo.biblionumber %]"
>-                                                           class="selectspecific"
>-                                                           disabled="disabled"
>-                                                           value="Specific"
>-                                                    />
>-                                                  [% ELSE %]
>-                                                    <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>-                                                           id="reqspecific_[% bibitemloo.biblionumber %]"
>-                                                           class="selectspecific"
>-                                                           value="Specific"
>-                                                    />
>-                                                  [% END %]
>-                                                  <label for="reqspecific_[% bibitemloo.biblionumber %]">A specific copy</label>
>-                                                </li>
>-                                            </ul>
>-                                          </td>
>-                                        [% END %][% END %]
>-
>-                    [% UNLESS ( singleBranchMode ) %]
>-                        [% IF ( bibitemloo.holdable ) %]
>+                          <ul>
>+                              <li>
>+                                  <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>+                                         id="reqany_[% bibitemloo.biblionumber %]"
>+                                         class="selectany"
>+                                         value="Any"
>+                                         checked="checked"
>+                                  />
>+                                <label for="reqany_[% bibitemloo.biblionumber %]">Next available copy</label>
>+                              </li>
>+                        [% IF ( bibitemloo.itemholdable ) %]
>+                              <li>
>+                                  <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]"
>+                                         id="reqspecific_[% bibitemloo.biblionumber %]"
>+                                         class="selectspecific"
>+                                         value="Specific"
>+                                  />
>+                                <label for="reqspecific_[% bibitemloo.biblionumber %]">A specific copy</label>
>+                              </li>
>+                        [% END %]
>+                          </ul>
>+                        </td>
>+
>+                        [% UNLESS ( singleBranchMode ) %]
> 			    [% IF ( choose_branch ) %]
>-                                          <td class="branch">
>-                         [% UNLESS ( bibitemloo.holdable ) %]
>-                            <select name="branch" id="branch_[% bibitemloo.biblionumber %]" disabled="disabled">
>-                              [% FOREACH branchChoicesLoo IN bibitemloo.branchChoicesLoop %]
>-                                [% IF ( branchChoicesLoo.selected ) %]
>-                                  <option value="[% branchChoicesLoo.value %]" selected="selected">[% branchChoicesLoo.branchname %]</option>
>-                                [% ELSE %]
>-                                  <option value="[% branchChoicesLoo.value %]">[% branchChoicesLoo.branchname %]</option>
>-                                [% END %]
>-                              [% END %]
>-                          </select>
>-                          [% ELSE %]
>+                        <td class="branch">
>                             <select name="branch" id="branch_[% bibitemloo.biblionumber %]">
>                               [% FOREACH branchChoicesLoo IN bibitemloo.branchChoicesLoop %]
>                                 [% IF ( branchChoicesLoo.selected ) %]
>@@ -408,15 +388,15 @@
>                                 [% END %]
>                               [% END %]
>                             </select>
>-                          [% END %]
>                        </td>
>-			    [% END %]
>-		        [% END %]
>-                    [% END %]
>+                            [% END %]
>+                        [% END %]
>+                    [% ELSIF NOT none_available %]
>+                       <td colspan="[% extra_cols %]">&nbsp;</td>
>+                    [% END # holdable%]
>                   </tr>
> 
>-                  [% IF ( OPACItemHolds ) %]
>-                  [% IF ( bibitemloo.holdable ) %]
>+                    [% IF ( bibitemloo.itemholdable ) %]
>                     <tr class="copiesrow" id="copiesrow_[% bibitemloo.biblionumber %]">
>                       <td>&nbsp;</td>
>                       <td colspan="[% itemtable_colspan %]">
>@@ -495,11 +475,9 @@
>                         </table>
>                       </td>
>                     </tr>
>-                  [% END %]
>-                  [% END %]
>-                [% END %]
>-              </table>
>-              [% END %] 
>+                    [% END # itemholdable%]
>+                [% END %]
>+              </table>
>             </div>
> 
>             [% UNLESS ( message ) %]
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results-grouped.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results-grouped.tt
>index a461800..5ab1860 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results-grouped.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results-grouped.tt
>@@ -269,18 +269,14 @@ function highlightOn() {
> 
> 				<p>
>                                 [% IF ( RequestOnOpac ) %]
>-					[% UNLESS ( GROUP_RESULT.norequests ) %]
>-						[% IF ( opacuserlogin ) %]
>-							[% IF ( AllowOnShelfHolds ) %]
>+                                    [% UNLESS ( GROUP_RESULT.norequests ) %]
>+                                        [% IF ( opacuserlogin ) %]
>+                                            [% IF ( GROUP_RESULT.holdable ) %]
>                                 <a class="hold" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% GROUP_RESULT.biblionumber %]">Place hold</a>
>-							[% ELSE %]
>-								[% IF ( GROUP_RESULT.itemsissued ) %]
>-                                    <a class="hold" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% GROUP_RESULT.biblionumber %]">Place hold</a>
>-								[% END %]
>-							[% END %]
>-						[% END %]
>-					[% END %]
>-				[% END %]
>+                                            [% END %]
>+                                        [% END %]
>+                                    [% END %]
>+                                [% END %]
> 
> 				[% IF ( opacbookbag || virtualshelves ) %]<input type="checkbox" name="biblionumber" value="[% GROUP_RESULT.biblionumber %]" title="Click to add to cart" /> <label for="bib[% GROUP_RESULT.biblionumber %]">[% END %]<img src="[% themelang %]/images/[% GROUP_RESULT.itemtype %].gif" alt="[% GROUP_RESULT.ccode %]" title="[% GROUP_RESULT.ccode %]" />[% IF ( opacbookbag || virtualshelves ) %]</label>[% END %] [% IF ( GROUP_RESULT.classification ) %]
>                                     <a href="/cgi-bin/koha/opac-search.pl?q=callnum:[% GROUP_RESULT.classification |url %]">
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
>index 30682c4..f93363a 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt
>@@ -623,12 +623,8 @@ $(document).ready(function(){
>                 [% IF ( RequestOnOpac ) %]
>                     [% UNLESS ( SEARCH_RESULT.norequests ) %]
>                         [% IF ( opacuserlogin ) %]
>-                            [% IF ( AllowOnShelfHolds ) %]
>+                            [% IF ( SEARCH_RESULT.holdable ) %]
>                                 <a class="hold" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% SEARCH_RESULT.biblionumber %]">Place hold</a>
>-                            [% ELSE %]
>-                                [% IF ( SEARCH_RESULT.itemsissued ) %]
>-                                    <a class="hold" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% SEARCH_RESULT.biblionumber %]">Place hold</a>
>-                                [% END %]
>                             [% END %]
>                         [% END %]
>                     [% END %]
>diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>index d6c1ed5..d968e0f 100644
>--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt
>@@ -406,7 +406,7 @@ $(function() {
>       [% IF ( RequestOnOpac ) %]
>           [% UNLESS ( itemsloo.norequests ) %]
>             [% IF ( opacuserlogin ) %]
>-              [% IF ( AllowOnShelfHolds ) %]
>+              [% IF ( itemsloo.allow_onshelf_holds ) %]
>                 <a class="hold" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% itemsloo.biblionumber %]">Place hold</a>
>               [% ELSE %]
>                 [% IF ( itemsloo.itemsissued ) %]
>diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl
>index be62879..cf55113 100755
>--- a/opac/opac-ISBDdetail.pl
>+++ b/opac/opac-ISBDdetail.pl
>@@ -70,17 +70,13 @@ my $biblionumber = $query->param('biblionumber');
> $biblionumber = int($biblionumber);
> 
> # get biblionumbers stored in the cart
>-my @cart_list;
>-
>-if($query->cookie("bib_list")){
>-    my $cart_list = $query->cookie("bib_list");
>-    @cart_list = split(/\//, $cart_list);
>+if(my $cart_list = $query->cookie("bib_list")){
>+    my @cart_list = split(/\//, $cart_list);
>     if ( grep {$_ eq $biblionumber} @cart_list) {
>         $template->param( incart => 1 );
>     }
> }
> 
>-$template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') );
> $template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) );
> 
> my $marcflavour      = C4::Context->preference("marcflavour");
>@@ -163,7 +159,6 @@ foreach ( @$reviews ) {
> 
> $template->param(
>     RequestOnOpac       => C4::Context->preference("RequestOnOpac"),
>-    AllowOnShelfHolds   => C4::Context->preference('AllowOnShelfHolds'),
>     norequests   => $norequests,
>     ISBD         => $res,
>     biblionumber => $biblionumber,
>diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl
>index ac13a9d..4afc011 100755
>--- a/opac/opac-MARCdetail.pl
>+++ b/opac/opac-MARCdetail.pl
>@@ -83,17 +83,13 @@ $template->param(
> );
> 
> # get biblionumbers stored in the cart
>-my @cart_list;
>-
>-if($query->cookie("bib_list")){
>-    my $cart_list = $query->cookie("bib_list");
>-    @cart_list = split(/\//, $cart_list);
>+if(my $cart_list = $query->cookie("bib_list")){
>+    my @cart_list = split(/\//, $cart_list);
>     if ( grep {$_ eq $biblionumber} @cart_list) {
>         $template->param( incart => 1 );
>     }
> }
> 
>-$template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') );
> $template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) );
> 
> # adding the $RequestOnOpac param
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index bb9cbd5..d629ffc 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -388,8 +388,6 @@ if ($session->param('busc')) {
> }
> 
> 
>-
>-$template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') );
> $template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) );
> 
> 
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index 07b4f76..d598b16 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -400,6 +400,7 @@ foreach my $biblioNum (@biblionumbers) {
> 
>     $biblioLoopIter{itemLoop} = [];
>     my $numCopiesAvailable = 0;
>+    my $numCopiesOPACAvailable = 0;
>     foreach my $itemInfo (@{$biblioData->{itemInfos}}) {
>         my $itemNum = $itemInfo->{itemnumber};
>         my $itemLoopIter = {};
>@@ -492,16 +493,26 @@ foreach my $biblioNum (@biblionumbers) {
> 
>         my $branch = ( C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary' ) ? $itemInfo->{'homebranch'} : $borr->{'branchcode'};
> 
>-        my $branchitemrule = GetBranchItemRule( $branch, $itemInfo->{'itype'} );
>-        my $policy_holdallowed = 1;
>-
>-        if ( $branchitemrule->{'holdallowed'} == 0 ||
>-                ( $branchitemrule->{'holdallowed'} == 1 && $borr->{'branchcode'} ne $itemInfo->{'homebranch'} ) ) {
>-            $policy_holdallowed = 0;
>+        my $policy_holdallowed = !$itemLoopIter->{already_reserved};
>+        if ($policy_holdallowed) {
>+            if (my $branchitemrule = GetBranchItemRule( $branch, $itemInfo->{'itype'} )) {
>+                $policy_holdallowed =
>+                  ($branchitemrule->{'holdallowed'} == 2) ||
>+                  ($branchitemrule->{'holdallowed'} == 1
>+                      && $borr->{'branchcode'} eq $itemInfo->{'homebranch'});
>+            } else {
>+                $policy_holdallowed = 0; # No rule - not allowed
>+            }
>         }
>-
>-        if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) and ($itemLoopIter->{already_reserved} ne 1)) {
>-            $itemLoopIter->{available} = 1;
>+        $policy_holdallowed &&=
>+            IsAvailableForItemLevelRequest($itemInfo,$borr) &&
>+            CanItemBeReserved($borrowernumber,$itemNum);
>+
>+        if ($policy_holdallowed) {
>+            if ( OPACItemHoldsAllowed( $itemInfo, $borr ) ) {
>+                $itemLoopIter->{available} = 1;
>+                $numCopiesOPACAvailable++;
>+            }
>             $numCopiesAvailable++;
>         }
> 
>@@ -527,15 +538,11 @@ foreach my $biblioNum (@biblionumbers) {
>         $numBibsAvailable++;
>         $biblioLoopIter{bib_available} = 1;
>         $biblioLoopIter{holdable} = 1;
>-        $anyholdable = 1;
>+        $biblioLoopIter{itemholdable} = 1 if $numCopiesOPACAvailable;
>     }
>     if ($biblioLoopIter{already_reserved}) {
>         $biblioLoopIter{holdable} = undef;
>-        $anyholdable = undef;
>-    }
>-    if(not CanBookBeReserved($borrowernumber,$biblioNum)){
>-        $biblioLoopIter{holdable} = undef;
>-        $anyholdable = undef;
>+        $biblioLoopIter{itemholdable} = undef;
>     }
>     if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) {
>         $biblioLoopIter{holdable} = undef;
>@@ -543,7 +550,11 @@ foreach my $biblioNum (@biblionumbers) {
>         $anyholdable = undef;
>     }
> 
>+    $biblioLoopIter{holdable} &&= CanBookBeReserved($borrowernumber,$biblioNum);
>+
>     push @$biblioLoop, \%biblioLoopIter;
>+
>+    $anyholdable = 1 if $biblioLoopIter{holdable};
> }
> 
> if ( $numBibsAvailable == 0 || !$anyholdable) {
>@@ -551,9 +562,6 @@ if ( $numBibsAvailable == 0 || !$anyholdable) {
> }
> 
> my $itemTableColspan = 7;
>-if (! $template->{VARS}->{'OPACItemHolds'}) {
>-    $itemTableColspan--;
>-}
> if (! $template->{VARS}->{'singleBranchMode'}) {
>     $itemTableColspan--;
> }
>diff --git a/opac/opac-search.pl b/opac/opac-search.pl
>index 4744ac6..ca9624d 100755
>--- a/opac/opac-search.pl
>+++ b/opac/opac-search.pl
>@@ -51,6 +51,8 @@ use C4::Tags qw(get_tags);
> use C4::Branch; # GetBranches
> use C4::SocialData;
> use C4::Ratings;
>+use C4::Members;
>+use C4::Reserves;
> 
> use POSIX qw(ceil floor strftime);
> use URI::Escape;
>@@ -124,7 +126,7 @@ if (C4::Context->preference("marcflavour") eq "UNIMARC" ) {
> elsif (C4::Context->preference("marcflavour") eq "MARC21" ) {
>     $template->param('usmarc' => 1);
> }
>-$template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') );
>+
> $template->param( 'OPACNoResultsFound' => C4::Context->preference('OPACNoResultsFound') );
> 
> $template->param(
>@@ -516,8 +518,11 @@ if ($@ || $error) {
>     exit;
> }
> 
>+my $borrower = $borrowernumber ? GetMember( borrowernumber => $borrowernumber ) : undef;
>+
> # At this point, each server has given us a result set
> # now we build that set for template display
>+my %allow_onshelf_holds;
> my @sup_results_array;
> for (my $i=0;$i<@servers;$i++) {
>     my $server = $servers[$i];
>@@ -532,11 +537,23 @@ for (my $i=0;$i<@servers;$i++) {
>                 # we need to set the offset parameter of searchResults to 0
>                 my @group_results = searchResults( 'opac', $query_desc, $group->{'group_count'},$results_per_page, 0, $scan,
>                                                    $group->{"RECORDS"});
>+                if ($borrower) {
>+                    $_->{holdable} =
>+                        IsAvailableForItemLevelRequest($_, $borrower) &&
>+                        OPACItemHoldsAllowed($_, $borrower)
>+                      foreach @group_results;
>+                }
>                 push @newresults, { group_label => $group->{'group_label'}, GROUP_RESULTS => \@group_results };
>             }
>         } else {
>             @newresults = searchResults('opac', $query_desc, $hits, $results_per_page, $offset, $scan,
>                                         $results_hashref->{$server}->{"RECORDS"});
>+            if ($borrower) {
>+                $_->{holdable} =
>+                    IsAvailableForItemLevelRequest($_, $borrower) &&
>+                    OPACItemHoldsAllowed($_, $borrower)
>+                  foreach @newresults;
>+            }
>         }
> 
>         # must define a value for size if not present in DB
>diff --git a/reserve/request.pl b/reserve/request.pl
>index 506d5fa..4f4ae8d 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -464,7 +464,7 @@ foreach my $biblionumber (@biblionumbers) {
>             if (
>                    $policy_holdallowed
>                 && !$item->{cantreserve}
>-                && IsAvailableForItemLevelRequest($itemnumber)
>+                && IsAvailableForItemLevelRequest($item, $borrowerinfo)
>                 && CanItemBeReserved(
>                     $borrowerinfo->{borrowernumber}, $itemnumber
>                 )
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 5786: 4706 | 4751 | 4755 | 6898 | 7018 | 7034 | 7044 | 11116 | 11137 | 11184 | 11185 | 11190 | 11903 | 11992 | 12323 | 12440 | 12441 | 15648 | 34080 | 34534 | 34535 | 34536 | 35545 | 35546 | 35547 | 35548 | 35550 | 35567 | 35568 | 35596 | 35597 | 35598 | 35599 | 36383 | 36384 | 36405 | 36617 | 36618 | 36619 | 37253 | 37263 | 37292 | 37373