From 8a6891fb095ed030238a70b4ade0b256c99b06b4 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. While doing so, I moved the code to charge the fee (in AddReserve) to a small new sub ChargeReserveFee. There is no change in behavior. The follow-up patch adds unit tests. 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? Signed-off-by: Joonas Kylmälä Signed-off-by: Jonathan Druart --- C4/Reserves.pm | 132 +++++++++++++++++---------------------------------------- 1 file changed, 38 insertions(+), 94 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index a300a4a..e8638a9 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,9 @@ sub AddReserve { $notes, $checkitem, $found, $waitingdate, $expdate ); my $reserve_id = $sth->{mysql_insertid}; + # add a reserve fee if needed + my $fee = GetReserveFee( $borrowernumber, $biblionumber ); + ChargeReserveFee( $borrowernumber, $fee, $title ); # Send e-mail to librarian if syspref is active if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){ @@ -655,91 +640,50 @@ sub GetOtherReserves { return ( $messages, $nextreservinfo ); } -=head2 GetReserveFee +=head2 ChargeReserveFee - $fee = GetReserveFee($borrowernumber,$biblionumber,$biblionumber); + $fee = ChargeReserveFee( $borrowernumber, $fee, $title ); -Calculate the fee for a reserve + Charge the fee for a reserve (if $fee > 0) =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, $fee, $title ) = @_; + return if !$fee; + 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; + my $dbh = C4::Context->dbh; + my $nextacctno = &getnextacctno( $borrowernumber ); + $dbh->do( $accquery, undef, ( $borrowernumber, $nextacctno, $fee, "Reserve Charge - $title", $fee ) ); +} - if ( $fee > 0 ) { +=head2 GetReserveFee - # 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++; - } + $fee = GetReserveFee( $borrowernumber, $biblionumber ); - 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; - } - } + Calculate the fee for a reserve (if applicable). + +=cut + +sub GetReserveFee { + my ( $borrowernumber, $biblionumber ) = @_; + my $borquery = qq{ +SELECT reservefee FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode WHERE borrowernumber = ? + }; + + 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 + $fee = 0 if $cnt1 - $cnt2 > 0 && $cnt3 == 0; } return $fee; } -- 2.1.0