Bugzilla – Attachment 18465 Details for
Bug 9394
Use reserve_id where possible
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9394 - Use reserve_id where possible
Bug-9394---Use-reserveid-where-possible.patch (text/plain), 56.94 KB, created by
Jonathan Druart
on 2013-05-29 13:03:53 UTC
(
hide
)
Description:
Bug 9394 - Use reserve_id where possible
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2013-05-29 13:03:53 UTC
Size:
56.94 KB
patch
obsolete
>From f7ac70fac678270b322ff3ecdf198fe2ed77b650 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 15 Jan 2013 10:13:52 -0500 >Subject: [PATCH] Bug 9394 - Use reserve_id where possible > >This patch switches from using a combination of >biblionumber/borrowernumber to using reserve_id where possible. > >Test Plan: >1) Apply patch >2) Run t/db_dependent/Holds.t > >Signed-off-by: Maxime Pelletier <maxime.pelletier@libeo.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> >--- > C4/Biblio.pm | 2 +- > C4/ILSDI/Services.pm | 2 +- > C4/Reserves.pm | 511 ++++++++++---------- > C4/SIP/ILS/Transaction/Hold.pm | 23 +- > circ/branchtransfers.pl | 10 +- > circ/circulation.pl | 2 +- > .../prog/en/modules/reserve/request.tt | 21 +- > koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt | 2 +- > opac/opac-modrequest.pl | 9 +- > reserve/modrequest.pl | 28 +- > reserve/request.pl | 69 +-- > serials/routing-preview.pl | 7 +- > t/db_dependent/Holds.t | 172 +++++++ > 13 files changed, 526 insertions(+), 332 deletions(-) > create mode 100755 t/db_dependent/Holds.t > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 7af8619..e94b0eb 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -438,7 +438,7 @@ sub DelBiblio { > require C4::Reserves; > my ($count, $reserves) = C4::Reserves::GetReservesFromBiblionumber($biblionumber); > foreach my $res ( @$reserves ) { >- C4::Reserves::CancelReserve( $res->{'biblionumber'}, $res->{'itemnumber'}, $res->{'borrowernumber'} ); >+ C4::Reserves::CancelReserve({ reserve_id => $res->{'reserve_id'} }); > } > > # Delete in Zebra. Be careful NOT to move this line after _koha_delete_biblio >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index 68cbdb3..75340a4 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -762,7 +762,7 @@ sub CancelHold { > return { code => 'NotCanceled' } unless any { $itemnumber eq $_ } @reserveditems; > > # Cancel the reserve >- CancelReserve( $itemnumber, undef, $borrowernumber ); >+ CancelReserve({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); > > return { code => 'Canceled' }; > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 5ff12e4..813b1cc 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -93,7 +93,8 @@ BEGIN { > @ISA = qw(Exporter); > @EXPORT = qw( > &AddReserve >- >+ >+ &GetReserve > &GetReservesFromItemnumber > &GetReservesFromBiblionumber > &GetReservesFromBorrowernumber >@@ -244,9 +245,27 @@ sub AddReserve { > return; # FIXME: why not have a useful return value? > } > >+=head2 GetReserve >+ >+ $res = GetReserve( $reserve_id ); >+ >+=cut >+ >+sub GetReserve { >+ my ($reserve_id) = @_; >+ #warn "C4::Reserves::GetReserve( $reserve_id )"; >+ >+ my $dbh = C4::Context->dbh; >+ my $query = "SELECT * FROM reserves WHERE reserve_id = ?"; >+ my $sth = $dbh->prepare( $query ); >+ $sth->execute( $reserve_id ); >+ my $res = $sth->fetchrow_hashref(); >+ return $res; >+} >+ > =head2 GetReservesFromBiblionumber > >- ($count, $title_reserves) = &GetReserves($biblionumber); >+ ($count, $title_reserves) = GetReservesFromBiblionumber($biblionumber); > > This function gets the list of reservations for one C<$biblionumber>, returning a count > of the reserves and an arrayref pointing to the reserves for C<$biblionumber>. >@@ -260,7 +279,8 @@ sub GetReservesFromBiblionumber { > > # Find the desired items in the reserves > my $query = " >- SELECT branchcode, >+ SELECT reserve_id, >+ branchcode, > timestamp AS rtimestamp, > priority, > biblionumber, >@@ -328,7 +348,7 @@ sub GetReservesFromBiblionumber { > > =head2 GetReservesFromItemnumber > >- ( $reservedate, $borrowernumber, $branchcode ) = GetReservesFromItemnumber($itemnumber); >+ ( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = GetReservesFromItemnumber($itemnumber); > > TODO :: Description here > >@@ -338,7 +358,7 @@ sub GetReservesFromItemnumber { > my ( $itemnumber, $all_dates ) = @_; > my $dbh = C4::Context->dbh; > my $query = " >- SELECT reservedate,borrowernumber,branchcode >+ SELECT reservedate,borrowernumber,branchcode,reserve_id > FROM reserves > WHERE itemnumber=? > "; >@@ -347,8 +367,8 @@ sub GetReservesFromItemnumber { > } > my $sth_res = $dbh->prepare($query); > $sth_res->execute($itemnumber); >- my ( $reservedate, $borrowernumber,$branchcode ) = $sth_res->fetchrow_array; >- return ( $reservedate, $borrowernumber, $branchcode ); >+ my ( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = $sth_res->fetchrow_array; >+ return ( $reservedate, $borrowernumber, $branchcode, $reserve_id ); > } > > =head2 GetReservesFromBorrowernumber >@@ -511,11 +531,11 @@ sub GetReserveCount { > > my $dbh = C4::Context->dbh; > >- my $query = ' >+ my $query = " > SELECT COUNT(*) AS counter > FROM reserves >- WHERE borrowernumber = ? >- '; >+ WHERE borrowernumber = ? >+ "; > my $sth = $dbh->prepare($query); > $sth->execute($borrowernumber); > my $row = $sth->fetchrow_hashref; >@@ -542,8 +562,7 @@ sub GetOtherReserves { > #minus priorities of others reservs > ModReserveMinusPriority( > $itemnumber, >- $checkreserves->{'borrowernumber'}, >- $iteminfo->{'biblionumber'} >+ $checkreserves->{'reserve_id'}, > ); > > #launch the subroutine dotransfer >@@ -560,8 +579,7 @@ sub GetOtherReserves { > $messages->{'waiting'} = 1; > ModReserveMinusPriority( > $itemnumber, >- $checkreserves->{'borrowernumber'}, >- $iteminfo->{'biblionumber'} >+ $checkreserves->{'reserve_id'}, > ); > ModReserveStatus($itemnumber,'W'); > } >@@ -687,7 +705,7 @@ sub GetReservesToBranch { > my ( $frombranch ) = @_; > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare( >- "SELECT borrowernumber,reservedate,itemnumber,timestamp >+ "SELECT reserve_id,borrowernumber,reservedate,itemnumber,timestamp > FROM reserves > WHERE priority='0' > AND branchcode=?" >@@ -710,22 +728,24 @@ sub GetReservesToBranch { > > sub GetReservesForBranch { > my ($frombranch) = @_; >- my $dbh = C4::Context->dbh; >- my $query = "SELECT borrowernumber,reservedate,itemnumber,waitingdate >+ my $dbh = C4::Context->dbh; >+ >+ my $query = " >+ SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate > FROM reserves > WHERE priority='0' >- AND found='W' "; >- if ($frombranch){ >- $query .= " AND branchcode=? "; >- } >+ AND found='W' >+ "; >+ $query .= " AND branchcode=? " if ( $frombranch ); > $query .= "ORDER BY waitingdate" ; >+ > my $sth = $dbh->prepare($query); > if ($frombranch){ >- $sth->execute($frombranch); >- } >- else { >- $sth->execute(); >- } >+ $sth->execute($frombranch); >+ } else { >+ $sth->execute(); >+ } >+ > my @transreserv; > my $i = 0; > while ( my $data = $sth->fetchrow_hashref ) { >@@ -744,6 +764,10 @@ If several reserves exist, the reserve with the lower priority is given. > > =cut > >+## FIXME: I don't think this does what it thinks it does. >+## It only ever checks the first reserve result, even though >+## multiple reserves for that bib can have the itemnumber set >+## the sub is only used once in the codebase. > sub GetReserveStatus { > my ($itemnumber, $biblionumber) = @_; > >@@ -904,7 +928,7 @@ sub CancelExpiredReserves { > $sth->execute(); > > while ( my $res = $sth->fetchrow_hashref() ) { >- CancelReserve( $res->{'biblionumber'}, '', $res->{'borrowernumber'} ); >+ CancelReserve({ reserve_id => $res->{'reserve_id'} }); > } > > # Cancel reserves that have been waiting too long >@@ -921,7 +945,7 @@ sub CancelExpiredReserves { > manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge); > } > >- CancelReserve( $res->{'biblionumber'}, '', $res->{'borrowernumber'} ); >+ CancelReserve({ reserve_id => $res->{'reserve_id'} }); > } > } > >@@ -947,109 +971,60 @@ sub AutoUnsuspendReserves { > > =head2 CancelReserve > >- &CancelReserve($biblionumber, $itemnumber, $borrowernumber); >+ CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber ] }); > > Cancels a reserve. > >-Use either C<$biblionumber> or C<$itemnumber> to specify the item to >-cancel, but not both: if both are given, C<&CancelReserve> uses >-C<$itemnumber>. >+=cut > >-C<$borrowernumber> is the borrower number of the patron on whose >-behalf the book was reserved. >+sub CancelReserve { >+ my ( $params ) = @_; > >-If C<$biblionumber> was given, C<&CancelReserve> also adjusts the >-priorities of the other people who are waiting on the book. >+ my $reserve_id = $params->{'reserve_id'}; >+ $reserve_id = GetReserveId( $params ) unless ( $reserve_id ); > >-=cut >+ return unless ( $reserve_id ); > >-sub CancelReserve { >- my ( $biblio, $item, $borr ) = @_; > my $dbh = C4::Context->dbh; >- if ( $item and $borr ) { >- # removing a waiting reserve record.... >- # update the database... >- my $query = " >- UPDATE reserves >- SET cancellationdate = now(), >- found = Null, >- priority = 0 >- WHERE itemnumber = ? >- AND borrowernumber = ? >- "; >- my $sth = $dbh->prepare($query); >- $sth->execute( $item, $borr ); >- $sth->finish; >- $query = " >- INSERT INTO old_reserves >- SELECT * FROM reserves >- WHERE itemnumber = ? >- AND borrowernumber = ? >- "; >- $sth = $dbh->prepare($query); >- $sth->execute( $item, $borr ); >- $query = " >- DELETE FROM reserves >- WHERE itemnumber = ? >- AND borrowernumber = ? >- "; >- $sth = $dbh->prepare($query); >- $sth->execute( $item, $borr ); >- } >- else { >- # removing a reserve record.... >- # get the prioritiy on this record.... >- my $priority; >- my $query = qq/ >- SELECT priority FROM reserves >- WHERE biblionumber = ? >- AND borrowernumber = ? >- AND cancellationdate IS NULL >- AND itemnumber IS NULL >- /; >- my $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borr ); >- ($priority) = $sth->fetchrow_array; >- $sth->finish; >- $query = qq/ >- UPDATE reserves >- SET cancellationdate = now(), >- found = Null, >- priority = 0 >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >- >- # update the database, removing the record... >- $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borr ); >- $sth->finish; > >- $query = qq/ >- INSERT INTO old_reserves >- SELECT * FROM reserves >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >- $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borr ); >+ my $query = " >+ UPDATE reserves >+ SET cancellationdate = now(), >+ found = Null, >+ priority = 0 >+ WHERE reserve_id = ? >+ "; >+ my $sth = $dbh->prepare($query); >+ $sth->execute( $reserve_id ); >+ $sth->finish; > >- $query = qq/ >- DELETE FROM reserves >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >- $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borr ); >+ $query = " >+ INSERT INTO old_reserves >+ SELECT * FROM reserves >+ WHERE reserve_id = ? >+ "; >+ $sth = $dbh->prepare($query); >+ $sth->execute( $reserve_id ); > >- # now fix the priority on the others.... >- _FixPriority( $biblio, $borr ); >- } >+ $query = " >+ DELETE FROM reserves >+ WHERE reserve_id = ? >+ "; >+ $sth = $dbh->prepare($query); >+ $sth->execute( $reserve_id ); >+ >+ # now fix the priority on the others.... >+ _FixPriority( $reserve_id ); > } > > =head2 ModReserve > >- ModReserve($rank, $biblio, $borrower, $branch[, $itemnumber]) >+ ModReserve({ rank => $rank, >+ reserve_id => $reserve_id, >+ branchcode => $branchcode >+ [, itemnumber => $itemnumber ] >+ [, biblionumber => $biblionumber, $borrowernumber => $borrowernumber ] >+ }); > > Change a hold request's priority or cancel it. > >@@ -1079,59 +1054,67 @@ itemnumber and supplying itemnumber. > =cut > > sub ModReserve { >- #subroutine to update a reserve >- my ( $rank, $biblio, $borrower, $branch , $itemnumber, $suspend_until) = @_; >- return if $rank eq "W"; >- return if $rank eq "n"; >+ my ( $params ) = @_; >+ >+ my $rank = $params->{'rank'}; >+ my $reserve_id = $params->{'reserve_id'}; >+ my $branchcode = $params->{'branchcode'}; >+ my $itemnumber = $params->{'itemnumber'}; >+ my $suspend_until = $params->{'suspend_until'}; >+ my $borrowernumber = $params->{'borrowernumber'}; >+ my $biblionumber = $params->{'biblionumber'}; >+ >+ return if $rank eq "W"; >+ return if $rank eq "n"; >+ >+ return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) ); >+ $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber }) unless ( $reserve_id ); >+ > my $dbh = C4::Context->dbh; > if ( $rank eq "del" ) { >- my $query = qq/ >+ my $query = " > UPDATE reserves > SET cancellationdate=now() >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >+ WHERE reserve_id = ? >+ "; > my $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borrower ); >+ $sth->execute( $reserve_id ); > $sth->finish; >- $query = qq/ >+ $query = " > INSERT INTO old_reserves > SELECT * > FROM reserves >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >+ WHERE reserve_id = ? >+ "; > $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borrower ); >- $query = qq/ >+ $sth->execute( $reserve_id ); >+ $query = " > DELETE FROM reserves >- WHERE biblionumber = ? >- AND borrowernumber = ? >- /; >+ WHERE reserve_id = ? >+ "; > $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borrower ); >+ $sth->execute( $reserve_id ); > > } > elsif ($rank =~ /^\d+/ and $rank > 0) { > my $query = " > UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL >- WHERE biblionumber = ? >- AND borrowernumber = ? >+ WHERE reserve_id = ? > "; > my $sth = $dbh->prepare($query); >- $sth->execute( $rank, $branch,$itemnumber, $biblio, $borrower); >+ $sth->execute( $rank, $branchcode, $itemnumber, $reserve_id ); > $sth->finish; > > if ( defined( $suspend_until ) ) { > if ( $suspend_until ) { > $suspend_until = C4::Dates->new( $suspend_until )->output("iso"); >- $dbh->do("UPDATE reserves SET suspend = 1, suspend_until = ? WHERE biblionumber = ? AND borrowernumber = ?", undef, ( $suspend_until, $biblio, $borrower ) ); >+ $dbh->do("UPDATE reserves SET suspend = 1, suspend_until = ? WHERE reserve_id = ?", undef, ( $suspend_until, $reserve_id ) ); > } else { >- $dbh->do("UPDATE reserves SET suspend_until = NULL WHERE biblionumber = ? AND borrowernumber = ?", undef, ( $biblio, $borrower ) ); >+ $dbh->do("UPDATE reserves SET suspend_until = NULL WHERE reserve_id = ?", undef, ( $reserve_id ) ); > } > } > >- _FixPriority( $biblio, $borrower, $rank); >+ _FixPriority( $reserve_id, $rank ); > } > } > >@@ -1151,6 +1134,7 @@ sub ModReserveFill { > my ($res) = @_; > my $dbh = C4::Context->dbh; > # fill in a reserve record.... >+ my $reserve_id = $res->{'reserve_id'}; > my $biblionumber = $res->{'biblionumber'}; > my $borrowernumber = $res->{'borrowernumber'}; > my $resdate = $res->{'reservedate'}; >@@ -1199,7 +1183,7 @@ sub ModReserveFill { > # now fix the priority on the others (if the priority wasn't > # already sorted!).... > unless ( $priority == 0 ) { >- _FixPriority( $biblionumber, $borrowernumber ); >+ _FixPriority( $reserve_id ); > } > } > >@@ -1309,7 +1293,7 @@ sub ModReserveCancelAll { > my ( $itemnumber, $borrowernumber ) = @_; > > #step 1 : cancel the reservation >- my $CancelReserve = CancelReserve( undef, $itemnumber, $borrowernumber ); >+ my $CancelReserve = CancelReserve({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); > > #step 2 launch the subroutine of the others reserves > ( $messages, $nextreservinfo ) = GetOtherReserves($itemnumber); >@@ -1321,30 +1305,29 @@ sub ModReserveCancelAll { > > &ModReserveMinusPriority($itemnumber,$borrowernumber,$biblionumber) > >-Reduce the values of queuded list >+Reduce the values of queued list > > =cut > > sub ModReserveMinusPriority { >- my ( $itemnumber, $borrowernumber, $biblionumber ) = @_; >+ my ( $itemnumber, $reserve_id ) = @_; > > #first step update the value of the first person on reserv > my $dbh = C4::Context->dbh; > my $query = " > UPDATE reserves > SET priority = 0 , itemnumber = ? >- WHERE borrowernumber=? >- AND biblionumber=? >+ WHERE reserve_id = ? > "; > my $sth_upd = $dbh->prepare($query); >- $sth_upd->execute( $itemnumber, $borrowernumber, $biblionumber ); >+ $sth_upd->execute( $itemnumber, $reserve_id ); > # second step update all others reservs >- _FixPriority($biblionumber, $borrowernumber, '0'); >+ _FixPriority( $reserve_id, '0'); > } > > =head2 GetReserveInfo > >- &GetReserveInfo($borrowernumber,$biblionumber); >+ &GetReserveInfo($reserve_id); > > Get item and borrower details for a current hold. > Current implementation this query should have a single result. >@@ -1352,49 +1335,47 @@ Current implementation this query should have a single result. > =cut > > sub GetReserveInfo { >- my ( $borrowernumber, $biblionumber ) = @_; >+ my ( $reserve_id ) = @_; > my $dbh = C4::Context->dbh; >- my $strsth="SELECT >- reservedate, >- reservenotes, >- reserves.borrowernumber, >- reserves.biblionumber, >- reserves.branchcode, >- reserves.waitingdate, >- notificationdate, >- reminderdate, >- priority, >- found, >- firstname, >- surname, >- phone, >- email, >- address, >- address2, >- cardnumber, >- city, >- zipcode, >- biblio.title, >- biblio.author, >- items.holdingbranch, >- items.itemcallnumber, >- items.itemnumber, >- items.location, >- barcode, >- notes >- FROM reserves >- LEFT JOIN items USING(itemnumber) >- LEFT JOIN borrowers USING(borrowernumber) >- LEFT JOIN biblio ON (reserves.biblionumber=biblio.biblionumber) >- WHERE >- reserves.borrowernumber=? >- AND reserves.biblionumber=?"; >- my $sth = $dbh->prepare($strsth); >- $sth->execute($borrowernumber,$biblionumber); >- >- my $data = $sth->fetchrow_hashref; >- return $data; >+ my $strsth="SELECT >+ reserve_id, >+ reservedate, >+ reservenotes, >+ reserves.borrowernumber, >+ reserves.biblionumber, >+ reserves.branchcode, >+ reserves.waitingdate, >+ notificationdate, >+ reminderdate, >+ priority, >+ found, >+ firstname, >+ surname, >+ phone, >+ email, >+ address, >+ address2, >+ cardnumber, >+ city, >+ zipcode, >+ biblio.title, >+ biblio.author, >+ items.holdingbranch, >+ items.itemcallnumber, >+ items.itemnumber, >+ items.location, >+ barcode, >+ notes >+ FROM reserves >+ LEFT JOIN items USING(itemnumber) >+ LEFT JOIN borrowers USING(borrowernumber) >+ LEFT JOIN biblio ON (reserves.biblionumber=biblio.biblionumber) >+ WHERE reserves.reserve_id = ?"; >+ my $sth = $dbh->prepare($strsth); >+ $sth->execute($reserve_id); > >+ my $data = $sth->fetchrow_hashref; >+ return $data; > } > > =head2 IsAvailableForItemLevelRequest >@@ -1471,7 +1452,7 @@ sub IsAvailableForItemLevelRequest { > > =head2 AlterPriority > >- AlterPriority( $where, $borrowernumber, $biblionumber, $reservedate ); >+ AlterPriority( $where, $reserve_id ); > > This function changes a reserve's priority up, down, to the top, or to the bottom. > Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was placed >@@ -1479,29 +1460,25 @@ Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was > =cut > > sub AlterPriority { >- my ( $where, $borrowernumber, $biblionumber ) = @_; >+ my ( $where, $reserve_id ) = @_; > > my $dbh = C4::Context->dbh; > >- ## Find this reserve >- my $sth = $dbh->prepare('SELECT * FROM reserves WHERE biblionumber = ? AND borrowernumber = ? AND cancellationdate IS NULL'); >- $sth->execute( $biblionumber, $borrowernumber ); >- my $reserve = $sth->fetchrow_hashref(); >- $sth->finish(); >+ my $reserve = GetReserve( $reserve_id ); > > if ( $where eq 'up' || $where eq 'down' ) { > > my $priority = $reserve->{'priority'}; > $priority = $where eq 'up' ? $priority - 1 : $priority + 1; >- _FixPriority( $biblionumber, $borrowernumber, $priority ) >+ _FixPriority( $reserve_id, $priority ) > > } elsif ( $where eq 'top' ) { > >- _FixPriority( $biblionumber, $borrowernumber, '1' ) >+ _FixPriority( $reserve_id, '1' ) > > } elsif ( $where eq 'bottom' ) { > >- _FixPriority( $biblionumber, $borrowernumber, '999999' ) >+ _FixPriority( $reserve_id, '999999' ) > > } > } >@@ -1515,27 +1492,20 @@ This function sets the lowestPriority field to true if is false, and false if it > =cut > > sub ToggleLowestPriority { >- my ( $borrowernumber, $biblionumber ) = @_; >+ my ( $reserve_id ) = @_; > > my $dbh = C4::Context->dbh; > >- my $sth = $dbh->prepare( >- "UPDATE reserves SET lowestPriority = NOT lowestPriority >- WHERE biblionumber = ? >- AND borrowernumber = ?" >- ); >- $sth->execute( >- $biblionumber, >- $borrowernumber, >- ); >+ my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); >+ $sth->execute( $reserve_id ); > $sth->finish; > >- _FixPriority( $biblionumber, $borrowernumber, '999999' ); >+ _FixPriority( $reserve_id, '999999' ); > } > > =head2 ToggleSuspend > >- ToggleSuspend( $borrowernumber, $biblionumber ); >+ ToggleSuspend( $reserve_id ); > > This function sets the suspend field to true if is false, and false if it is true. > If the reserve is currently suspended with a suspend_until date, that date will >@@ -1544,7 +1514,7 @@ be cleared when it is unsuspended. > =cut > > sub ToggleSuspend { >- my ( $borrowernumber, $biblionumber, $suspend_until ) = @_; >+ my ( $reserve_id, $suspend_until ) = @_; > > $suspend_until = output_pref( dt_from_string( $suspend_until ), 'iso' ) if ( $suspend_until ); > >@@ -1555,14 +1525,12 @@ sub ToggleSuspend { > my $sth = $dbh->prepare( > "UPDATE reserves SET suspend = NOT suspend, > suspend_until = CASE WHEN suspend = 0 THEN NULL ELSE $do_until END >- WHERE biblionumber = ? >- AND borrowernumber = ? >+ WHERE reserve_id = ? > "); > > my @params; > push( @params, $suspend_until ) if ( $suspend_until ); >- push( @params, $biblionumber ); >- push( @params, $borrowernumber ); >+ push( @params, $reserve_id ); > > $sth->execute( @params ); > $sth->finish; >@@ -1627,7 +1595,7 @@ sub SuspendAll { > > =head2 _FixPriority > >- &_FixPriority($biblio,$borrowernumber,$rank,$ignoreSetLowestRank); >+ &_FixPriority( $reserve_id, $rank, $ignoreSetLowestRank); > > Only used internally (so don't export it) > Changed how this functions works # >@@ -1639,41 +1607,39 @@ in new priority rank > =cut > > sub _FixPriority { >- my ( $biblio, $borrowernumber, $rank, $ignoreSetLowestRank ) = @_; >+ my ( $reserve_id, $rank, $ignoreSetLowestRank ) = @_; > my $dbh = C4::Context->dbh; >- if ( $rank eq "del" ) { >- CancelReserve( $biblio, undef, $borrowernumber ); >- } >- if ( $rank eq "W" || $rank eq "0" ) { >+ >+ my $res = GetReserve( $reserve_id ); >+ >+ if ( $rank eq "del" ) { >+ CancelReserve({ reserve_id => $reserve_id }); >+ } >+ elsif ( $rank eq "W" || $rank eq "0" ) { > > # make sure priority for waiting or in-transit items is 0 >- my $query = qq/ >+ my $query = " > UPDATE reserves > SET priority = 0 >- WHERE biblionumber = ? >- AND borrowernumber = ? >- AND found IN ('W', 'T') >- /; >+ WHERE reserve_id = ? >+ AND found IN ('W', 'T') >+ "; > my $sth = $dbh->prepare($query); >- $sth->execute( $biblio, $borrowernumber ); >+ $sth->execute( $reserve_id ); > } > my @priority; > my @reservedates; > > # get whats left >-# FIXME adding a new security in returned elements for changing priority, >-# now, we don't care anymore any reservations with itemnumber linked (suppose a waiting reserve) >- # This is wrong a waiting reserve has W set >- # The assumption that having an itemnumber set means waiting is wrong and should be corrected any place it occurs >- my $query = qq/ >- SELECT borrowernumber, reservedate, constrainttype >+ my $query = " >+ SELECT reserve_id, borrowernumber, reservedate, constrainttype > FROM reserves > WHERE biblionumber = ? >- AND ((found <> 'W' AND found <> 'T') or found is NULL) >+ AND ((found <> 'W' AND found <> 'T') OR found IS NULL) > ORDER BY priority ASC >- /; >+ "; > my $sth = $dbh->prepare($query); >- $sth->execute($biblio); >+ $sth->execute( $res->{'biblionumber'} ); > while ( my $line = $sth->fetchrow_hashref ) { > push( @reservedates, $line ); > push( @priority, $line ); >@@ -1683,7 +1649,7 @@ sub _FixPriority { > my $i; > my $key = -1; # to allow for 0 to be a valid result > for ( $i = 0 ; $i < @priority ; $i++ ) { >- if ( $borrowernumber == $priority[$i]->{'borrowernumber'} ) { >+ if ( $reserve_id == $priority[$i]->{'reserve_id'} ) { > $key = $i; # save the index > last; > } >@@ -1699,29 +1665,25 @@ sub _FixPriority { > > # now fix the priority on those that are left.... > $query = " >- UPDATE reserves >- SET priority = ? >- WHERE biblionumber = ? >- AND borrowernumber = ? >- AND reservedate = ? >- AND found IS NULL >+ UPDATE reserves >+ SET priority = ? >+ WHERE reserve_id = ? > "; > $sth = $dbh->prepare($query); > for ( my $j = 0 ; $j < @priority ; $j++ ) { > $sth->execute( >- $j + 1, $biblio, >- $priority[$j]->{'borrowernumber'}, >- $priority[$j]->{'reservedate'} >+ $j + 1, >+ $priority[$j]->{'reserve_id'} > ); > $sth->finish; > } > >- $sth = $dbh->prepare( "SELECT borrowernumber FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); >+ $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( $biblio, $res->{'borrowernumber'}, '999999', 1 ); >+ _FixPriority( $res->{'reserve_id'}, '999999', 1 ); > } > } > } >@@ -1760,7 +1722,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, itemnumber) >@@ -2028,8 +1991,11 @@ sub MoveReserve { > RevertWaitingStatus({ itemnumber => $itemnumber }); > } > elsif ( $cancelreserve eq 'cancel' || $cancelreserve ) { # cancel reserves on this item >- CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'}); >- CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'}); >+ CancelReserve({ >+ biblionumber => $res->{'biblionumber'}, >+ itemnumber => $res->{'itemnumber'}, >+ borrowernumber => $res->{'borrowernumber'} >+ }); > } > } > } >@@ -2045,7 +2011,7 @@ This shifts the holds from C<$from_biblio> to C<$to_biblio> and reorders them by > sub MergeHolds { > my ( $dbh, $to_biblio, $from_biblio ) = @_; > my $sth = $dbh->prepare( >- "SELECT count(*) as reservenumber FROM reserves WHERE biblionumber = ?" >+ "SELECT count(*) as reserve_id FROM reserves WHERE biblionumber = ?" > ); > $sth->execute($from_biblio); > if ( my $data = $sth->fetchrow_hashref() ) { >@@ -2138,6 +2104,41 @@ sub RevertWaitingStatus { > return $sth->execute( $reserve->{'reserve_id'} ); > } > >+=head2 GetReserveId >+ >+ $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber [, itemnumber => $itemnumber ] }); >+ >+ Returnes the first reserve id that matches the given criteria >+ >+=cut >+ >+sub GetReserveId { >+ my ( $params ) = @_; >+ >+ return unless ( ( $params->{'biblionumber'} || $params->{'itemnumber'} ) && $params->{'borrowernumber'} ); >+ >+ my $dbh = C4::Context->dbh(); >+ >+ my $sql = "SELECT reserve_id FROM reserves WHERE "; >+ >+ my @params; >+ my @limits; >+ foreach my $key ( keys %$params ) { >+ if ( defined( $params->{$key} ) ) { >+ push( @limits, "$key = ?" ); >+ push( @params, $params->{$key} ); >+ } >+ } >+ >+ $sql .= join( " AND ", @limits ); >+ >+ my $sth = $dbh->prepare( $sql ); >+ $sth->execute( @params ); >+ my $row = $sth->fetchrow_hashref(); >+ >+ return $row->{'reserve_id'}; >+} >+ > =head2 ReserveSlip > > ReserveSlip($branchcode, $borrowernumber, $biblionumber) >diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm >index a2ca13e..0ff842fb 100644 >--- a/C4/SIP/ILS/Transaction/Hold.pm >+++ b/C4/SIP/ILS/Transaction/Hold.pm >@@ -85,10 +85,13 @@ sub drop_hold { > return $self; > } > my $bib = GetBiblioFromItemNumber(undef, $self->{item}->id); >- # FIXME: figure out if it is a item or title hold. Till then, cancel both. >- CancelReserve($bib->{biblionumber}, undef, $borrower->{borrowernumber}); >- CancelReserve(undef, $self->{item}->id, $borrower->{borrowernumber}); >- # unfortunately no meaningful return value here either >+ >+ CancelReserve({ >+ biblionumber => $bib->{biblionumber}, >+ itemnumber => $self->{item}->id, >+ borrowernumber => $borrower->{borrowernumber} >+ }); >+ > $self->ok(1); > return $self; > } >@@ -119,17 +122,11 @@ sub change_hold { > return $self; > } > my $bibno = $bib->{biblionumber}; >- ModReserve($bibno, $borrower->{borrowernumber}, $branch, GetBiblioItemByBiblioNumber($bibno)); >- # unfortunately no meaningful return value >- # ModReserve needs to be fixed to maintain priority by iteself (Feb 2008) >+ ModReserve({ biblionumber => $bibno, borrowernumber => $borrower->{borrowernumber}, branchcode => $branch }); >+ > $self->ok(1); > return $self; > } >+ > 1; > __END__ >- >-# 11 friggin arguments >-AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$startdate,$notes,$title,$checkitem,$found) >- >-ModReserve($rank, $biblio, $borrower, $branch , $itemnumber) >- >diff --git a/circ/branchtransfers.pl b/circ/branchtransfers.pl >index ad7aa9a..3be7bb4 100755 >--- a/circ/branchtransfers.pl >+++ b/circ/branchtransfers.pl >@@ -80,7 +80,10 @@ my $ignoreRs = 0; > # Deal with the requests.... > if ( $request eq "KillWaiting" ) { > my $item = $query->param('itemnumber'); >- CancelReserve( 0, $item, $borrowernumber ); >+ CancelReserve({ >+ itemnumber => $item, >+ borrowernumber => $borrowernumber >+ }); > $cancelled = 1; > $reqmessage = 1; > } >@@ -93,7 +96,10 @@ elsif ( $request eq "SetWaiting" ) { > } > elsif ( $request eq 'KillReserved' ) { > my $biblio = $query->param('biblionumber'); >- CancelReserve( $biblio, 0, $borrowernumber ); >+ CancelReserve({ >+ biblionumber => $biblio, >+ borrowernumber => $borrowernumber >+ }); > $cancelled = 1; > $reqmessage = 1; > } >diff --git a/circ/circulation.pl b/circ/circulation.pl >index ef0edf7..6eff7a2 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -359,7 +359,7 @@ if ($borrowernumber) { > $getreserv{nottransfered} = 0; > > $getreserv{reservedate} = format_date( $num_res->{'reservedate'} ); >- $getreserv{reservenumber} = $num_res->{'reservenumber'}; >+ $getreserv{reserve_id} = $num_res->{'reserve_id'}; > $getreserv{title} = $getiteminfo->{'title'}; > $getreserv{itemtype} = $itemtypeinfo->{'description'}; > $getreserv{author} = $getiteminfo->{'author'}; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >index 122b2d0..f42bd29 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -564,6 +564,7 @@ function checkMultiHold() { > [% FOREACH reserveloo IN biblioloo.reserveloop %] > [% UNLESS ( loop.odd ) %]<tr class="highlight">[% ELSE %]<tr>[% END %] > <td> >+ <input type="hidden" name="reserve_id" value="[% reserveloo.reserve_id %]" /> > <input type="hidden" name="borrowernumber" value="[% reserveloo.borrowernumber %]" /> > <input type="hidden" name="biblionumber" value="[% reserveloo.biblionumber %]" /> > <select name="rank-request"> >@@ -583,19 +584,19 @@ function checkMultiHold() { > > [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] > <td style="white-space:nowrap;"> >- <a title="Move Hold Up" href="request.pl?action=move&where=up&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold Up" href="request.pl?action=move&where=up&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-up.png" border="0" alt="Go up" /> > </a> > >- <a title="Move hold to top" href="request.pl?action=move&where=top&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold To Top" href="request.pl?action=move&where=top&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-top.png" border="0" alt="Go top" /> > </a> > >- <a title="Move hold to bottom" href="request.pl?action=move&where=bottom&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold To Bottom" href="request.pl?action=move&where=bottom&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-bottom.png" border="0" alt="Go bottom" /> > </a> > >- <a title="Move hold down" href="request.pl?action=move&where=down&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold Down" href="request.pl?action=move&where=down&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-down.png" border="0" alt="Go down" /> > </a> > </td> >@@ -675,7 +676,7 @@ function checkMultiHold() { > > [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] > <td> >- <a title="Toggle lowest priority" href="request.pl?action=setLowestPriority&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Toggle Lowest Priority" href="request.pl?action=setLowestPriority&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > [% IF ( reserveloo.lowestPriority ) %] > <img src="/intranet-tmpl/[% theme %]/img/go-bottom.png" border="0" alt="Unset Lowest Priority" /> > [% ELSE %] >@@ -686,7 +687,7 @@ function checkMultiHold() { > [% END %] > > <td> >- <a title="Cancel hold" href="request.pl?action=cancel&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Cancel Hold" href="request.pl?action=cancel&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/x.png" border="0" alt="Cancel" /> > </a> > </td> >@@ -694,12 +695,12 @@ function checkMultiHold() { > [% IF SuspendHoldsIntranet %] > <td> > [% UNLESS ( reserveloo.wait ) %] >- <input type="button" value="[% IF ( reserveloo.suspend ) %]Unsuspend[% ELSE %]Suspend[% END %]" onclick="window.location.href='request.pl?action=toggleSuspend&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]&suspend_until=' + $('#suspend_until_[% reserveloo.borrowernumber %]').val()" /> >+ <input type="button" value="[% IF ( reserveloo.suspend ) %]Unsuspend[% ELSE %]Suspend[% END %]" onclick="window.location.href='request.pl?action=toggleSuspend&reserve_id=[% reserveloo.reserve_id %]&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]&suspend_until=' + $('#suspend_until_[% reserveloo.reserve_id %]').val()" /> > > [% IF AutoResumeSuspendedHolds %] >- <label for="suspend_until_[% reserveloo.borrowernumber %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label> >- <input name="suspend_until" id="suspend_until_[% reserveloo.borrowernumber %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" /> >- <a href='#' onclick="document.getElementById('suspend_until_[% reserveloo.borrowernumber %]').value='';">Clear Date</a> >+ <label for="suspend_until_[% reserveloo.reserve_id %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label> >+ <input name="suspend_until" id="suspend_until_[% reserveloo.reserve_id %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" /> >+ <a href='#' onclick="document.getElementById('suspend_until_[% reserveloo.reserve_id %]').value='';">Clear Date</a> > [% END %] > [% ELSE %] > <input type="hidden" name="suspend_until" value="" /> >diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt >index f1d3435..0b1b85d 100644 >--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt >+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-user.tt >@@ -420,7 +420,7 @@ $.tablesorter.addParser({ > [% IF ( RESERVE.cancelable ) %] > <form action="/cgi-bin/koha/opac-modrequest.pl" method="post"> > <input type="hidden" name="biblionumber" value="[% RESERVE.biblionumber %]" /> >- <input type="hidden" name="reservenumber" value="[% RESERVE.reservenumber %]" /> >+ <input type="hidden" name="reserve_id" value="[% RESERVE.reserve_id %]" /> > <input type="submit" name="submit" class="icon delete cancel" value="Cancel" onclick="return confirmDelete(MSG_CONFIRM_DELETE_HOLD);" /></form> > [% ELSE %] > [% END %] >diff --git a/opac/opac-modrequest.pl b/opac/opac-modrequest.pl >index 7e2f2e5..7ec11eb 100755 >--- a/opac/opac-modrequest.pl >+++ b/opac/opac-modrequest.pl >@@ -30,6 +30,7 @@ use C4::Output; > use C4::Reserves; > use C4::Auth; > my $query = new CGI; >+ > my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > { > template_name => "opac-account.tmpl", >@@ -41,8 +42,10 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > } > ); > >-my $biblionumber = $query->param('biblionumber'); >-if ($biblionumber and $borrowernumber) { >- CancelReserve($biblionumber, '', $borrowernumber); >+my $reserve_id = $query->param('reserve_id'); >+ >+if ($reserve_id && $borrowernumber) { >+ CancelReserve({ reserve_id => $reserve_id }); > } >+ > print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-holds"); >diff --git a/reserve/modrequest.pl b/reserve/modrequest.pl >index 03ef064..2f0b8d3 100755 >--- a/reserve/modrequest.pl >+++ b/reserve/modrequest.pl >@@ -41,23 +41,24 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > } > ); > >-my @rank=$query->param('rank-request'); >-my @biblionumber=$query->param('biblionumber'); >-my @borrower=$query->param('borrowernumber'); >-my @branch=$query->param('pickup'); >-my @itemnumber=$query->param('itemnumber'); >+my @reserve_id = $query->param('reserve_id'); >+my @rank = $query->param('rank-request'); >+my @biblionumber = $query->param('biblionumber'); >+my @borrower = $query->param('borrowernumber'); >+my @branch = $query->param('pickup'); >+my @itemnumber = $query->param('itemnumber'); > my @suspend_until=$query->param('suspend_until'); > my $multi_hold = $query->param('multi_hold'); > my $biblionumbers = $query->param('biblionumbers'); > my $count=@rank; > >-my $CancelBiblioNumber=$query->param('CancelBiblioNumber'); >-my $CancelBorrowerNumber=$query->param('CancelBorrowerNumber'); >-my $CancelItemnumber=$query->param('CancelItemnumber'); >+my $CancelBiblioNumber = $query->param('CancelBiblioNumber'); >+my $CancelBorrowerNumber = $query->param('CancelBorrowerNumber'); >+my $CancelItemnumber = $query->param('CancelItemnumber'); > > # 2 possibilitys : cancel an item reservation, or modify or cancel the queded list > >-# 1) cancel an item reservation by fonction ModReserveCancelAll (in reserves.pm) >+# 1) cancel an item reservation by function ModReserveCancelAll (in reserves.pm) > if ($CancelBorrowerNumber) { > ModReserveCancelAll($CancelItemnumber, $CancelBorrowerNumber); > $biblionumber[0] = $CancelBiblioNumber, >@@ -67,9 +68,16 @@ if ($CancelBorrowerNumber) { > else { > for (my $i=0;$i<$count;$i++){ > undef $itemnumber[$i] unless $itemnumber[$i] ne ''; >- ModReserve($rank[$i],$biblionumber[$i],$borrower[$i],$branch[$i],$itemnumber[$i],$suspend_until[$i]); #from C4::Reserves >+ ModReserve({ >+ rank => $rank[$i], >+ reserve_id => $reserve_id[$i], >+ branchcode => $branch[$i], >+ itemnumber => $itemnumber[$i], >+ suspend_until => $suspend_until[$i] >+ }); > } > } >+ > my $from=$query->param('from'); > $from ||= q{}; > if ( $from eq 'borrower'){ >diff --git a/reserve/request.pl b/reserve/request.pl >index a0cc055..703131d 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -87,22 +87,18 @@ $action ||= q{}; > > if ( $action eq 'move' ) { > my $where = $input->param('where'); >- my $borrowernumber = $input->param('borrowernumber'); >- my $biblionumber = $input->param('biblionumber'); >- AlterPriority( $where, $borrowernumber, $biblionumber ); >+ my $reserve_id = $input->param('reserve_id'); >+ AlterPriority( $where, $reserve_id ); > } elsif ( $action eq 'cancel' ) { >- my $borrowernumber = $input->param('borrowernumber'); >- my $biblionumber = $input->param('biblionumber'); >- CancelReserve( $biblionumber, '', $borrowernumber ); >+ my $reserve_id = $input->param('reserve_id'); >+ CancelReserve({ reserve_id => $reserve_id }); > } elsif ( $action eq 'setLowestPriority' ) { >- my $borrowernumber = $input->param('borrowernumber'); >- my $biblionumber = $input->param('biblionumber'); >- ToggleLowestPriority( $borrowernumber, $biblionumber ); >+ my $reserve_id = $input->param('reserve_id'); >+ ToggleLowestPriority( $reserve_id ); > } elsif ( $action eq 'toggleSuspend' ) { >- my $borrowernumber = $input->param('borrowernumber'); >- my $biblionumber = $input->param('biblionumber'); >+ my $reserve_id = $input->param('reserve_id'); > my $suspend_until = $input->param('suspend_until'); >- ToggleSuspend( $borrowernumber, $biblionumber, $suspend_until ); >+ ToggleSuspend( $reserve_id, $suspend_until ); > } > > if ($findborrower) { >@@ -136,7 +132,7 @@ if ($borrowernumber_hold && !$action) { > GetReserveCount( $borrowerinfo->{'borrowernumber'} ); > > if ( C4::Context->preference('maxreserves') && ($number_reserves >= C4::Context->preference('maxreserves')) ) { >- $warnings = 1; >+ $warnings = 1; > $maxreserves = 1; > } > >@@ -150,7 +146,7 @@ if ($borrowernumber_hold && !$action) { > > # check if the borrower make the reserv in a different branch > if ( $borrowerinfo->{'branchcode'} ne C4::Context->userenv->{'branch'} ) { >- $messages = 1; >+ $messages = 1; > $diffbranch = 1; > } > >@@ -172,8 +168,8 @@ if ($borrowernumber_hold && !$action) { > maxreserves => $maxreserves, > expiry => $expiry, > diffbranch => $diffbranch, >- messages => $messages, >- warnings => $warnings >+ messages => $messages, >+ warnings => $warnings > ); > } > >@@ -227,12 +223,12 @@ my @biblioloop = (); > foreach my $biblionumber (@biblionumbers) { > > my %biblioloopiter = (); >- my $maxreserves; >+ my $maxreserves; > >- my $dat = GetBiblioData($biblionumber); >+ my $dat = GetBiblioData($biblionumber); > > unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { >- $warnings = 1; >+ $warnings = 1; > $maxreserves = 1; > } > >@@ -245,7 +241,8 @@ foreach my $biblionumber (@biblionumbers) { > # get existing reserves ..... > my ( $count, $reserves ) = GetReservesFromBiblionumber($biblionumber,1); > my $totalcount = $count; >- my $alreadyreserved; >+ my $holds_count = 0; >+ my $alreadyreserved = 0; > > foreach my $res (@$reserves) { > if ( defined $res->{found} && $res->{found} eq 'W' ) { >@@ -253,20 +250,23 @@ foreach my $biblionumber (@biblionumbers) { > } > > if ( defined $borrowerinfo && ($borrowerinfo->{borrowernumber} eq $res->{borrowernumber}) ) { >- $warnings = 1; >+ $holds_count++; >+ } >+ } >+ >+ if ( $holds_count ) { > $alreadyreserved = 1; >+ $warnings = 1; > $biblioloopiter{warn} = 1; > $biblioloopiter{alreadyres} = 1; >- } > } > > $template->param( alreadyreserved => $alreadyreserved, >- messages => $messages, >- warnings => $warnings, >- maxreserves=>$maxreserves, >- alreadypossession => $alreadypossession, >- ); >- >+ messages => $messages, >+ warnings => $warnings, >+ maxreserves => $maxreserves, >+ alreadypossession => $alreadypossession, >+ ); > > # FIXME think @optionloop, is maybe obsolete, or must be switchable by a systeme preference fixed rank or not > # make priorities options >@@ -292,11 +292,11 @@ foreach my $biblionumber (@biblionumbers) { > if (my $items = get_itemnumbers_of($biblionumber)->{$biblionumber}){ > @itemnumbers = @$items; > } >- my @hostitems = get_hostitemnumbers_of($biblionumber); >- if (@hostitems){ >- $template->param('hostitemsflag' => 1); >- push(@itemnumbers, @hostitems); >- } >+ my @hostitems = get_hostitemnumbers_of($biblionumber); >+ if (@hostitems){ >+ $template->param('hostitemsflag' => 1); >+ push(@itemnumbers, @hostitems); >+ } > > if (!@itemnumbers) { > $template->param('noitems' => 1); >@@ -374,7 +374,7 @@ foreach my $biblionumber (@biblionumbers) { > } > > # checking reserve >- my ($reservedate,$reservedfor,$expectedAt) = GetReservesFromItemnumber($itemnumber); >+ my ($reservedate,$reservedfor,$expectedAt,$reserve_id) = GetReservesFromItemnumber($itemnumber); > my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); > > if ( defined $reservedate ) { >@@ -570,6 +570,7 @@ foreach my $biblionumber (@biblionumbers) { > $reserve{'optionloop'} = \@optionloop; > $reserve{'suspend'} = $res->{'suspend'}; > $reserve{'suspend_until'} = $res->{'suspend_until'}; >+ $reserve{'reserve_id'} = $res->{'reserve_id'}; > > if ( C4::Context->preference('IndependentBranches') && $flags->{'superlibrarian'} != 1 ) { > $reserve{'branchloop'} = [ GetBranchDetail($res->{'branchcode'}) ]; >diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl >index 437c852..154d477 100755 >--- a/serials/routing-preview.pl >+++ b/serials/routing-preview.pl >@@ -87,7 +87,12 @@ if($ok){ > my $reserve = $sth->fetchrow_hashref; > > if($routing->{borrowernumber} == $reserve->{borrowernumber}){ >- ModReserve($routing->{ranking},$biblio,$routing->{borrowernumber},$branch); >+ ModReserve({ >+ rank => $routing->{ranking}, >+ biblionumber => $biblio, >+ borrowernumber => $routing->{borrowernumber}, >+ branchcode => $branch >+ }); > } else { > AddReserve($branch,$routing->{borrowernumber},$biblio,$const,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); > } >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >new file mode 100755 >index 0000000..801a55c >--- /dev/null >+++ b/t/db_dependent/Holds.t >@@ -0,0 +1,172 @@ >+#!/usr/bin/perl >+ >+use strict; >+use warnings; >+use C4::Branch; >+ >+use Test::More tests => 18; >+use MARC::Record; >+use C4::Biblio; >+use C4::Items; >+ >+BEGIN { >+ use FindBin; >+ use lib $FindBin::Bin; >+ use_ok('C4::Reserves'); >+} >+ >+my $borrowers_count = 5; >+ >+# Setup Test------------------------ >+# Helper biblio. >+diag("Creating biblio instance for testing."); >+my ($bibnum, $title, $bibitemnum) = create_helper_biblio(); >+ >+# Helper item for that biblio. >+diag("Creating item instance for testing."); >+my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => 'CPL', holdingbranch => 'CPL' } , $bibnum); >+ >+# Get a borrower >+my $dbh = C4::Context->dbh; >+my $query = "SELECT borrowernumber FROM borrowers LIMIT $borrowers_count"; >+my $sth = $dbh->prepare($query); >+$sth->execute; >+my @borrowernumbers; >+while ( my $row = $sth->fetchrow_hashref ) { >+ push( @borrowernumbers, $row->{'borrowernumber'} ); >+} >+ >+my $biblionumber = $bibnum; >+ >+my @branches = GetBranchesLoop(); >+my $branch = $branches[0][0]{value}; >+ >+# Create five item level holds >+foreach my $borrowernumber ( @borrowernumbers ) { >+ AddReserve( >+ $branch, >+ $borrowernumber, >+ $biblionumber, >+ my $constraint = 'a', >+ my $bibitems = q{}, >+ my $priority, >+ my $resdate, >+ my $expdate, >+ my $notes = q{}, >+ $title, >+ my $checkitem = $itemnumber, >+ my $found, >+ ); >+} >+ >+ >+my ($count, $reserves) = GetReservesFromBiblionumber($biblionumber); >+is( $count, $borrowers_count, "Test GetReserves()" ); >+ >+ >+my ( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = GetReservesFromItemnumber($itemnumber); >+ok($reserve_id, "Test GetReservesFromItemnumber()"); >+ >+ >+my ( $reserve ) = GetReservesFromBorrowernumber($borrowernumbers[0]); >+ok( $reserve->{'borrowernumber'} eq $borrowernumbers[0], "Test GetReservesFromBorrowernumber()"); >+ >+ >+ok( GetReserveCount( $borrowernumbers[0] ), "Test GetReserveCount()" ); >+ >+ >+CancelReserve({ 'reserve_id' => $reserve_id }); >+($count, $reserves) = GetReservesFromBiblionumber($biblionumber); >+ok( $count == $borrowers_count - 1, "Test CancelReserve()" ); >+ >+ >+( $reservedate, $borrowernumber, $branchcode, $reserve_id ) = GetReservesFromItemnumber($itemnumber); >+ModReserve({ >+ reserve_id => $reserve_id, >+ rank => '4', >+ branchcode => $branch, >+ itemnumber => $itemnumber, >+ suspend_until => C4::Dates->new("2013-01-01","iso")->output(), >+}); >+$reserve = GetReserve( $reserve_id ); >+ok( $reserve->{'priority'} eq '4', "Test GetReserve(), priority changed correctly" ); >+ok( $reserve->{'suspend'}, "Test GetReserve(), suspend hold" ); >+ok( $reserve->{'suspend_until'} eq '2013-01-01 00:00:00', "Test GetReserve(), suspend until date" ); >+ >+ToggleSuspend( $reserve_id ); >+$reserve = GetReserve( $reserve_id ); >+ok( !$reserve->{'suspend'}, "Test ToggleSuspend(), no date" ); >+ >+ToggleSuspend( $reserve_id, '2012-01-01' ); >+$reserve = GetReserve( $reserve_id ); >+ok( $reserve->{'suspend_until'} eq '2012-01-01 00:00:00', "Test ToggleSuspend(), with date" ); >+ >+AutoUnsuspendReserves(); >+$reserve = GetReserve( $reserve_id ); >+ok( !$reserve->{'suspend'}, "Test AutoUnsuspendReserves()" ); >+ >+# Add a new hold for the borrower whose hold we canceled earlier, this time at the bib level >+AddReserve( >+ $branch, >+ $borrowernumber, >+ $biblionumber, >+ my $constraint = 'a', >+ my $bibitems = q{}, >+ my $priority, >+ my $resdate, >+ my $expdate, >+ my $notes = q{}, >+ $title, >+ my $checkitem, >+ my $found, >+); >+( $reserve ) = GetReservesFromBorrowernumber($borrowernumber); >+ModReserveMinusPriority( $itemnumber, $reserve->{'reserve_id'} ); >+( $reserve ) = GetReservesFromBorrowernumber($borrowernumber); >+ok( $reserve->{'itemnumber'} eq $itemnumber, "Test ModReserveMinusPriority()" ); >+ >+ >+my $reserve2 = GetReserveInfo( $reserve->{'reserve_id'} ); >+ok( $reserve->{'reserve_id'} eq $reserve2->{'reserve_id'}, "Test GetReserveInfo()" ); >+ >+ >+($count, $reserves) = GetReservesFromBiblionumber($biblionumber,1); >+$reserve = $reserves->[1]; >+AlterPriority( 'top', $reserve->{'reserve_id'} ); >+$reserve = GetReserve( $reserve->{'reserve_id'} ); >+ok( $reserve->{'priority'} eq '1', "Test AlterPriority(), move to top" ); >+ >+AlterPriority( 'down', $reserve->{'reserve_id'} ); >+$reserve = GetReserve( $reserve->{'reserve_id'} ); >+ok( $reserve->{'priority'} eq '2', "Test AlterPriority(), move down" ); >+ >+AlterPriority( 'up', $reserve->{'reserve_id'} ); >+$reserve = GetReserve( $reserve->{'reserve_id'} ); >+ok( $reserve->{'priority'} eq '1', "Test AlterPriority(), move up" ); >+ >+AlterPriority( 'bottom', $reserve->{'reserve_id'} ); >+$reserve = GetReserve( $reserve->{'reserve_id'} ); >+ok( $reserve->{'priority'} eq '5', "Test AlterPriority(), move to bottom" ); >+ >+# Delete the reserves >+diag("Deleting holds."); >+$dbh->do("DELETE FROM reserves WHERE biblionumber = ?", undef, ( $biblionumber ) ); >+ >+# Delete item. >+diag("Deleting item testing instance."); >+DelItem($dbh, $bibnum, $itemnumber); >+ >+# Delete helper Biblio. >+diag("Deleting biblio testing instance."); >+DelBiblio($bibnum); >+ >+# Helper method to set up a Biblio. >+sub create_helper_biblio { >+ my $bib = MARC::Record->new(); >+ my $title = 'Silence in the library'; >+ $bib->append_fields( >+ MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), >+ MARC::Field->new('245', ' ', ' ', a => $title), >+ ); >+ return ($bibnum, $title, $bibitemnum) = AddBiblio($bib, ''); >+} >-- >1.7.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18465">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 9394
:
14601
|
14860
|
14863
|
15730
|
15733
|
16676
|
17447
|
18216
|
18318
|
18465
|
18466
|
18492
|
18621
|
19893
|
19894
|
19895
|
19896
|
19897
|
19898
|
19899
|
19902
|
19904
|
19905
|
19906
|
19907
|
19908
|
19909