Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 14702: Refactor GetReserveFee

Browse source 
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Marcel de Rooy 2015-08-21 11:44:55 +02:00 committed by Tomas Cohen Arazi
parent 7f65aaac74
commit 44a4e043a5
1 changed files with 38 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

132
C4/Reserves.pm
View file

@ -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 ChargeReserveFee
$fee = ChargeReserveFee( $borrowernumber, $fee, $title );
Charge the fee for a reserve (if $fee > 0)
=cut
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 $dbh = C4::Context->dbh;
my $nextacctno = &getnextacctno( $borrowernumber );
$dbh->do( $accquery, undef, ( $borrowernumber, $nextacctno, $fee, "Reserve Charge - $title", $fee ) );
}
=head2 GetReserveFee
$fee = GetReserveFee($borrowernumber,$biblionumber,$biblionumber);
$fee = GetReserveFee( $borrowernumber, $biblionumber );
Calculate the fee for a reserve
Calculate 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 = ?
my ( $borrowernumber, $biblionumber ) = @_;
my $borquery = qq{
SELECT reservefee FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode WHERE borrowernumber = ?
};
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
$fee = 0 if $cnt1 - $cnt2 > 0 && $cnt3 == 0;
}
return $fee;
}