From 785d24a026d9f5427f087d5c31192b9ce730cced Mon Sep 17 00:00:00 2001 From: Srdjan Date: Mon, 30 Jul 2012 12:04:01 +1200 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 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 --- C4/Auth.pm | 1 - C4/ILSDI/Services.pm | 2 +- C4/Items.pm | 2 + C4/Reserves.pm | 155 +++++++++++++---- 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 | 11 +- .../prog/en/includes/opac-detail-sidebar.inc | 8 +- .../opac-tmpl/prog/en/modules/opac-reserve.tt | 175 ++++++++------------ .../prog/en/modules/opac-results-grouped.tt | 6 +- .../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 | 43 +++-- opac/opac-search.pl | 19 ++- reserve/request.pl | 2 +- 26 files changed, 306 insertions(+), 223 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index b6dc2f5..7c03233 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -412,7 +412,6 @@ sub get_template_and_user { OPACAmazonReviews => C4::Context->preference("OPACAmazonReviews"), 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 4eed936..13b1276 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -485,7 +485,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 a9c0a16..f319099 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 585d1e5..71ef88c 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -123,6 +123,8 @@ BEGIN { &AutoUnsuspendReserves &IsAvailableForItemLevelRequest + + &OPACItemHoldsAllowed &AlterPriority &ToggleLowestPriority @@ -489,7 +491,6 @@ sub CanItemBeReserved{ if(my $rowcount = $sthcount->fetchrow_hashref()){ $reservecount = $rowcount->{count}; } - # we check if it's ok or not if( $reservecount < $allowedreserves ){ return 1; @@ -1375,7 +1376,7 @@ 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 @@ -1385,12 +1386,8 @@ item-level hold request. An item is available if * 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 $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; + 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')); - 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 ); @@ -1967,6 +1996,58 @@ 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 $dbh = C4::Context->dbh; + 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 c765cf0..cdf8c30 100644 --- a/C4/VirtualShelves/Page.pm +++ b/C4/VirtualShelves/Page.pm @@ -238,7 +238,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 f45ea43..faf9781 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 e12bd5f..0f43bf4 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"; @@ -375,7 +374,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 69304e1..6a7773d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -999,6 +999,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 6a53379..a8f71f2 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -193,7 +193,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'); @@ -226,7 +225,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 608a1bf..5c6a86b 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -2092,7 +2092,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'"); @@ -5566,6 +5565,41 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { SetVersion ($DBversion); } + + +$DBversion = '3.09.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 e07fc0d..4812754 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 @@ -268,12 +268,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 4a4d4de..21bcbe0 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 @@ -384,12 +384,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 b2f62f7..fa6715f 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,6 +76,9 @@ for="tobranch">Clone these rules to: Clone these rules to: Delete @@ -167,7 +173,10 @@ for="tobranch">Clone these rules to: - + + + + 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 85d44d7..16577fa 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 ) %] -
  • Place hold
  • - [% ELSE %] - [% IF ( ItemsIssued ) %] -
  • Place hold
  • - [% END %] - [% END %] +
  • Place hold
  • [% 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 cdd201b..8f68352 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt @@ -209,23 +209,22 @@ [% IF ( bad_data ) %]
    ERROR: Internal error: incomplete hold request.
    [% END %] - [% ELSE %] - [% IF ( none_available ) %] + [% ELSIF ( none_available ) %]
    Sorry, none of these items can be placed on hold.
    - [% END %] [% END %] [% UNLESS ( message ) %][% UNLESS ( none_available ) %]

    Confirm holds for: [% FOREACH USER_INF IN USER_INFO %] [% USER_INF.firstname %] [% USER_INF.surname %] ([% USER_INF.cardnumber %]) [% END %] -

    [% END %] - [% IF (RESERVE_CHARGE) %] + + [% IF (RESERVE_CHARGE) %]
    There is a charge of [% RESERVE_CHARGE %] for placing this hold
    - [% END %] + [% END %] + [% END %][% END %]
    @@ -234,38 +233,47 @@
    + [% extra_cols = 2 %] - [% UNLESS ( none_available ) %] + + [% UNLESS ( none_available ) %] + [% END %] - [% UNLESS ( item_level_itypes ) %] + [% UNLESS ( item_level_itypes ) %] - [% END %] + [% END %] + [% UNLESS ( none_available ) %] [% IF showholds && showpriority %] + [% extra_cols = extra_cols + 1 %] [% ELSIF showholds %] + [% extra_cols = extra_cols + 1 %] [% ELSIF showpriority %] + [% extra_cols = extra_cols + 1 %] [% END %] [% IF ( reserve_in_future ) %] + [% extra_cols = extra_cols + 1 %] [% END %] - [% IF ( OPACItemHolds ) %] - [% END %] [% UNLESS ( singleBranchMode ) %] [% IF ( choose_branch ) %] + [% extra_cols = extra_cols + 1 %] [% END %] [% END %] - [% ELSE %][% END %] + [% END %] + [% FOREACH bibitemloo IN bibitemloop %] - [% IF ( bibitemloo.holdable ) %] - - [% ELSE %] - [% UNLESS ( none_available ) %][% END %] - [% END %] - [% IF ( bibitemloo.holdable ) %] + [% END %] + - [% IF ( bibitemloo.holdable ) %] - [% UNLESS ( item_level_itypes ) %] [% END %] + [% IF ( bibitemloo.holdable ) %] + [% IF showholds || showpriority %] [% END %] + - [% IF ( bibitemloo.holdable ) %] - - [% IF ( OPACItemHolds ) %] + - [% END %][% END %] - - [% UNLESS ( singleBranchMode ) %] - [% IF ( bibitemloo.holdable ) %] +
      +
    • + + +
    • + [% IF ( bibitemloo.itemholdable ) %] +
    • + + +
    • + [% END %] +
    + + + [% UNLESS ( singleBranchMode ) %] [% IF ( choose_branch ) %] - - [% END %] - [% END %] - [% END %] + [% END %] + [% END %] + [% ELSIF NOT none_available %] + + [% END # holdable%] - [% IF ( OPACItemHolds ) %] - [% IF ( bibitemloo.holdable ) %] + [% IF ( bibitemloo.itemholdable ) %]
    HoldTitleItem typeHolds and priorityHoldsPriorityHold starts on dateHold not needed afterPickup location
    Title
    + [% UNLESS none_available %] + + [% IF ( bibitemloo.holdable ) %] @@ -276,38 +284,35 @@ value="any" /> -  [% ELSE %][% END %] + [% ELSE %] +   + [% END %] + [% bibitemloo.title |html %][% IF ( bibitemloo.subtitle ) %] [% FOREACH subtitl IN bibitemloo.subtitle %][% subtitl.subfield %][% END %][% END %] [% IF ( bibitemloo.author ) %], by [% bibitemloo.author %][% END %] [% UNLESS ( bibitemloo.holdable ) %] - +
    [% IF ( bibitemloo.already_reserved ) %] -
    You have already requested this title.
    + You have already requested this title. + [% ELSIF ( bibitemloo.bib_available ) %] + No available items. [% ELSE %] - [% UNLESS ( bibitemloo.bib_available ) %] -
    No available items.
    - [% ELSE %] -
    This title cannot be requested.
    - [% END %] - [% END %] - - + This title cannot be requested. [% END %] - +
    + [% END %]
    [% IF ( bibitemloo.imageurl ) %][% END %] [% bibitemloo.description %] [% IF showpriority %] [% bibitemloo.rank %] [% END %] @@ -332,68 +337,36 @@

    Clear date

    -
    - [% UNLESS ( bibitemloo.holdable ) %] - - [% ELSE %] + - [% END %]  
      @@ -490,11 +463,9 @@
    - [% END %] - [% END %] - [% END %] - - [% END %] + [% END # itemholdable%] + [% END %] +
    [% 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 99c31bd..598927c 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 @@ -260,12 +260,8 @@ function highlightOn() { [% IF ( RequestOnOpac ) %] [% UNLESS ( GROUP_RESULT.norequests ) %] [% IF ( opacuserlogin ) %] - [% IF ( AllowOnShelfHolds ) %] + [% IF ( GROUP_RESULT.holdable ) %] Place hold - [% ELSE %] - [% IF ( GROUP_RESULT.itemsissued ) %] - Place hold - [% END %] [% END %] [% END %] [% END %] 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 67b1686..f064057 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-results.tt @@ -567,12 +567,8 @@ $(document).ready(function(){ [% IF ( RequestOnOpac ) %] [% UNLESS ( SEARCH_RESULT.norequests ) %] [% IF ( opacuserlogin ) %] - [% IF ( AllowOnShelfHolds ) %] + [% IF ( SEARCH_RESULT.holdable ) %] Place hold - [% ELSE %] - [% IF ( SEARCH_RESULT.itemsissued ) %] - Place hold - [% 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 716f2e1..75762f2 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tt @@ -382,7 +382,7 @@ $(function() { [% IF ( RequestOnOpac ) %] [% UNLESS ( itemsloo.norequests ) %] [% IF ( opacuserlogin ) %] - [% 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 435859d..9f4333b 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -70,17 +70,13 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $biblionumber = $query->param('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 2d9dc25..34c9125 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -82,17 +82,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 0bafabd..080734c 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -387,8 +387,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 1fb4df0..a8bc22c 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -380,6 +380,7 @@ foreach my $biblioNum (@biblionumbers) { $biblioLoopIter{itemLoop} = []; my $numCopiesAvailable = 0; + my $numCopiesOPACAvailable = 0; foreach my $itemInfo (@{$biblioData->{itemInfos}}) { my $itemNum = $itemInfo->{itemnumber}; my $itemLoopIter = {}; @@ -472,16 +473,26 @@ foreach my $biblioNum (@biblionumbers) { my $branch = C4::Circulation::_GetCircControlBranch($itemLoopIter, $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) 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++; } @@ -507,18 +518,17 @@ 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; } + $biblioLoopIter{holdable} &&= CanBookBeReserved($borrowernumber,$biblioNum); push @$biblioLoop, \%biblioLoopIter; + + $anyholdable = 1 if $biblioLoopIter{holdable}; } if ( $numBibsAvailable == 0 || !$anyholdable) { @@ -526,9 +536,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 1e5a0d7..aa74f68 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( @@ -505,8 +507,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]; @@ -521,11 +526,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 c34a614..a922c8f 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -456,7 +456,7 @@ foreach my $biblionumber (@biblionumbers) { if ( $policy_holdallowed && !$item->{cantreserve} - && IsAvailableForItemLevelRequest($itemnumber) + && IsAvailableForItemLevelRequest($item, $borrowerinfo) && CanItemBeReserved( $borrowerinfo->{borrowernumber}, $itemnumber ) -- 1.7.2.5