From 84ac34a65d129820bc478515ecea39fdd3cbe065 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 20 Mar 2012 12:50:39 -0400 Subject: [PATCH] Bug 7710 - multiple holds per title - Part 2 - Switch to using reservenumber Where necessary, switch from using biblionumber/borrowernumber to using reservenumber to reference individual reserves. --- C4/Reserves.pm | 335 ++++++++------------ .../prog/en/modules/reserve/request.tt | 13 +- opac/opac-modrequest.pl | 6 +- reserve/modrequest.pl | 21 +- reserve/request.pl | 46 ++-- 5 files changed, 180 insertions(+), 241 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 276658c..45b7a52 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -90,7 +90,8 @@ BEGIN { @ISA = qw(Exporter); @EXPORT = qw( &AddReserve - + + &GetReserve &GetReservesFromItemnumber &GetReservesFromBiblionumber &GetReservesFromBorrowernumber @@ -237,6 +238,24 @@ sub AddReserve { return; # FIXME: why not have a useful return value? } +=head2 GetReserve + + $res = GetReserve( $reservenumber ); + +=cut + +sub GetReserve { + my ($reservenumber) = @_; + #warn "C4::Reserves::GetReserve( $reservenumber )"; + + my $dbh = C4::Context->dbh; + my $query = "SELECT * FROM reserves WHERE reservenumber = ?"; + my $sth = $dbh->prepare( $query ); + $sth->execute( $reservenumber ); + my $res = $sth->fetchrow_hashref(); + return $res; +} + =head2 GetReservesFromBiblionumber ($count, $title_reserves) = &GetReserves($biblionumber); @@ -253,7 +272,8 @@ sub GetReservesFromBiblionumber { # Find the desired items in the reserves my $query = " - SELECT branchcode, + SELECT reservenumber, + branchcode, timestamp AS rtimestamp, priority, biblionumber, @@ -319,7 +339,7 @@ sub GetReservesFromBiblionumber { =head2 GetReservesFromItemnumber - ( $reservedate, $borrowernumber, $branchcode ) = GetReservesFromItemnumber($itemnumber); + ( $reservedate, $borrowernumber, $branchcode, $reservenumber ) = GetReservesFromItemnumber($itemnumber); TODO :: Description here @@ -329,7 +349,7 @@ sub GetReservesFromItemnumber { my ( $itemnumber, $all_dates ) = @_; my $dbh = C4::Context->dbh; my $query = " - SELECT reservedate,borrowernumber,branchcode + SELECT reservedate,borrowernumber,branchcode,reservenumber FROM reserves WHERE itemnumber=? "; @@ -502,11 +522,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; @@ -678,7 +698,7 @@ sub GetReservesToBranch { my ( $frombranch ) = @_; my $dbh = C4::Context->dbh; my $sth = $dbh->prepare( - "SELECT borrowernumber,reservedate,itemnumber,timestamp + "SELECT reservenumber,borrowernumber,reservedate,itemnumber,timestamp FROM reserves WHERE priority='0' AND branchcode=?" @@ -701,22 +721,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 reservenumber,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 ) { @@ -896,109 +918,49 @@ sub CancelExpiredReserves { =head2 CancelReserve - &CancelReserve($biblionumber, $itemnumber, $borrowernumber); + &CancelReserve( $reservenumber ); 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>. - -C<$borrowernumber> is the borrower number of the patron on whose -behalf the book was reserved. - -If C<$biblionumber> was given, C<&CancelReserve> also adjusts the -priorities of the other people who are waiting on the book. - =cut sub CancelReserve { - my ( $biblio, $item, $borr ) = @_; + my ( $reservenumber ) = @_; 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 reservenumber = ? + "; + my $sth = $dbh->prepare($query); + $sth->execute( $reservenumber ); + $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 reservenumber = ? + "; + $sth = $dbh->prepare($query); + $sth->execute( $reservenumber ); - # now fix the priority on the others.... - _FixPriority( $biblio, $borr ); - } + $query = " + DELETE FROM reserves + WHERE reservenumber = ? + "; + $sth = $dbh->prepare($query); + $sth->execute( $reservenumber ); + + # now fix the priority on the others.... + _FixPriority( $reservenumber ); } =head2 ModReserve - ModReserve($rank, $biblio, $borrower, $branch[, $itemnumber]) + ModReserve($rank, $reservenumber, $branch[, $itemnumber]) Change a hold request's priority or cancel it. @@ -1029,48 +991,44 @@ itemnumber and supplying itemnumber. sub ModReserve { #subroutine to update a reserve - my ( $rank, $biblio, $borrower, $branch , $itemnumber) = @_; + my ( $rank, $reservenumber, $branch , $itemnumber) = @_; return if $rank eq "W"; return if $rank eq "n"; my $dbh = C4::Context->dbh; if ( $rank eq "del" ) { - my $query = qq/ + my $query = " UPDATE reserves SET cancellationdate=now() - WHERE biblionumber = ? - AND borrowernumber = ? - /; + WHERE reservenumber = ? + "; my $sth = $dbh->prepare($query); - $sth->execute( $biblio, $borrower ); + $sth->execute( $reservenumber ); $sth->finish; - $query = qq/ + $query = " INSERT INTO old_reserves SELECT * FROM reserves - WHERE biblionumber = ? - AND borrowernumber = ? - /; + WHERE reservenumber = ? + "; $sth = $dbh->prepare($query); - $sth->execute( $biblio, $borrower ); - $query = qq/ + $sth->execute( $reservenumber ); + $query = " DELETE FROM reserves - WHERE biblionumber = ? - AND borrowernumber = ? - /; + WHERE reservenumber = ? + "; $sth = $dbh->prepare($query); - $sth->execute( $biblio, $borrower ); + $sth->execute( $reservenumber ); } elsif ($rank =~ /^\d+/ and $rank > 0) { - my $query = qq/ - UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL - WHERE biblionumber = ? - AND borrowernumber = ? - /; + my $query = " + UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = ?, found = NULL, waitingdate = NULL + WHERE reservenumber = ? + "; my $sth = $dbh->prepare($query); - $sth->execute( $rank, $branch,$itemnumber, $biblio, $borrower); + $sth->execute( $rank, $branch, $itemnumber, $reservenumber ); $sth->finish; - _FixPriority( $biblio, $borrower, $rank); + _FixPriority( $reservenumber, $rank ); } } @@ -1090,6 +1048,7 @@ sub ModReserveFill { my ($res) = @_; my $dbh = C4::Context->dbh; # fill in a reserve record.... + my $reservenumber = $res->{'reservenumber'}; my $biblionumber = $res->{'biblionumber'}; my $borrowernumber = $res->{'borrowernumber'}; my $resdate = $res->{'reservedate'}; @@ -1138,7 +1097,7 @@ sub ModReserveFill { # now fix the priority on the others (if the priority wasn't # already sorted!).... unless ( $priority == 0 ) { - _FixPriority( $biblionumber, $borrowernumber ); + _FixPriority( $reservenumber ); } } @@ -1265,20 +1224,19 @@ Reduce the values of queuded list =cut sub ModReserveMinusPriority { - my ( $itemnumber, $borrowernumber, $biblionumber ) = @_; + my ( $itemnumber, $reservenumber ) = @_; #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 reservenumber = ? "; my $sth_upd = $dbh->prepare($query); - $sth_upd->execute( $itemnumber, $borrowernumber, $biblionumber ); + $sth_upd->execute( $itemnumber, $reservenumber ); # second step update all others reservs - _FixPriority($biblionumber, $borrowernumber, '0'); + _FixPriority( $reservenumber, '0'); } =head2 GetReserveInfo @@ -1291,7 +1249,7 @@ Current implementation this query should have a single result. =cut sub GetReserveInfo { - my ( $borrowernumber, $biblionumber ) = @_; + my ( $reservenumber ) = @_; my $dbh = C4::Context->dbh; my $strsth="SELECT reservedate, @@ -1325,11 +1283,9 @@ sub GetReserveInfo { LEFT JOIN items USING(itemnumber) LEFT JOIN borrowers USING(borrowernumber) LEFT JOIN biblio ON (reserves.biblionumber=biblio.biblionumber) - WHERE - reserves.borrowernumber=? - AND reserves.biblionumber=?"; + WHERE reserves.reservenumber = ?"; my $sth = $dbh->prepare($strsth); - $sth->execute($borrowernumber,$biblionumber); + $sth->execute($reservenumber); my $data = $sth->fetchrow_hashref; return $data; @@ -1418,29 +1374,25 @@ Input: $where is 'up', 'down', 'top' or 'bottom'. Biblionumber, Date reserve was =cut sub AlterPriority { - my ( $where, $borrowernumber, $biblionumber ) = @_; - + my ( $where, $reservenumber ) = @_; + #warn "C4::Reserves::AlterPriority( $where, $reservenumber )"; 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( $reservenumber ); if ( $where eq 'up' || $where eq 'down' ) { my $priority = $reserve->{'priority'}; $priority = $where eq 'up' ? $priority - 1 : $priority + 1; - _FixPriority( $biblionumber, $borrowernumber, $priority ) + _FixPriority( $reservenumber, $priority ) } elsif ( $where eq 'top' ) { - _FixPriority( $biblionumber, $borrowernumber, '1' ) + _FixPriority( $reservenumber, '1' ) } elsif ( $where eq 'bottom' ) { - _FixPriority( $biblionumber, $borrowernumber, '999999' ) + _FixPriority( $reservenumber, '999999' ) } } @@ -1454,27 +1406,20 @@ This function sets the lowestPriority field to true if is false, and false if it =cut sub ToggleLowestPriority { - my ( $borrowernumber, $biblionumber ) = @_; + my ( $reservenumber ) = @_; 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 reservenumber = ?"); + $sth->execute( $reservenumber ); $sth->finish; - _FixPriority( $biblionumber, $borrowernumber, '999999' ); + _FixPriority( $reservenumber, '999999' ); } =head2 _FixPriority - &_FixPriority($biblio,$borrowernumber,$rank,$ignoreSetLowestRank); + &_FixPriority( $reservenumber, $rank, $ignoreSetLowestRank); Only used internally (so don't export it) Changed how this functions works # @@ -1486,41 +1431,39 @@ in new priority rank =cut sub _FixPriority { - my ( $biblio, $borrowernumber, $rank, $ignoreSetLowestRank ) = @_; + my ( $reservenumber, $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( $reservenumber ); + + if ( $rank eq "del" ) { + CancelReserve( $reservenumber ); + } + 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 reservenumber = ? + AND found IN ('W', 'T') + "; my $sth = $dbh->prepare($query); - $sth->execute( $biblio, $borrowernumber ); + $sth->execute( $reservenumber ); } 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 reservenumber, 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 ); @@ -1530,7 +1473,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 ( $reservenumber == $priority[$i]->{'reservenumber'} ) { $key = $i; # save the index last; } @@ -1546,29 +1489,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 reservenumber = ? "; $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]->{'reservenumber'} ); $sth->finish; } - $sth = $dbh->prepare( "SELECT borrowernumber FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); + $sth = $dbh->prepare( "SELECT reservenumber 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->{'reservenumber'}, '999999', 1 ); } } } @@ -1835,7 +1774,7 @@ sub _ShiftPriorityByDateAndPriority { =head2 MoveReserve - MoveReserve( $itemnumber, $borrowernumber, $cancelreserve ) + MoveReserve( $reservenumber, $cancelreserve ) Use when checking out an item to handle reserves If $cancelreserve boolean is set to true, it will remove existing reserve 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 2771c6f..d4cf225 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -601,6 +601,7 @@ function checkMultiHold() { [% FOREACH reserveloo IN biblioloo.reserveloop %] [% UNLESS ( loop.odd ) %][% ELSE %][% END %] +