From 0886604b4d18deac8420681e35b57be3884f4277 Mon Sep 17 00:00:00 2001 From: Pasi Kallinen Date: Wed, 26 Mar 2014 13:20:24 +0200 Subject: [PATCH] Allow multiple reserves for the same patron. This requires rather large changes to C4/Reserves.pm, because it didn't use the reserve_id to distinguish the reserves. -Remove all vestiges of reserveconstraints -table from the code. -Make Reserves use ID numbers as much as possible, instead of matching with patron number, bibnum, and date. -Allow the same patron reserve the same item multiple times, but only via staff client. --- C4/Reserves.pm | 199 +++++++++------------ installer/data/mysql/sysprefs.sql | 1 + .../en/modules/admin/preferences/circulation.pref | 6 + reserve/placerequest.pl | 16 +- reserve/request.pl | 2 +- 5 files changed, 96 insertions(+), 128 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index eb1ae2b..baa6f96 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -187,7 +187,6 @@ sub AddReserve { "Reserve Charge - $title", $fee ); } - #if ($const eq 'a'){ my $query = qq/ INSERT INTO reserves (borrowernumber,biblionumber,reservedate,branchcode,constrainttype, @@ -232,19 +231,6 @@ sub AddReserve { } } - #} - ($const eq "o" || $const eq "e") or return; # FIXME: why not have a useful return value? - $query = qq/ - INSERT INTO reserveconstraints - (borrowernumber,biblionumber,reservedate,biblioitemnumber) - VALUES - (?,?,?,?) - /; - $sth = $dbh->prepare($query); # keep prepare outside the loop! - foreach (@$bibitems) { - $sth->execute($borrowernumber, $biblionumber, $resdate, $_); - } - return; # FIXME: why not have a useful return value? } @@ -325,48 +311,8 @@ sub GetReservesFromBiblionumber { $query .= "ORDER BY priority"; my $sth = $dbh->prepare($query); $sth->execute( @params ); - my @results; - my $i = 0; - while ( my $data = $sth->fetchrow_hashref ) { - - # FIXME - What is this doing? How do constraints work? - if ($data->{constrainttype} eq 'o') { - $query = ' - SELECT biblioitemnumber - FROM reserveconstraints - WHERE biblionumber = ? - AND borrowernumber = ? - AND reservedate = ? - '; - my $csth = $dbh->prepare($query); - $csth->execute($data->{biblionumber}, $data->{borrowernumber}, $data->{reservedate}); - my @bibitemno; - while ( my $bibitemnos = $csth->fetchrow_array ) { - 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; - if ( $count > 1 ) { - $bdata = GetBiblioItemData( $bibitemno[$i] ); # FIXME: This doesn't make sense. - $i++; # $i can increase each pass, but the next @bibitemno might be smaller? - } - else { - # Look up the book we just found. - $bdata = GetBiblioItemData( $bibitemno[0] ); - } - # Add the results of this latest search to the current - # results. - # FIXME - An 'each' would probably be more efficient. - foreach my $key ( keys %$bdata ) { - $data->{$key} = $bdata->{$key}; - } - } - push @results, $data; - } - return \@results; + my $results = $sth->fetchall_arrayref({}); + return $results; } =head2 GetReservesFromItemnumber @@ -1161,45 +1107,39 @@ sub ModReserveFill { my $borrowernumber = $res->{'borrowernumber'}; my $resdate = $res->{'reservedate'}; + $reserve_id = GetReserveId({biblionumber => $biblionumber, + borrowernumber => $borrowernumber, + reservedate => $resdate}) if (!$reserve_id); + return if (!$reserve_id); + # get the priority on this record.... my $priority; my $query = "SELECT priority FROM reserves - WHERE biblionumber = ? - AND borrowernumber = ? - AND reservedate = ?"; + WHERE reserve_id = ?"; my $sth = $dbh->prepare($query); - $sth->execute( $biblionumber, $borrowernumber, $resdate ); + $sth->execute( $reserve_id ); ($priority) = $sth->fetchrow_array; # update the database... $query = "UPDATE reserves SET found = 'F', priority = 0 - WHERE biblionumber = ? - AND reservedate = ? - AND borrowernumber = ? - "; + WHERE reserve_id = ?"; $sth = $dbh->prepare($query); - $sth->execute( $biblionumber, $resdate, $borrowernumber ); + $sth->execute( $reserve_id ); # move to old_reserves $query = "INSERT INTO old_reserves SELECT * FROM reserves - WHERE biblionumber = ? - AND reservedate = ? - AND borrowernumber = ? - "; + WHERE reserve_id = ?"; $sth = $dbh->prepare($query); - $sth->execute( $biblionumber, $resdate, $borrowernumber ); + $sth->execute( $reserve_id ); $query = "DELETE FROM reserves - WHERE biblionumber = ? - AND reservedate = ? - AND borrowernumber = ? - "; + WHERE reserve_id = ?"; $sth = $dbh->prepare($query); - $sth->execute( $biblionumber, $resdate, $borrowernumber ); - + $sth->execute( $reserve_id ); + # now fix the priority on the others (if the priority wasn't # already sorted!).... unless ( $priority == 0 ) { @@ -1259,15 +1199,14 @@ sub ModReserveAffect { $sth->execute($itemnumber); my ($biblionumber) = $sth->fetchrow; - # get request - need to find out if item is already - # waiting in order to not send duplicate hold filled notifications - my $reserve_id = GetReserveId({ - borrowernumber => $borrowernumber, - biblionumber => $biblionumber, - }); + my $reserve_id = GetReserveNextEligible({ + borrowernumber => $borrowernumber, + biblionumber => $biblionumber, + itemnumber => $itemnumber + }); + return unless defined $reserve_id; my $request = GetReserveInfo($reserve_id); - my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0; # If we affect a reserve that has to be transfered, don't set to Waiting my $query; @@ -1277,8 +1216,7 @@ sub ModReserveAffect { SET priority = 0, itemnumber = ?, found = 'T' - WHERE borrowernumber = ? - AND biblionumber = ? + WHERE reserve_id = ? "; } else { @@ -1289,13 +1227,16 @@ sub ModReserveAffect { found = 'W', waitingdate = NOW(), itemnumber = ? - WHERE borrowernumber = ? - AND biblionumber = ? + WHERE reserve_id = ? "; } $sth = $dbh->prepare($query); - $sth->execute( $itemnumber, $borrowernumber,$biblionumber); - _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo && !$already_on_shelf ); + $sth->execute( $itemnumber, $reserve_id ); + _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber, $reserve_id ) if ( !$transferToDo ); + + if ( $request && $request->{priority} != 0 ) { + _FixPriority({ reserve_id => $reserve_id }); + } if ( C4::Context->preference("ReturnToShelvingCart") ) { CartToShelf( $itemnumber ); @@ -1515,7 +1456,7 @@ sub AlterPriority { =head2 ToggleLowestPriority - ToggleLowestPriority( $borrowernumber, $biblionumber ); + ToggleLowestPriority( $reserve_id ); This function sets the lowestPriority field to true if is false, and false if it is true. @@ -1819,7 +1760,8 @@ sub _Findgroupreserve { reserves.priority AS priority, reserves.timestamp AS timestamp, biblioitems.biblioitemnumber AS biblioitemnumber, - reserves.itemnumber AS itemnumber + reserves.itemnumber AS itemnumber, + reserves.reserve_id AS reserve_id FROM reserves JOIN biblioitems USING (biblionumber) JOIN hold_fill_targets USING (biblionumber, borrowernumber) @@ -1849,21 +1791,18 @@ sub _Findgroupreserve { reserves.reservenotes AS reservenotes, reserves.priority AS priority, reserves.timestamp AS timestamp, - reserveconstraints.biblioitemnumber AS biblioitemnumber, + biblioitems.biblioitemnumber AS biblioitemnumber, reserves.itemnumber AS itemnumber FROM reserves - LEFT JOIN reserveconstraints ON reserves.biblionumber = reserveconstraints.biblionumber + JOIN biblioitems USING (biblionumber) WHERE reserves.biblionumber = ? - AND ( ( reserveconstraints.biblioitemnumber = ? - AND reserves.borrowernumber = reserveconstraints.borrowernumber - AND reserves.reservedate = reserveconstraints.reservedate ) - OR reserves.constrainttype='a' ) + AND (reserves.constrainttype='a') AND (reserves.itemnumber IS NULL OR reserves.itemnumber = ?) AND reserves.reservedate <= DATE_ADD(NOW(),INTERVAL ? DAY) AND suspend = 0 /; $sth = $dbh->prepare($query); - $sth->execute( $biblio, $bibitem, $itemnumber, $lookahead||0); + $sth->execute( $biblio, $itemnumber, $lookahead||0); @results = (); while ( my $data = $sth->fetchrow_hashref ) { push( @results, $data ); @@ -1873,7 +1812,7 @@ sub _Findgroupreserve { =head2 _koha_notify_reserve - _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ); + _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber, $reserve_id ); Sends a notification to the patron that their hold has been filled (through ModReserveAffect, _not_ ModReserveFill) @@ -1881,7 +1820,7 @@ ModReserveAffect, _not_ ModReserveFill) =cut sub _koha_notify_reserve { - my ($itemnumber, $borrowernumber, $biblionumber) = @_; + my ($itemnumber, $borrowernumber, $biblionumber, $reserve_id) = @_; my $dbh = C4::Context->dbh; my $borrower = C4::Members::GetMember(borrowernumber => $borrowernumber); @@ -1903,8 +1842,9 @@ sub _koha_notify_reserve { FROM reserves WHERE borrowernumber = ? AND biblionumber = ? + AND reserve_id = ? "); - $sth->execute( $borrowernumber, $biblionumber ); + $sth->execute( $borrowernumber, $biblionumber, $reserve_id ); my $reserve = $sth->fetchrow_hashref; my $branch_details = GetBranchDetail( $reserve->{'branchcode'} ); @@ -1995,18 +1935,16 @@ sub _ShiftPriorityByDateAndPriority { # Shift the priority up by one; works in conjunction with the next SQL statement $query = "UPDATE reserves SET priority = priority+1 - WHERE biblionumber = ? - AND borrowernumber = ? - AND reservedate = ? + WHERE reserve_id = ? AND found IS NULL"; my $sth_update = $dbh->prepare( $query ); # Select all reserves for the biblio with priority greater than $new_priority, and order greatest to least - $query = "SELECT borrowernumber, reservedate FROM reserves WHERE priority >= ? AND biblionumber = ? ORDER BY priority DESC"; + $query = "SELECT reserve_id FROM reserves WHERE priority >= ? AND biblionumber = ? ORDER BY priority DESC"; $sth = $dbh->prepare( $query ); $sth->execute( $new_priority, $biblio ); while ( my $row = $sth->fetchrow_hashref ) { - $sth_update->execute( $biblio, $row->{borrowernumber}, $row->{reservedate} ); + $sth_update->execute( $row->{reserve_id} ); } return $new_priority; # so the caller knows what priority they wind up receiving @@ -2056,11 +1994,7 @@ sub MoveReserve { RevertWaitingStatus({ itemnumber => $itemnumber }); } elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item - CancelReserve({ - biblionumber => $res->{'biblionumber'}, - itemnumber => $res->{'itemnumber'}, - borrowernumber => $res->{'borrowernumber'} - }); + CancelReserve({ reserve_id => $res->{'reserve_id'} }); } } } @@ -2136,6 +2070,7 @@ sub RevertWaitingStatus { SELECT * FROM reserves WHERE itemnumber = ? AND found IS NOT NULL + ORDER BY priority "; my $sth = $dbh->prepare( $query ); $sth->execute( $itemnumber ); @@ -2171,7 +2106,7 @@ sub RevertWaitingStatus { =head2 GetReserveId - $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber [, itemnumber => $itemnumber ] }); + $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber [, itemnumber => $itemnumber ] } ); Returnes the first reserve id that matches the given criteria @@ -2197,6 +2132,8 @@ sub GetReserveId { $sql .= join( " AND ", @limits ); + $sql .= " ORDER BY priority"; + my $sth = $dbh->prepare( $sql ); $sth->execute( @params ); my $row = $sth->fetchrow_hashref(); @@ -2204,6 +2141,44 @@ sub GetReserveId { return $row->{'reserve_id'}; } +=head2 GetReserveNextEligible + + GetReserveNextEligible({ biblionumber => $bibnum, borrowernumber => $bornum + [, itemnumber => $itemnum ] + }) + + Returns the next eligible reserve id, or undef. + + If reserve is for a certain item, it is eligible only if it matches the itemnumber. + If a matching reserve is already waiting, return undef. + +=cut + +sub GetReserveNextEligible { + my ( $params ) = @_; + + return unless ( $params->{'biblionumber'} && $params->{'borrowernumber'} ); + + my $dbh = C4::Context->dbh(); + + my $sql = "SELECT reserve_id, found, itemnumber + FROM reserves + WHERE biblionumber=? + AND borrowernumber=? + ORDER BY priority"; + + my $sth = $dbh->prepare( $sql ); + $sth->execute( $params->{'biblionumber'}, $params->{'borrowernumber'} ); + + while ( my $row = $sth->fetchrow_hashref() ) { + next if ($row->{'itemnumber'} && ($row->{'itemnumber'} ne $params->{'itemnumber'})); + return if ($row->{'found'} =~ /[WF]/); # Waiting or Found(?) are never eligible + return $row->{'reserve_id'}; + } + + return; +} + =head2 ReserveSlip ReserveSlip($branchcode, $borrowernumber, $biblionumber) diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 661910b..ac504ad 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -18,6 +18,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AllowHoldsOnPatronsPossessions','1',NULL,'Allow holds on records that patron have items of it','YesNo'), ('AllowItemsOnHoldCheckout','0','','Do not generate RESERVE_WAITING and RESERVED warning when checking out items reserved to someone else. This allows self checkouts for those items.','YesNo'), ('AllowMultipleCovers','0','1','Allow multiple cover images to be attached to each bibliographic record.','YesNo'), +('AllowMultipleHolds','0','1','Allow staff to place multiple holds for the same item or biblio for the same patron.','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'), 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 7d44f79..753f530 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 @@ -69,6 +69,12 @@ Circulation: no: "Don't allow" - staff to delete messages added from other libraries. - + - pref: AllowMultipleHolds + choices: + yes: Allow + no: "Don't allow" + - staff to place multiple holds for the same item or biblio for the same patron. + - - Show the - pref: numReturnedItemsToShow class: integer diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 3fe459c..30f6077 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -38,11 +38,6 @@ my $input = CGI->new(); checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); my @bibitems=$input->param('biblioitem'); -# FIXME I think reqbib does not exist anymore, it's used in line 82, to AddReserve of contraint type 'o' -# I bet it's a 2.x feature, reserving a given biblioitem, that is useless in Koha 3.0 -# we can remove this line, the AddReserve of constrainttype 'o', -# and probably remove the reserveconstraint table as well, I never could fill anything in this table. -my @reqbib=$input->param('reqbib'); my $biblionumber=$input->param('biblionumber'); my $borrowernumber=$input->param('borrowernumber'); my $notes=$input->param('notes'); @@ -112,16 +107,7 @@ if ($type eq 'str8' && $borrower){ AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber], $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); } else { - if ($input->param('request') eq 'any'){ - # place a request on 1st available - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); - } elsif ($reqbib[0] ne ''){ - # FIXME : elsif probably never reached, (see top of the script) - # place a request on a given item - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); - } else { - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); - } + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); } } diff --git a/reserve/request.pl b/reserve/request.pl index 1b303c7..a3c6475 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -213,7 +213,7 @@ foreach my $biblionumber (@biblionumbers) { } if ( $holds_count ) { - $alreadyreserved = 1; + $alreadyreserved = 1 if (!C4::Context->preference('AllowMultipleHolds')); $biblioloopiter{warn} = 1; $biblioloopiter{alreadyres} = 1; } -- 1.8.3.2