From 3242bfacd615072cd81b3c06d6f90d8493148306 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 21 Aug 2015 11:44:55 +0200 Subject: [PATCH] Bug 14702: Refactor GetReserveFee Content-Type: text/plain; charset=utf-8 The code of GetReserveFee was not very clear. What it did was: check if there are some items not issued. If so and there are no holds, calculate no fee. Since AddReserves only calls this sub, we can move the code for charging the fee to this sub. I renamed it to ChargeReserveFee. There is no change in behavior. Test plan: [1] Make sure that a patron category (X) includes a hold fee. [2] Select a biblio with 2 items. [3] Issue one item to another patron. [4] Place a hold on this biblio by patron with category X. No charge? [5] Cancel the hold from the previous step. [6] Use another patron to place another hold on this biblio. [7] Place hold again by patron with category X. Is it charged? [8] Cancel that hold again. Issue the second item to another patron. [9] Place hold again by patron with category X. Is it charged again? --- C4/Reserves.pm | 124 ++++++++++++-------------------------------------------- 1 file changed, 27 insertions(+), 97 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index f0fc845..86a5252 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -103,7 +103,6 @@ BEGIN { &GetReservesForBranch &GetReservesToBranch &GetReserveCount - &GetReserveFee &GetReserveInfo &GetReserveStatus @@ -156,9 +155,6 @@ sub AddReserve { $bibitems, $priority, $resdate, $expdate, $notes, $title, $checkitem, $found ) = @_; - my $fee = - GetReserveFee($borrowernumber, $biblionumber, - $bibitems ); my $dbh = C4::Context->dbh; $resdate = format_date_in_iso( $resdate ) if ( $resdate ); $resdate = C4::Dates->today( 'iso' ) unless ( $resdate ); @@ -178,21 +174,7 @@ sub AddReserve { $waitingdate = $resdate; } - #eval { # updates take place here - if ( $fee > 0 ) { - my $nextacctno = &getnextacctno( $borrowernumber ); - my $query = qq{ - INSERT INTO accountlines - (borrowernumber,accountno,date,amount,description,accounttype,amountoutstanding) - VALUES - (?,?,now(),?,?,'Res',?) - }; - my $usth = $dbh->prepare($query); - $usth->execute( $borrowernumber, $nextacctno, $fee, - "Reserve Charge - $title", $fee ); - } - my $query = qq{ INSERT INTO reserves (borrowernumber,biblionumber,reservedate,branchcode, @@ -207,6 +189,8 @@ sub AddReserve { $notes, $checkitem, $found, $waitingdate, $expdate ); my $reserve_id = $sth->{mysql_insertid}; + # add a reserve fee if needed + ChargeReserveFee( $borrowernumber, $biblionumber, $title ); # Send e-mail to librarian if syspref is active if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){ @@ -650,93 +634,39 @@ sub GetOtherReserves { return ( $messages, $nextreservinfo ); } -=head2 GetReserveFee +=head2 ChargeReserveFee - $fee = GetReserveFee($borrowernumber,$biblionumber,$biblionumber); + $fee = ChargeReserveFee( $borrowernumber, $biblionumber, $title ); -Calculate the fee for a reserve + Charge the fee for a reserve (if applicable). =cut -sub GetReserveFee { - my ($borrowernumber, $biblionumber, $bibitems ) = @_; - - #check for issues; - my $dbh = C4::Context->dbh; - my $query = qq{ - SELECT * FROM borrowers - LEFT JOIN categories ON borrowers.categorycode = categories.categorycode - WHERE borrowernumber = ? +sub ChargeReserveFee { + my ( $borrowernumber, $biblionumber, $title ) = @_; + my $borquery = qq{ +SELECT reservefee FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode WHERE borrowernumber = ? + }; + my $accquery = qq{ +INSERT INTO accountlines ( borrowernumber, accountno, date, amount, description, accounttype, amountoutstanding ) VALUES (?, ?, NOW(), ?, ?, 'Res', ?) }; - my $sth = $dbh->prepare($query); - $sth->execute($borrowernumber); - my $data = $sth->fetchrow_hashref; - my $fee = $data->{'reservefee'}; - my $cntitems = @- > $bibitems; - - if ( $fee > 0 ) { - - # check for items on issue - # first find biblioitem records - my @biblioitems; - my $sth1 = $dbh->prepare( - "SELECT * FROM biblio LEFT JOIN biblioitems on biblio.biblionumber = biblioitems.biblionumber - WHERE (biblio.biblionumber = ?)" - ); - $sth1->execute($biblionumber); - while ( my $data1 = $sth1->fetchrow_hashref ) { - my $found = 0; - my $x = 0; - while ( $x < $cntitems ) { - if ( @$bibitems->{'biblioitemnumber'} == - $data->{'biblioitemnumber'} ) - { - $found = 1; - } - $x++; - } - if ( $found == 0 ) { - push @biblioitems, $data1; - } - } - my $cntitemsfound = @biblioitems; - my $issues = 0; - my $x = 0; - my $allissued = 1; - while ( $x < $cntitemsfound ) { - my $bitdata = $biblioitems[$x]; - my $sth2 = $dbh->prepare( - "SELECT * FROM items - WHERE biblioitemnumber = ?" - ); - $sth2->execute( $bitdata->{'biblioitemnumber'} ); - while ( my $itdata = $sth2->fetchrow_hashref ) { - my $sth3 = $dbh->prepare( - "SELECT * FROM issues - WHERE itemnumber = ?" - ); - $sth3->execute( $itdata->{'itemnumber'} ); - if ( my $isdata = $sth3->fetchrow_hashref ) { - } - else { - $allissued = 0; - } - } - $x++; - } - if ( $allissued == 0 ) { - my $rsth = - $dbh->prepare("SELECT * FROM reserves WHERE biblionumber = ?"); - $rsth->execute($biblionumber); - if ( my $rdata = $rsth->fetchrow_hashref ) { - } - else { - $fee = 0; - } - } + my $dbh = C4::Context->dbh; + my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) ); + if( $fee && $fee > 0 ) { + # This is a reconstruction of the old code: + # Calculate number of items, items issued and holds + my ( $cnt1 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM items WHERE biblionumber=?", undef, ( $biblionumber ) ); + my ( $cnt2 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM issues LEFT JOIN items USING (itemnumber) WHERE biblionumber=?", undef, ( $biblionumber )); + my ( $cnt3 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>?", undef, ( $biblionumber, $borrowernumber ) ); + # If not all items are issued and there are no holds: charge no fee + # NOTE: Lost, damaged, not-for-loan, etc. are just ignored here + return if $cnt1 - $cnt2 > 0 && $cnt3 == 0; + + # Here we need to charge the fee + my $nextacctno = &getnextacctno( $borrowernumber ); + $dbh->do( $accquery, undef, ( $borrowernumber, $nextacctno, $fee, "Reserve Charge - $title", $fee ) ); } - return $fee; } =head2 GetReservesToBranch -- 1.7.10.4