Bugzilla – Attachment 27121 Details for
Bug 12085
Allow staff to place multiple reserves for the same patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Allow multiple reserves for the same patron
0001-Allow-multiple-reserves-for-the-same-patron.patch (text/plain), 19.56 KB, created by
paxed
on 2014-04-15 09:28:03 UTC
(
hide
)
Description:
Allow multiple reserves for the same patron
Filename:
MIME Type:
Creator:
paxed
Created:
2014-04-15 09:28:03 UTC
Size:
19.56 KB
patch
obsolete
>From 0886604b4d18deac8420681e35b57be3884f4277 Mon Sep 17 00:00:00 2001 >From: Pasi Kallinen <pasi.kallinen@pttk.fi> >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 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=27121">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12085
:
27121
|
34522
|
34523