From 2075e13bf7ce90ce5334fad8581a58ce6cd2bb27 Mon Sep 17 00:00:00 2001 From: slef Date: Mon, 15 Dec 2003 16:07:36 +0000 Subject: [PATCH] DBI call fix for bug 662 --- C4/Reserves2.pm | 246 +++++++++++++++++++----------------------------- 1 file changed, 96 insertions(+), 150 deletions(-) diff --git a/C4/Reserves2.pm b/C4/Reserves2.pm index b584b91734..e2f7576dab 100755 --- a/C4/Reserves2.pm +++ b/C4/Reserves2.pm @@ -110,6 +110,7 @@ sub FindReserves { # FIXME - These three bits of SQL seem to contain a fair amount of # redundancy. Wouldn't it be better to have a @clauses array, add # one or two clauses as necessary, then join(" AND ", @clauses) ? + # FIXME: not keen on quote() and interpolation either, but it looks safe if ($bib ne ''){ $bib = $dbh->quote($bib); if ($bor ne ''){ @@ -148,16 +149,11 @@ sub FindReserves { while (my $data=$sth->fetchrow_hashref){ # FIXME - What is this if-statement doing? How do constraints work? if ($data->{'constrainttype'} eq 'o') { - my $conquery = "SELECT biblioitemnumber FROM reserveconstraints + my $csth=$dbh->prepare("SELECT biblioitemnumber FROM reserveconstraints WHERE biblionumber = ? AND borrowernumber = ? - AND reservedate = ?"; - my $csth=$dbh->prepare($conquery); - # FIXME - Why use separate variables for this? - my $bibn = $data->{'biblionumber'}; - my $born = $data->{'borrowernumber'}; - my $resd = $data->{'reservedate'}; - $csth->execute($bibn, $born, $resd); + AND reservedate = ?"); + $csth->execute($data->{'biblionumber'}, $data->{'borrowernumber'}, $data->{'reservedate'}); my ($bibitemno) = $csth->fetchrow_array; $csth->finish; # Look up the book we just found. @@ -296,50 +292,42 @@ sub CancelReserve { #warn "In CancelReserve"; if (($item and $borr) and (not $biblio)) { # removing a waiting reserve record.... - $item = $dbh->quote($item); - $borr = $dbh->quote($borr); # update the database... - # FIXME - Use $dbh->do() - my $query = "update reserves set cancellationdate = now(), + my $sth = $dbh->prepare("update reserves set cancellationdate = now(), found = Null, priority = 0 - where itemnumber = $item - and borrowernumber = $borr"; - my $sth = $dbh->prepare($query); - $sth->execute; + where itemnumber = ? + and borrowernumber = ?"); + $sth->execute($item,$borr); $sth->finish; } if (($biblio and $borr) and (not $item)) { # removing a reserve record.... - my $q_biblio = $dbh->quote($biblio); - $borr = $dbh->quote($borr); # get the prioritiy on this record.... my $priority; { - my $query = "SELECT priority FROM reserves - WHERE biblionumber = $q_biblio - AND borrowernumber = $borr + my $sth=$dbh->prepare("SELECT priority FROM reserves + WHERE biblionumber = ? + AND borrowernumber = ? AND cancellationdate is NULL - AND (found <> 'F' or found is NULL)"; - my $sth=$dbh->prepare($query); - $sth->execute; + AND (found <> 'F' or found is NULL)"); + $sth->execute($biblio,$borr); ($priority) = $sth->fetchrow_array; $sth->finish; } # update the database, removing the record... { - my $query = "update reserves set cancellationdate = now(), + my $sth = $dbh->prepare("update reserves set cancellationdate = now(), found = Null, priority = 0 - where biblionumber = $q_biblio - and borrowernumber = $borr + where biblionumber = ? + and borrowernumber = ? and cancellationdate is NULL - and (found <> 'F' or found is NULL)"; - my $sth = $dbh->prepare($query); - $sth->execute; + and (found <> 'F' or found is NULL)"); + $sth->execute($biblio,$borr); $sth->finish; } @@ -411,19 +399,11 @@ sub fixpriority { my ($count, $reserves) = FindReserves($biblio); foreach my $rec (@$reserves) { if ($rec->{'priority'} > $priority) { - # FIXME - Rewrite this without so much duplication and - # redundancy - my $newpr = $rec->{'priority'}; $newpr = $dbh->quote($newpr - 1); - my $nbib = $rec->{'biblionumber'}; $nbib = $dbh->quote($nbib); - my $nbor = $rec->{'borrowernumber'}; $nbor = $dbh->quote($nbor); - my $nresd = $rec->{'reservedate'}; $nresd = $dbh->quote($nresd); - my $query = "UPDATE reserves SET priority = $newpr - WHERE biblionumber = $nbib - AND borrowernumber = $nbor - AND reservedate = $nresd"; - #warn $query; - my $sth = $dbh->prepare($query); - $sth->execute; + my $sth = $dbh->prepare("UPDATE reserves SET priority = ? + WHERE biblionumber = ? + AND borrowernumber = ? + AND reservedate = ?"); + $sth->execute($rec->{'priority'},$rec->{'biblionumber'},$rec->{'borrowernumber'},$rec->{'reservedate'}); $sth->finish; } } @@ -433,34 +413,28 @@ sub fixpriority { sub ReserveWaiting { my ($item, $borr) = @_; my $dbh = C4::Context->dbh; - $item = $dbh->quote($item); - $borr = $dbh->quote($borr); # get priority and biblionumber.... - my $query = "SELECT reserves.priority as priority, + my $sth = $dbh->prepare("SELECT reserves.priority as priority, reserves.biblionumber as biblionumber, reserves.branchcode as branchcode, reserves.timestamp as timestamp FROM reserves,items WHERE reserves.biblionumber = items.biblionumber - AND items.itemnumber = $item - AND reserves.borrowernumber = $borr + AND items.itemnumber = ? + AND reserves.borrowernumber = ? AND reserves.cancellationdate is NULL - AND (reserves.found <> 'F' or reserves.found is NULL)"; - my $sth = $dbh->prepare($query); - $sth->execute; + AND (reserves.found <> 'F' or reserves.found is NULL)"); + $sth->execute($item,$borr); my $data = $sth->fetchrow_hashref; $sth->finish; my $biblio = $data->{'biblionumber'}; my $timestamp = $data->{'timestamp'}; - my $q_biblio = $dbh->quote($biblio); - my $q_timestamp = $dbh->quote($timestamp); # update reserves record.... - $query = "UPDATE reserves SET priority = 0, found = 'W', itemnumber = $item - WHERE borrowernumber = $borr - AND biblionumber = $q_biblio - AND timestamp = $q_timestamp"; - $sth = $dbh->prepare($query); - $sth->execute; + $sth = $dbh->prepare("UPDATE reserves SET priority = 0, found = 'W', itemnumber = ? + WHERE borrowernumber = ? + AND biblionumber = ? + AND timestamp = ?"); + $sth->execute($item,$borr,$biblio,$timestamp); $sth->finish; # now fix up the remaining priorities.... fixpriority($data->{'priority'}, $biblio); @@ -472,22 +446,17 @@ sub ReserveWaiting { sub CheckWaiting { my ($borr)=@_; my $dbh = C4::Context->dbh; - $borr = $dbh->quote($borr); my @itemswaiting; - my $query = "SELECT * FROM reserves - WHERE borrowernumber = $borr + my $sth = $dbh->prepare("SELECT * FROM reserves + WHERE borrowernumber = ? AND reserves.found = 'W' - AND cancellationdate is NULL"; - my $sth = $dbh->prepare($query); - $sth->execute(); - # FIXME - Use 'push' - my $cnt=0; + AND cancellationdate is NULL"); + $sth->execute($borr); if (my $data=$sth->fetchrow_hashref) { - $itemswaiting[$cnt] =$data; - $cnt ++; + push(@itemswaiting,$data); } $sth->finish; - return ($cnt,\@itemswaiting); + return (scalar(@itemswaiting),\@itemswaiting); } =item Findgroupreserve @@ -513,7 +482,7 @@ C. sub Findgroupreserve { my ($bibitem,$biblio)=@_; my $dbh = C4::Context->dbh; - my $query = "SELECT reserves.biblionumber AS biblionumber, + my $sth=$dbh->prepare("SELECT reserves.biblionumber AS biblionumber, reserves.borrowernumber AS borrowernumber, reserves.reservedate AS reservedate, reserves.branchcode AS branchcode, @@ -532,18 +501,14 @@ sub Findgroupreserve { AND reserves.reservedate =reserveconstraints.reservedate ) OR reserves.constrainttype='a' ) AND reserves.cancellationdate is NULL - AND (reserves.found <> 'F' or reserves.found is NULL)"; - my $sth=$dbh->prepare($query); + AND (reserves.found <> 'F' or reserves.found is NULL)"); $sth->execute($biblio, $bibitem); - # FIXME - $i is unnecessary and bogus - my $i=0; my @results; while (my $data=$sth->fetchrow_hashref){ - $results[$i]=$data; # FIXME - Use push - $i++; + push(@results,$data); } $sth->finish; - return($i,@results); + return(scalar(@results),@results); } # FIXME - A somewhat different version of this function appears in @@ -562,21 +527,18 @@ sub CreateReserve { if ($fee > 0) { # print $fee; my $nextacctno = &getnextacctno($env,$borrnum,$dbh); - my $updquery = "insert into accountlines + my $usth = $dbh->prepare("insert into accountlines (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding) values - ($borrnum,$nextacctno,now(),$fee,'Reserve Charge - $title','Res',$fee)"; - my $usth = $dbh->prepare($updquery); - $usth->execute; + (?,?,now(),?,?,'Res',?)"); + $usth->execute($borrnum,$nextacctno,$fee,'Reserve Charge - $title',$fee); $usth->finish; } #if ($const eq 'a'){ - my $query="insert into reserves + my $sth = $dbh->prepare("insert into reserves (borrowernumber,biblionumber,reservedate,branchcode,constrainttype,priority,reservenotes) - values -('$borrnum','$biblionumber','$resdate','$branch','$const','$priority','$notes')"; - my $sth = $dbh->prepare($query); - $sth->execute(); + values (?,?,?,?,?,?,?)"); + $sth->execute($borrnum,$biblionumber,$resdate,$branch,$const,$priority,$notes); $sth->finish; #} if (($const eq "o") || ($const eq "e")) { @@ -584,13 +546,11 @@ sub CreateReserve { my $i = 0; while ($i < $numitems) { my $biblioitem = @$bibitems[$i]; - my $query = "insert into + my $sth = $dbh->prepare("insert into reserveconstraints (borrowernumber,biblionumber,reservedate,biblioitemnumber) - values - ('$borrnum','$biblionumber','$resdate','$biblioitem')"; - my $sth = $dbh->prepare($query); - $sth->execute(); + values (?,?,?,?)"); + $sth->execute($borrnum,$biblionumber,$resdate,$biblioitem); $sth->finish; $i++; } @@ -608,10 +568,9 @@ sub CalcReserveFee { #check for issues; my $dbh = C4::Context->dbh; my $const = lc substr($constraint,0,1); - my $query = "SELECT * FROM borrowers,categories + my $sth = $dbh->prepare("SELECT * FROM borrowers,categories WHERE (borrowernumber = ?) - AND (borrowers.categorycode = categories.categorycode)"; - my $sth = $dbh->prepare($query); + AND (borrowers.categorycode = categories.categorycode)"); $sth->execute($borrnum); my $data = $sth->fetchrow_hashref; $sth->finish(); @@ -621,10 +580,9 @@ sub CalcReserveFee { # check for items on issue # first find biblioitem records my @biblioitems; - my $query1 = "SELECT * FROM biblio,biblioitems + my $sth1 = $dbh->prepare("SELECT * FROM biblio,biblioitems WHERE (biblio.biblionumber = ?) - AND (biblio.biblionumber = biblioitems.biblionumber)"; - my $sth1 = $dbh->prepare($query1); + AND (biblio.biblionumber = biblioitems.biblionumber)"); $sth1->execute($biblionumber); while (my $data1=$sth1->fetchrow_hashref) { if ($const eq "a") { @@ -656,16 +614,13 @@ sub CalcReserveFee { my $allissued = 1; while ($x < $cntitemsfound) { my $bitdata = $biblioitems[$x]; - my $query2 = "SELECT * FROM items - WHERE biblioitemnumber = ?"; - my $sth2 = $dbh->prepare($query2); + my $sth2 = $dbh->prepare("SELECT * FROM items + WHERE biblioitemnumber = ?"); $sth2->execute($bitdata->{'biblioitemnumber'}); while (my $itdata=$sth2->fetchrow_hashref) { - my $query3 = "SELECT * FROM issues + my $sth3 = $dbh->prepare("SELECT * FROM issues WHERE itemnumber = ? - AND returndate IS NULL"; - - my $sth3 = $dbh->prepare($query3); + AND returndate IS NULL"); $sth3->execute($itdata->{'itemnumber'}); if (my $isdata=$sth3->fetchrow_hashref) { } else { @@ -675,8 +630,7 @@ sub CalcReserveFee { $x++; } if ($allissued == 0) { - my $rquery = "SELECT * FROM reserves WHERE biblionumber = ?"; - my $rsth = $dbh->prepare($rquery); + my $rsth = $dbh->prepare("SELECT * FROM reserves WHERE biblionumber = ?"); $rsth->execute($biblionumber); if (my $rdata = $rsth->fetchrow_hashref) { } else { @@ -692,11 +646,10 @@ sub CalcReserveFee { sub getnextacctno { my ($env,$bornumber,$dbh)=@_; my $nextaccntno = 1; - my $query = "select * from accountlines - where (borrowernumber = '$bornumber') - order by accountno desc"; - my $sth = $dbh->prepare($query); - $sth->execute; + my $sth = $dbh->prepare("select * from accountlines + where (borrowernumber = ?) + order by accountno desc"); + $sth->execute($bornumber); if (my $accdata=$sth->fetchrow_hashref){ $nextaccntno = $accdata->{'accountno'} + 1; } @@ -709,38 +662,34 @@ sub updatereserves{ #subroutine to update a reserve my ($rank,$biblio,$borrower,$del,$branch)=@_; my $dbh = C4::Context->dbh; - my $query="Update reserves "; if ($del == 0){ - $query.="set priority='$rank',branchcode='$branch' where - biblionumber=$biblio and borrowernumber=$borrower"; + my $sth = $dbh->prepare("Update reserves set priority=?,branchcode=? where + biblionumber=? and borrowernumber=?"); + $sth->execute($rank,$branch,$biblio,$borrower); + $sth->finish(); } else { - $query="Select * from reserves where biblionumber=$biblio and - borrowernumber=$borrower"; - my $sth=$dbh->prepare($query); - $sth->execute; + my $sth=$dbh->prepare("Select * from reserves where biblionumber=? and + borrowernumber=?"); + $sth->execute($biblio,$borrower); my $data=$sth->fetchrow_hashref; - $sth->finish; - $query="Select * from reserves where biblionumber=$biblio and - priority > '$data->{'priority'}' and cancellationdate is NULL - order by priority"; - my $sth2=$dbh->prepare($query) || die $dbh->errstr; - $sth2->execute || die $sth2->errstr; - while (my $data=$sth2->fetchrow_hashref){ + $sth->finish(); + $sth=$dbh->prepare("Select * from reserves where biblionumber=? and + priority > ? and cancellationdate is NULL + order by priority") || die $dbh->errstr; + $sth->execute($biblio,$data->{'priority'}) || die $sth->errstr; + while (my $data=$sth->fetchrow_hashref){ $data->{'priority'}--; - $query="Update reserves set priority=$data->{'priority'} where - biblionumber=$data->{'biblionumber'} and - borrowernumber=$data->{'borrowernumber'}"; - my $sth3=$dbh->prepare($query); - $sth3->execute || die $sth3->errstr; - $sth3->finish; + my $sth3=$dbh->prepare("Update reserves set priority=? + where biblionumber=? and borrowernumber=?"); + $sth3->execute($data->{'priority'},$data->{'biblionumber'},$data->{'borrowernumber'}) || die $sth3->errstr; + $sth3->finish(); } - $sth2->finish; - $query="update reserves set cancellationdate=now() where biblionumber=$biblio - and borrowernumber=$borrower"; + $sth->finish(); + $sth=$dbh->prepare("update reserves set cancellationdate=now() where biblionumber=? + and borrowernumber=?"); + $sth->execute($biblio,$borrower); + $sth->finish; } - my $sth=$dbh->prepare($query); - $sth->execute; - $sth->finish; } # XXX - POD @@ -751,21 +700,19 @@ sub UpdateReserve { return if $rank eq "n"; my $dbh = C4::Context->dbh; if ($rank eq "del") { - my $query = "UPDATE reserves SET cancellationdate=now() + my $sth=$dbh->prepare("UPDATE reserves SET cancellationdate=now() WHERE biblionumber = ? AND borrowernumber = ? AND cancellationdate is NULL - AND (found <> 'F' or found is NULL)"; - my $sth=$dbh->prepare($query); + AND (found <> 'F' or found is NULL)"); $sth->execute($biblio, $borrower); $sth->finish; } else { - my $query = "UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = NULL, found = NULL + my $sth=$dbh->prepare("UPDATE reserves SET priority = ? ,branchcode = ?, itemnumber = NULL, found = NULL WHERE biblionumber = ? AND borrowernumber = ? AND cancellationdate is NULL - AND (found <> 'F' or found is NULL)"; - my $sth=$dbh->prepare($query); + AND (found <> 'F' or found is NULL)"); $sth->execute($rank, $branch, $biblio, $borrower); $sth->finish; } @@ -775,13 +722,12 @@ sub UpdateReserve { sub getreservetitle { my ($biblio,$bor,$date,$timestamp)=@_; my $dbh = C4::Context->dbh; - my $query="Select * from reserveconstraints,biblioitems where + my $sth=$dbh->prepare("Select * from reserveconstraints,biblioitems where reserveconstraints.biblioitemnumber=biblioitems.biblioitemnumber - and reserveconstraints.biblionumber=$biblio and reserveconstraints.borrowernumber - = $bor and reserveconstraints.reservedate='$date' and - reserveconstraints.timestamp=$timestamp"; - my $sth=$dbh->prepare($query); - $sth->execute; + and reserveconstraints.biblionumber=? and reserveconstraints.borrowernumber + = ? and reserveconstraints.reservedate=? and + reserveconstraints.timestamp=?"); + $sth->execute($biblio,$bor,$date,$timestamp); my $data=$sth->fetchrow_hashref; $sth->finish; return($data); -- 2.39.5