Bugzilla – Attachment 42165 Details for
Bug 14702
Refactor GetReserveFee
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14702: Refactor GetReserveFee
Bug-14702-Refactor-GetReserveFee.patch (text/plain), 7.87 KB, created by
Jonathan Druart
on 2015-09-01 14:31:17 UTC
(
hide
)
Description:
Bug 14702: Refactor GetReserveFee
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2015-09-01 14:31:17 UTC
Size:
7.87 KB
patch
obsolete
>From 8a6891fb095ed030238a70b4ade0b256c99b06b4 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >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ä <j.kylmala@gmail.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14702
:
41779
|
41780
|
41781
|
41853
|
41854
|
41856
|
41857
|
42005
|
42006
|
42015
|
42016
|
42017
|
42153
|
42154
|
42155
|
42156
| 42165 |
42166
|
42167
|
42191