From c2c331c06e3c000f909b404722b4b4af2c9b29ea Mon Sep 17 00:00:00 2001 From: Srdjan Date: Mon, 25 Feb 2013 16:43:00 +1300 Subject: [PATCH] Bug 5786 - Move AllowOnShelfHolds system preference to the Circulation Matrix 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 I have tested this patch left, right and upside down for the last several months. All tests have passed. Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize --- C4/Auth.pm | 1 - C4/Circulation.pm | 8 +- C4/ILSDI/Services.pm | 2 +- C4/Items.pm | 2 + C4/Reserves.pm | 233 +++++++++++++------- C4/UsageStats.pm | 2 - C4/VirtualShelves/Page.pm | 4 +- admin/smart-rules.pl | 18 +- .../mysql/it-IT/necessari/system_preferences.sql | 1 - installer/data/mysql/kohastructure.sql | 2 + 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 | 7 - .../prog/en/modules/admin/smart-rules.tt | 6 + .../bootstrap/en/includes/opac-detail-sidebar.inc | 6 +- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 48 ++-- .../bootstrap/en/modules/opac-results-grouped.tt | 10 +- .../opac-tmpl/bootstrap/en/modules/opac-results.tt | 8 +- .../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 2 +- opac/opac-ISBDdetail.pl | 27 ++- opac/opac-MARCdetail.pl | 18 +- opac/opac-detail.pl | 22 +- opac/opac-reserve.pl | 41 ++-- opac/opac-search.pl | 19 +- reserve/request.pl | 11 +- t/db_dependent/Circulation.t | 2 +- t/db_dependent/Circulation_Issuingrule.t | 6 + t/db_dependent/Reserves.t | 24 +- 30 files changed, 362 insertions(+), 214 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index e6c121a..9a5149c 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -471,7 +471,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"), OPACURLOpenInNewWindow => "" . C4::Context->preference("OPACURLOpenInNewWindow"), OPACUserCSS => "" . C4::Context->preference("OPACUserCSS"), diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 14dfb0e..36c6fbb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1468,10 +1468,10 @@ Returns a hashref from the issuingrules table. sub GetIssuingRule { my ( $borrowertype, $itemtype, $branchcode ) = @_; my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare( "select * from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null" ); + my $sth = $dbh->prepare( "select * from issuingrules where categorycode=? and itemtype=? and branchcode=?" ); my $irule; - $sth->execute( $borrowertype, $itemtype, $branchcode ); + $sth->execute( $borrowertype, $itemtype, $branchcode ); $irule = $sth->fetchrow_hashref; return $irule if defined($irule) ; @@ -2699,8 +2699,10 @@ sub CanBookBeRenewed { # by pushing all the elements onto an array and removing the duplicates. my @reservable; foreach my $b (@borrowernumbers) { + my ( $borr ) = C4::Members::GetMemberDetails( $b ); foreach my $i (@itemnumbers) { - if ( IsAvailableForItemLevelRequest($i) + my $item = GetItem($i); + if ( IsAvailableForItemLevelRequest($item, $borr) && CanItemBeReserved( $b, $i ) && !IsItemOnHoldAndFound($i) ) { diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index b917a82..e7acbad 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -491,7 +491,7 @@ sub GetServices { my $canbookbereserved = CanBookBeReserved( $borrower, $biblionumber ); if ($canbookbereserved eq 'OK') { 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 33143a5..0ed7fe0 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -174,6 +174,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 a188f96..71d4fc5 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -70,13 +70,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 @@ -106,9 +106,9 @@ BEGIN { &GetReserveFee &GetReserveInfo &GetReserveStatus - + &GetOtherReserves - + &ModReserveFill &ModReserveAffect &ModReserve @@ -116,7 +116,7 @@ BEGIN { &ModReserveCancelAll &ModReserveMinusPriority &MoveReserve - + &CheckReserves &CanBookBeReserved &CanItemBeReserved @@ -127,7 +127,9 @@ BEGIN { &AutoUnsuspendReserves &IsAvailableForItemLevelRequest - + + &OPACItemHoldsAllowed + &AlterPriority &ToggleLowestPriority @@ -140,7 +142,7 @@ BEGIN { IsItemOnHoldAndFound ); @EXPORT_OK = qw( MergeHolds ); -} +} =head2 AddReserve @@ -351,7 +353,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; @@ -491,7 +493,7 @@ sub CanBookBeReserved{ sub CanItemBeReserved{ my ($borrowernumber, $itemnumber) = @_; - + my $dbh = C4::Context->dbh; my $ruleitemtype; # itemtype of the matching issuing rule my $allowedreserves = 0; @@ -513,18 +515,18 @@ sub CanItemBeReserved{ my $itemtypefield = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype"; # 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) @@ -536,7 +538,7 @@ sub CanItemBeReserved{ my $branchcode = ""; my $branchfield = "reserves.branchcode"; - + if( $controlbranch eq "ItemHomeLibrary" ){ $branchfield = "items.homebranch"; $branchcode = $item->{homebranch}; @@ -553,9 +555,9 @@ sub CanItemBeReserved{ }else{ $ruleitemtype = '*'; } - + # we retrieve count - + $querycount .= "AND $branchfield = ?"; $querycount .= " AND $itemtypefield = ?" if ($ruleitemtype ne "*"); @@ -566,12 +568,11 @@ sub CanItemBeReserved{ }else{ $sthcount->execute($borrowernumber, $branchcode, $ruleitemtype); } - + my $reservecount = "0"; if(my $rowcount = $sthcount->fetchrow_hashref()){ $reservecount = $rowcount->{count}; } - # we check if it's ok or not if( $reservecount >= $allowedreserves ){ return 'tooManyReserves'; @@ -955,7 +956,7 @@ sub CheckReserves { LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype "; } - + if ($item) { $sth = $dbh->prepare("$select WHERE itemnumber = ?"); $sth->execute($item); @@ -1053,7 +1054,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 " ); @@ -1062,7 +1063,7 @@ sub CancelExpiredReserves { while ( my $res = $sth->fetchrow_hashref() ) { CancelReserve({ reserve_id => $res->{'reserve_id'} }); } - + # Cancel reserves that have been waiting too long if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); @@ -1188,12 +1189,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. @@ -1305,7 +1306,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 ) { @@ -1350,7 +1351,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 @@ -1511,22 +1512,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 @@ -1537,48 +1534,80 @@ 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 = _get_itype($item); + my $notforloan_per_itemtype + = $dbh->selectrow_array("SELECT notforloan FROM itemtypes WHERE itemtype = ?", + undef, $itype); + + return 0 if + $notforloan_per_itemtype || + $item->{itemlost} || + $item->{notforloan} > 0 || + $item->{wthdrawn} || + ($item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems')); + + + return 1 if _OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch}); + + return $item->{onloan} || GetReserveStatus($item->{itemnumber}) eq "Waiting"; +} + +=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 ($item, $borrower) = @_; + + my $itype = _get_itype($item); + return _OnShelfHoldsAllowed($itype,$borrower->{categorycode},$item->{holdingbranch}); +} + +sub _get_itype { + my $item = shift; + + 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 $dbh = C4::Context->dbh; + 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}; + } } + return $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->{withdrawn} or - $notforloan_per_itemtype; - +sub _OnShelfHoldsAllowed { + my ($itype,$borrowercategory,$branchcode) = @_; - if (C4::Context->preference('AllowOnShelfHolds')) { - return $available_per_item; - } else { - return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "Waiting")); - } + my $rule = C4::Circulation::GetIssuingRule($borrowercategory, $itype, $branchcode); + return $rule->{onshelfholds}; } =head2 AlterPriority @@ -1846,7 +1875,7 @@ sub _FixPriority { $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); $sth->execute(); - + unless ( $ignoreSetLowestRank ) { while ( my $res = $sth->fetchrow_hashref() ) { _FixPriority({ @@ -1914,7 +1943,7 @@ sub _Findgroupreserve { unless any{ $data->{borrowernumber} eq $_ } @$ignore_borrowers ; } return @results if @results; - + # check for title-level targetted match my $title_level_target_query = qq{ SELECT reserves.biblionumber AS biblionumber, @@ -1999,7 +2028,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 = C4::Members::GetNoticeEmailAddress($borrowernumber); @@ -2119,6 +2148,54 @@ 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; + my $opacitemholds = uc substr ($data->{opacitemholds}, 0, 1); + return '' if $opacitemholds eq 'N'; + return $opacitemholds; +} + =head2 MoveReserve MoveReserve( $itemnumber, $borrowernumber, $cancelreserve ) diff --git a/C4/UsageStats.pm b/C4/UsageStats.pm index 627fba5..4960d51 100644 --- a/C4/UsageStats.pm +++ b/C4/UsageStats.pm @@ -155,7 +155,6 @@ sub BuildReport { AllowHoldPolicyOverride AllowHoldsOnDamagedItems AllowHoldsOnPatronsPossessions - AllowOnShelfHolds AutoResumeSuspendedHolds canreservefromotherbranches decreaseLoanHighHolds @@ -265,7 +264,6 @@ sub BuildReport { AllowPurchaseSuggestionBranchChoice OpacAllowPublicListCreation OpacAllowSharingPrivateLists - OPACItemHolds OpacRenewalAllowed OpacRenewalBranch OPACViewOthersSuggestions diff --git a/C4/VirtualShelves/Page.pm b/C4/VirtualShelves/Page.pm index 052839d..55bade3 100644 --- a/C4/VirtualShelves/Page.pm +++ b/C4/VirtualShelves/Page.pm @@ -30,6 +30,7 @@ use Data::Dumper; use C4::VirtualShelves qw/:DEFAULT ShelvesMax/; use C4::Biblio; use C4::Items; +use C4::Reserves; use C4::Koha; use C4::Auth qw/get_session/; use C4::Members; @@ -238,7 +239,6 @@ sub shelfpage { my ($shelfnumber2,$shelfname,$owner,$category,$sorton) = GetShelf($shelfnumber); $template->param( - 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds'), 'DisplayMultiPlaceHold' => C4::Context->preference('DisplayMultiPlaceHold'), ); if (C4::Context->preference('TagsEnabled')) { @@ -259,6 +259,7 @@ sub shelfpage { direction => $direction, ); ( $items, $totitems ) = GetShelfContents( $shelfnumber, $shelflimit, $shelfoffset, $sortfield, $direction ); + my $borrower = GetMember( 'borrowernumber' => $loggedinuser ); for my $this_item (@$items) { my $biblionumber = $this_item->{'biblionumber'}; my $record = GetMarcBiblio($biblionumber); @@ -296,6 +297,7 @@ sub shelfpage { }); } + $this_item->{'allow_onshelf_holds'} = C4::Reserves::OnShelfHoldsAllowed($this_item, $borrower); } if($type eq 'intranet'){ # Build drop-down list for 'Add To:' menu... diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 942e9bf..eed932d 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, renewalperiod, norenewalbefore, auto_renew, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, maxsuspensiondays, firstremind, chargeperiod,rentaldiscount, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'); - my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, maxsuspensiondays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, renewalperiod=?, norenewalbefore=?, auto_renew=?, 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, norenewalbefore, auto_renew, reservesallowed, issuelength, lengthunit, hardduedate, hardduedatecompare, fine, finedays, maxsuspensiondays, firstremind, chargeperiod,rentaldiscount, onshelfholds, opacitemholds, overduefinescap) VALUES(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)'); + my $sth_update=$dbh->prepare("UPDATE issuingrules SET fine=?, finedays=?, maxsuspensiondays=?, firstremind=?, chargeperiod=?, maxissueqty=?, renewalsallowed=?, renewalperiod=?, norenewalbefore=?, auto_renew=?, reservesallowed=?, issuelength=?, lengthunit=?, hardduedate=?, hardduedatecompare=?, rentaldiscount=?, onshelfholds=?, opacitemholds=?, 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 $maxsuspensiondays = $input->param('maxsuspensiondays'); @@ -120,6 +120,7 @@ elsif ($op eq 'add') { $norenewalbefore = undef if $norenewalbefore eq '0' or $norenewalbefore =~ /^\s*$/; my $auto_renew = $input->param('auto_renew') eq 'yes' ? 1 : 0; 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'); @@ -128,15 +129,16 @@ elsif ($op eq 'add') { $hardduedate = format_date_in_iso($hardduedate); my $hardduedatecompare = $input->param('hardduedatecompare'); my $rentaldiscount = $input->param('rentaldiscount'); + 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, $maxsuspensiondays, $firstremind, $chargeperiod, $maxissueqty, $renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount,$overduefinescap, $br,$bor,$cat); + $sth_update->execute($fine, $finedays, $maxsuspensiondays, $firstremind, $chargeperiod, $maxissueqty, $renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed, $issuelength,$lengthunit, $hardduedate,$hardduedatecompare,$rentaldiscount, $onshelfholds, $opacitemholds, $overduefinescap, $br,$bor,$itemtype); } else { - $sth_insert->execute($br,$bor,$cat,$maxissueqty,$renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays, $maxsuspensiondays, $firstremind,$chargeperiod,$rentaldiscount,$overduefinescap); + $sth_insert->execute($br,$bor,$itemtype,$maxissueqty,$renewalsallowed, $renewalperiod, $norenewalbefore, $auto_renew, $reservesallowed,$issuelength,$lengthunit,$hardduedate,$hardduedatecompare,$fine,$finedays, $maxsuspensiondays, $firstremind,$chargeperiod,$rentaldiscount,$onshelfholds,$opacitemholds,$overduefinescap); } } elsif ($op eq "set-branch-defaults") { @@ -391,7 +393,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/installer/data/mysql/it-IT/necessari/system_preferences.sql b/installer/data/mysql/it-IT/necessari/system_preferences.sql index df446bc..94a00b9 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 82868e7..a9f174b 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1192,6 +1192,8 @@ 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(28,6) 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 char(1) NOT NULL default 'N', -- allow opac users to place specific items on hold 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 89707ef..9254982 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -25,7 +25,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AllowMultipleIssuesOnABiblio',1,'Allow/Don\'t allow patrons to check out multiple items from one biblio','','YesNo'), ('AllowNotForLoanOverride','0','','If ON, Koha will allow the librarian to loan a not for loan item.','YesNo'), ('AllowOfflineCirculation','0','','If on, enables HTML5 offline circulation functionality.','YesNo'), -('AllowOnShelfHolds','0','','Allow hold requests to be placed on items that are not on loan','YesNo'), ('AllowPKIAuth','None','None|Common Name|emailAddress','Use the field from a client-side SSL certificate to look a user in the Koha database','Choice'), ('AllowPurchaseSuggestionBranchChoice','0','1','Allow user to choose branch when making a purchase suggestion','YesNo'), ('AllowRenewalIfOtherItemsAvailable','0',NULL,'If enabled, allow a patron to renew an item with unfilled holds if other available items can fill that hold.','YesNo'), @@ -262,7 +261,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('OpacHiddenItems','','','This syspref allows to define custom rules for hiding specific items at opac. See docs/opac/OpacHiddenItems.txt for more informations.','Textarea'), ('OpacHighlightedWords','1','','If Set, then queried words are higlighted in OPAC','YesNo'), ('OpacHoldNotes','0','','Show hold notes on OPAC','YesNo'), -('OPACItemHolds','1','0|1|force','Allow OPAC users to place hold on specific items. If No, users can only request next available copy. If Yes, users can choose between next available and specific copy. If Force, users *must* choose a specific copy.','Choice'), ('OpacItemLocation','callnum','callnum|ccode|location','Show the shelving location of items in the opac','Choice'), ('OPACItemsResultsDisplay','0','','If OFF : show only the status of items in result list.If ON : show full location of items (branch+location+callnumber) as in staff interface','YesNo'), ('OpacKohaUrl','1',NULL,'Show \'Powered by Koha\' text on OPAC footer.',NULL), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 4653c02..05589c4 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'"); @@ -9666,6 +9665,41 @@ if ( CheckVersion($DBversion) ) { $dbh->do(q|SET foreign_key_checks = 1|);; print "Upgrade to $DBversion done (Bug 11944 - Convert DB tables to utf8_unicode_ci)\n"; + SetVersion($DBversion); +} + +$DBversion = '3.19.00.XXX'; +if (C4::Context->preference("Version") < TransformToNum($DBversion)) { + # First create the column + $dbh->do("ALTER TABLE issuingrules ADD onshelfholds tinyint(1) default 0 NOT NULL"); + # 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"); + } + # Remove from the systempreferences table + $dbh->do("DELETE FROM systempreferences WHERE variable = 'AllowOnShelfHolds'"); + + # First create the column + $dbh->do("ALTER TABLE issuingrules ADD opacitemholds char(1) DEFAULT 'N' NOT NULL"); + # Now update the column + my $opacitemholds = C4::Context->preference("OPACItemHolds") || ''; + if (lc ($opacitemholds) eq 'force') { + $opacitemholds = 'F'; + } + else { + $opacitemholds = $opacitemholds ? 'Y' : 'N'; + } + # Set allow for all rules + $dbh->do("UPDATE issuingrules SET opacitemholds=1"); + + # Remove from the systempreferences table + $dbh->do("DELETE FROM systempreferences WHERE variable = 'OPACItemHolds'"); + + print "Upgrade to $DBversion done (Move AllowOnShelfHolds to circulation matrix; Move OPACItemHolds system preference to circulation matrix)\n"; SetVersion ($DBversion); } 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 a296bf6..a16f231 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 @@ -439,12 +439,6 @@ Circulation: no: "Don't allow" - hold requests to be placed on and filled by 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 f0c0951..de64fac 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 @@ -494,13 +494,6 @@ OPAC: # choices: # - If ON, enables subject cloud on OPAC - - - pref: OPACItemHolds - choices: - no: "Don't allow" - yes: Allow - force: Force - - 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. If this is forced, users must put a hold on a specific 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 8bc5809..1397b1d 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 @@ -156,6 +156,8 @@ for="tobranch">Clone these rules to:   @@ -218,6 +220,8 @@ for="tobranch">Clone these rules to: Edit @@ -275,6 +279,8 @@ for="tobranch">Clone these rules to: + + diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc index 3efa10c..8f8ffa5 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc @@ -2,12 +2,8 @@ [% UNLESS ( norequests ) %] [% IF Koha.Preference( 'opacuserlogin' ) == 1 %] [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %] - [% IF ( AllowOnShelfHolds ) %] + [% IF ( AllowOnShelfHolds OR ItemsIssued ) %]
  • Place hold
    • - [% ELSE %] - [% IF ( ItemsIssued ) %] -
  • Place hold
    • - [% END %] [% END %] [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt index 6558e6e..3aa4cd1 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -243,26 +243,22 @@ [% END # / IF OpacHoldNotes %] - [% IF OPACItemHolds == '1' or OPACItemHolds == 'force' %] +
  • +
    • + [% IF bibitemloo.itemholdable %] - [% END # / IF OPACItemHolds %] + [% END # / IF bibitemloo.itemholdable %] - [% IF OPACItemHolds == '1' || OPACItemHolds == 'force' %] + [% IF bibitemloo.itemholdable %] @@ -380,7 +376,7 @@ [% END # / FOREACH itemLoo IN bibitemloo.itemLoop%]
    Select a specific item:
    - [% END # / IF ( OPACItemHolds )%] + [% END # / IF ( bibitemloo.itemholdable )%] [% END # / IF ( bibitemloo.holdable ) %] @@ -460,12 +456,10 @@ // Hides all 'specific copy' table rows on load. $(".copiesrow").hide(); - [% IF OPACItemHolds == 'force' %] - [% FOREACH bibitemloo IN bibitemloop %] - [% IF bibitemloo.holdable %] - $("#toggle-hold-options-[% bibitemloo.biblionumber %]").click(); - $("#copiesrow_[% bibitemloo.biblionumber %]").show(); - [% END %] + [% FOREACH bibitemloo IN bibitemloop %] + [% IF bibitemloo.force_hold %] + $("#toggle-hold-options-[% bibitemloo.biblionumber %]").click(); + $("#copiesrow_[% bibitemloo.biblionumber %]").show(); [% END %] [% END %] @@ -523,20 +517,16 @@ $("#place_on_hdr").show(); - [% IF OPACItemHolds == '1' %] - $(".place_on_type").show(); - // onload, selectany is checked - $(".selectany").attr("checked", "checked"); - [% END %] + $(".place_on_type").show(); + // onload, selectany is checked + $(".selectany").attr("checked", "checked"); // If the user is *allowed* to choose a specific item // The first one is preselected - [% IF OPACItemHolds =="1" %] - $("table.copiesrow").each(function(){ - var id = suffixOf($(this).attr("id"), "_"); - select_first_available(id); - }); - [% END %] + $("table.copiesrow").each(function(){ + var id = suffixOf($(this).attr("id"), "_"); + select_first_available(id); + }); // On confirmsjs change $(".confirmjs").change(function(){ diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results-grouped.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results-grouped.tt index 9b8cd2b..05d022a 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results-grouped.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results-grouped.tt @@ -227,14 +227,8 @@ href="/cgi-bin/koha/opac-rss.pl?[% query_cgi %][% limit_cgi |html %]" />

    [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %] [% UNLESS ( GROUP_RESULT.norequests ) %] - [% IF Koha.Preference( 'opacuserlogin' ) == 1 %] - [% IF ( AllowOnShelfHolds ) %] - Place hold - [% ELSE %] - [% IF ( GROUP_RESULT.itemsissued ) %] - Place hold - [% END %] - [% END %] + [% IF Koha.Preference( 'opacuserlogin' ) == 1 && GROUP_RESULT.holdable %] + Place hold [% END %] [% END %] [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt index 9a15d84..acfc31a 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt @@ -471,13 +471,9 @@

    [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %] [% UNLESS ( SEARCH_RESULT.norequests ) %] - [% IF ( ( Koha.Preference( 'opacuserlogin' ) == 1 ) && AllowOnShelfHolds ) %] + [% IF ( Koha.Preference( 'opacuserlogin' ) == 1 ) && SEARCH_RESULT.holdable %] Place hold - [% ELSE %] - [% IF ( SEARCH_RESULT.itemsissued ) %] - Place hold - [% END %] - [% END # / IF opacuserlogin && AllowOnShelfHolds %] + [% END # / IF opacuserlogin && holdable %] [% END # UNLESS SEARCH_RESULT.norequests %] [% END # IF RequestOnOpac %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt index 4b929d2..cfabd90 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt @@ -418,7 +418,7 @@ [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %] [% UNLESS ( itemsloo.norequests ) %] [% IF Koha.Preference( 'opacuserlogin' ) == 1 %] - [% IF ( AllowOnShelfHolds ) %] + [% IF ( itemsloo.allow_onshelf_holds ) %] Place hold [% ELSE %] [% IF ( itemsloo.itemsissued ) %] diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index d45ebc3..33cf4f3 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -49,6 +49,7 @@ use CGI qw ( -utf8 ); use MARC::Record; use C4::Biblio; use C4::Items; +use C4::Reserves; use C4::Acquisition; use C4::Review; use C4::Serials; # uses getsubscriptionfrom biblionumber @@ -70,17 +71,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"); @@ -148,16 +145,22 @@ $template->param( ); my $norequests = 1; +my $allow_onshelf_holds; my $res = GetISBDView($biblionumber, "opac"); my $itemtypes = GetItemTypes(); +my $borrower = GetMember( 'borrowernumber' => $loggedinuser ); for my $itm (@items) { $norequests = 0 - if ( (not $itm->{'withdrawn'} ) - && (not $itm->{'itemlost'} ) - && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'} ) - && (not $itemtypes->{$itm->{'itype'}}->{notforloan} ) - && ($itm->{'itemnumber'} ) ); + if $norequests + && !$itm->{'withdrawn'} + && !$itm->{'itemlost'} + && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'}) + && !$itemtypes->{$itm->{'itype'}}->{notforloan} + && $itm->{'itemnumber'}; + + $allow_onshelf_holds = C4::Reserves::OnShelfHoldsAllowed($itm, $borrower) + unless $allow_onshelf_holds; } my $reviews = getreviews( $biblionumber, 1 ); @@ -173,7 +176,7 @@ foreach ( @$reviews ) { $template->param( RequestOnOpac => C4::Context->preference("RequestOnOpac"), - AllowOnShelfHolds => C4::Context->preference('AllowOnShelfHolds'), + AllowOnShelfHolds => $allow_onshelf_holds, norequests => $norequests, ISBD => $res, biblionumber => $biblionumber, diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 23c802a..6a5e8a6 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -52,6 +52,8 @@ use CGI qw ( -utf8 ); use MARC::Record; use C4::Biblio; use C4::Items; +use C4::Reserves; +use C4::Members; use C4::Acquisition; use C4::Koha; use List::MoreUtils qw/any/; @@ -103,17 +105,21 @@ $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') ); +my $allow_onshelf_holds; +my $borrower = GetMember( 'borrowernumber' => $loggedinuser ); +for my $itm (@all_items) { + $allow_onshelf_holds = C4::Reserves::OnShelfHoldsAllowed($itm, $borrower); + last if $allow_onshelf_holds; +} + +$template->param( 'AllowOnShelfHolds' => $allow_onshelf_holds ); $template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) ); # adding the $RequestOnOpac param diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index b8cd194..e0824d8 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -437,12 +437,7 @@ if ($session->param('busc')) { } - -$template->param( 'AllowOnShelfHolds' => C4::Context->preference('AllowOnShelfHolds') ); $template->param( 'ItemsIssued' => CountItemsIssued( $biblionumber ) ); - - - $template->param('OPACShowCheckoutName' => C4::Context->preference("OPACShowCheckoutName") ); $template->param('OPACShowBarcode' => C4::Context->preference("OPACShowBarcode") ); @@ -627,15 +622,21 @@ if ( not $viewallitems and @items > $max_items_to_display ) { items_count => scalar( @items ), ); } else { + my $allow_onshelf_holds; + my $borrower = GetMember( 'borrowernumber' => $borrowernumber ); for my $itm (@items) { $itm->{holds_count} = $item_reserves{ $itm->{itemnumber} }; $itm->{priority} = $priority{ $itm->{itemnumber} }; $norequests = 0 - if ( (not $itm->{'withdrawn'} ) - && (not $itm->{'itemlost'} ) - && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'} ) - && (not $itemtypes->{$itm->{'itype'}}->{notforloan} ) - && ($itm->{'itemnumber'} ) ); + if $norequests + && !$itm->{'withdrawn'} + && !$itm->{'itemlost'} + && ($itm->{'itemnotforloan'}<0 || not $itm->{'itemnotforloan'}) + && !$itemtypes->{$itm->{'itype'}}->{notforloan} + && $itm->{'itemnumber'}; + + $allow_onshelf_holds = C4::Reserves::OnShelfHoldsAllowed($itm, $borrower) + unless $allow_onshelf_holds; # get collection code description, too my $ccode = $itm->{'ccode'}; @@ -691,6 +692,7 @@ if ( not $viewallitems and @items > $max_items_to_display ) { push @itemloop, $itm; } } + $template->param( 'AllowOnShelfHolds' => $allow_onshelf_holds ); } # Display only one tab if one items list is empty diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 6c381f3..be6b90e 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -423,6 +423,7 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{itemLoop} = []; my $numCopiesAvailable = 0; + my $numCopiesOPACAvailable = 0; foreach my $itemInfo (@{$biblioData->{itemInfos}}) { my $itemNum = $itemInfo->{itemnumber}; my $itemLoopIter = {}; @@ -517,16 +518,27 @@ foreach my $biblioNum (@biblionumbers) { my $branch = GetReservesControlBranch( $itemInfo, $borr ); - 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) eq 'OK' and ($itemLoopIter->{already_reserved} ne 1)) { - $itemLoopIter->{available} = 1; + $policy_holdallowed &&= + IsAvailableForItemLevelRequest($itemInfo,$borr) && + CanItemBeReserved($borrowernumber,$itemNum) eq 'OK'; + + if ($policy_holdallowed) { + if ( my $hold_allowed = OPACItemHoldsAllowed( $itemInfo, $borr ) ) { + $itemLoopIter->{available} = 1; + $numCopiesOPACAvailable++; + $biblioLoopIter{force_hold} = 1 if $hold_allowed eq 'F'; + } $numCopiesAvailable++; } @@ -545,23 +557,22 @@ foreach my $biblioNum (@biblionumbers) { $numBibsAvailable++; $biblioLoopIter{bib_available} = 1; $biblioLoopIter{holdable} = 1; + $biblioLoopIter{itemholdable} = 1 if $numCopiesOPACAvailable; } if ($biblioLoopIter{already_reserved}) { $biblioLoopIter{holdable} = undef; - } - my $canReserve = CanBookBeReserved($borrowernumber,$biblioNum); - unless ($canReserve eq 'OK') { - $biblioLoopIter{holdable} = undef; - $biblioLoopIter{ $canReserve } = 1; + $biblioLoopIter{itemholdable} = undef; } if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) { $biblioLoopIter{holdable} = undef; $biblioLoopIter{already_patron_possession} = 1; } - if( $biblioLoopIter{holdable} ){ $anyholdable++; } + $biblioLoopIter{holdable} &&= CanBookBeReserved($borrowernumber,$biblioNum) eq 'OK'; push @$biblioLoop, \%biblioLoopIter; + + $anyholdable = 1 if $biblioLoopIter{holdable}; } if ( $numBibsAvailable == 0 || $anyholdable == 0) { diff --git a/opac/opac-search.pl b/opac/opac-search.pl index b0feaeb..472d133 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -42,6 +42,8 @@ use C4::Branch; # GetBranches use C4::SocialData; use C4::Ratings; use C4::External::OverDrive; +use C4::Members; +use C4::Reserves; use POSIX qw(ceil floor strftime); use URI::Escape; @@ -136,7 +138,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( @@ -567,8 +569,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]; @@ -583,11 +588,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; + } } $hits = 0 unless @newresults; diff --git a/reserve/request.pl b/reserve/request.pl index ce0e70f..b4b749e 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -334,9 +334,6 @@ foreach my $biblionumber (@biblionumbers) { $item->{hosttitle} = GetBiblioData($item->{biblionumber})->{title}; } - # add information - $item->{itemcallnumber} = $item->{itemcallnumber}; - # if the item is currently on loan, we display its return date and # change the background color my $issues= GetItemIssue($itemnumber); @@ -347,9 +344,9 @@ foreach my $biblionumber (@biblionumbers) { # checking reserve my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber); - my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); - if ( defined $reservedate ) { + my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); + $item->{backgroundcolor} = 'reserved'; $item->{reservedate} = format_date($reservedate); $item->{ReservedForBorrowernumber} = $reservedfor; @@ -394,7 +391,7 @@ foreach my $biblionumber (@biblionumbers) { } # If there is no loan, return and transfer, we show a checkbox. - $item->{notforloan} = $item->{notforloan} || 0; + $item->{notforloan} ||= 0; # if independent branches is on we need to check if the person can reserve # for branches they arent logged in to @@ -424,7 +421,7 @@ foreach my $biblionumber (@biblionumbers) { if ( $policy_holdallowed && !$item->{cantreserve} - && IsAvailableForItemLevelRequest($itemnumber) + && IsAvailableForItemLevelRequest($item, $borrowerinfo) && CanItemBeReserved( $borrowerinfo->{borrowernumber}, $itemnumber ) eq 'OK' diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 58dbc2b..242cf59 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -291,7 +291,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); ); # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds - C4::Context->set_preference('AllowOnShelfHolds', 1 ); + C4::Context->dbh->do("UPDATE issuingrules SET onshelfholds = 1"); C4::Context->set_preference('AllowRenewalIfOtherItemsAvailable', 1 ); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber); is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds'); diff --git a/t/db_dependent/Circulation_Issuingrule.t b/t/db_dependent/Circulation_Issuingrule.t index 6e4f44f..b1d9d72 100644 --- a/t/db_dependent/Circulation_Issuingrule.t +++ b/t/db_dependent/Circulation_Issuingrule.t @@ -132,6 +132,8 @@ my $sampleissuingrule1 = { itemtype => 'BOOK', categorycode => $samplecat->{categorycode}, maxsuspensiondays => 0, + onshelfholds => 0, + opacitemholds => 'N', }; my $sampleissuingrule2 = { branchcode => $samplebranch2->{branchcode}, @@ -158,6 +160,8 @@ my $sampleissuingrule2 = { chargename => 'Null', restrictedtype => 'Null', maxsuspensiondays => 0, + onshelfholds => 1, + opacitemholds => 'Y', }; my $sampleissuingrule3 = { branchcode => $samplebranch1->{branchcode}, @@ -184,6 +188,8 @@ my $sampleissuingrule3 = { chargename => 'Null', restrictedtype => 'Null', maxsuspensiondays => 0, + onshelfholds => 1, + opacitemholds => 'F', }; $query = 'INSERT INTO issuingrules ( branchcode, diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 4676a60..fa0cf94 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 53; +use Test::More tests => 56; use MARC::Record; use DateTime::Duration; @@ -506,6 +506,28 @@ is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Res ####### EO Bug 13113 <<< #### +my $item = GetItem($itemnumber); + +ok( C4::Reserves::IsAvailableForItemLevelRequest($item, $borrower), "Reserving a book on item level" ); + +my $itype = C4::Reserves::_get_itype($item); +my $categorycode = $borrower->{categorycode}; +my $holdingbranch = $item->{holdingbranch}; +my $rule = C4::Circulation::GetIssuingRule($categorycode, $itype, $holdingbranch); + +$dbh->do( + "UPDATE issuingrules SET onshelfholds = 1 WHERE categorycode = ? AND itemtype= ? and branchcode = ?", + undef, + $rule->{categorycode}, $rule->{itemtype}, $rule->{branchcode} +); +ok( C4::Reserves::OnShelfHoldsAllowed($item, $borrower), "OnShelfHoldsAllowed() allowed" ); +$dbh->do( + "UPDATE issuingrules SET onshelfholds = 0 WHERE categorycode = ? AND itemtype= ? and branchcode = ?", + undef, + $rule->{categorycode}, $rule->{itemtype}, $rule->{branchcode} +); +ok( !C4::Reserves::OnShelfHoldsAllowed($item, $borrower), "OnShelfHoldsAllowed() disallowed" ); + $dbh->rollback; sub count_hold_print_messages { -- 1.7.10.4